Move activity implementation now uses ChildActivity

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

@@ -144,8 +144,47 @@ namespace OpenRA.Mods.Common.Activities
 			}
 		}
 
+		public override bool Cancel(Actor self, bool keepQueue = false)
+		{
+			if (ChildActivity == null)
+				return base.Cancel(self, keepQueue);
+
+			// Although MoveFirstHalf and MoveSecondHalf can't be interrupted,
+			// we prevent them from moving forever by removing the path.
+			if (path != null)
+				path.Clear();
+
+			// Remove queued activities
+			if (!keepQueue && NextInQueue != null)
+				NextInQueue = null;
+
+			// In current implementation, ChildActivity can be Turn, MoveFirstHalf and MoveSecondHalf.
+			// Turn may be interrupted freely while they are turning.
+			// Unlike Turn, MoveFirstHalf and MoveSecondHalf are not Interruptable, but clearing the
+			// path guarantees that they will return as soon as possible, once the actor is back in a
+			// valid position.
+			// This means that it is safe to unconditionally return true, which avoids breaking parent
+			// activities that rely on cancellation succeeding (but not necessarily immediately
+			ChildActivity.Cancel(self, false);
+
+			return true;
+		}
+
 		public override Activity Tick(Actor self)
 		{
+			// ChildActivity is the top priority, unlike other activities.
+			// Even if this activity is canceled, we must let the child be run so that units
+			// will not end up in an odd place.
+			if (ChildActivity != null)
+			{
+				ChildActivity = ActivityUtils.RunActivity(self, ChildActivity);
+
+				// Child activities such as Turn might have finished.
+				// If we "return this" in this situation, the unit loses one tick and pauses movement briefly.
+				if (ChildActivity != null)
+					return this;
+			}
+
 			// If the actor is inside a tunnel then we must let them move
 			// all the way through before moving to the next activity
 			if (IsCanceled && self.Location.Layer != CustomMovementLayerType.Tunnel)
@@ -185,30 +224,33 @@ namespace OpenRA.Mods.Common.Activities
 			if (firstFacing != mobile.Facing)
 			{
 				path.Add(nextCell.Value.First);
-				return ActivityUtils.SequenceActivities(new Turn(self, firstFacing), this);
+				QueueChild(new Turn(self, firstFacing));
+				return this;
 			}
-			else
-			{
-				mobile.SetLocation(mobile.FromCell, mobile.FromSubCell, nextCell.Value.First, nextCell.Value.Second);
 
-				var map = self.World.Map;
+			mobile.SetLocation(mobile.FromCell, mobile.FromSubCell, nextCell.Value.First, nextCell.Value.Second);
-				var from = (mobile.FromCell.Layer == 0 ? map.CenterOfCell(mobile.FromCell) :
-					self.World.GetCustomMovementLayers()[mobile.FromCell.Layer].CenterOfCell(mobile.FromCell)) +
-					map.Grid.OffsetOfSubCell(mobile.FromSubCell);
 
-				var to = Util.BetweenCells(self.World, mobile.FromCell, mobile.ToCell) +
+			var map = self.World.Map;
-					(map.Grid.OffsetOfSubCell(mobile.FromSubCell) + map.Grid.OffsetOfSubCell(mobile.ToSubCell)) / 2;
+			var from = (mobile.FromCell.Layer == 0 ? map.CenterOfCell(mobile.FromCell) :
+				self.World.GetCustomMovementLayers()[mobile.FromCell.Layer].CenterOfCell(mobile.FromCell)) +
+				map.Grid.OffsetOfSubCell(mobile.FromSubCell);
 
-				var move = new MoveFirstHalf(
+			var to = Util.BetweenCells(self.World, mobile.FromCell, mobile.ToCell) +
-					this,
+				(map.Grid.OffsetOfSubCell(mobile.FromSubCell) + map.Grid.OffsetOfSubCell(mobile.ToSubCell)) / 2;
-					from,
-					to,
-					mobile.Facing,
-					mobile.Facing,
-					0);
 
-				return move;
+			QueueChild(new MoveFirstHalf(
-			}
+				this,
+				from,
+				to,
+				mobile.Facing,
+				mobile.Facing,
+				0));
+
+			// While carrying out one Move order, MoveSecondHalf finishes its work from time to time and returns null.
+			// That causes the ChildActivity to be null and makes us return to this part of code.
+			// If we only queue the activity and not run it, units will lose one tick and pause briefly!
+			ChildActivity = ActivityUtils.RunActivity(self, ChildActivity);
+			return this;
 		}
 
 		[Conditional("SANITY_CHECKS")]
@@ -316,6 +358,7 @@ namespace OpenRA.Mods.Common.Activities
 				ToFacing = toFacing;
 				moveFraction = startingFraction;
 				MoveFractionTotal = (to - from).Length;
+				IsInterruptible = false; // See comments in Move.Cancel()
 
 				// Calculate an elliptical arc that joins from and to
 				var delta = Util.NormalizeFacing(fromFacing - toFacing);
@@ -338,17 +381,6 @@ namespace OpenRA.Mods.Common.Activities
 				}
 			}
 
-			public override bool Cancel(Actor self, bool keepQueue = false)
-			{
-				Move.Cancel(self, keepQueue);
-				return base.Cancel(self);
-			}
-
-			public override void Queue(Activity activity)
-			{
-				Move.Queue(activity);
-			}
-
 			public override Activity Tick(Actor self)
 			{
 				var ret = InnerTick(self, Move.mobile);
@@ -356,9 +388,14 @@ namespace OpenRA.Mods.Common.Activities
 
 				if (moveFraction > MoveFractionTotal)
 					moveFraction = MoveFractionTotal;
+
 				UpdateCenterLocation(self, Move.mobile);
 
-				return ret;
+				if (ret == this)
+					return ret;
+
+				Queue(ret);
+				return NextActivity;
 			}
 
 			Activity InnerTick(Actor self, Mobile mobile)
@@ -367,11 +404,7 @@ namespace OpenRA.Mods.Common.Activities
 				if (moveFraction <= MoveFractionTotal)
 					return this;
 
-				var next = OnComplete(self, mobile, Move);
+				return OnComplete(self, mobile, Move);
-				if (next != null)
-					return next;
-
-				return Move;
 			}
 
 			void UpdateCenterLocation(Actor self, Mobile mobile)