Merge pull request #11956 from obrakmann/fix11953_fix-aircraft-reservation-madness

Fix aircraft reservation madness

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

@@ -19,6 +19,7 @@ namespace OpenRA.Mods.Common.Activities
 	public class FlyAttack : Activity
 	{
 		readonly Target target;
+		readonly Aircraft aircraft;
 		readonly AttackPlane attackPlane;
 		readonly AmmoPool[] ammoPools;
 
@@ -28,6 +29,7 @@ namespace OpenRA.Mods.Common.Activities
 		public FlyAttack(Actor self, Target target)
 		{
 			this.target = target;
+			aircraft = self.Trait<Aircraft>();
 			attackPlane = self.TraitOrDefault<AttackPlane>();
 			ammoPools = self.TraitsImplementing<AmmoPool>().ToArray();
 			ticksUntilTurn = attackPlane.AttackPlaneInfo.AttackTurnDelay;
@@ -38,10 +40,9 @@ namespace OpenRA.Mods.Common.Activities
 			if (!target.IsValidFor(self))
 				return NextActivity;
 
-			// Move to the next activity only if all ammo pools are depleted and none reload automatically
 			// TODO: This should check whether there is ammo left that is actually suitable for the target
 			if (ammoPools.All(x => !x.Info.SelfReloads && !x.HasAmmo()))
-				return ActivityUtils.SequenceActivities(new ReturnToBase(self), NextActivity);
+				return ActivityUtils.SequenceActivities(new ReturnToBase(self), this);
 
 			if (attackPlane != null)
 				attackPlane.DoAttack(self, target);
@@ -56,6 +57,10 @@ namespace OpenRA.Mods.Common.Activities
 					inner = ActivityUtils.SequenceActivities(new FlyTimed(ticksUntilTurn, self), new Fly(self, target), new FlyTimed(ticksUntilTurn, self));
 				else
 					inner = ActivityUtils.SequenceActivities(new Fly(self, target), new FlyTimed(ticksUntilTurn, self));
+
+				// HACK: This needs to be done in this round-about way because TakeOff doesn't behave as expected when it doesn't have a NextActivity.
+				if (self.World.Map.DistanceAboveTerrain(self.CenterPosition).Length < aircraft.Info.MinAirborneAltitude)
+					inner = ActivityUtils.SequenceActivities(new TakeOff(self), inner);
 			}
 
 			inner = ActivityUtils.RunActivity(self, inner);

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

@@ -65,9 +65,9 @@ namespace OpenRA.Mods.Common.Activities
 				return ActivityUtils.SequenceActivities(new HeliFly(self, newTarget));
 			}
 
-			// If any AmmoPool is depleted and no weapon is valid against target, return to helipad to reload and then move to next activity
+			// If any AmmoPool is depleted and no weapon is valid against target, return to helipad to reload and then resume the activity
 			if (ammoPools.Any(x => !x.Info.SelfReloads && !x.HasAmmo()) && !attackHeli.HasAnyValidWeapons(target))
-				return ActivityUtils.SequenceActivities(new HeliReturnToBase(self), NextActivity);
+				return ActivityUtils.SequenceActivities(new HeliReturnToBase(self), this);
 
 			var dist = target.CenterPosition - self.CenterPosition;
 

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

@@ -19,6 +19,7 @@ namespace OpenRA.Mods.Common.Activities
 	public class ResupplyAircraft : Activity
 	{
 		readonly Aircraft aircraft;
+		Activity inner;
 
 		public ResupplyAircraft(Actor self)
 		{
@@ -27,22 +28,45 @@ namespace OpenRA.Mods.Common.Activities
 
 		public override Activity Tick(Actor self)
 		{
-			var host = aircraft.GetActorBelow();
+			if (IsCanceled)
-
-			if (host == null)
 				return NextActivity;
 
-			if (aircraft.IsPlane)
+			if (inner == null)
-				return ActivityUtils.SequenceActivities(
+			{
-					aircraft.GetResupplyActivities(host)
+				var host = aircraft.GetActorBelow();
-					.Append(new CallFunc(() => aircraft.UnReserve()))
-					.Append(new WaitFor(() => NextActivity != null || Reservable.IsReserved(host)))
-					.Append(new TakeOff(self))
-					.Append(NextActivity).ToArray());
 
-			// If is helicopter move away as soon as the resupply ends
+				if (host == null)
-			return ActivityUtils.SequenceActivities(
+					return NextActivity;
-				aircraft.GetResupplyActivities(host).Append(new TakeOff(self)).Append(NextActivity).ToArray());
+
+				if (aircraft.IsPlane)
+				{
+					inner = ActivityUtils.SequenceActivities(
+						aircraft.GetResupplyActivities(host)
+						.Append(new CallFunc(() => aircraft.MayYieldReservation = true))
+						.Append(new WaitFor(() => NextActivity != null || aircraft.ReservedActor == null))
+						.ToArray());
+				}
+				else
+				{
+					// Helicopters should take off from their helipad immediately after resupplying.
+					// HACK: NextActivity needs to be appended here because otherwise TakeOff does stupid things.
+					inner = ActivityUtils.SequenceActivities(
+						aircraft.GetResupplyActivities(host).Append(new TakeOff(self)).Append(NextActivity).ToArray());
+				}
+			}
+			else
+				inner = ActivityUtils.RunActivity(self, inner);
+
+			// The inner == NextActivity check is needed here because of the TakeOff issue mentioned in the comment above.
+			return inner == null || inner == NextActivity ? NextActivity : this;
+		}
+
+		public override void Cancel(Actor self)
+		{
+			if (!IsCanceled && inner != null)
+				inner.Cancel(self);
+
+			base.Cancel(self);
 		}
 	}
 }

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

@@ -104,13 +104,14 @@ namespace OpenRA.Mods.Common.Traits
 
 		UpgradeManager um;
 		IDisposable reservation;
-		Actor reservedActor;
 		IEnumerable<int> speedModifiers;
 
 		[Sync] public int Facing { get; set; }
 		[Sync] public WPos CenterPosition { get; private set; }
 		public CPos TopLeft { get { return self.World.Map.CellContaining(CenterPosition); } }
 		public int TurnSpeed { get { return Info.TurnSpeed; } }
+		public Actor ReservedActor { get; private set; }
+		public bool MayYieldReservation;
 
 		bool airborne;
 		bool cruising;
@@ -190,7 +191,7 @@ namespace OpenRA.Mods.Common.Traits
 
 			if (reservation != null)
 			{
-				var distanceFromReservationActor = (reservedActor.CenterPosition - self.CenterPosition).HorizontalLength;
+				var distanceFromReservationActor = (ReservedActor.CenterPosition - self.CenterPosition).HorizontalLength;
 				if (distanceFromReservationActor < Info.WaitDistanceFromResupplyBase.Length)
 					return WVec.Zero;
 			}
@@ -277,7 +278,7 @@ namespace OpenRA.Mods.Common.Traits
 			if (reservable != null)
 			{
 				reservation = reservable.Reserve(target, self, this);
-				reservedActor = target;
+				ReservedActor = target;
 			}
 		}
 
@@ -288,7 +289,8 @@ namespace OpenRA.Mods.Common.Traits
 
 			reservation.Dispose();
 			reservation = null;
-			reservedActor = null;
+			ReservedActor = null;
+			MayYieldReservation = false;
 
 			if (self.World.Map.DistanceAboveTerrain(CenterPosition).Length <= Info.LandAltitude.Length)
 				self.QueueActivity(new TakeOff(self));
@@ -614,8 +616,6 @@ namespace OpenRA.Mods.Common.Traits
 
 				self.QueueActivity(new ResupplyAircraft(self));
 			}
-			else
-				UnReserve();
 		}
 
 		#endregion

OpenRA.Mods.Common/Traits/Buildings/Reservable.cs

@@ -29,11 +29,19 @@ namespace OpenRA.Mods.Common.Traits
 				return;		/* nothing to do */
 
 			if (!Target.FromActor(reservedFor).IsValidFor(self))
-				reservedFor = null;		/* not likely to arrive now. */
+			{
+				/* Not likely to arrive now. */
+				reservedForAircraft.UnReserve();
+				reservedFor = null;
+				reservedForAircraft = null;
+			}
 		}
 
 		public IDisposable Reserve(Actor self, Actor forActor, Aircraft forAircraft)
 		{
+			if (reservedForAircraft != null && reservedForAircraft.MayYieldReservation)
+				reservedForAircraft.UnReserve();
+
 			reservedFor = forActor;
 			reservedForAircraft = forAircraft;
 
@@ -53,7 +61,7 @@ namespace OpenRA.Mods.Common.Traits
 		public static bool IsReserved(Actor a)
 		{
 			var res = a.TraitOrDefault<Reservable>();
-			return res != null && res.reservedFor != null;
+			return res != null && res.reservedForAircraft != null && !res.reservedForAircraft.MayYieldReservation;
 		}
 
 		public void Disposing(Actor self)