Merge pull request #8622 from reaperrr/hv-refactor2

Refactored harvester resource search

OpenRA.Mods.Common/Activities/FindResources.cs

@@ -8,7 +8,6 @@
  */
 #endregion
 
-using System;
 using System.Collections.Generic;
 using System.Drawing;
 using System.Linq;
@@ -58,91 +57,111 @@ namespace OpenRA.Mods.Common.Activities
 			if (harv.IsFull)
 				return Util.SequenceActivities(deliver, NextActivity);
 
-			// Determine where to search from and how far to search:
+			var closestHarvestablePosition = ClosestHarvestablePos(self);
-			var searchFromLoc = harv.LastOrderLocation ?? (harv.LastLinkedProc ?? harv.LinkedProc ?? self).Location;
-			var searchRadius = harv.LastOrderLocation.HasValue ? harvInfo.SearchFromOrderRadius : harvInfo.SearchFromProcRadius;
-			var searchRadiusSquared = searchRadius * searchRadius;
 
-			// Find harvestable resources nearby:
+			// If no harvestable position could be found, either deliver the remaining resources
-			var path = pathFinder.FindPath(
+			// or get out of the way and do not disturb.
-				PathSearch.Search(self.World, mobileInfo, self, true)
+			if (!closestHarvestablePosition.HasValue)
-					.WithHeuristic(loc =>
 			{
-						// Avoid this cell:
+				if (!harv.IsEmpty)
-						if (avoidCell.HasValue && loc == avoidCell.Value)
+					return deliver;
-							return EstimateDistance(loc, searchFromLoc) + Constants.CellCost;
 
-						// Don't harvest out of range:
+				var cachedPosition = self.Location;
-						var distSquared = (loc - searchFromLoc).LengthSquared;
+				harv.UnblockRefinery(self);
-						if (distSquared > searchRadiusSquared)
-							return EstimateDistance(loc, searchFromLoc) + Constants.CellCost * 2;
 
-						// Get the resource at this location:
+				// Only do this if UnblockRefinery did nothing.
-						var resType = resLayer.GetResource(loc);
+				if (self.Location == cachedPosition)
+				{
+					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);
+				}
+
+				var randFrames = self.World.SharedRandom.Next(100, 175);
+				return Util.SequenceActivities(NextActivity, new Wait(randFrames), this);
+			}
+			else
+			{
+				var next = this;
+
+				// Attempt to claim a resource as ours
+				if (territory != null)
+				{
+					if (!territory.ClaimResource(self, closestHarvestablePosition.Value))
+						return Util.SequenceActivities(new Wait(25), next);
+				}
+
+				// If not given a direct order, assume ordered to the first resource location we find:
+				if (!harv.LastOrderLocation.HasValue)
+					harv.LastOrderLocation = closestHarvestablePosition;
+
+				self.SetTargetLine(Target.FromCell(self.World, closestHarvestablePosition.Value), Color.Red, false);
+
+				var notify = self.TraitsImplementing<INotifyHarvesterAction>();
+
+				foreach (var n in notify)
+					n.MovingToResources(self, closestHarvestablePosition.Value, next);
+
+				return Util.SequenceActivities(mobile.MoveTo(closestHarvestablePosition.Value, 1), new HarvestResource(self), next);
+			}
+		}
+
+		bool IsHarvestable(Actor self, CPos pos)
+		{
+			var resType = resLayer.GetResource(pos);
 			if (resType == null)
-							return EstimateDistance(loc, searchFromLoc) + Constants.CellCost;
+				return false;
 
 			// Can the harvester collect this kind of resource?
 			if (!harvInfo.Resources.Contains(resType.Info.Name))
-							return EstimateDistance(loc, searchFromLoc) + Constants.CellCost;
+				return false;
 
 			if (territory != null)
 			{
 				// Another harvester has claimed this resource:
 				ResourceClaim claim;
-							if (territory.IsClaimedByAnyoneElse(self, loc, out claim))
+				if (territory.IsClaimedByAnyoneElse(self as Actor, pos, out claim))
-								return EstimateDistance(loc, searchFromLoc) + Constants.CellCost;
+					return false;
 			}
 
+			return true;
+		}
+
+		/// <summary>
+		/// Finds the closest harvestable pos between the current position of the harvester
+		/// and the last order location
+		/// </summary>
+		CPos? ClosestHarvestablePos(Actor self)
+		{
+			if (IsHarvestable(self, self.Location))
+				return self.Location;
+
+			// Determine where to search from and how far to search:
+			var searchFromLoc = harv.LastOrderLocation ?? (harv.LastLinkedProc ?? harv.LinkedProc ?? self).Location;
+			var searchRadius = harv.LastOrderLocation.HasValue ? harvInfo.SearchFromOrderRadius : harvInfo.SearchFromProcRadius;
+			var searchRadiusSquared = searchRadius * searchRadius;
+
+			var search = PathSearch.Search(self.World, mobileInfo, self, true,
+				loc => IsHarvestable(self, loc))
+				.WithCustomCost(loc =>
+				{
+					if ((avoidCell.HasValue && loc == avoidCell.Value) ||
+						(loc - self.Location).LengthSquared > searchRadiusSquared)
+						return int.MaxValue;
+
 					return 0;
 				})
-					.FromPoint(self.Location));
+				.FromPoint(self.Location)
+				.FromPoint(searchFromLoc);
 
-			var next = this;
+			// Find any harvestable resources:
+			var path = pathFinder.FindPath(search);
 
-			if (path.Count == 0)
+			if (path.Count > 0)
-			{
+				return path[0];
-				if (!harv.IsEmpty)
-					return deliver;
-				else
-				{
-					// Get out of the way if we are:
-					harv.UnblockRefinery(self);
-					var randFrames = self.World.SharedRandom.Next(90, 160);
-					if (NextActivity != null)
-						return Util.SequenceActivities(NextActivity, new Wait(randFrames), next);
-					else
-						return Util.SequenceActivities(new Wait(randFrames), next);
-				}
-			}
 
-			// Attempt to claim a resource as ours:
+			return null;
-			if (territory != null)
-			{
-				if (!territory.ClaimResource(self, path[0]))
-					return Util.SequenceActivities(new Wait(25), next);
-			}
-
-			// If not given a direct order, assume ordered to the first resource location we find:
-			if (harv.LastOrderLocation == null)
-				harv.LastOrderLocation = path[0];
-
-			self.SetTargetLine(Target.FromCell(self.World, path[0]), Color.Red, false);
-
-			var notify = self.TraitsImplementing<INotifyHarvesterAction>();
-			foreach (var n in notify)
-				n.MovingToResources(self, path[0], next);
-
-			return Util.SequenceActivities(mobile.MoveTo(path[0], 1), new HarvestResource(self), next);
-		}
-
-		// Diagonal distance heuristic
-		static int EstimateDistance(CPos here, CPos destination)
-		{
-			var diag = Math.Min(Math.Abs(here.X - destination.X), Math.Abs(here.Y - destination.Y));
-			var straight = Math.Abs(here.X - destination.X) + Math.Abs(here.Y - destination.Y);
-
-			return Constants.CellCost * straight + (Constants.DiagonalCellCost - 2 * Constants.CellCost) * diag;
 		}
 
 		public override IEnumerable<Target> GetTargets(Actor self)

OpenRA.Mods.Common/Pathfinder/BasePathSearch.cs

@@ -110,6 +110,7 @@ namespace OpenRA.Mods.Common.Pathfinder
 		public bool Debug { get; set; }
 		string id;
 		protected Func<CPos, int> heuristic;
+		protected Func<CPos, bool> isGoal;
 
 		// This member is used to compute the ID of PathSearch.
 		// Essentially, it represents a collection of the initial
@@ -194,8 +195,7 @@ namespace OpenRA.Mods.Common.Pathfinder
 
 		public bool IsTarget(CPos location)
 		{
-			var locInfo = Graph[location];
+			return isGoal(location);
-			return locInfo.EstimatedTotal - locInfo.CostSoFar == 0;
 		}
 
 		public abstract CPos Expand();

OpenRA.Mods.Common/Pathfinder/Constants.cs

@@ -25,5 +25,7 @@ namespace OpenRA.Mods.Common.Pathfinder
 		/// a unit took to move one cell diagonally)
 		/// </summary>
 		public const int DiagonalCellCost = 177;
+
+		public const int InvalidNode = int.MaxValue;
 	}
 }

OpenRA.Mods.Common/Pathfinder/PathGraph.cs

@@ -23,7 +23,7 @@ namespace OpenRA.Mods.Common.Pathfinder
 		/// <summary>
 		/// Gets all the Connections for a given node in the graph
 		/// </summary>
-		ICollection<GraphConnection> GetConnections(CPos position);
+		IEnumerable<GraphConnection> GetConnections(CPos position);
 
 		/// <summary>
 		/// Retrieves an object given a node in the graph
@@ -102,7 +102,7 @@ namespace OpenRA.Mods.Common.Pathfinder
 			new[] { new CVec(1, -1), new CVec(1, 0), new CVec(-1, 1), new CVec(0, 1), new CVec(1, 1) },
 		};
 
-		public ICollection<GraphConnection> GetConnections(CPos position)
+		public IEnumerable<GraphConnection> GetConnections(CPos position)
 		{
 			var previousPos = cellInfo[position].PreviousPos;
 
@@ -116,7 +116,7 @@ namespace OpenRA.Mods.Common.Pathfinder
 			{
 				var neighbor = position + directions[i];
 				var movementCost = GetCostToNode(neighbor, directions[i]);
-				if (movementCost != InvalidNode)
+				if (movementCost != Constants.InvalidNode)
 					validNeighbors.AddLast(new GraphConnection(position, neighbor, movementCost));
 			}
 
@@ -137,7 +137,7 @@ namespace OpenRA.Mods.Common.Pathfinder
 				return CalculateCellCost(destNode, direction, movementCost);
 			}
 
-			return InvalidNode;
+			return Constants.InvalidNode;
 		}
 
 		int CalculateCellCost(CPos neighborCPos, CVec direction, int movementCost)
@@ -148,7 +148,13 @@ namespace OpenRA.Mods.Common.Pathfinder
 				cellCost = (cellCost * 34) / 24;
 
 			if (CustomCost != null)
-				cellCost += CustomCost(neighborCPos);
+			{
+				var customCost = CustomCost(neighborCPos);
+				if (customCost == Constants.InvalidNode)
+					return Constants.InvalidNode;
+
+				cellCost += customCost;
+			}
 
 			// directional bonuses for smoother flow!
 			if (LaneBias != 0)
@@ -184,15 +190,8 @@ namespace OpenRA.Mods.Common.Pathfinder
 
 		public CellInfo this[CPos pos]
 		{
-			get
+			get { return cellInfo[pos]; }
-			{
+			set { cellInfo[pos] = value; }
-				return cellInfo[pos];
-			}
-
-			set
-			{
-				cellInfo[pos] = value;
-			}
 		}
 	}
 }

OpenRA.Mods.Common/Pathfinder/PathSearch.cs

@@ -8,6 +8,7 @@
  */
 #endregion
 
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using OpenRA.Mods.Common.Traits;
@@ -32,10 +33,13 @@ namespace OpenRA.Mods.Common.Pathfinder
 			considered = new LinkedList<Pair<CPos, int>>();
 		}
 
-		public static IPathSearch Search(World world, MobileInfo mi, Actor self, bool checkForBlocked)
+		public static IPathSearch Search(World world, MobileInfo mi, Actor self, bool checkForBlocked, Func<CPos, bool> goalCondition)
 		{
 			var graph = new PathGraph(CellInfoLayerManager.Instance.NewLayer(world.Map), mi, self, world, checkForBlocked);
-			return new PathSearch(graph);
+			var search = new PathSearch(graph);
+			search.isGoal = goalCondition;
+			search.heuristic = loc => 0;
+			return search;
 		}
 
 		public static IPathSearch FromPoint(World world, MobileInfo mi, Actor self, CPos from, CPos target, bool checkForBlocked)
@@ -46,6 +50,12 @@ namespace OpenRA.Mods.Common.Pathfinder
 				heuristic = DefaultEstimator(target)
 			};
 
+			search.isGoal = loc =>
+			{
+				var locInfo = search.Graph[loc];
+				return locInfo.EstimatedTotal - locInfo.CostSoFar == 0;
+			};
+
 			if (world.Map.Contains(from))
 				search.AddInitialCell(from);
 
@@ -60,6 +70,12 @@ namespace OpenRA.Mods.Common.Pathfinder
 				heuristic = DefaultEstimator(target)
 			};
 
+			search.isGoal = loc =>
+			{
+				var locInfo = search.Graph[loc];
+				return locInfo.EstimatedTotal - locInfo.CostSoFar == 0;
+			};
+
 			foreach (var sl in froms.Where(sl => world.Map.Contains(sl)))
 				search.AddInitialCell(sl);
 
@@ -88,12 +104,8 @@ namespace OpenRA.Mods.Common.Pathfinder
 			var currentCell = Graph[currentMinNode];
 			Graph[currentMinNode] = new CellInfo(currentCell.CostSoFar, currentCell.EstimatedTotal, currentCell.PreviousPos, CellStatus.Closed);
 
-			if (Graph.CustomCost != null)
+			if (Graph.CustomCost != null && Graph.CustomCost(currentMinNode) == Constants.InvalidNode)
-			{
-				var c = Graph.CustomCost(currentMinNode);
-				if (c == int.MaxValue)
 				return currentMinNode;
-			}
 
 			foreach (var connection in Graph.GetConnections(currentMinNode))
 			{

OpenRA.Mods.Common/Traits/Harvester.cs

@@ -26,6 +26,9 @@ namespace OpenRA.Mods.Common.Traits
 		[Desc("How long (in ticks) to wait until (re-)checking for a nearby available DeliveryBuilding if not yet linked to one.")]
 		public readonly int SearchForDeliveryBuildingDelay = 125;
 
+		[Desc("Cell to move to when automatically unblocking DeliveryBuilding.")]
+		public readonly CVec UnblockCell = new CVec(0, 4);
+
 		[Desc("How much resources it can carry.")]
 		public readonly int Capacity = 28;
 
@@ -164,7 +167,7 @@ namespace OpenRA.Mods.Common.Traits
 
 						// 4 harvesters clogs up the refinery's delivery location:
 						if (occupancy >= 3)
-							return int.MaxValue;
+							return Constants.InvalidNode;
 
 						// Prefer refineries with less occupancy (multiplier is to offset distance cost):
 						return occupancy * 12;
@@ -196,7 +199,8 @@ namespace OpenRA.Mods.Common.Traits
 				if (self.Location == deliveryLoc)
 				{
 					// Get out of the way:
-					var moveTo = LastHarvestedCell ?? (deliveryLoc + new CVec(0, 4));
+					var unblockCell = LastHarvestedCell ?? (deliveryLoc + info.UnblockCell);
+					var moveTo = mobile.NearestMoveableCell(unblockCell, 1, 5);
 					self.QueueActivity(mobile.MoveTo(moveTo, 1));
 					self.SetTargetLine(Target.FromCell(self.World, moveTo), Color.Gray, false);
 
@@ -209,8 +213,7 @@ namespace OpenRA.Mods.Common.Traits
 					foreach (var n in notify)
 						n.MovingToResources(self, moveTo, next);
 
-					self.QueueActivity(new FindResources(self));
+					self.QueueActivity(next);
-					return;
 				}
 			}
 		}
@@ -320,51 +323,46 @@ namespace OpenRA.Mods.Common.Traits
 
 				self.CancelActivity();
 
+				CPos? loc;
 				if (order.TargetLocation != CPos.Zero)
 				{
-					var loc = order.TargetLocation;
+					loc = order.TargetLocation;
-					var territory = self.World.WorldActor.TraitOrDefault<ResourceClaimLayer>();
 
+					var territory = self.World.WorldActor.TraitOrDefault<ResourceClaimLayer>();
 					if (territory != null)
 					{
 						// Find the nearest claimable cell to the order location (useful for group-select harvest):
-						loc = mobile.NearestCell(loc, p => mobile.CanEnterCell(p) && territory.ClaimResource(self, p), 1, 6);
+						loc = mobile.NearestCell(loc.Value, p => mobile.CanEnterCell(p) && territory.ClaimResource(self, p), 1, 6);
 					}
 					else
 					{
 						// Find the nearest cell to the order location (useful for group-select harvest):
 						var taken = new HashSet<CPos>();
-						loc = mobile.NearestCell(loc, p => mobile.CanEnterCell(p) && taken.Add(p), 1, 6);
+						loc = mobile.NearestCell(loc.Value, p => mobile.CanEnterCell(p) && taken.Add(p), 1, 6);
 					}
-
-					self.QueueActivity(mobile.MoveTo(loc, 0));
-					self.SetTargetLine(Target.FromCell(self.World, loc), Color.Red);
-
-					var notify = self.TraitsImplementing<INotifyHarvesterAction>();
-					var next = new FindResources(self);
-					foreach (var n in notify)
-						n.MovingToResources(self, loc, next);
-
-					LastOrderLocation = loc;
 				}
 				else
 				{
 					// A bot order gives us a CPos.Zero TargetLocation, so find some good resources for him:
-					var loc = FindNextResourceForBot(self);
+					loc = FindNextResourceForBot(self);
 
 					// No more resources? Oh well.
 					if (!loc.HasValue)
 						return;
+				}
 
-					self.QueueActivity(mobile.MoveTo(loc.Value, 0));
+				var next = new FindResources(self);
+				self.QueueActivity(next);
 				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);
+
 				LastOrderLocation = loc;
-				}
 
 				// This prevents harvesters returning to an empty patch when the player orders them to a new patch:
 				LastHarvestedCell = LastOrderLocation;
-				self.QueueActivity(new FindResources(self));
 			}
 			else if (order.OrderString == "Deliver")
 			{
@@ -413,32 +411,31 @@ namespace OpenRA.Mods.Common.Traits
 
 			// Find any harvestable resources:
 			var path = self.World.WorldActor.Trait<IPathFinder>().FindPath(
-				PathSearch.Search(self.World, mobileInfo, self, true)
+				PathSearch.Search(self.World, mobileInfo, self, true,
-					.WithHeuristic(loc =>
+					loc =>
 					{
-						// Get the resource at this location:
 						var resType = resLayer.GetResource(loc);
 						if (resType == null)
-							return Constants.CellCost;
+							return false;
 
 						// Can the harvester collect this kind of resource?
 						if (!harvInfo.Resources.Contains(resType.Info.Name))
-							return Constants.CellCost;
+							return false;
 
 						if (territory != null)
 						{
 							// Another harvester has claimed this resource:
 							ResourceClaim claim;
 							if (territory.IsClaimedByAnyoneElse(self, loc, out claim))
-								return Constants.CellCost;
+								return false;
 						}
 
-						return 0;
+						return true;
 					})
 					.FromPoint(self.Location));
 
 			if (path.Count == 0)
-				return (CPos?)null;
+				return null;
 
 			return path[0];
 		}

OpenRA.Mods.D2k/Traits/Carryable.cs

@@ -10,6 +10,7 @@
 
 using System.Linq;
 using OpenRA.Activities;
+using OpenRA.Mods.Common.Activities;
 using OpenRA.Mods.Common.Traits;
 using OpenRA.Traits;
 
@@ -113,8 +114,16 @@ namespace OpenRA.Mods.D2k.Traits
 			locked = false;
 
 			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.HasTrait<Harvester>() && !(afterLandActivity is DeliverResources);
+				if (findResources)
+					self.QueueActivity(new FindResources(self));
+				else
 					self.QueueActivity(false, afterLandActivity);
 			}
+		}
 
 		public bool Reserve(Actor carrier)
 		{
@@ -148,7 +157,7 @@ namespace OpenRA.Mods.D2k.Traits
 			if (locked || !WantsTransport)
 				return false;
 
-			// Last change to change our mind...
+			// Last chance to change our mind...
 			var destPos = self.World.Map.CenterOfCell(Destination);
 			if ((self.CenterPosition - destPos).LengthSquared < info.MinDistance.LengthSquared)
 			{