Use MoveWithinRange for attack activities.

Fixes #2104. Fixes #2923. Fixes #4455.

OpenRA.Mods.RA/Activities/Attack.cs

@@ -16,25 +16,27 @@ namespace OpenRA.Mods.RA.Activities
 	/* non-turreted attack */
 	public class Attack : Activity
 	{
-		protected Target Target;
+		protected readonly Target Target;
-		WRange Range;
+		readonly AttackBase attack;
-		bool AllowMovement;
+		readonly IMove move;
-
+		readonly IFacing facing;
-		int nextPathTime;
+		readonly WRange minRange;
-
+		readonly WRange maxRange;
-		const int delayBetweenPathingAttempts = 20;
-		const int delaySpread = 5;
 
 		public Attack(Actor self, Target target, WRange minRange, WRange maxRange, bool allowMovement)
 		{
 			Target = target;
-			Range = maxRange;
+			this.minRange = minRange;
-			AllowMovement = allowMovement;
+			this.maxRange = maxRange;
+
+			attack = self.Trait<AttackBase>();
+			facing = self.Trait<IFacing>();
+
+			move = allowMovement ? self.TraitOrDefault<IMove>() : null;
 		}
 
 		public override Activity Tick(Actor self)
 		{
-			var attack = self.Trait<AttackBase>();
 			var ret = InnerTick(self, attack);
 			attack.IsAttacking = (ret == this);
 			return ret;
@@ -49,27 +51,23 @@ namespace OpenRA.Mods.RA.Activities
 			if (!Target.IsValidFor(self) || type == TargetType.FrozenActor)
 				return NextActivity;
 
-			// TODO: This is horrible, and probably wrong. Work out what it is trying to solve, then redo it properly.
+			// Drop the target if it moves under the shroud / fog.
-			if (type == TargetType.Actor && Target.Actor.HasTrait<Mobile>() && !self.Owner.Shroud.IsTargetable(Target.Actor))
+			// HACK: This would otherwise break targeting frozen actors
+			// The problem is that Shroud.IsTargetable returns false (as it should) for
+			// frozen actors, but we do want to explicitly target the underlying actor here.
+			if (type == TargetType.Actor && !Target.Actor.HasTrait<FrozenUnderFog>() && !self.Owner.Shroud.IsTargetable(Target.Actor))
 				return NextActivity;
 
-			if (!Target.IsInRange(self.CenterPosition, Range))
+			// Try to move within range
-			{
+			if (move != null && (!Target.IsInRange(self.CenterPosition, maxRange) || Target.IsInRange(self.CenterPosition, minRange)))
-				if (--nextPathTime > 0)
+				return Util.SequenceActivities(move.MoveWithinRange(Target, minRange, maxRange), this);
-					return this;
-
-				nextPathTime = self.World.SharedRandom.Next(delayBetweenPathingAttempts - delaySpread,
-					delayBetweenPathingAttempts + delaySpread);
-
-				return (AllowMovement) ? Util.SequenceActivities(self.Trait<IMove>().MoveWithinRange(Target, Range), this) : NextActivity;
-			}
 
 			var desiredFacing = Util.GetFacing(Target.CenterPosition - self.CenterPosition, 0);
-			var facing = self.Trait<IFacing>();
 			if (facing.Facing != desiredFacing)
 				return Util.SequenceActivities(new Turn(desiredFacing), this);
 
 			attack.DoAttack(self, Target);
+
 			return this;
 		}
 	}

OpenRA.Mods.RA/Activities/Follow.cs

@@ -1,6 +1,6 @@
 #region Copyright & License Information
 /*
- * Copyright 2007-2011 The OpenRA Developers (see AUTHORS)
+ * Copyright 2007-2014 The OpenRA Developers (see AUTHORS)
  * This file is part of OpenRA, which is free software. It is made
  * available to you under the terms of the GNU General Public License
  * as published by the Free Software Foundation. For more information,
@@ -8,25 +8,25 @@
  */
 #endregion
 
+using System.Collections.Generic;
 using OpenRA.Traits;
 
 namespace OpenRA.Mods.RA.Activities
 {
 	public class Follow : Activity
 	{
-		Target target;
+		readonly Target target;
-		IMove move;
+		readonly WRange minRange;
-		WRange range;
+		readonly WRange maxRange;
-		int nextPathTime;
+		readonly IMove move;
-
-		const int delayBetweenPathingAttempts = 20;
-		const int delaySpread = 5;
 
 		public Follow(Actor self, Target target, WRange minRange, WRange maxRange)
 		{
 			this.target = target;
+			this.minRange = minRange;
+			this.maxRange = maxRange;
+
 			move = self.Trait<IMove>();
-			this.range = maxRange;
 		}
 
 		public override Activity Tick(Actor self)
@@ -34,13 +34,17 @@ namespace OpenRA.Mods.RA.Activities
 			if (IsCanceled || !target.IsValidFor(self))
 				return NextActivity;
 
-			if (target.IsInRange(self.CenterPosition, range) || --nextPathTime > 0)
+			var cachedPosition = target.CenterPosition;
-				return this;
+			var path = move.MoveWithinRange(target, minRange, maxRange);
 
-			nextPathTime = self.World.SharedRandom.Next(delayBetweenPathingAttempts - delaySpread,
+			// We are already in range, so wait until the target moves before doing anything
-				delayBetweenPathingAttempts + delaySpread);
+			if (target.IsInRange(self.CenterPosition, maxRange) && !target.IsInRange(self.CenterPosition, minRange))
+			{
+				var wait = new WaitFor(() => !target.IsValidFor(self) || target.CenterPosition != cachedPosition);
+				return Util.SequenceActivities(wait, path, this);
+			}
 
-			return Util.SequenceActivities(move.MoveWithinRange(target, range), this);
+			return Util.SequenceActivities(path, this);
 		}
 	}
 }

OpenRA.Mods.RA/Move/Mobile.cs

@@ -591,8 +591,8 @@ namespace OpenRA.Mods.RA.Move
 		public Activity ScriptedMove(CPos cell) { return new Move(cell); }
 		public Activity MoveTo(CPos cell, int nearEnough) { return new Move(cell, nearEnough); }
 		public Activity MoveTo(CPos cell, Actor ignoredActor) { return new Move(cell, ignoredActor); }
-		public Activity MoveWithinRange(Target target, WRange range) { return new Move(target, range); }
+		public Activity MoveWithinRange(Target target, WRange range) { return new MoveWithinRange(self, target, WRange.Zero, range); }
-		public Activity MoveWithinRange(Target target, WRange minRange, WRange maxRange) { return new Move(target, maxRange); }
+		public Activity MoveWithinRange(Target target, WRange minRange, WRange maxRange) { return new MoveWithinRange(self, target, minRange, maxRange); }
 		public Activity MoveFollow(Actor self, Target target, WRange minRange, WRange maxRange) { return new Follow(self, target, minRange, maxRange); }
 		public Activity MoveTo(Func<List<CPos>> pathFunc) { return new Move(pathFunc); }