Rework aircraft rally point handling.

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

@@ -9,7 +9,6 @@
  */
 #endregion
 
-using System;
 using System.Collections.Generic;
 using OpenRA.Activities;
 using OpenRA.Mods.Common.Traits;
@@ -22,40 +21,24 @@ namespace OpenRA.Mods.Common.Activities
 	{
 		readonly Aircraft aircraft;
 		readonly IMove move;
-		Target target;
+		Target fallbackTarget;
-		bool moveToRallyPoint;
+		bool movedToTarget = false;
-		bool assignTargetOnFirstRun;
 
-		public TakeOff(Actor self, Target target)
+		public TakeOff(Actor self, Target fallbackTarget)
 		{
 			aircraft = self.Trait<Aircraft>();
 			move = self.Trait<IMove>();
-			this.target = target;
+			this.fallbackTarget = fallbackTarget;
 		}
 
 		public TakeOff(Actor self)
-			: this(self, Target.Invalid)
+			: this(self, Target.Invalid) { }
-		{
-			assignTargetOnFirstRun = true;
-		}
 
 		protected override void OnFirstRun(Actor self)
 		{
 			if (aircraft.ForceLanding)
 				return;
 
-			if (assignTargetOnFirstRun)
-			{
-				var host = aircraft.GetActorBelow();
-				var rp = host != null ? host.TraitOrDefault<RallyPoint>() : null;
-
-				var rallyPointDestination = rp != null ? rp.Location :
-					(host != null ? self.World.Map.CellContaining(host.CenterPosition) : self.Location);
-
-				target = Target.FromCell(self.World, rallyPointDestination);
-				moveToRallyPoint = self.CurrentActivity.NextActivity == null && rallyPointDestination != self.Location;
-			}
-
 			// We are taking off, so remove reservation and influence in ground cells.
 			aircraft.UnReserve();
 			aircraft.RemoveInfluence();
@@ -82,21 +65,16 @@ namespace OpenRA.Mods.Common.Activities
 					Fly.VerticalTakeOffOrLandTick(self, aircraft, aircraft.Facing, aircraft.Info.CruiseAltitude);
 					return false;
 				}
-				else
+
-				{
 				Fly.FlyTick(self, aircraft, aircraft.Facing, aircraft.Info.CruiseAltitude);
 				return false;
 			}
-			}
 
-			// Checking for NextActivity == null again in case another activity was queued while taking off
+			// Only move to the fallback target if we don't have anything better to do
-			if (moveToRallyPoint && NextActivity == null)
+			if (NextActivity == null && fallbackTarget.IsValidFor(self) && !movedToTarget)
 			{
-				if (!aircraft.Info.VTOL && assignTargetOnFirstRun)
+				QueueChild(new AttackMoveActivity(self, () => move.MoveToTarget(self, fallbackTarget, targetLineColor: Color.OrangeRed)));
-					return true;
+				movedToTarget = true;
-
-				QueueChild(new AttackMoveActivity(self, () => move.MoveToTarget(self, target, targetLineColor: Color.OrangeRed)));
-				moveToRallyPoint = false;
 				return false;
 			}
 
@@ -105,11 +83,11 @@ namespace OpenRA.Mods.Common.Activities
 
 		public override IEnumerable<TargetLineNode> TargetLineNodes(Actor self)
 		{
-			if (ChildActivity == null && moveToRallyPoint)
+			if (ChildActivity != null)
-				yield return new TargetLineNode(target, Color.OrangeRed);
-			else
 				foreach (var n in ChildActivity.TargetLineNodes(self))
 					yield return n;
+			else
+				yield return new TargetLineNode(fallbackTarget, Color.OrangeRed);
 		}
 	}
 }

OpenRA.Mods.Common/Activities/Resupply.cs

@@ -169,13 +169,22 @@ namespace OpenRA.Mods.Common.Activities
 
 		void OnResupplyEnding(Actor self, bool isHostInvalid = false)
 		{
+			var rp = host.Actor.TraitOrDefault<RallyPoint>();
 			if (aircraft != null)
 			{
-				aircraft.AllowYieldingReservation();
+				if (wasRepaired || isHostInvalid || (!stayOnResupplier && aircraft.Info.TakeOffOnResupply))
-				if (wasRepaired || isHostInvalid ||
+				{
-					(!stayOnResupplier && aircraft.Info.TakeOffOnResupply))
+					if (rp != null)
+						QueueChild(move.MoveTo(rp.Location, repairableNear != null ? null : host.Actor));
+					else
 						QueueChild(new TakeOff(self));
 				}
+
+				// Aircraft without TakeOffOnResupply remain on the resupplier until something else needs it
+				// The rally point location is queried by the aircraft before it takes off
+				else
+					aircraft.AllowYieldingReservation();
+			}
 			else if (!stayOnResupplier && !isHostInvalid)
 			{
 				// If there's no next activity, move to rallypoint if available, else just leave host if Repairable.
@@ -183,7 +192,6 @@ namespace OpenRA.Mods.Common.Activities
 				// If there's a next activity and we're not RepairableNear, first leave host if the next activity is not a Move.
 				if (self.CurrentActivity.NextActivity == null)
 				{
-					var rp = host.Actor.TraitOrDefault<RallyPoint>();
 					if (rp != null)
 						QueueChild(move.MoveTo(rp.Location, repairableNear != null ? null : host.Actor));
 					else if (repairableNear == null)

OpenRA.Mods.Common/Traits/Air/Aircraft.cs

@@ -357,7 +357,7 @@ namespace OpenRA.Mods.Common.Traits
 				MakeReservation(host);
 
 				if (Info.TakeOffOnCreation)
-					self.QueueActivity(new TakeOff(self));
+					UnReserve(true);
 			}
 
 			// Add land activity if LandOnCondition resolves to true and the actor can land at the current location.
@@ -521,14 +521,22 @@ namespace OpenRA.Mods.Common.Traits
 			if (reservation == null)
 				return;
 
+			// Move to the host's rally point if it has one
+			var rp = ReservedActor != null ? ReservedActor.TraitOrDefault<RallyPoint>() : null;
+
 			reservation.Dispose();
 			reservation = null;
 			ReservedActor = null;
 			MayYieldReservation = false;
 
 			if (takeOff && self.World.Map.DistanceAboveTerrain(CenterPosition).Length <= LandAltitude.Length)
+			{
+				if (rp != null)
+					self.QueueActivity(new TakeOff(self, Target.FromCell(self.World, rp.Location)));
+				else
 					self.QueueActivity(new TakeOff(self));
 			}
+		}
 
 		bool AircraftCanEnter(Actor a, TargetModifiers modifiers)
 		{