Prevent movement pausing at invalid position.

OpenRA.Mods.Common/Activities/Air/FlyIdle.cs

@@ -23,12 +23,14 @@ namespace OpenRA.Mods.Common.Activities
 		readonly int turnSpeed;
 		int remainingTicks;
 
-		public FlyIdle(Actor self, int ticks = -1)
+		public FlyIdle(Actor self, int ticks = -1, bool tickIdle = true)
 		{
 			aircraft = self.Trait<Aircraft>();
-			tickIdles = self.TraitsImplementing<INotifyIdle>().ToArray();
 			turnSpeed = aircraft.Info.IdleTurnSpeed > -1 ? aircraft.Info.IdleTurnSpeed : aircraft.TurnSpeed;
 			remainingTicks = ticks;
+
+			if (tickIdle)
+				tickIdles = self.TraitsImplementing<INotifyIdle>().ToArray();
 		}
 
 		public override bool Tick(Actor self)
@@ -42,6 +44,7 @@ namespace OpenRA.Mods.Common.Activities
 			if (remainingTicks > 0)
 				remainingTicks--;
 
+			if (tickIdles != null)
 				foreach (var tickIdle in tickIdles)
 					tickIdle.TickIdle(self);
 

OpenRA.Mods.Common/Activities/Move/Move.cs

@@ -210,6 +210,13 @@ namespace OpenRA.Mods.Common.Activities
 
 			var nextCell = path[path.Count - 1];
 
+			// Something else might have moved us, so the path is no longer valid.
+			if (!Util.AreAdjacentCells(mobile.ToCell, nextCell))
+			{
+				path = EvalPath(BlockedByActor.Immovable);
+				return null;
+			}
+
 			var containsTemporaryBlocker = WorldUtils.ContainsTemporaryBlocker(self.World, nextCell, self);
 
 			// Next cell in the move is blocked by another actor
@@ -407,9 +414,6 @@ namespace OpenRA.Mods.Common.Activities
 
 			public override bool Tick(Actor self)
 			{
-				if (Move.mobile.IsTraitDisabled || Move.mobile.IsTraitPaused)
-					return false;
-
 				var ret = InnerTick(self, Move.mobile);
 
 				if (moveFraction > MoveFractionTotal)
@@ -491,7 +495,7 @@ namespace OpenRA.Mods.Common.Activities
 				var nextCell = parent.PopPath(self);
 				if (nextCell != null)
 				{
-					if (IsTurn(mobile, nextCell.Value.First, map))
+					if (!mobile.IsTraitPaused && !mobile.IsTraitDisabled && IsTurn(mobile, nextCell.Value.First, map))
 					{
 						var nextSubcellOffset = map.Grid.OffsetOfSubCell(nextCell.Value.Second);
 						var ret = new MoveFirstHalf(

OpenRA.Mods.Common/Activities/PickupUnit.cs

@@ -9,6 +9,7 @@
  */
 #endregion
 
+using System;
 using System.Collections.Generic;
 using OpenRA.Activities;
 using OpenRA.Mods.Common.Traits;
@@ -20,20 +21,18 @@ namespace OpenRA.Mods.Common.Activities
 	public class PickupUnit : Activity
 	{
 		readonly Actor cargo;
-		readonly IMove movement;
-
 		readonly Carryall carryall;
-		readonly IFacing carryallFacing;
-
 		readonly Carryable carryable;
 		readonly IFacing carryableFacing;
 		readonly BodyOrientation carryableBody;
 
 		readonly int delay;
 
-		enum PickupState { Intercept, LockCarryable, Pickup }
+		// TODO: Expose this to yaml
+		readonly WDist targetLockRange = WDist.FromCells(4);
 
-		PickupState state;
+		enum PickupState { Intercept, LockCarryable, Pickup }
+		PickupState state = PickupState.Intercept;
 
 		public PickupUnit(Actor self, Actor cargo, int delay)
 		{
@@ -43,16 +42,20 @@ namespace OpenRA.Mods.Common.Activities
 			carryableFacing = cargo.Trait<IFacing>();
 			carryableBody = cargo.Trait<BodyOrientation>();
 
-			movement = self.Trait<IMove>();
 			carryall = self.Trait<Carryall>();
-			carryallFacing = self.Trait<IFacing>();
 
-			state = PickupState.Intercept;
+			ChildHasPriority = false;
 		}
 
 		protected override void OnFirstRun(Actor self)
 		{
-			carryall.ReserveCarryable(self, cargo);
+			if (carryall.ReserveCarryable(self, cargo))
+			{
+				// Fly to the target and wait for it to be locked for pickup
+				// These activities will be cancelled and replaced by Land once the target has been locked
+				QueueChild(new Fly(self, Target.FromActor(cargo)));
+				QueueChild(new FlyIdle(self, tickIdle: false));
+			}
 		}
 
 		public override bool Tick(Actor self)
@@ -74,26 +77,21 @@ namespace OpenRA.Mods.Common.Activities
 				return true;
 			}
 
-			if (carryall.State != Carryall.CarryallState.Reserved)
-				return true;
-
-			switch (state)
-			{
-				case PickupState.Intercept:
-					QueueChild(movement.MoveWithinRange(Target.FromActor(cargo), WDist.FromCells(4)));
+			// Wait until we are near the target before we try to lock it
+			var distSq = (cargo.CenterPosition - self.CenterPosition).HorizontalLengthSquared;
+			if (state == PickupState.Intercept && distSq <= targetLockRange.LengthSquared)
 				state = PickupState.LockCarryable;
-					return false;
 
-				case PickupState.LockCarryable:
-					if (!carryable.LockForPickup(cargo, self))
-						Cancel(self);
-
-					state = PickupState.Pickup;
-					return false;
-
-				case PickupState.Pickup:
+			if (state == PickupState.LockCarryable)
 			{
-					// Land at the target location
+				var lockResponse = carryable.LockForPickup(cargo, self);
+				if (lockResponse == LockResponse.Failed)
+					Cancel(self);
+				else if (lockResponse == LockResponse.Success)
+				{
+					// Pickup position and facing are now known - swap the fly/wait activity with Land
+					ChildActivity.Cancel(self);
+
 					var localOffset = carryall.OffsetForCarryable(self, cargo).Rotate(carryableBody.QuantizeOrientation(self, cargo.Orientation));
 					QueueChild(new Land(self, Target.FromActor(cargo), -carryableBody.LocalToWorld(localOffset), carryableFacing.Facing));
 
@@ -104,11 +102,13 @@ namespace OpenRA.Mods.Common.Activities
 					// Remove our carryable from world
 					QueueChild(new AttachUnit(self, cargo));
 					QueueChild(new TakeOff(self));
-					return false;
+
+					state = PickupState.Pickup;
 				}
 			}
 
-			return true;
+			// Return once we are in the pickup state and the pickup activities have completed
+			return TickChild(self) && state == PickupState.Pickup;
 		}
 
 		public override IEnumerable<TargetLineNode> TargetLineNodes(Actor self)

OpenRA.Mods.Common/Traits/AutoCarryable.cs

@@ -102,10 +102,10 @@ namespace OpenRA.Mods.Common.Traits
 		}
 
 		// Prepare for transport pickup
-		public override bool LockForPickup(Actor self, Actor carrier)
+		public override LockResponse LockForPickup(Actor self, Actor carrier)
 		{
-			if (state == State.Locked || !WantsTransport)
-				return false;
+			if ((state == State.Locked && Carrier != carrier) || !WantsTransport)
+				return LockResponse.Failed;
 
 			// Last chance to change our mind...
 			var delta = self.World.Map.CenterOfCell(Destination.Value) - self.CenterPosition;
@@ -113,7 +113,7 @@ namespace OpenRA.Mods.Common.Traits
 			{
 				// Cancel pickup
 				MovementCancelled(self);
-				return false;
+				return LockResponse.Failed;
 			}
 
 			return base.LockForPickup(self, carrier);

OpenRA.Mods.Common/Traits/CaptureManager.cs

@@ -221,6 +221,11 @@ namespace OpenRA.Mods.Common.Traits
 			if (captures == null)
 				return false;
 
+			// HACK: Make sure the target is not moving and at its normal position with respect to the cell grid
+			var enterMobile = target.TraitOrDefault<Mobile>();
+			if (enterMobile != null && enterMobile.IsMovingBetweenCells)
+				return false;
+
 			if (progressWatchers.Any() || targetManager.progressWatchers.Any())
 			{
 				currentTargetTotal = captures.Info.CaptureDelay;

OpenRA.Mods.Common/Traits/Carryable.cs

@@ -35,6 +35,8 @@ namespace OpenRA.Mods.Common.Traits
 		public override object Create(ActorInitializer init) { return new Carryable(init.Self, this); }
 	}
 
+	public enum LockResponse { Success, Pending, Failed }
+
 	public class Carryable : ConditionalTrait<CarryableInfo>
 	{
 		ConditionManager conditionManager;
@@ -42,6 +44,8 @@ namespace OpenRA.Mods.Common.Traits
 		int carriedToken = ConditionManager.InvalidConditionToken;
 		int lockedToken = ConditionManager.InvalidConditionToken;
 
+		Mobile mobile;
+
 		public Actor Carrier { get; private set; }
 		public bool Reserved { get { return state != State.Free; } }
 		public CPos? Destination { get; protected set; }
@@ -57,6 +61,7 @@ namespace OpenRA.Mods.Common.Traits
 		protected override void Created(Actor self)
 		{
 			conditionManager = self.Trait<ConditionManager>();
+			mobile = self.TraitOrDefault<Mobile>();
 
 			base.Created(self);
 		}
@@ -111,18 +116,25 @@ namespace OpenRA.Mods.Common.Traits
 		}
 
 		// Prepare for transport pickup
-		public virtual bool LockForPickup(Actor self, Actor carrier)
+		public virtual LockResponse LockForPickup(Actor self, Actor carrier)
 		{
-			if (state == State.Locked)
-				return false;
+			if (state == State.Locked && Carrier != carrier)
+				return LockResponse.Failed;
 
+			if (state != State.Locked)
+			{
 				state = State.Locked;
 				Carrier = carrier;
 
 				if (lockedToken == ConditionManager.InvalidConditionToken && !string.IsNullOrEmpty(Info.LockedCondition))
 					lockedToken = conditionManager.GrantCondition(self, Info.LockedCondition);
+			}
 
-			return true;
+			// Make sure we are not moving and at our normal position with respect to the cell grid
+			if (mobile != null && mobile.IsMovingBetweenCells)
+				return LockResponse.Pending;
+
+			return LockResponse.Success;
 		}
 	}
 }

OpenRA.Mods.Common/Traits/Mobile.cs

@@ -184,6 +184,11 @@ namespace OpenRA.Mods.Common.Traits
 		public bool TurnToMove;
 		public bool IsBlocking { get; private set; }
 
+		public bool IsMovingBetweenCells
+		{
+			get { return FromCell != ToCell; }
+		}
+
 		#region IFacing
 		[Sync]
 		public int Facing

OpenRA.Mods.Common/Util.cs

@@ -141,6 +141,12 @@ namespace OpenRA.Mods.Common
 			}
 		}
 
+		public static bool AreAdjacentCells(CPos a, CPos b)
+		{
+			var offset = b - a;
+			return Math.Abs(offset.X) < 2 && Math.Abs(offset.Y) < 2;
+		}
+
 		public static IEnumerable<CPos> ExpandFootprint(IEnumerable<CPos> cells, bool allowDiagonal)
 		{
 			return cells.SelectMany(c => Neighbours(c, allowDiagonal)).Distinct();