Make Tick return bool

OpenRA.Game/Activities/Activity.cs

@@ -21,8 +21,7 @@ public enum ActivityState { Queued, Active, Canceling, Done }
 	/*
 	 * Things to be aware of when writing activities:
 	 *
-	 * - Use "return NextActivity" at least once somewhere in the tick method.
-	 * - Do not use "return new SomeActivity()" as that will break the queue. Queue the new activity and use "return NextActivity" instead.
+	 * - Use "return true" at least once somewhere in the tick method.
 	 * - Do not "reuse" (with "SequenceActivities", for example) activity objects that have already started running.
 	 *   Queue a new instance instead.
 	 * - Avoid calling actor.CancelActivity(). It is almost always a bug. Call activity.Cancel() instead.
@@ -32,11 +31,13 @@ public abstract class Activity
 		public ActivityState State { get; private set; }
 
 		protected Activity ChildActivity { get; private set; }
-		public Activity NextActivity { get; protected set; }
+		public Activity NextActivity { get; private set; }
 
 		public bool IsInterruptible { get; protected set; }
 		public bool ChildHasPriority { get; protected set; }
 		public bool IsCanceling { get { return State == ActivityState.Canceling; } }
+		bool finishing;
+		bool lastRun;
 
 		public Activity()
 		{
@@ -55,38 +56,48 @@ public Activity TickOuter(Actor self)
 				State = ActivityState.Active;
 			}
 
+			// Only run the parent tick when the child is done.
+			// We must always let the child finish on its own before continuing.
 			if (ChildHasPriority)
 			{
-				ChildActivity = ActivityUtils.RunActivity(self, ChildActivity);
-				if (ChildActivity != null)
-					return this;
+				lastRun = TickChild(self) && (finishing || Tick(self));
+				finishing |= lastRun;
 			}
 
-			var ret = Tick(self);
+			// The parent determines whether the child gets a chance at ticking.
+			else
+				lastRun = Tick(self);
 
 			// Avoid a single tick delay if the childactivity was just queued.
-			if (ChildActivity != null && (ChildActivity.State == ActivityState.Queued || !ChildHasPriority))
-			{
-				ChildActivity = ActivityUtils.RunActivity(self, ChildActivity);
-				if (!ChildHasPriority && ChildActivity == null)
-					ret = NextActivity;
-			}
+			if (ChildActivity != null && ChildActivity.State == ActivityState.Queued)
+				lastRun = TickChild(self);
 
-			if (ret != this)
+			if (lastRun)
 			{
 				State = ActivityState.Done;
 				OnLastRun(self);
+				return NextActivity;
 			}
 
-			return ret;
+			return this;
+		}
+
+		protected bool TickChild(Actor self)
+		{
+			ChildActivity = ActivityUtils.RunActivity(self, ChildActivity);
+			if (ChildActivity != null)
+				return false;
+
+			return true;
 		}
 
 		/// <summary>
-		/// Runs every timestep as long as this activity is active.
+		/// Runs every timestep as long as this activity is active. Returns true
+		/// when the activity is finished.
 		/// </summary>
-		public virtual Activity Tick(Actor self)
+		public virtual bool Tick(Actor self)
 		{
-			return NextActivity;
+			return true;
 		}
 
 		/// <summary>

OpenRA.Game/Activities/CallFunc.cs

@@ -24,10 +24,10 @@ public CallFunc(Action a, bool interruptible)
 
 		Action a;
 
-		public override Activity Tick(Actor self)
+		public override bool Tick(Actor self)
 		{
 			if (a != null) a();
-			return NextActivity;
+			return true;
 		}
 	}
 }

OpenRA.Mods.Cnc/Activities/LayMines.cs

@@ -37,10 +37,10 @@ public LayMines(Actor self, CPos[] minefield)
 			this.minefield = minefield;
 		}
 
-		public override Activity Tick(Actor self)
+		public override bool Tick(Actor self)
 		{
 			if (IsCanceling)
-				return NextActivity;
+				return true;
 
 			if (rearmableInfo != null && ammoPools.Any(p => p.Info.Name == info.AmmoPoolName && !p.HasAmmo()))
 			{
@@ -50,20 +50,20 @@ public override Activity Tick(Actor self)
 					.ClosestTo(self);
 
 				if (rearmTarget == null)
-					return NextActivity;
+					return true;
 
 				// Add a CloseEnough range of 512 to the Rearm/Repair activities in order to ensure that we're at the host actor
 				QueueChild(new MoveAdjacentTo(self, Target.FromActor(rearmTarget)));
 				QueueChild(movement.MoveTo(self.World.Map.CellContaining(rearmTarget.CenterPosition), rearmTarget));
 				QueueChild(new Resupply(self, rearmTarget, new WDist(512)));
-				return this;
+				return false;
 			}
 
 			if ((minefield == null || minefield.Contains(self.Location)) && ShouldLayMine(self, self.Location))
 			{
 				LayMine(self);
 				QueueChild(new Wait(20)); // A little wait after placing each mine, for show
-				return this;
+				return false;
 			}
 
 			if (minefield != null && minefield.Length > 0)
@@ -75,13 +75,13 @@ public override Activity Tick(Actor self)
 					if (ShouldLayMine(self, p))
 					{
 						QueueChild(movement.MoveTo(p, 0));
-						return this;
+						return false;
 					}
 				}
 			}
 
 			// TODO: Return somewhere likely to be safe (near rearm building) so we're not sitting out in the minefield.
-			return NextActivity;
+			return true;
 		}
 
 		static bool ShouldLayMine(Actor self, CPos p)

OpenRA.Mods.Cnc/Activities/Leap.cs

@@ -70,11 +70,11 @@ protected override void OnFirstRun(Actor self)
 			attack.GrantLeapCondition(self);
 		}
 
-		public override Activity Tick(Actor self)
+		public override bool Tick(Actor self)
 		{
 			// Correct the visual position after we jumped
 			if (canceled || jumpComplete)
-				return NextActivity;
+				return true;
 
 			if (target.Type != TargetType.Invalid)
 				targetPosition = target.CenterPosition;
@@ -100,7 +100,7 @@ public override Activity Tick(Actor self)
 				QueueChild(mobile.VisualMove(self, position, self.World.Map.CenterOfSubCell(destinationCell, destinationSubCell)));
 			}
 
-			return this;
+			return false;
 		}
 
 		protected override void OnLastRun(Actor self)

OpenRA.Mods.Cnc/Activities/LeapAttack.cs

@@ -57,10 +57,10 @@ protected override void OnFirstRun(Actor self)
 			attack.IsAiming = true;
 		}
 
-		public override Activity Tick(Actor self)
+		public override bool Tick(Actor self)
 		{
 			if (IsCanceling)
-				return NextActivity;
+				return true;
 
 			bool targetIsHiddenActor;
 			target = target.Recalculate(self.Owner, out targetIsHiddenActor);
@@ -80,35 +80,35 @@ public override Activity Tick(Actor self)
 
 			// Target is hidden or dead, and we don't have a fallback position to move towards
 			if (useLastVisibleTarget && !lastVisibleTarget.IsValidFor(self))
-				return NextActivity;
+				return true;
 
 			var pos = self.CenterPosition;
 			var checkTarget = useLastVisibleTarget ? lastVisibleTarget : target;
 
 			if (!checkTarget.IsInRange(pos, lastVisibleMaxRange) || checkTarget.IsInRange(pos, lastVisibleMinRange))
 			{
 				if (!allowMovement || lastVisibleMaxRange == WDist.Zero || lastVisibleMaxRange < lastVisibleMinRange)
-					return NextActivity;
+					return true;
 
 				QueueChild(mobile.MoveWithinRange(target, lastVisibleMinRange, lastVisibleMaxRange, checkTarget.CenterPosition, Color.Red));
-				return this;
+				return false;
 			}
 
 			// Ready to leap, but target isn't visible
 			if (targetIsHiddenActor || target.Type != TargetType.Actor)
-				return NextActivity;
+				return true;
 
 			// Target is not valid
 			if (!target.IsValidFor(self) || !attack.HasAnyValidWeapons(target))
-				return NextActivity;
+				return true;
 
 			var edible = target.Actor.TraitOrDefault<EdibleByLeap>();
 			if (edible == null || !edible.CanLeap(self))
-				return NextActivity;
+				return true;
 
 			// Can't leap yet
 			if (attack.Armaments.All(a => a.IsReloading))
-				return this;
+				return false;
 
 			// Use CenterOfSubCell with ToSubCell instead of target.Centerposition
 			// to avoid continuous facing adjustments as the target moves
@@ -121,13 +121,13 @@ public override Activity Tick(Actor self)
 			if (mobile.Facing != desiredFacing)
 			{
 				QueueChild(new Turn(self, desiredFacing));
-				return this;
+				return false;
 			}
 
 			QueueChild(new Leap(self, target, mobile, targetMobile, info.Speed.Length, attack, edible));
 
 			// Re-queue the child activities to kill the target if it didn't die in one go
-			return this;
+			return false;
 		}
 
 		protected override void OnLastRun(Actor self)

OpenRA.Mods.Cnc/Activities/Teleport.cs

@@ -52,15 +52,15 @@ public Teleport(Actor teleporter, CPos destination, int? maximumDistance,
 				IsInterruptible = false;
 		}
 
-		public override Activity Tick(Actor self)
+		public override bool Tick(Actor self)
 		{
 			var pc = self.TraitOrDefault<PortableChrono>();
 			if (teleporter == self && pc != null && (!pc.CanTeleport || IsCanceling))
 			{
 				if (killOnFailure)
 					self.Kill(teleporter, killDamageTypes);
 
-				return NextActivity;
+				return true;
 			}
 
 			var bestCell = ChooseBestDestinationCell(self, destination);
@@ -69,7 +69,7 @@ public override Activity Tick(Actor self)
 				if (killOnFailure)
 					self.Kill(teleporter, killDamageTypes);
 
-				return NextActivity;
+				return true;
 			}
 
 			destination = bestCell.Value;
@@ -112,7 +112,7 @@ public override Activity Tick(Actor self)
 					building.PlayCustomAnimation(teleporter, "active");
 			}
 
-			return NextActivity;
+			return true;
 		}
 
 		CPos? ChooseBestDestinationCell(Actor self, CPos destination)

OpenRA.Mods.Cnc/Activities/VoxelHarvesterDockSequence.cs

@@ -27,7 +27,7 @@ public VoxelHarvesterDockSequence(Actor self, Actor refinery, int dockAngle, boo
 			spriteOverlay = refinery.TraitOrDefault<WithDockingOverlay>();
 		}
 
-		public override Activity OnStateDock(Actor self)
+		public override void OnStateDock(Actor self)
 		{
 			body.Docked = true;
 
@@ -41,11 +41,9 @@ public override Activity OnStateDock(Actor self)
 			}
 			else
 				dockingState = DockingState.Loop;
-
-			return this;
 		}
 
-		public override Activity OnStateUndock(Actor self)
+		public override void OnStateUndock(Actor self)
 		{
 			dockingState = DockingState.Wait;
 
@@ -63,8 +61,6 @@ public override Activity OnStateUndock(Actor self)
 				dockingState = DockingState.Complete;
 				body.Docked = false;
 			}
-
-			return this;
 		}
 	}
 }

OpenRA.Mods.Cnc/Traits/Attack/AttackTDGunboatTurreted.cs

@@ -46,27 +46,27 @@ public AttackTDGunboatTurretedActivity(Actor self, Target target, bool allowMove
 				this.forceAttack = forceAttack;
 			}
 
-			public override Activity Tick(Actor self)
+			public override bool Tick(Actor self)
 			{
 				if (IsCanceling || !target.IsValidFor(self))
-					return NextActivity;
+					return true;
 
 				if (attack.IsTraitDisabled)
-					return this;
+					return false;
 
 				var weapon = attack.ChooseArmamentsForTarget(target, forceAttack).FirstOrDefault();
 				if (weapon != null)
 				{
 					// Check that AttackTDGunboatTurreted hasn't cancelled the target by modifying attack.Target
 					// Having both this and AttackTDGunboatTurreted modify that field is a horrible hack.
 					if (hasTicked && attack.RequestedTarget.Type == TargetType.Invalid)
-						return NextActivity;
+						return true;
 
 					attack.RequestedTarget = target;
 					hasTicked = true;
 				}
 
-				return NextActivity;
+				return false;
 			}
 		}
 	}

OpenRA.Mods.Cnc/Traits/Attack/AttackTesla.cs

@@ -89,13 +89,13 @@ public ChargeAttack(AttackTesla attack, Target target)
 				this.target = target;
 			}
 
-			public override Activity Tick(Actor self)
+			public override bool Tick(Actor self)
 			{
 				if (IsCanceling || !attack.CanAttack(self, target))
-					return NextActivity;
+					return true;
 
 				if (attack.charges == 0)
-					return this;
+					return false;
 
 				foreach (var notify in self.TraitsImplementing<INotifyTeslaCharging>())
 					notify.Charging(self, target);
@@ -105,7 +105,7 @@ public override Activity Tick(Actor self)
 
 				QueueChild(new Wait(attack.info.InitialChargeDelay));
 				QueueChild(new ChargeFire(attack, target));
-				return this;
+				return false;
 			}
 		}
 
@@ -120,18 +120,18 @@ public ChargeFire(AttackTesla attack, Target target)
 				this.target = target;
 			}
 
-			public override Activity Tick(Actor self)
+			public override bool Tick(Actor self)
 			{
 				if (IsCanceling || !attack.CanAttack(self, target))
-					return NextActivity;
+					return true;
 
 				if (attack.charges == 0)
-					return NextActivity;
+					return true;
 
 				attack.DoAttack(self, target);
 
 				QueueChild(new Wait(attack.info.ChargeDelay));
-				return this;
+				return false;
 			}
 		}
 	}