Port EnterTransport to the new Enter activity.

This dramatically simplifies the reservation logic,
which seemed to be needlessly complicated. This may
regress unexpected edge-cases.

OpenRA.Mods.Common/Activities/EnterTransport.cs

@@ -11,67 +11,134 @@
 
 using System;
 using System.Drawing;
+using System.Linq;
+using OpenRA.Activities;
 using OpenRA.Mods.Common.Traits;
 using OpenRA.Traits;
 
 namespace OpenRA.Mods.Common.Activities
 {
-	class EnterTransport : LegacyEnter
+	class EnterTransport : Enter
 	{
 		readonly Passenger passenger;
-		readonly int maxTries;
-		Actor transport;
-		Cargo cargo;
 
-		public EnterTransport(Actor self, Actor transport, int maxTries = 0, bool repathWhileMoving = true)
+		Actor enterActor;
-			: base(self, transport, EnterBehaviour.Exit, maxTries, repathWhileMoving, Color.Green)
+		Cargo enterCargo;
+
+		public EnterTransport(Actor self, Target target)
+			: base(self, target, Color.Green)
 		{
-			this.transport = transport;
-			this.maxTries = maxTries;
-			cargo = transport.Trait<Cargo>();
 			passenger = self.Trait<Passenger>();
 		}
 
-		protected override void Unreserve(Actor self, bool abort) { passenger.Unreserve(self); }
+		protected override bool TryStartEnter(Actor self, Actor targetActor)
-		protected override bool CanReserve(Actor self) { return cargo.Unloading || cargo.CanLoad(transport, self); }
-		protected override ReserveStatus Reserve(Actor self)
 		{
-			var status = base.Reserve(self);
+			enterActor = targetActor;
-			if (status != ReserveStatus.Ready)
+			enterCargo = targetActor.TraitOrDefault<Cargo>();
-				return status;
+
-			if (passenger.Reserve(self, cargo))
+			// Make sure we can still enter the transport
-				return ReserveStatus.Ready;
+			// (but not before, because this may stop the actor in the middle of nowhere)
-			return ReserveStatus.Pending;
+			if (enterCargo == null || !passenger.Reserve(self, enterCargo))
+			{
+				Cancel(self, true);
+				return false;
 			}
 
-		protected override void OnInside(Actor self)
+			return true;
+		}
+
+		protected override void OnEnterComplete(Actor self, Actor targetActor)
 		{
 			self.World.AddFrameEndTask(w =>
 			{
-				if (self.IsDead || transport.IsDead || !cargo.CanLoad(transport, self))
+				// Make sure the target hasn't changed while entering
+				// OnEnterComplete is only called if targetActor is alive
+				if (targetActor != enterActor)
 					return;
 
-				cargo.Load(transport, self);
+				if (!enterCargo.CanLoad(enterActor, self))
-				w.Remove(self);
+					return;
-			});
 
-			Done(self);
+				enterCargo.Load(enterActor, self);
+				w.Remove(self);
 
 				// Preemptively cancel any activities to avoid an edge-case where successively queued
 				// EnterTransports corrupt the actor state. Activities are cancelled again on unload
 				self.CancelActivity();
+			});
 		}
 
-		protected override bool TryGetAlternateTarget(Actor self, int tries, ref Target target)
+		protected override void OnCancel(Actor self)
 		{
-			if (tries > maxTries)
+			passenger.Unreserve(self);
+		}
+
+		protected override void OnLastRun(Actor self)
+		{
+			passenger.Unreserve(self);
+		}
+	}
+
+	class EnterTransports : Activity
+	{
+		readonly string type;
+		readonly Passenger passenger;
+
+		Activity enterTransport;
+
+		public EnterTransports(Actor self, Target primaryTarget)
+		{
+			passenger = self.Trait<Passenger>();
+			if (primaryTarget.Type == TargetType.Actor)
+				type = primaryTarget.Actor.Info.Name;
+
+			enterTransport = new EnterTransport(self, primaryTarget);
+		}
+
+		public override Activity Tick(Actor self)
+		{
+			if (enterTransport != null)
+			{
+				enterTransport = ActivityUtils.RunActivity(self, enterTransport);
+				if (enterTransport != null)
+					return this;
+			}
+
+			// Try and find a new transport nearby
+			if (IsCanceled || string.IsNullOrEmpty(type))
+				return NextActivity;
+
+			Func<Actor, bool> isValidTransport = a =>
+			{
+				var c = a.TraitOrDefault<Cargo>();
+				return c != null && c.Info.Types.Contains(passenger.Info.CargoType) &&
+				       (c.Unloading || c.CanLoad(a, self));
+			};
+
+			var candidates = self.World.FindActorsInCircle(self.CenterPosition, passenger.Info.AlternateTransportScanRange)
+				.Where(isValidTransport)
+				.ToList();
+
+			// Prefer transports of the same type as the primary
+			var transport = candidates.Where(a => a.Info.Name == type).ClosestTo(self);
+			if (transport == null)
+				transport = candidates.ClosestTo(self);
+
+			if (transport != null)
+			{
+				enterTransport = ActivityUtils.RunActivity(self, new EnterTransport(self, Target.FromActor(transport)));
+				return this;
+			}
+
+			return NextActivity;
+		}
+
+		public override bool Cancel(Actor self, bool keepQueue = false)
+		{
+			if (!IsCanceled && enterTransport != null && !enterTransport.Cancel(self))
 				return false;
-			var type = target.Actor.Info.Name;
+
-			return TryGetAlternateTargetInCircle(
+			return base.Cancel(self, keepQueue);
-				self, passenger.Info.AlternateTransportScanRange,
-				t => { transport = t.Actor; cargo = t.Actor.Trait<Cargo>(); }, // update transport and cargo
-				a => { var c = a.TraitOrDefault<Cargo>(); return c != null && c.Info.Types.Contains(passenger.Info.CargoType) && (c.Unloading || c.CanLoad(a, self)); },
-				new Func<Actor, bool>[] { a => a.Info.Name == type }); // Prefer transports of the same type
 		}
 	}
 }

OpenRA.Mods.Common/Scripting/Properties/MobileProperties.cs

@@ -60,7 +60,7 @@ namespace OpenRA.Mods.Common.Scripting
 		[Desc("Move to and enter the transport.")]
 		public void EnterTransport(Actor transport)
 		{
-			Self.QueueActivity(new EnterTransport(Self, transport, 1, false));
+			Self.QueueActivity(new EnterTransport(Self, Target.FromActor(transport)));
 		}
 
 		[Desc("Whether the actor can move (false if immobilized).")]

OpenRA.Mods.Common/Traits/Passenger.cs

@@ -32,9 +32,6 @@ namespace OpenRA.Mods.Common.Traits
 			"Default - use force move modifier (Alt) to disable.")]
 		public readonly AlternateTransportsMode AlternateTransportsMode = AlternateTransportsMode.Force;
 
-		[Desc("Number of retries using alternate transports.")]
-		public readonly int MaxAlternateTransportAttempts = 1;
-
 		[Desc("Range from self for looking for an alternate transport (default: 5.5 cells).")]
 		public readonly WDist AlternateTransportScanRange = WDist.FromCells(11) / 2;
 
@@ -161,9 +158,11 @@ namespace OpenRA.Mods.Common.Traits
 			if (!order.Queued)
 				self.CancelActivity();
 
-			var transports = order.OrderString == "EnterTransports";
 			self.SetTargetLine(order.Target, Color.Green);
-			self.QueueActivity(new EnterTransport(self, targetActor, transports ? Info.MaxAlternateTransportAttempts : 0, !transports));
+			if (order.OrderString == "EnterTransports")
+				self.QueueActivity(new EnterTransports(self, order.Target));
+			else
+				self.QueueActivity(new EnterTransport(self, order.Target));
 		}
 
 		public bool Reserve(Actor self, Cargo cargo)