Introduce a new CarryableHarvester trait

OpenRA.Mods.Common/Activities/DeliverResources.cs

@@ -65,8 +65,7 @@ namespace OpenRA.Mods.Common.Activities
 			self.SetTargetLine(Target.FromActor(proc), Color.Green, false);
 			if (self.Location != proc.Location + iao.DeliveryOffset)
 			{
-				var notify = self.TraitsImplementing<INotifyHarvesterAction>();
+				foreach (var n in self.TraitsImplementing<INotifyHarvesterAction>())
-				foreach (var n in notify)
 					n.MovingToRefinery(self, proc, this);
 
 				return ActivityUtils.SequenceActivities(movement.MoveTo(proc.Location + iao.DeliveryOffset, 0), this);

OpenRA.Mods.Common/Activities/FindResources.cs

@@ -55,10 +55,8 @@ namespace OpenRA.Mods.Common.Activities
 			if (NextInQueue != null)
 				return NextInQueue;
 
-			var deliver = new DeliverResources(self);
-
 			if (harv.IsFull)
-				return ActivityUtils.SequenceActivities(deliver, NextActivity);
+				return ActivityUtils.SequenceActivities(new DeliverResources(self), NextActivity);
 
 			var closestHarvestablePosition = ClosestHarvestablePos(self);
 
@@ -67,20 +65,18 @@ namespace OpenRA.Mods.Common.Activities
 			if (!closestHarvestablePosition.HasValue)
 			{
 				if (!harv.IsEmpty)
-					return deliver;
+					return new DeliverResources(self);
 
 				harv.LastSearchFailed = true;
 
 				var unblockCell = harv.LastHarvestedCell ?? (self.Location + harvInfo.UnblockCell);
 				var moveTo = mobile.NearestMoveableCell(unblockCell, 2, 5);
 				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
+				foreach (var n in self.TraitsImplementing<INotifyHarvesterAction>())
-				var notify = self.TraitsImplementing<INotifyHarvesterAction>();
-				foreach (var n in notify)
 					n.MovingToResources(self, moveTo, this);
 
+				self.SetTargetLine(Target.FromCell(self.World, moveTo), Color.Gray, false);
 				var randFrames = self.World.SharedRandom.Next(100, 175);
 
 				// Avoid creating an activity cycle
@@ -100,13 +96,10 @@ namespace OpenRA.Mods.Common.Activities
 				if (!harv.LastOrderLocation.HasValue)
 					harv.LastOrderLocation = closestHarvestablePosition;
 
-				self.SetTargetLine(Target.FromCell(self.World, closestHarvestablePosition.Value), Color.Red, false);
+				foreach (var n in self.TraitsImplementing<INotifyHarvesterAction>())
-
-				// 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, this);
 
+				self.SetTargetLine(Target.FromCell(self.World, closestHarvestablePosition.Value), Color.Red, false);
 				return ActivityUtils.SequenceActivities(mobile.MoveTo(closestHarvestablePosition.Value, 1), new HarvestResource(self), this);
 			}
 		}

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

@@ -316,6 +316,7 @@
     <Compile Include="Traits\CaptureProgressBlink.cs" />
     <Compile Include="Traits\RepairsUnits.cs" />
     <Compile Include="Traits\Burns.cs" />
+    <Compile Include="Traits\CarryableHarvester.cs" />
     <Compile Include="Traits\ChangesTerrain.cs" />
     <Compile Include="Traits\CommandBarBlacklist.cs" />
     <Compile Include="Traits\Conditions\GrantExternalConditionToCrusher.cs" />

OpenRA.Mods.Common/Traits/AutoCarryable.cs

@@ -25,7 +25,7 @@ namespace OpenRA.Mods.Common.Traits
 		public override object Create(ActorInitializer init) { return new AutoCarryable(init.Self, this); }
 	}
 
-	public class AutoCarryable : Carryable, INotifyHarvesterAction, ICallForTransport
+	public class AutoCarryable : Carryable, ICallForTransport
 	{
 		readonly AutoCarryableInfo info;
 		Activity afterLandActivity;
@@ -38,21 +38,6 @@ namespace OpenRA.Mods.Common.Traits
 
 		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, Actor refineryActor, Activity next)
-		{
-			var iao = refineryActor.Trait<IAcceptResources>();
-			RequestTransport(self, refineryActor.Location + iao.DeliveryOffset, 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); }

OpenRA.Mods.Common/Traits/CarryableHarvester.cs

@@ -0,0 +1,57 @@
+#region Copyright & License Information
+/*
+ * Copyright 2007-2018 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;
+using System.Linq;
+using OpenRA.Activities;
+using OpenRA.Traits;
+
+namespace OpenRA.Mods.Common.Traits
+{
+	public class CarryableHarvesterInfo : ITraitInfo
+	{
+		public object Create(ActorInitializer init) { return new CarryableHarvester(); }
+	}
+
+	public class CarryableHarvester : INotifyCreated, INotifyHarvesterAction
+	{
+		ICallForTransport[] transports;
+
+		void INotifyCreated.Created(Actor self)
+		{
+			transports = self.TraitsImplementing<ICallForTransport>().ToArray();
+		}
+
+		void INotifyHarvesterAction.MovingToResources(Actor self, CPos targetCell, Activity next)
+		{
+			foreach (var t in transports)
+				t.RequestTransport(self, targetCell, next);
+		}
+
+		void INotifyHarvesterAction.MovingToRefinery(Actor self, Actor refineryActor, Activity next)
+		{
+			var iao = refineryActor.Trait<IAcceptResources>();
+			var location = refineryActor.Location + iao.DeliveryOffset;
+			foreach (var t in transports)
+				t.RequestTransport(self, location, next);
+		}
+
+		void INotifyHarvesterAction.MovementCancelled(Actor self)
+		{
+			foreach (var t in transports)
+				t.MovementCancelled(self);
+		}
+
+		void INotifyHarvesterAction.Harvested(Actor self, ResourceType resource) { }
+		void INotifyHarvesterAction.Docked() { }
+		void INotifyHarvesterAction.Undocked() { }
+	}
+}

OpenRA.Mods.Common/Traits/Harvester.cs

@@ -83,9 +83,9 @@ namespace OpenRA.Mods.Common.Traits
 		readonly Mobile mobile;
 		readonly ResourceLayer resLayer;
 		readonly ResourceClaimLayer claimLayer;
-		Dictionary<ResourceTypeInfo, int> contents = new Dictionary<ResourceTypeInfo, int>();
+		readonly Dictionary<ResourceTypeInfo, int> contents = new Dictionary<ResourceTypeInfo, int>();
-		INotifyHarvesterAction[] notify;
 		bool idleSmart = true;
+		INotifyHarvesterAction[] notifyHarvesterAction;
 		int idleDuration;
 
 		[Sync] public bool LastSearchFailed;
@@ -119,7 +119,7 @@ namespace OpenRA.Mods.Common.Traits
 
 		void INotifyCreated.Created(Actor self)
 		{
-			notify = self.TraitsImplementing<INotifyHarvesterAction>().ToArray();
+			notifyHarvesterAction = self.TraitsImplementing<INotifyHarvesterAction>().ToArray();
 
 			// Note: This is queued in a FrameEndTask because otherwise the activity is dropped/overridden while moving out of a factory.
 			if (Info.SearchOnCreation)
@@ -233,11 +233,8 @@ namespace OpenRA.Mods.Common.Traits
 					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
+					// FindResources takes care of calling INotifyHarvesterAction
-					var notify = self.TraitsImplementing<INotifyHarvesterAction>();
+					self.QueueActivity(new FindResources(self));
-					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);
@@ -381,12 +378,10 @@ namespace OpenRA.Mods.Common.Traits
 					loc = self.Location;
 				}
 
-				var findResources = new FindResources(self);
-				self.QueueActivity(findResources);
 				self.SetTargetLine(Target.FromCell(self.World, loc.Value), Color.Red);
 
-				foreach (var n in notify)
+				// FindResources takes care of calling INotifyHarvesterAction
-					n.MovingToResources(self, loc.Value, findResources);
+				self.QueueActivity(new FindResources(self));
 
 				LastOrderLocation = loc;
 
@@ -414,16 +409,14 @@ namespace OpenRA.Mods.Common.Traits
 				self.SetTargetLine(order.Target, Color.Green);
 
 				self.CancelActivity();
+				self.QueueActivity(new DeliverResources(self));
 
-				var deliver = new DeliverResources(self);
+				foreach (var n in notifyHarvesterAction)
-				self.QueueActivity(deliver);
+					n.MovingToRefinery(self, targetActor, new DeliverResources(self));
-
-				foreach (var n in notify)
-					n.MovingToRefinery(self, targetActor, deliver);
 			}
 			else if (order.OrderString == "Stop" || order.OrderString == "Move")
 			{
-				foreach (var n in notify)
+				foreach (var n in notifyHarvesterAction)
 					n.MovementCancelled(self);
 
 				// Turn off idle smarts to obey the stop/move:

OpenRA.Mods.Common/Traits/Render/WithHarvestOverlay.cs

@@ -50,7 +50,7 @@ namespace OpenRA.Mods.Common.Traits.Render
 				p => ZOffsetFromCenter(self, p, 0)), info.Palette);
 		}
 
-		public void Harvested(Actor self, ResourceType resource)
+		void INotifyHarvesterAction.Harvested(Actor self, ResourceType resource)
 		{
 			if (visible)
 				return;
@@ -59,11 +59,11 @@ namespace OpenRA.Mods.Common.Traits.Render
 			anim.PlayThen(info.Sequence, () => visible = false);
 		}
 
-		public void MovingToResources(Actor self, CPos targetCell, Activity next) { }
+		void INotifyHarvesterAction.MovingToResources(Actor self, CPos targetCell, Activity next) { }
-		public void MovingToRefinery(Actor self, Actor targetRefinery, Activity next) { }
+		void INotifyHarvesterAction.MovingToRefinery(Actor self, Actor targetRefinery, Activity next) { }
-		public void MovementCancelled(Actor self) { }
+		void INotifyHarvesterAction.MovementCancelled(Actor self) { }
-		public void Docked() { }
+		void INotifyHarvesterAction.Docked() { }
-		public void Undocked() { }
+		void INotifyHarvesterAction.Undocked() { }
 
 		public static int ZOffsetFromCenter(Actor self, WPos pos, int offset)
 		{

mods/d2k/rules/vehicles.yaml

@@ -73,6 +73,7 @@ harvester:
 		BaleUnloadDelay: 5
 		SearchFromProcRadius: 30
 		SearchFromOrderRadius: 15
+	CarryableHarvester:
 	Health:
 		HP: 45000
 	Armor: