Overhaul Carryall behaviour, adding support for manual control.

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

@@ -18,7 +18,7 @@ namespace OpenRA.Mods.Common.Activities
 	{
 		int remainingTicks;
 
-		public FlyCircleTimed(int ticks, Actor self) : base(self)
+		public FlyCircleTimed(Actor self, int ticks) : base(self)
 		{
 			remainingTicks = ticks;
 		}

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

@@ -17,12 +17,18 @@ namespace OpenRA.Mods.Common.Activities
 	public class HeliLand : Activity
 	{
 		readonly Aircraft helicopter;
-		bool requireSpace;
+		readonly WDist landAltitude;
+		readonly bool requireSpace;
+
 		bool playedSound;
 
 		public HeliLand(Actor self, bool requireSpace)
+			: this(self, requireSpace, self.Info.TraitInfo<AircraftInfo>().LandAltitude) { }
+
+		public HeliLand(Actor self, bool requireSpace, WDist landAltitude)
 		{
 			this.requireSpace = requireSpace;
+			this.landAltitude = landAltitude;
 			helicopter = self.Trait<Aircraft>();
 		}
 
@@ -40,7 +46,7 @@ namespace OpenRA.Mods.Common.Activities
 				playedSound = true;
 			}
 
-			if (HeliFly.AdjustAltitude(self, helicopter, helicopter.Info.LandAltitude))
+			if (HeliFly.AdjustAltitude(self, helicopter, landAltitude))
 				return this;
 
 			return NextActivity;

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

@@ -109,7 +109,7 @@ namespace OpenRA.Mods.Common.Activities
 				if (nearestAfld != null)
 					return ActivityUtils.SequenceActivities(
 						new Fly(self, Target.FromActor(nearestAfld), WDist.Zero, plane.Info.WaitDistanceFromResupplyBase),
-						new FlyCircleTimed(plane.Info.NumberOfTicksToVerifyAvailableAirport, self),
+						new FlyCircleTimed(self, plane.Info.NumberOfTicksToVerifyAvailableAirport),
 						this);
 				else
 					return NextActivity;

OpenRA.Mods.Common/Activities/DeliverResources.cs

@@ -65,9 +65,8 @@ namespace OpenRA.Mods.Common.Activities
 			if (self.Location != proc.Location + iao.DeliveryOffset)
 			{
 				var notify = self.TraitsImplementing<INotifyHarvesterAction>();
-				var next = new DeliverResources(self);
 				foreach (var n in notify)
-					n.MovingToRefinery(self, proc.Location + iao.DeliveryOffset, next);
+					n.MovingToRefinery(self, proc.Location + iao.DeliveryOffset, this);
 
 				return ActivityUtils.SequenceActivities(movement.MoveTo(proc.Location + iao.DeliveryOffset, 0), this);
 			}

OpenRA.Mods.Common/Activities/DeliverUnit.cs

@@ -9,6 +9,7 @@
  */
 #endregion
 
+using System.Linq;
 using OpenRA.Activities;
 using OpenRA.Mods.Common.Traits;
 using OpenRA.Traits;
@@ -18,86 +19,145 @@ namespace OpenRA.Mods.Common.Activities
 	public class DeliverUnit : Activity
 	{
 		readonly Actor self;
-		readonly Actor cargo;
-		readonly IMove movement;
 		readonly Carryable carryable;
 		readonly Carryall carryall;
-		readonly Aircraft aircraft;
 		readonly IPositionable positionable;
-		readonly IFacing cargoFacing;
+		readonly BodyOrientation body;
-		readonly IFacing selfFacing;
+		readonly IFacing carryableFacing;
+		readonly IFacing carryallFacing;
+		readonly CPos destination;
 
-		enum State { Transport, Land, Release }
+		enum State { Transport, Land, Wait, Release, TakeOff, Aborted }
 
 		State state;
+		Activity innerActivity;
 
-		public DeliverUnit(Actor self)
+		public DeliverUnit(Actor self, CPos destination)
 		{
-			carryall = self.Trait<Carryall>();
 			this.self = self;
-			cargo = carryall.Carrying;
+			this.destination = destination;
-			movement = self.Trait<IMove>();
+
-			carryable = cargo.Trait<Carryable>();
+			carryallFacing = self.Trait<IFacing>();
-			aircraft = self.Trait<Aircraft>();
+			carryall = self.Trait<Carryall>();
-			positionable = cargo.Trait<IPositionable>();
+			body = self.Trait<BodyOrientation>();
-			cargoFacing = cargo.Trait<IFacing>();
+
-			selfFacing = self.Trait<IFacing>();
+			carryable = carryall.Carryable.Trait<Carryable>();
+			positionable = carryall.Carryable.Trait<IPositionable>();
+			carryableFacing = carryall.Carryable.Trait<IFacing>();
 			state = State.Transport;
 		}
 
-		// Find a suitable location to drop our carryable
+		CPos? FindDropLocation(CPos targetCell, WDist maxSearchDistance)
-		CPos GetLocationToDrop(CPos requestedPosition)
 		{
-			if (positionable.CanEnterCell(requestedPosition))
+			// The easy case
-				return requestedPosition;
+			if (positionable.CanEnterCell(targetCell))
+				return targetCell;
 
-			var candidateCells = Util.AdjacentCells(self.World, Target.FromCell(self.World, requestedPosition));
+			var cellRange = (maxSearchDistance.Length + 1023) / 1024;
-
+			var centerPosition = self.World.Map.CenterOfCell(targetCell);
-			// TODO: This will behave badly if there is no suitable drop point nearby
+			foreach (var c in self.World.Map.FindTilesInCircle(targetCell, cellRange))
-			do
 			{
-				foreach (var c in candidateCells)
+				if (!positionable.CanEnterCell(c))
-					if (positionable.CanEnterCell(c))
+					continue;
-						return c;
 
-				// Expanding dropable cells search area
+				var delta = self.World.Map.CenterOfCell(c) - centerPosition;
-				// TODO: This also includes all of the cells we have just checked
+				if (delta.LengthSquared < maxSearchDistance.LengthSquared)
-				candidateCells = Util.ExpandFootprint(candidateCells, true);
+					return c;
-			} while (true);
+			}
+
+			return null;
 		}
 
 		// Check if we can drop the unit at our current location.
 		bool CanDropHere()
 		{
-			return positionable.CanEnterCell(self.Location);
+			var localOffset = carryall.CarryableOffset.Rotate(body.QuantizeOrientation(self, self.Orientation));
+			var targetCell = self.World.Map.CellContaining(self.CenterPosition + body.LocalToWorld(localOffset));
+			return positionable.CanEnterCell(targetCell);
 		}
 
 		public override Activity Tick(Actor self)
 		{
-			if (cargo.IsDead || !carryall.IsBusy)
+			if (innerActivity != null)
 			{
-				carryall.UnreserveCarryable();
+				innerActivity = ActivityUtils.RunActivity(self, innerActivity);
-				return NextActivity;
+				return this;
 			}
 
+			if (IsCanceled)
+				return NextActivity;
+
+			if ((carryall.State == Carryall.CarryallState.Idle || carryall.Carryable.IsDead) && state != State.TakeOff)
+				state = State.Aborted;
+
 			switch (state)
 			{
 				case State.Transport:
-					var targetl = GetLocationToDrop(carryable.Destination);
+				{
+					var targetLocation = FindDropLocation(destination, carryall.Info.DropRange);
+
+					// Can't land, so wait at the target until something changes
+					if (!targetLocation.HasValue)
+					{
+						innerActivity = ActivityUtils.SequenceActivities(
+							new HeliFly(self, Target.FromCell(self.World, destination)),
+							new Wait(25));
+
+						return this;
+					}
+
+					var targetPosition = self.World.Map.CenterOfCell(targetLocation.Value);
+
+					var localOffset = carryall.CarryableOffset.Rotate(body.QuantizeOrientation(self, self.Orientation));
+					var carryablePosition = self.CenterPosition + body.LocalToWorld(localOffset);
+					if ((carryablePosition - targetPosition).HorizontalLengthSquared != 0)
+					{
+						// For non-zero offsets the drop position depends on the carryall facing
+						// We therefore need to predict/correct for the facing *at the drop point*
+						if (carryall.CarryableOffset.HorizontalLengthSquared != 0)
+						{
+							var facing = (targetPosition - self.CenterPosition).Yaw.Facing;
+							localOffset = carryall.CarryableOffset.Rotate(body.QuantizeOrientation(self, WRot.FromFacing(facing)));
+							innerActivity = ActivityUtils.SequenceActivities(
+								new HeliFly(self, Target.FromPos(targetPosition - body.LocalToWorld(localOffset))),
+								new Turn(self, facing));
+
+							return this;
+						}
+
+						innerActivity = new HeliFly(self, Target.FromPos(targetPosition));
+						return this;
+					}
+
 					state = State.Land;
-					return ActivityUtils.SequenceActivities(movement.MoveTo(targetl, 0), this);
+					return this;
+				}
 
 				case State.Land:
+				{
 					if (!CanDropHere())
 					{
 						state = State.Transport;
 						return this;
 					}
 
-					if (HeliFly.AdjustAltitude(self, aircraft, aircraft.Info.LandAltitude))
+					// Make sure that the carried actor is on the ground before releasing it
+					var localOffset = carryall.CarryableOffset.Rotate(body.QuantizeOrientation(self, self.Orientation));
+					var carryablePosition = self.CenterPosition + body.LocalToWorld(localOffset);
+					if (self.World.Map.DistanceAboveTerrain(carryablePosition) != WDist.Zero)
+					{
+						innerActivity = new HeliLand(self, false, -new WDist(carryall.CarryableOffset.Z));
 						return this;
+					}
+
+					state = carryall.Info.UnloadingDelay > 0 ? State.Wait : State.Release;
+					return this;
+				}
+
+				case State.Wait:
 					state = State.Release;
-					return ActivityUtils.SequenceActivities(new Wait(15), this);
+					innerActivity = new Wait(carryall.Info.UnloadingDelay, false);
+					return this;
 
 				case State.Release:
 					if (!CanDropHere())
@@ -107,7 +167,15 @@ namespace OpenRA.Mods.Common.Activities
 					}
 
 					Release();
-					return NextActivity;
+					state = State.TakeOff;
+					return this;
+
+				case State.TakeOff:
+					return ActivityUtils.SequenceActivities(new HeliFly(self, Target.FromPos(self.CenterPosition)), NextActivity);
+
+				case State.Aborted:
+					carryall.UnreserveCarryable(self);
+					break;
 			}
 
 			return NextActivity;
@@ -115,24 +183,29 @@ namespace OpenRA.Mods.Common.Activities
 
 		void Release()
 		{
-			positionable.SetPosition(cargo, self.Location, SubCell.FullCell);
+			var localOffset = carryall.CarryableOffset.Rotate(body.QuantizeOrientation(self, self.Orientation));
-			cargoFacing.Facing = selfFacing.Facing;
+			var targetPosition = self.CenterPosition + body.LocalToWorld(localOffset);
+			var targetLocation = self.World.Map.CellContaining(targetPosition);
+			positionable.SetPosition(carryall.Carryable, targetLocation, SubCell.FullCell);
+			carryableFacing.Facing = carryallFacing.Facing;
 
 			// Put back into world
 			self.World.AddFrameEndTask(w =>
 			{
-				cargo.World.Add(cargo);
+				var cargo = carryall.Carryable;
-				carryall.UnreserveCarryable();
+				w.Add(cargo);
+				carryall.DetachCarryable(self);
+				carryable.UnReserve(cargo);
+				carryable.Detached(cargo);
 			});
-
-			// Unlock carryable
-			carryall.CarryableReleased();
-			carryable.Dropped();
 		}
 
 		public override void Cancel(Actor self)
 		{
-			// TODO: Drop the unit at the nearest available cell
+			if (innerActivity != null)
+				innerActivity.Cancel(self);
+
+			base.Cancel(self);
 		}
 	}
 }

OpenRA.Mods.Common/Activities/FindResources.cs

@@ -73,19 +73,19 @@ namespace OpenRA.Mods.Common.Activities
 				self.QueueActivity(mobile.MoveTo(moveTo, 1));
 				self.SetTargetLine(Target.FromCell(self.World, moveTo), Color.Gray, false);
 
+				// TODO: The harvest-deliver-return sequence is a horrible mess of duplicated code and edge-cases
+				var notify = self.TraitsImplementing<INotifyHarvesterAction>();
+				foreach (var n in notify)
+					n.MovingToResources(self, moveTo, this);
+
 				var randFrames = self.World.SharedRandom.Next(100, 175);
 				return ActivityUtils.SequenceActivities(NextActivity, new Wait(randFrames), this);
 			}
 			else
 			{
-				var next = this;
-
 				// Attempt to claim a resource as ours
-				if (territory != null)
+				if (territory != null && !territory.ClaimResource(self, closestHarvestablePosition.Value))
-				{
+					return ActivityUtils.SequenceActivities(new Wait(25), this);
-					if (!territory.ClaimResource(self, closestHarvestablePosition.Value))
-						return ActivityUtils.SequenceActivities(new Wait(25), next);
-				}
 
 				// If not given a direct order, assume ordered to the first resource location we find:
 				if (!harv.LastOrderLocation.HasValue)
@@ -93,12 +93,12 @@ namespace OpenRA.Mods.Common.Activities
 
 				self.SetTargetLine(Target.FromCell(self.World, closestHarvestablePosition.Value), Color.Red, false);
 
+				// TODO: The harvest-deliver-return sequence is a horrible mess of duplicated code and edge-cases
 				var notify = self.TraitsImplementing<INotifyHarvesterAction>();
-
 				foreach (var n in notify)
-					n.MovingToResources(self, closestHarvestablePosition.Value, next);
+					n.MovingToResources(self, closestHarvestablePosition.Value, this);
 
-				return ActivityUtils.SequenceActivities(mobile.MoveTo(closestHarvestablePosition.Value, 1), new HarvestResource(self), next);
+				return ActivityUtils.SequenceActivities(mobile.MoveTo(closestHarvestablePosition.Value, 1), new HarvestResource(self), this);
 			}
 		}
 

OpenRA.Mods.Common/Activities/PickupUnit.cs

@@ -19,87 +19,150 @@ namespace OpenRA.Mods.Common.Activities
 	{
 		readonly Actor cargo;
 		readonly IMove movement;
-		readonly Carryable carryable;
-		readonly Carryall carryall;
-		readonly Aircraft aircraft;
-		readonly IFacing cargoFacing;
-		readonly IFacing selfFacing;
 
-		enum State { Intercept, LockCarryable, MoveToCarryable, Turn, Pickup, TakeOff }
+		readonly Carryall carryall;
+		readonly IFacing carryallFacing;
+
+		readonly Carryable carryable;
+		readonly IFacing carryableFacing;
+		readonly BodyOrientation carryableBody;
+
+		readonly int delay;
+
+		enum State { Intercept, LockCarryable, MoveToCarryable, Turn, Land, Wait, Pickup, Aborted }
 
 		State state;
+		Activity innerActivity;
 
-		public PickupUnit(Actor self, Actor cargo)
+		public PickupUnit(Actor self, Actor cargo, int delay)
 		{
 			this.cargo = cargo;
+			this.delay = delay;
 			carryable = cargo.Trait<Carryable>();
-			cargoFacing = cargo.Trait<IFacing>();
+			carryableFacing = cargo.Trait<IFacing>();
+			carryableBody = cargo.Trait<BodyOrientation>();
+
 			movement = self.Trait<IMove>();
 			carryall = self.Trait<Carryall>();
-			aircraft = self.Trait<Aircraft>();
+			carryallFacing = self.Trait<IFacing>();
-			selfFacing = self.Trait<IFacing>();
+
 			state = State.Intercept;
 		}
 
 		public override Activity Tick(Actor self)
 		{
-			if (cargo.IsDead || !carryall.IsBusy)
+			if (innerActivity != null)
 			{
-				carryall.UnreserveCarryable();
+				innerActivity = ActivityUtils.RunActivity(self, innerActivity);
+				return this;
+			}
+
+			if (cargo != carryall.Carryable)
+				return NextActivity;
+
+			if (cargo.IsDead || IsCanceled)
+			{
+				carryall.UnreserveCarryable(self);
 				return NextActivity;
 			}
 
+			if (carryall.State == Carryall.CarryallState.Idle)
+				return NextActivity;
+
 			switch (state)
 			{
 				case State.Intercept:
+					innerActivity = movement.MoveWithinRange(Target.FromActor(cargo), WDist.FromCells(4));
 					state = State.LockCarryable;
-					return ActivityUtils.SequenceActivities(movement.MoveWithinRange(Target.FromActor(cargo), WDist.FromCells(4)), this);
+					return this;
 
 				case State.LockCarryable:
-					// Last check
+					state = State.MoveToCarryable;
-					if (carryable.StandbyForPickup(self))
+					if (!carryable.LockForPickup(cargo, self))
+						state = State.Aborted;
+					return this;
+
+				case State.MoveToCarryable:
+				{
+					// Line up with the attachment point
+					var localOffset = carryall.OffsetForCarryable(self, cargo).Rotate(carryableBody.QuantizeOrientation(self, cargo.Orientation));
+					var targetPosition = cargo.CenterPosition - carryableBody.LocalToWorld(localOffset);
+					if ((self.CenterPosition - targetPosition).HorizontalLengthSquared != 0)
+					{
+						// Run the first tick of the move activity immediately to avoid a one-frame pause
+						innerActivity = ActivityUtils.RunActivity(self, new HeliFly(self, Target.FromPos(targetPosition)));
+						return this;
+					}
+
+					state = State.Turn;
+					return this;
+				}
+
+				case State.Turn:
+					if (carryallFacing.Facing != carryableFacing.Facing)
+					{
+						innerActivity = new Turn(self, carryableFacing.Facing);
+						return this;
+					}
+
+					state = State.Land;
+					return this;
+
+				case State.Land:
+				{
+					var localOffset = carryall.OffsetForCarryable(self, cargo).Rotate(carryableBody.QuantizeOrientation(self, cargo.Orientation));
+					var targetPosition = cargo.CenterPosition - carryableBody.LocalToWorld(localOffset);
+					if ((self.CenterPosition - targetPosition).HorizontalLengthSquared != 0 || carryallFacing.Facing != carryableFacing.Facing)
 					{
 						state = State.MoveToCarryable;
 						return this;
 					}
 
-					// We got cancelled
+					if (targetPosition.Z != self.CenterPosition.Z)
-					carryall.UnreserveCarryable();
-					return NextActivity;
-
-				case State.MoveToCarryable: // We arrived, move on top
-					if (self.Location == cargo.Location)
 					{
-						state = State.Turn;
+						innerActivity = new HeliLand(self, false, self.World.Map.DistanceAboveTerrain(targetPosition));
 						return this;
 					}
 
-					return ActivityUtils.SequenceActivities(movement.MoveTo(cargo.Location, 0), this);
+					state = delay > 0 ? State.Wait : State.Pickup;
+					return this;
+				}
 
-				case State.Turn: // Align facing and Land
+				case State.Wait:
-					if (selfFacing.Facing != cargoFacing.Facing)
-						return ActivityUtils.SequenceActivities(new Turn(self, cargoFacing.Facing), this);
 					state = State.Pickup;
-					return ActivityUtils.SequenceActivities(new HeliLand(self, false), new Wait(10), this);
+					innerActivity = new Wait(delay, false);
+					return this;
 
 				case State.Pickup:
 					// Remove our carryable from world
-					self.World.AddFrameEndTask(w => cargo.World.Remove(cargo));
+					Attach(self);
-					carryall.AttachCarryable(cargo);
-					state = State.TakeOff;
-					return this;
-				case State.TakeOff:
-					if (HeliFly.AdjustAltitude(self, aircraft, aircraft.Info.CruiseAltitude))
-						return this;
 					return NextActivity;
+
+				case State.Aborted:
+					// We got cancelled
+					carryall.UnreserveCarryable(self);
+					break;
 			}
 
 			return NextActivity;
 		}
 
+		void Attach(Actor self)
+		{
+			self.World.AddFrameEndTask(w =>
+			{
+				cargo.World.Remove(cargo);
+				carryable.Attached(cargo);
+				carryall.AttachCarryable(self, cargo);
+			});
+		}
+
 		public override void Cancel(Actor self)
 		{
-			// TODO: Drop the unit at the nearest available cell
+			if (innerActivity != null)
+				innerActivity.Cancel(self);
+
+			base.Cancel(self);
 		}
 	}
 }

OpenRA.Mods.Common/Activities/WaitForTransport.cs

@@ -43,9 +43,6 @@ namespace OpenRA.Mods.Common.Activities
 
 		public override void Cancel(Actor self)
 		{
-			if (transportable != null)
-				transportable.WantsTransport = false;
-
 			if (inner != null)
 				inner.Cancel(self);
 		}

OpenRA.Mods.Common/OpenRA.Mods.Common.csproj

@@ -780,6 +780,8 @@
     <Compile Include="UtilityCommands\ListInstallShieldCabContentsCommand.cs" />
     <Compile Include="FileFormats\InstallShieldCABCompression.cs" />
     <Compile Include="ModContent.cs" />
+    <Compile Include="Traits\AutoCarryable.cs" />
+    <Compile Include="Traits\AutoCarryall.cs" />
   </ItemGroup>
   <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
   <PropertyGroup>

OpenRA.Mods.Common/Orders/AircraftMoveOrderTargeter.cs

@@ -15,10 +15,10 @@ using OpenRA.Traits;
 
 namespace OpenRA.Mods.Common.Orders
 {
-	class AircraftMoveOrderTargeter : IOrderTargeter
+	public class AircraftMoveOrderTargeter : IOrderTargeter
 	{
-		public string OrderID { get { return "Move"; } }
+		public string OrderID { get; protected set; }
-		public int OrderPriority { get { return 4; } }
+		public int OrderPriority { get; protected set; }
 		public bool TargetOverridesSelection(TargetModifiers modifiers)
 		{
 			return modifiers.HasModifier(TargetModifiers.ForceMove);
@@ -26,9 +26,14 @@ namespace OpenRA.Mods.Common.Orders
 
 		readonly AircraftInfo info;
 
-		public AircraftMoveOrderTargeter(AircraftInfo info) { this.info = info; }
+		public AircraftMoveOrderTargeter(AircraftInfo info)
+		{
+			this.info = info;
+			OrderID = "Move";
+			OrderPriority = 4;
+		}
 
-		public bool CanTarget(Actor self, Target target, List<Actor> othersAtTarget, ref TargetModifiers modifiers, ref string cursor)
+		public virtual bool CanTarget(Actor self, Target target, List<Actor> othersAtTarget, ref TargetModifiers modifiers, ref string cursor)
 		{
 			if (target.Type != TargetType.Terrain)
 				return false;

OpenRA.Mods.Common/Traits/AutoCarryable.cs

@@ -0,0 +1,147 @@
+#region Copyright & License Information
+/*
+ * Copyright 2007-2016 The OpenRA Developers (see AUTHORS)
+ * This file is part of OpenRA, which is free software. It is made
+ * available to you under the terms of the GNU General Public License
+ * as published by the Free Software Foundation, either version 3 of
+ * the License, or (at your option) any later version. For more
+ * information, see COPYING.
+ */
+#endregion
+
+using System.Linq;
+using OpenRA.Activities;
+using OpenRA.Mods.Common.Activities;
+using OpenRA.Traits;
+
+namespace OpenRA.Mods.Common.Traits
+{
+	[Desc("Can be carried by units with the trait `Carryall`.")]
+	public class AutoCarryableInfo : CarryableInfo
+	{
+		[Desc("Required distance away from destination before requesting a pickup. Default is 6 cells.")]
+		public readonly WDist MinDistance = WDist.FromCells(6);
+
+		public override object Create(ActorInitializer init) { return new AutoCarryable(init.Self, this); }
+	}
+
+	public class AutoCarryable : Carryable, INotifyHarvesterAction, ICallForTransport
+	{
+		readonly AutoCarryableInfo info;
+		Activity afterLandActivity;
+
+		public AutoCarryable(Actor self, AutoCarryableInfo info)
+			: base(self, info)
+		{
+			this.info = info;
+		}
+
+		public WDist MinimumDistance { get { return info.MinDistance; } }
+
+		void INotifyHarvesterAction.MovingToResources(Actor self, CPos targetCell, Activity next) { RequestTransport(self, targetCell, next); }
+		void INotifyHarvesterAction.MovingToRefinery(Actor self, CPos targetCell, Activity next) { RequestTransport(self, targetCell, next); }
+		void INotifyHarvesterAction.MovementCancelled(Actor self) { MovementCancelled(self); }
+
+		// We do not handle Harvested notification
+		void INotifyHarvesterAction.Harvested(Actor self, ResourceType resource) { }
+		void INotifyHarvesterAction.Docked() { }
+		void INotifyHarvesterAction.Undocked() { }
+
+		// No longer want to be carried
+		void ICallForTransport.MovementCancelled(Actor self) { MovementCancelled(self); }
+		void ICallForTransport.RequestTransport(Actor self, CPos destination, Activity afterLandActivity) { RequestTransport(self, destination, afterLandActivity); }
+
+		void MovementCancelled(Actor self)
+		{
+			if (state == State.Locked)
+				return;
+
+			Destination = null;
+			afterLandActivity = null;
+
+			// TODO: We could implement something like a carrier.Trait<Carryall>().CancelTransportNotify(self) and call it here
+		}
+
+		void RequestTransport(Actor self, CPos destination, Activity afterLandActivity)
+		{
+			var delta = self.World.Map.CenterOfCell(destination) - self.CenterPosition;
+			if (delta.HorizontalLengthSquared < info.MinDistance.LengthSquared)
+			{
+				Destination = null;
+				return;
+			}
+
+			Destination = destination;
+			this.afterLandActivity = afterLandActivity;
+
+			if (state != State.Free)
+				return;
+
+			// Inform all idle carriers
+			var carriers = self.World.ActorsWithTrait<Carryall>()
+				.Where(c => c.Trait.State == Carryall.CarryallState.Idle && !c.Actor.IsDead && c.Actor.Owner == self.Owner && c.Actor.IsInWorld)
+				.OrderBy(p => (self.Location - p.Actor.Location).LengthSquared);
+
+			// Enumerate idle carriers to find the first that is able to transport us
+			foreach (var carrier in carriers)
+				if (carrier.Trait.RequestTransportNotify(carrier.Actor, self, destination))
+					return;
+		}
+
+		// This gets called by carrier after we touched down
+		public override void Detached(Actor self)
+		{
+			if (!attached)
+				return;
+
+			Destination = null;
+
+			if (afterLandActivity != null)
+			{
+				// HACK: Harvesters need special treatment to avoid getting stuck on resource fields,
+				// so if a Harvester's afterLandActivity is not DeliverResources, queue a new FindResources activity
+				var findResources = self.Info.HasTraitInfo<HarvesterInfo>() && !(afterLandActivity is DeliverResources);
+				if (findResources)
+					self.QueueActivity(new FindResources(self));
+				else
+					self.QueueActivity(false, afterLandActivity);
+			}
+
+			base.Detached(self);
+		}
+
+		public override bool Reserve(Actor self, Actor carrier)
+		{
+			if (Reserved || !WantsTransport)
+				return false;
+
+			var delta = self.World.Map.CenterOfCell(Destination.Value) - self.CenterPosition;
+			if (delta.HorizontalLengthSquared < info.MinDistance.LengthSquared)
+			{
+				// Cancel pickup
+				MovementCancelled(self);
+				return false;
+			}
+
+			return base.Reserve(self, carrier);
+		}
+
+		// Prepare for transport pickup
+		public override bool LockForPickup(Actor self, Actor carrier)
+		{
+			if (state == State.Locked || !WantsTransport)
+				return false;
+
+			// Last chance to change our mind...
+			var delta = self.World.Map.CenterOfCell(Destination.Value) - self.CenterPosition;
+			if (delta.HorizontalLengthSquared < info.MinDistance.LengthSquared)
+			{
+				// Cancel pickup
+				MovementCancelled(self);
+				return false;
+			}
+
+			return base.LockForPickup(self, carrier);
+		}
+	}
+}

OpenRA.Mods.Common/Traits/AutoCarryall.cs

@@ -0,0 +1,110 @@
+#region Copyright & License Information
+/*
+ * Copyright 2007-2016 The OpenRA Developers (see AUTHORS)
+ * This file is part of OpenRA, which is free software. It is made
+ * available to you under the terms of the GNU General Public License
+ * as published by the Free Software Foundation, either version 3 of
+ * the License, or (at your option) any later version. For more
+ * information, see COPYING.
+ */
+#endregion
+
+using System.Linq;
+using OpenRA.Mods.Common.Activities;
+using OpenRA.Traits;
+
+namespace OpenRA.Mods.Common.Traits
+{
+	[Desc("Automatically transports harvesters with the Carryable trait between resource fields and refineries.")]
+	public class AutoCarryallInfo : CarryallInfo
+	{
+		public override object Create(ActorInitializer init) { return new AutoCarryall(init.Self, this); }
+	}
+
+	public class AutoCarryall : Carryall, INotifyBecomingIdle
+	{
+		bool busy;
+
+		public AutoCarryall(Actor self, AutoCarryallInfo info)
+			: base(self, info) { }
+
+		void INotifyBecomingIdle.OnBecomingIdle(Actor self)
+		{
+			busy = false;
+			FindCarryableForTransport(self);
+
+			// TODO: This should be handled by the aircraft trait
+			if (!busy)
+				self.QueueActivity(new HeliFlyCircle(self));
+		}
+
+		// A carryable notifying us that he'd like to be carried
+		public override bool RequestTransportNotify(Actor self, Actor carryable, CPos destination)
+		{
+			if (busy)
+				return false;
+
+			if (ReserveCarryable(self, carryable))
+			{
+				self.QueueActivity(false, new PickupUnit(self, carryable, 0));
+				self.QueueActivity(true, new DeliverUnit(self, destination));
+				return true;
+			}
+
+			return false;
+		}
+
+		bool IsBestAutoCarryallForCargo(Actor self, Actor candidateCargo)
+		{
+			// Find carriers
+			var carriers = self.World.ActorsHavingTrait<AutoCarryall>(c => !c.busy)
+				.Where(a => a.Owner == self.Owner && a.IsInWorld);
+
+			return carriers.ClosestTo(candidateCargo) == self;
+		}
+
+		void FindCarryableForTransport(Actor self)
+		{
+			if (!self.IsInWorld)
+				return;
+
+			// Get all carryables who want transport
+			var carryables = self.World.ActorsWithTrait<Carryable>().Where(p =>
+			{
+				var actor = p.Actor;
+				if (actor == null)
+					return false;
+
+				if (actor.Owner != self.Owner)
+					return false;
+
+				if (actor.IsDead)
+					return false;
+
+				var trait = p.Trait;
+				if (trait.Reserved)
+					return false;
+
+				if (!trait.WantsTransport)
+					return false;
+
+				if (actor.IsIdle)
+					return false;
+
+				return true;
+			}).OrderBy(p => (self.Location - p.Actor.Location).LengthSquared);
+
+			foreach (var p in carryables)
+			{
+				// Check if its actually me who's the best candidate
+				if (IsBestAutoCarryallForCargo(self, p.Actor) && ReserveCarryable(self, p.Actor))
+				{
+					busy = true;
+					self.QueueActivity(false, new PickupUnit(self, p.Actor, 0));
+					self.QueueActivity(true, new DeliverUnit(self, p.Trait.Destination.Value));
+					break;
+				}
+			}
+		}
+	}
+}

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

@@ -54,12 +54,10 @@ namespace OpenRA.Mods.Common.Traits
 			CreateActors(actorName, carrierActorName, out cargo, out carrier);
 
 			var carryable = cargo.Trait<Carryable>();
-			carryable.Destination = location;
+			carryable.Reserve(cargo, carrier);
-			carryable.Reserve(carrier);
 
-			carrier.Trait<Carryall>().AttachCarryable(cargo);
+			carrier.Trait<Carryall>().AttachCarryable(carrier, cargo);
-
+			carrier.QueueActivity(new DeliverUnit(carrier, location));
-			carrier.QueueActivity(new DeliverUnit(carrier));
 			carrier.QueueActivity(new HeliFly(carrier, Target.FromCell(self.World, self.World.Map.ChooseRandomEdgeCell(self.World.SharedRandom))));
 			carrier.QueueActivity(new RemoveSelf());
 

OpenRA.Mods.Common/Traits/Carryable.cs

@@ -9,191 +9,90 @@
  */
 #endregion
 
-using System.Linq;
-using OpenRA.Activities;
 using OpenRA.Mods.Common.Activities;
 using OpenRA.Traits;
 
 namespace OpenRA.Mods.Common.Traits
 {
-	[Desc("Can be carried by units with the trait `Carryall`.")]
+	[Desc("Can be carried by actors with the `Carryall` trait.")]
 	public class CarryableInfo : ITraitInfo, Requires<UpgradeManagerInfo>
 	{
-		[Desc("Required distance away from destination before requesting a pickup. Default is 6 cells.")]
-		public readonly WDist MinDistance = WDist.FromCells(6);
-
 		[UpgradeGrantedReference]
 		[Desc("The upgrades to grant to self while waiting or being carried.")]
 		public readonly string[] CarryableUpgrades = { };
 
-		public object Create(ActorInitializer init) { return new Carryable(init.Self, this); }
+		[Desc("Carryall attachment point relative to body.")]
+		public readonly WVec LocalOffset = WVec.Zero;
+
+		public virtual object Create(ActorInitializer init) { return new Carryable(init.Self, this); }
 	}
 
-	public class Carryable : INotifyHarvesterAction, ICallForTransport
+	public class Carryable
 	{
 		readonly CarryableInfo info;
-		readonly Actor self;
 		readonly UpgradeManager upgradeManager;
 
-		public bool Reserved { get; private set; }
+		public Actor Carrier { get; private set; }
+		public bool Reserved { get { return state != State.Free; } }
+		public CPos? Destination { get; protected set; }
+		public bool WantsTransport { get { return Destination != null; } }
 
-		// If we're locked there isn't much we can do. We'll have to wait for the carrier to finish with us. We should not move or get new orders!
+		protected enum State { Free, Reserved, Locked }
-		bool locked;
+		protected State state = State.Free;
-
+		protected bool attached;
-		void Unlock()
-		{
-			if (!locked)
-				return;
-
-			locked = false;
-			foreach (var u in info.CarryableUpgrades)
-				upgradeManager.RevokeUpgrade(self, u, this);
-		}
-
-		void Lock()
-		{
-			if (locked)
-				return;
-
-			locked = true;
-			foreach (var u in info.CarryableUpgrades)
-				upgradeManager.GrantUpgrade(self, u, this);
-		}
-
-		public bool WantsTransport { get; set; }
-		public CPos Destination;
-		Activity afterLandActivity;
 
 		public Carryable(Actor self, CarryableInfo info)
 		{
 			this.info = info;
-			this.self = self;
 			upgradeManager = self.Trait<UpgradeManager>();
-
-			locked = false;
-			Reserved = false;
-			WantsTransport = false;
 		}
 
-		public void MovingToResources(Actor self, CPos targetCell, Activity next) { RequestTransport(targetCell, next); }
+		public virtual void Attached(Actor self)
-		public void MovingToRefinery(Actor self, CPos targetCell, Activity next) { RequestTransport(targetCell, next); }
-
-		public WDist MinimumDistance { get { return info.MinDistance; } }
-
-		public void RequestTransport(CPos destination, Activity afterLandActivity)
 		{
-			var destPos = self.World.Map.CenterOfCell(destination);
+			if (attached)
-			if (destination == CPos.Zero || (self.CenterPosition - destPos).LengthSquared < info.MinDistance.LengthSquared)
-			{
-				WantsTransport = false; // Be sure to cancel any pending transports
-				return;
-			}
-
-			Destination = destination;
-			this.afterLandActivity = afterLandActivity;
-			WantsTransport = true;
-
-			if (locked || Reserved)
 				return;
 
-			// Inform all idle carriers
+			attached = true;
-			var carriers = self.World.ActorsWithTrait<Carryall>()
+			foreach (var u in info.CarryableUpgrades)
-				.Where(c => !c.Trait.IsBusy && !c.Actor.IsDead && c.Actor.Owner == self.Owner && c.Actor.IsInWorld)
+				upgradeManager.GrantUpgrade(self, u, this);
-				.OrderBy(p => (self.Location - p.Actor.Location).LengthSquared);
-
-			// Is any carrier able to transport the actor?
-			// Any will return once it finds a carrier that returns true.
-			carriers.Any(carrier => carrier.Trait.RequestTransportNotify(self));
-		}
-
-		// No longer want to be carried
-		public void MovementCancelled(Actor self)
-		{
-			if (locked)
-				return;
-
-			WantsTransport = false;
-			afterLandActivity = null;
-
-			// TODO: We could implement something like a carrier.Trait<Carryall>().CancelTransportNotify(self) and call it here
-		}
-
-		// We do not handle Harvested notification
-		public void Harvested(Actor self, ResourceType resource) { }
-		public void Docked() { }
-		public void Undocked() { }
-
-		public Actor GetClosestIdleCarrier()
-		{
-			// Find carriers
-			var carriers = self.World.ActorsHavingTrait<Carryall>(c => !c.IsBusy)
-				.Where(a => a.Owner == self.Owner && a.IsInWorld);
-
-			return carriers.ClosestTo(self);
 		}
 
 		// This gets called by carrier after we touched down
-		public void Dropped()
+		public virtual void Detached(Actor self)
 		{
-			WantsTransport = false;
+			if (!attached)
-			Unlock();
+				return;
 
-			if (afterLandActivity != null)
+			attached = false;
-			{
+			foreach (var u in info.CarryableUpgrades)
-				// HACK: Harvesters need special treatment to avoid getting stuck on resource fields,
+				upgradeManager.RevokeUpgrade(self, u, this);
-				// so if a Harvester's afterLandActivity is not DeliverResources, queue a new FindResources activity
-				var findResources = self.Info.HasTraitInfo<HarvesterInfo>() && !(afterLandActivity is DeliverResources);
-				if (findResources)
-					self.QueueActivity(new FindResources(self));
-				else
-					self.QueueActivity(false, afterLandActivity);
-			}
 		}
 
-		public bool Reserve(Actor carrier)
+		public virtual bool Reserve(Actor self, Actor carrier)
 		{
 			if (Reserved)
 				return false;
 
-			var destPos = self.World.Map.CenterOfCell(Destination);
+			state = State.Reserved;
-			if ((self.CenterPosition - destPos).LengthSquared < info.MinDistance.LengthSquared)
+			Carrier = carrier;
-			{
-				MovementCancelled(self);
-				return false;
-			}
-
-			Reserved = true;
-
 			return true;
 		}
 
-		public void UnReserve(Actor carrier)
+		public virtual void UnReserve(Actor self)
 		{
-			Reserved = false;
+			state = State.Free;
-			Unlock();
+			Carrier = null;
 		}
 
 		// Prepare for transport pickup
-		public bool StandbyForPickup(Actor carrier)
+		public virtual bool LockForPickup(Actor self, Actor carrier)
 		{
-			if (Destination == CPos.Zero)
+			if (state == State.Locked)
 				return false;
 
-			if (locked || !WantsTransport)
+			state = State.Locked;
-				return false;
+			Carrier = carrier;
-
+			self.QueueActivity(false, new WaitFor(() => state != State.Locked, false));
-			// Last chance to change our mind...
-			var destPos = self.World.Map.CenterOfCell(Destination);
-			if ((self.CenterPosition - destPos).LengthSquared < info.MinDistance.LengthSquared)
-			{
-				MovementCancelled(self);
-				return false;
-			}
-
-			// Cancel our activities
-			self.CancelActivity();
-			Lock();
-
 			return true;
 		}
 	}

OpenRA.Mods.Common/Traits/Carryall.cs

@@ -10,206 +10,336 @@
 #endregion
 
 using System.Collections.Generic;
+using System.Drawing;
 using System.Linq;
 using OpenRA.Graphics;
 using OpenRA.Mods.Common.Activities;
-using OpenRA.Mods.Common.Traits.Render;
+using OpenRA.Mods.Common.Graphics;
+using OpenRA.Mods.Common.Orders;
+using OpenRA.Primitives;
 using OpenRA.Traits;
 
 namespace OpenRA.Mods.Common.Traits
 {
-	[Desc("Automatically transports harvesters with the Carryable trait between resource fields and refineries.")]
+	[Desc("Transports actors with the `Carryable` trait.")]
-	public class CarryallInfo : ITraitInfo, Requires<BodyOrientationInfo>
+	public class CarryallInfo : ITraitInfo, Requires<BodyOrientationInfo>, Requires<AircraftInfo>
 	{
-		[Desc("Set to false when the carryall should not automatically get new jobs.")]
+		[Desc("Delay on the ground while attaching an actor to the carryall.")]
-		public readonly bool Automatic = true;
+		public readonly int LoadingDelay = 0;
 
-		public object Create(ActorInitializer init) { return new Carryall(init.Self, this); }
+		[Desc("Delay on the ground while detacting an actor to the carryall.")]
+		public readonly int UnloadingDelay = 0;
+
+		[Desc("Carryable attachment point relative to body.")]
+		public readonly WVec LocalOffset = WVec.Zero;
+
+		[Desc("Radius around the target drop location that are considered if the target tile is blocked.")]
+		public readonly WDist DropRange = WDist.FromCells(5);
+
+		[VoiceReference]
+		public readonly string Voice = "Action";
+
+		public virtual object Create(ActorInitializer init) { return new Carryall(init.Self, this); }
 	}
 
-	public class Carryall : INotifyBecomingIdle, INotifyKilled, ISync, IRender, INotifyActorDisposing
+	public class Carryall : INotifyKilled, ISync, IRender, INotifyActorDisposing, IIssueOrder, IResolveOrder, IOrderVoice
 	{
-		readonly Actor self;
+		public enum CarryallState
-		readonly WDist carryHeight;
+		{
-		readonly CarryallInfo info;
+			Idle,
+			Reserved,
+			Carrying
+		}
+
+		public readonly CarryallInfo Info;
+		readonly AircraftInfo aircraftInfo;
+		readonly BodyOrientation body;
+		readonly IMove move;
+		readonly IFacing facing;
 
 		// The actor we are currently carrying.
-		[Sync] public Actor Carrying { get; internal set; }
+		[Sync] public Actor Carryable { get; private set; }
-		public bool IsCarrying { get; internal set; }
+		public CarryallState State { get; private set; }
 
-		// TODO: Use ActorPreviews so that this can support actors with multiple sprites
+		IActorPreview[] carryablePreview = null;
-		Animation anim;
 
-		public bool IsBusy { get; internal set; }
+		/// <summary>Offset between the carryall's and the carried actor's CenterPositions</summary>
+		public WVec CarryableOffset { get; private set; }
 
 		public Carryall(Actor self, CarryallInfo info)
 		{
-			this.self = self;
+			this.Info = info;
-			this.info = info;
 
-			IsBusy = false;
+			Carryable = null;
-			IsCarrying = false;
+			State = CarryallState.Idle;
 
-			var helicopter = self.Info.TraitInfoOrDefault<AircraftInfo>();
+			aircraftInfo = self.Info.TraitInfoOrDefault<AircraftInfo>();
-			carryHeight = helicopter != null ? helicopter.LandAltitude : WDist.Zero;
+			body = self.Trait<BodyOrientation>();
+			move = self.Trait<IMove>();
+			facing = self.Trait<IFacing>();
 		}
 
-		public void OnBecomingIdle(Actor self)
+		void INotifyActorDisposing.Disposing(Actor self)
 		{
-			if (info.Automatic)
+			if (State == CarryallState.Carrying)
-				FindCarryableForTransport();
-
-			if (!IsBusy)
-				self.QueueActivity(new HeliFlyCircle(self));
-		}
-
-		// A carryable notifying us that he'd like to be carried
-		public bool RequestTransportNotify(Actor carryable)
-		{
-			if (IsBusy || !info.Automatic)
-				return false;
-
-			if (ReserveCarryable(carryable))
 			{
-				self.QueueActivity(false, new PickupUnit(self, carryable));
+				Carryable.Dispose();
-				self.QueueActivity(true, new DeliverUnit(self));
+				Carryable = null;
-				return true;
 			}
 
+			UnreserveCarryable(self);
+		}
+
+		void INotifyKilled.Killed(Actor self, AttackInfo e)
+		{
+			if (State == CarryallState.Carrying)
+			{
+				if (Carryable.IsInWorld && !Carryable.IsDead)
+					Carryable.Kill(e.Attacker);
+				Carryable = null;
+			}
+
+			UnreserveCarryable(self);
+		}
+
+		public virtual bool RequestTransportNotify(Actor self, Actor carryable, CPos destination)
+		{
 			return false;
 		}
 
-		void FindCarryableForTransport()
+		public virtual WVec OffsetForCarryable(Actor self, Actor carryable)
 		{
-			if (!self.IsInWorld)
+			return Info.LocalOffset - carryable.Info.TraitInfo<CarryableInfo>().LocalOffset;
-				return;
+		}
 
-			// Get all carryables who want transport
+		public virtual bool AttachCarryable(Actor self, Actor carryable)
-			var carryables = self.World.ActorsWithTrait<Carryable>()
+		{
-				.Where(p =>
+			if (State == CarryallState.Carrying)
-				{
+				return false;
-					var actor = p.Actor;
-					if (actor == null)
-						return false;
 
-					if (actor.Owner != self.Owner)
+			Carryable = carryable;
-						return false;
+			State = CarryallState.Carrying;
 
-					if (actor.IsDead)
+			CarryableOffset = OffsetForCarryable(self, carryable);
-						return false;
+			return true;
+		}
 
-					var trait = p.Trait;
+		public virtual void DetachCarryable(Actor self)
-					if (trait.Reserved)
+		{
-						return false;
+			UnreserveCarryable(self);
 
-					if (!trait.WantsTransport)
+			carryablePreview = null;
-						return false;
+			CarryableOffset = WVec.Zero;
+		}
 
-					if (actor.IsIdle)
+		public virtual bool ReserveCarryable(Actor self, Actor carryable)
-						return false;
+		{
+			if (State == CarryallState.Reserved)
+				UnreserveCarryable(self);
 
-					return true;
+			if (State != CarryallState.Idle || !carryable.Trait<Carryable>().Reserve(carryable, self))
-				})
+				return false;
-				.OrderBy(p => (self.Location - p.Actor.Location).LengthSquared);
 
-			foreach (var p in carryables)
+			Carryable = carryable;
+			State = CarryallState.Reserved;
+			return true;
+		}
+
+		public virtual void UnreserveCarryable(Actor self)
+		{
+			if (Carryable != null && Carryable.IsInWorld && !Carryable.IsDead)
+				Carryable.Trait<Carryable>().UnReserve(Carryable);
+
+			Carryable = null;
+			State = CarryallState.Idle;
+		}
+
+		IEnumerable<IRenderable> IRender.Render(Actor self, WorldRenderer wr)
+		{
+			if (State == CarryallState.Carrying)
 			{
-				// Check if its actually me who's the best candidate
+				if (carryablePreview == null)
-				if (p.Trait.GetClosestIdleCarrier() == self && ReserveCarryable(p.Actor))
 				{
-					self.QueueActivity(false, new PickupUnit(self, p.Actor));
+					var carryableInits = new TypeDictionary()
-					self.QueueActivity(true, new DeliverUnit(self));
+					{
-					break;
+						new OwnerInit(Carryable.Owner),
+						new DynamicFacingInit(() => facing.Facing),
+					};
+
+					foreach (var api in Carryable.TraitsImplementing<IActorPreviewInitModifier>())
+						api.ModifyActorPreviewInit(Carryable, carryableInits);
+
+					var init = new ActorPreviewInitializer(Carryable.Info, wr, carryableInits);
+					carryablePreview = Carryable.Info.TraitInfos<IRenderActorPreviewInfo>()
+						.SelectMany(rpi => rpi.RenderPreview(init))
+						.ToArray();
+				}
+
+				var offset = body.LocalToWorld(CarryableOffset.Rotate(body.QuantizeOrientation(self, self.Orientation)));
+				var previewRenderables = carryablePreview
+					.SelectMany(p => p.Render(wr, self.CenterPosition + offset))
+					.OrderBy(WorldRenderer.RenderableScreenZPositionComparisonKey);
+
+				foreach (var r in previewRenderables)
+					yield return r;
+			}
+		}
+
+		IEnumerable<IOrderTargeter> IIssueOrder.Orders
+		{
+			get
+			{
+				if (State != CarryallState.Carrying)
+					yield return new CarryallPickupOrderTargeter();
+				else
+					yield return new CarryallDeliverUnitTargeter(aircraftInfo, CarryableOffset);
+			}
+		}
+
+		Order IIssueOrder.IssueOrder(Actor self, IOrderTargeter order, Target target, bool queued)
+		{
+			if (order.OrderID == "PickupUnit")
+			{
+				if (target.Type == TargetType.FrozenActor)
+					return new Order(order.OrderID, self, queued) { ExtraData = target.FrozenActor.ID };
+
+				return new Order(order.OrderID, self, queued) { TargetActor = target.Actor };
+			}
+			else if (order.OrderID == "DeliverUnit")
+			{
+				return new Order(order.OrderID, self, queued) { TargetLocation = self.World.Map.CellContaining(target.CenterPosition) };
+			}
+			else if (order.OrderID == "Unload")
+			{
+				return new Order(order.OrderID, self, queued) { TargetLocation = self.World.Map.CellContaining(target.CenterPosition) };
+			}
+
+			return null;
+		}
+
+		void IResolveOrder.ResolveOrder(Actor self, Order order)
+		{
+			if (State == CarryallState.Carrying)
+			{
+				if (order.OrderString == "DeliverUnit")
+				{
+					var cell = self.World.Map.Clamp(order.TargetLocation);
+
+					if (!aircraftInfo.MoveIntoShroud && !self.Owner.Shroud.IsExplored(cell))
+						return;
+
+					var targetLocation = move.NearestMoveableCell(order.TargetLocation);
+					self.SetTargetLine(Target.FromCell(self.World, targetLocation), Color.Yellow);
+					self.QueueActivity(order.Queued, new DeliverUnit(self, targetLocation));
+				}
+				else if (order.OrderString == "Unload")
+				{
+					var targetLocation = move.NearestMoveableCell(self.Location);
+					self.SetTargetLine(Target.FromCell(self.World, targetLocation), Color.Yellow);
+					self.QueueActivity(order.Queued, new DeliverUnit(self, targetLocation));
+				}
+			}
+			else
+			{
+				if (order.OrderString == "PickupUnit")
+				{
+					var target = self.ResolveFrozenActorOrder(order, Color.Yellow);
+					if (target.Type != TargetType.Actor)
+						return;
+
+					if (!ReserveCarryable(self, target.Actor))
+						return;
+
+					if (!order.Queued)
+						self.CancelActivity();
+
+					self.SetTargetLine(target, Color.Yellow);
+					self.QueueActivity(order.Queued, new PickupUnit(self, target.Actor, Info.LoadingDelay));
 				}
 			}
 		}
 
-		// Reserve the carryable so its ours exclusively
+		string IOrderVoice.VoicePhraseForOrder(Actor self, Order order)
-		public bool ReserveCarryable(Actor carryable)
 		{
-			if (Carrying != null)
+			switch (order.OrderString)
-				return false;
-
-			if (carryable.Trait<Carryable>().Reserve(self))
 			{
-				Carrying = carryable;
+				case "DeliverUnit":
-				IsBusy = true;
+				case "Unload":
+				case "PickupUnit":
+					return Info.Voice;
+				default:
+					return null;
+			}
+		}
+
+		class CarryallPickupOrderTargeter : UnitOrderTargeter
+		{
+			public CarryallPickupOrderTargeter()
+				: base("PickupUnit", 5, "ability", false, true)
+			{
+			}
+
+			static bool CanTarget(Actor self, Actor target)
+			{
+				if (!target.AppearsFriendlyTo(self))
+					return false;
+				var carryable = target.TraitOrDefault<Carryable>();
+				if (carryable == null)
+					return false;
+				if (carryable.Reserved && carryable.Carrier != self)
+					return false;
 				return true;
 			}
 
-			return false;
+			public override bool CanTargetActor(Actor self, Actor target, TargetModifiers modifiers, ref string cursor)
-		}
-
-		// Unreserve the carryable
-		public void UnreserveCarryable()
-		{
-			if (Carrying != null)
 			{
-				if (Carrying.IsInWorld && !Carrying.IsDead)
+				return CanTarget(self, target);
-					Carrying.Trait<Carryable>().UnReserve(self);
-
-				Carrying = null;
 			}
 
-			CarryableReleased();
+			public override bool CanTargetFrozenActor(Actor self, FrozenActor target, TargetModifiers modifiers, ref string cursor)
-		}
-
-		// INotifyKilled
-		public void Killed(Actor self, AttackInfo e)
-		{
-			if (Carrying != null)
 			{
-				if (IsCarrying && Carrying.IsInWorld && !Carrying.IsDead)
+				return CanTarget(self, target.Actor);
-					Carrying.Kill(e.Attacker);
-
-				Carrying = null;
-			}
-
-			UnreserveCarryable();
-		}
-
-		public void Disposing(Actor self)
-		{
-			if (Carrying != null && IsCarrying)
-			{
-				Carrying.Dispose();
-				Carrying = null;
 			}
 		}
 
-		// Called when carryable is inside.
+		class CarryallDeliverUnitTargeter : AircraftMoveOrderTargeter
-		public void AttachCarryable(Actor carryable)
 		{
-			IsBusy = true;
+			readonly AircraftInfo aircraftInfo;
-			IsCarrying = true;
+			readonly WVec carryableOffset;
-			Carrying = carryable;
 
-			// Create a new animation for our carryable unit
+			public CarryallDeliverUnitTargeter(AircraftInfo aircraftInfo, WVec carryableOffset)
-			var rs = carryable.Trait<RenderSprites>();
+				: base(aircraftInfo)
-			anim = new Animation(self.World, rs.GetImage(carryable), RenderSprites.MakeFacingFunc(self));
-			anim.PlayRepeating("idle");
-			anim.IsDecoration = true;
-		}
-
-		// Called when released
-		public void CarryableReleased()
-		{
-			IsBusy = false;
-			IsCarrying = false;
-			anim = null;
-		}
-
-		public IEnumerable<IRenderable> Render(Actor self, WorldRenderer wr)
-		{
-			// Render the carryable below us TODO: Implement RenderSprites trait
-			if (anim != null && !self.World.FogObscures(self))
 			{
-				anim.Tick();
+				OrderID = "DeliverUnit";
-				var renderables = anim.Render(self.CenterPosition + new WVec(0, 0, -carryHeight.Length),
+				OrderPriority = 6;
-					wr.Palette("player" + Carrying.Owner.InternalName));
+				this.carryableOffset = carryableOffset;
+				this.aircraftInfo = aircraftInfo;
+			}
 
-				foreach (var rr in renderables)
+			public override bool CanTarget(Actor self, Target target, List<Actor> othersAtTarget, ref TargetModifiers modifiers, ref string cursor)
-					yield return rr;
+			{
+				if (modifiers.HasModifier(TargetModifiers.ForceMove))
+					return false;
+
+				var type = target.Type;
+				if (type == TargetType.Actor && self == target.Actor)
+				{
+					var altitude = self.World.Map.DistanceAboveTerrain(self.CenterPosition);
+					if (altitude.Length - carryableOffset.Z < aircraftInfo.MinAirborneAltitude)
+					{
+						cursor = "deploy";
+						OrderID = "Unload";
+						return true;
+					}
+				}
+				else if ((type == TargetType.Actor && target.Actor.Info.HasTraitInfo<BuildingInfo>())
+					|| (target.Type == TargetType.FrozenActor && target.FrozenActor.Info.HasTraitInfo<BuildingInfo>()))
+				{
+					cursor = "move-blocked";
+					return true;
+				}
+
+				return base.CanTarget(self, target, othersAtTarget, ref modifiers, ref cursor);
 			}
 		}
 	}

OpenRA.Mods.Common/Traits/Harvester.cs

@@ -150,7 +150,7 @@ namespace OpenRA.Mods.Common.Traits
 
 		public void ContinueHarvesting(Actor self)
 		{
-			// Move out of the refinery dock and continue harvesting:
+			// Move out of the refinery dock and continue harvesting
 			UnblockRefinery(self);
 			self.QueueActivity(new FindResources(self));
 		}
@@ -220,6 +220,13 @@ namespace OpenRA.Mods.Common.Traits
 					// Get out of the way:
 					var unblockCell = LastHarvestedCell ?? (deliveryLoc + Info.UnblockCell);
 					var moveTo = mobile.NearestMoveableCell(unblockCell, 1, 5);
+
+					// TODO: The harvest-deliver-return sequence is a horrible mess of duplicated code and edge-cases
+					var notify = self.TraitsImplementing<INotifyHarvesterAction>();
+					var findResources = new FindResources(self);
+					foreach (var n in notify)
+						n.MovingToResources(self, moveTo, findResources);
+
 					self.QueueActivity(mobile.MoveTo(moveTo, 1));
 					self.SetTargetLine(Target.FromCell(self.World, moveTo), Color.Gray, false);
 				}
@@ -363,13 +370,13 @@ namespace OpenRA.Mods.Common.Traits
 					loc = self.Location;
 				}
 
-				var next = new FindResources(self);
+				var findResources = new FindResources(self);
-				self.QueueActivity(next);
+				self.QueueActivity(findResources);
 				self.SetTargetLine(Target.FromCell(self.World, loc.Value), Color.Red);
 
 				var notify = self.TraitsImplementing<INotifyHarvesterAction>();
 				foreach (var n in notify)
-					n.MovingToResources(self, loc.Value, next);
+					n.MovingToResources(self, loc.Value, findResources);
 
 				LastOrderLocation = loc;
 
@@ -392,12 +399,12 @@ namespace OpenRA.Mods.Common.Traits
 
 				self.CancelActivity();
 
-				var next = new DeliverResources(self);
+				var deliver = new DeliverResources(self);
-				self.QueueActivity(next);
+				self.QueueActivity(deliver);
 
 				var notify = self.TraitsImplementing<INotifyHarvesterAction>();
 				foreach (var n in notify)
-					n.MovingToRefinery(self, order.TargetLocation, next);
+					n.MovingToRefinery(self, order.TargetLocation, deliver);
 			}
 			else if (order.OrderString == "Stop" || order.OrderString == "Move")
 			{

OpenRA.Mods.Common/Traits/Repairable.cs

@@ -149,7 +149,7 @@ namespace OpenRA.Mods.Common.Traits
 			if ((self.CenterPosition - target.CenterPosition).LengthSquared < transport.MinimumDistance.LengthSquared)
 				return;
 
-			transport.RequestTransport(targetCell, nextActivity);
+			transport.RequestTransport(self, targetCell, nextActivity);
 		}
 	}
 }

OpenRA.Mods.Common/TraitsInterfaces.cs

@@ -112,9 +112,9 @@ namespace OpenRA.Mods.Common.Traits
 	public interface ICallForTransport
 	{
 		WDist MinimumDistance { get; }
-		bool WantsTransport { get; set; }
+		bool WantsTransport { get; }
 		void MovementCancelled(Actor self);
-		void RequestTransport(CPos destination, Activity afterLandActivity);
+		void RequestTransport(Actor self, CPos destination, Activity afterLandActivity);
 	}
 
 	public interface IDeathActorInitModifier

mods/d2k/rules/aircraft.yaml

@@ -16,7 +16,6 @@ carryall.reinforce:
 		TurnSpeed: 4
 		LandableTerrainTypes: Sand, Rock, Transition, Spice, SpiceSand, Dune
 		Repulsable: False
-		LandAltitude: 100
 		LandWhenIdle: False
 		AirborneUpgrades: airborne
 		CanHover: True
@@ -31,7 +30,9 @@ carryall.reinforce:
 	SpawnActorOnDeath:
 		Actor: carryall.husk
 	Carryall:
-		Automatic: False
+		LoadingDelay: 10
+		UnloadingDelay: 15
+		LocalOffset: 0, 0, -128
 	RenderSprites:
 		Image: carryall
 	SelfHealing:
@@ -44,8 +45,11 @@ carryall.reinforce:
 
 carryall:
 	Inherits: carryall.reinforce
-	Carryall:
+	-Carryall:
-		Automatic: True
+	AutoCarryall:
+		LoadingDelay: 10
+		UnloadingDelay: 15
+		LocalOffset: 0, 0, -128
 	Buildable:
 		Queue: Aircraft
 		BuildPaletteOrder: 120

mods/d2k/rules/defaults.yaml

@@ -96,7 +96,7 @@
 	MustBeDestroyed:
 	Voiced:
 		VoiceSet: VehicleVoice
-	Carryable:
+	AutoCarryable:
 		CarryableUpgrades: notmobile
 	WithDecorationCarryable:
 		Image: pips