From afd8b9ab8675539eed1c7306166be435b906ae56 Mon Sep 17 00:00:00 2001 From: Paul Chote Date: Sat, 15 Jul 2017 12:36:41 +0000 Subject: [PATCH] Rework harvester resource claiming: * Maintains lists of claims, and only restricts reservations for friendly units. * Removes OnNotifyResourceClaimLost; it's not clear whether that is still useful, and it prevents future necessary cleanups. * Moves other code without changing behaviour. This fixed stale claims from dead units and enemy claims from preventing otherwise valid harvest activities. --- OpenRA.Mods.Common/AI/HackyAI.cs | 25 ++-- .../Activities/FindResources.cs | 16 ++- .../Activities/HarvestResource.cs | 13 +- OpenRA.Mods.Common/ActorExts.cs | 26 ---- OpenRA.Mods.Common/OpenRA.Mods.Common.csproj | 1 - OpenRA.Mods.Common/Traits/Harvester.cs | 52 ++++---- .../Traits/World/ResourceClaim.cs | 21 ---- .../Traits/World/ResourceClaimLayer.cs | 112 +++++------------- OpenRA.Mods.Common/TraitsInterfaces.cs | 5 - 9 files changed, 79 insertions(+), 192 deletions(-) delete mode 100644 OpenRA.Mods.Common/Traits/World/ResourceClaim.cs diff --git a/OpenRA.Mods.Common/AI/HackyAI.cs b/OpenRA.Mods.Common/AI/HackyAI.cs index 380e780ae4..e45586a30b 100644 --- a/OpenRA.Mods.Common/AI/HackyAI.cs +++ b/OpenRA.Mods.Common/AI/HackyAI.cs @@ -263,8 +263,7 @@ namespace OpenRA.Mods.Common.AI public Player Player { get; private set; } readonly DomainIndex domainIndex; - readonly ResourceLayer resLayer; - readonly ResourceClaimLayer territory; + readonly ResourceClaimLayer claimLayer; readonly IPathFinder pathfinder; readonly Func isEnemyUnit; @@ -311,8 +310,7 @@ namespace OpenRA.Mods.Common.AI return; domainIndex = World.WorldActor.Trait(); - resLayer = World.WorldActor.Trait(); - territory = World.WorldActor.TraitOrDefault(); + claimLayer = World.WorldActor.TraitOrDefault(); pathfinder = World.WorldActor.Trait(); isEnemyUnit = unit => @@ -769,19 +767,22 @@ namespace OpenRA.Mods.Common.AI return targets.MinByOrDefault(target => (target.Actor.CenterPosition - capturer.CenterPosition).LengthSquared); } - CPos FindNextResource(Actor harvester) + CPos FindNextResource(Actor actor, Harvester harv) { - var harvInfo = harvester.Info.TraitInfo(); - var mobileInfo = harvester.Info.TraitInfo(); + var mobileInfo = actor.Info.TraitInfo(); var passable = (uint)mobileInfo.GetMovementClass(World.Map.Rules.TileSet); + Func isValidResource = cell => + domainIndex.IsPassable(actor.Location, cell, mobileInfo, passable) && + harv.CanHarvestCell(actor, cell) && + claimLayer.CanClaimCell(actor, cell); + var path = pathfinder.FindPath( - PathSearch.Search(World, mobileInfo, harvester, true, - loc => domainIndex.IsPassable(harvester.Location, loc, mobileInfo, passable) && harvester.CanHarvestAt(loc, resLayer, harvInfo, territory)) + PathSearch.Search(World, mobileInfo, actor, true, isValidResource) .WithCustomCost(loc => World.FindActorsInCircle(World.Map.CenterOfCell(loc), Info.HarvesterEnemyAvoidanceRadius) - .Where(u => !u.IsDead && harvester.Owner.Stances[u.Owner] == Stance.Enemy) + .Where(u => !u.IsDead && actor.Owner.Stances[u.Owner] == Stance.Enemy) .Sum(u => Math.Max(WDist.Zero.Length, Info.HarvesterEnemyAvoidanceRadius.Length - (World.Map.CenterOfCell(loc) - u.CenterPosition).Length))) - .FromPoint(harvester.Location)); + .FromPoint(actor.Location)); if (path.Count == 0) return CPos.Zero; @@ -809,7 +810,7 @@ namespace OpenRA.Mods.Common.AI continue; // Tell the idle harvester to quit slacking: - var newSafeResourcePatch = FindNextResource(harvester); + var newSafeResourcePatch = FindNextResource(harvester, harv); BotDebug("AI: Harvester {0} is idle. Ordering to {1} in search for new resources.".F(harvester, newSafeResourcePatch)); QueueOrder(new Order("Harvest", harvester, false) { TargetLocation = newSafeResourcePatch }); } diff --git a/OpenRA.Mods.Common/Activities/FindResources.cs b/OpenRA.Mods.Common/Activities/FindResources.cs index cec2512fdf..219ece03c1 100644 --- a/OpenRA.Mods.Common/Activities/FindResources.cs +++ b/OpenRA.Mods.Common/Activities/FindResources.cs @@ -24,8 +24,7 @@ namespace OpenRA.Mods.Common.Activities readonly HarvesterInfo harvInfo; readonly Mobile mobile; readonly MobileInfo mobileInfo; - readonly ResourceLayer resLayer; - readonly ResourceClaimLayer territory; + readonly ResourceClaimLayer claimLayer; readonly IPathFinder pathFinder; readonly DomainIndex domainIndex; @@ -37,8 +36,7 @@ namespace OpenRA.Mods.Common.Activities harvInfo = self.Info.TraitInfo(); mobile = self.Trait(); mobileInfo = self.Info.TraitInfo(); - resLayer = self.World.WorldActor.Trait(); - territory = self.World.WorldActor.TraitOrDefault(); + claimLayer = self.World.WorldActor.Trait(); pathFinder = self.World.WorldActor.Trait(); domainIndex = self.World.WorldActor.Trait(); } @@ -90,8 +88,8 @@ namespace OpenRA.Mods.Common.Activities } else { - // Attempt to claim a resource as ours - if (territory != null && !territory.ClaimResource(self, closestHarvestablePosition.Value)) + // Attempt to claim the target cell + if (!claimLayer.TryClaimCell(self, closestHarvestablePosition.Value)) return ActivityUtils.SequenceActivities(new Wait(25), this); // If not given a direct order, assume ordered to the first resource location we find: @@ -115,7 +113,7 @@ namespace OpenRA.Mods.Common.Activities /// CPos? ClosestHarvestablePos(Actor self) { - if (self.CanHarvestAt(self.Location, resLayer, harvInfo, territory)) + if (harv.CanHarvestCell(self, self.Location) && claimLayer.CanClaimCell(self, self.Location)) return self.Location; // Determine where to search from and how far to search: @@ -126,8 +124,8 @@ namespace OpenRA.Mods.Common.Activities // Find any harvestable resources: var passable = (uint)mobileInfo.GetMovementClass(self.World.Map.Rules.TileSet); List path; - using (var search = PathSearch.Search(self.World, mobileInfo, self, true, - loc => domainIndex.IsPassable(self.Location, loc, mobileInfo, passable) && self.CanHarvestAt(loc, resLayer, harvInfo, territory)) + using (var search = PathSearch.Search(self.World, mobileInfo, self, true, loc => + domainIndex.IsPassable(self.Location, loc, mobileInfo, passable) && harv.CanHarvestCell(self, loc) && claimLayer.CanClaimCell(self, loc)) .WithCustomCost(loc => { if ((avoidCell.HasValue && loc == avoidCell.Value) || diff --git a/OpenRA.Mods.Common/Activities/HarvestResource.cs b/OpenRA.Mods.Common/Activities/HarvestResource.cs index ae1c916fad..77eca28ca6 100644 --- a/OpenRA.Mods.Common/Activities/HarvestResource.cs +++ b/OpenRA.Mods.Common/Activities/HarvestResource.cs @@ -20,7 +20,7 @@ namespace OpenRA.Mods.Common.Activities readonly Harvester harv; readonly HarvesterInfo harvInfo; readonly IFacing facing; - readonly ResourceClaimLayer territory; + readonly ResourceClaimLayer claimLayer; readonly ResourceLayer resLayer; readonly BodyOrientation body; @@ -30,7 +30,7 @@ namespace OpenRA.Mods.Common.Activities harvInfo = self.Info.TraitInfo(); facing = self.Trait(); body = self.Trait(); - territory = self.World.WorldActor.TraitOrDefault(); + claimLayer = self.World.WorldActor.Trait(); resLayer = self.World.WorldActor.Trait(); } @@ -38,8 +38,7 @@ namespace OpenRA.Mods.Common.Activities { if (IsCanceled) { - if (territory != null) - territory.UnclaimByActor(self); + claimLayer.RemoveClaim(self); return NextActivity; } @@ -47,8 +46,7 @@ namespace OpenRA.Mods.Common.Activities if (harv.IsFull) { - if (territory != null) - territory.UnclaimByActor(self); + claimLayer.RemoveClaim(self); return NextActivity; } @@ -64,8 +62,7 @@ namespace OpenRA.Mods.Common.Activities var resource = resLayer.Harvest(self.Location); if (resource == null) { - if (territory != null) - territory.UnclaimByActor(self); + claimLayer.RemoveClaim(self); return NextActivity; } diff --git a/OpenRA.Mods.Common/ActorExts.cs b/OpenRA.Mods.Common/ActorExts.cs index 913e35f8d5..38b985a83f 100644 --- a/OpenRA.Mods.Common/ActorExts.cs +++ b/OpenRA.Mods.Common/ActorExts.cs @@ -122,32 +122,6 @@ namespace OpenRA.Mods.Common NotifyBlocker(self, positions.SelectMany(p => self.World.ActorMap.GetActorsAt(p))); } - public static bool CanHarvestAt(this Actor self, CPos pos, ResourceLayer resLayer, HarvesterInfo harvInfo, - ResourceClaimLayer territory) - { - // Resources only exist in the ground layer - if (pos.Layer != 0) - return false; - - var resType = resLayer.GetResource(pos); - if (resType == null) - return false; - - // Can the harvester collect this kind of resource? - if (!harvInfo.Resources.Contains(resType.Info.Type)) - return false; - - if (territory != null) - { - // Another harvester has claimed this resource: - ResourceClaim claim; - if (territory.IsClaimedByAnyoneElse(self as Actor, pos, out claim)) - return false; - } - - return true; - } - public static CPos ClosestCell(this Actor self, IEnumerable cells) { return cells.MinByOrDefault(c => (self.Location - c).LengthSquared); diff --git a/OpenRA.Mods.Common/OpenRA.Mods.Common.csproj b/OpenRA.Mods.Common/OpenRA.Mods.Common.csproj index cacb933d83..f251250015 100644 --- a/OpenRA.Mods.Common/OpenRA.Mods.Common.csproj +++ b/OpenRA.Mods.Common/OpenRA.Mods.Common.csproj @@ -527,7 +527,6 @@ - diff --git a/OpenRA.Mods.Common/Traits/Harvester.cs b/OpenRA.Mods.Common/Traits/Harvester.cs index f0c69b265b..d802b1b852 100644 --- a/OpenRA.Mods.Common/Traits/Harvester.cs +++ b/OpenRA.Mods.Common/Traits/Harvester.cs @@ -78,10 +78,12 @@ namespace OpenRA.Mods.Common.Traits public class Harvester : IIssueOrder, IResolveOrder, IPips, IExplodeModifier, IOrderVoice, ISpeedModifier, ISync, INotifyCreated, - INotifyResourceClaimLost, INotifyIdle, INotifyBlockingMove, INotifyBuildComplete + INotifyIdle, INotifyBlockingMove, INotifyBuildComplete { public readonly HarvesterInfo Info; readonly Mobile mobile; + readonly ResourceLayer resLayer; + readonly ResourceClaimLayer claimLayer; Dictionary contents = new Dictionary(); bool idleSmart = true; int idleDuration; @@ -108,6 +110,9 @@ namespace OpenRA.Mods.Common.Traits { Info = info; mobile = self.Trait(); + resLayer = self.World.WorldActor.Trait(); + claimLayer = self.World.WorldActor.Trait(); + self.QueueActivity(new CallFunc(() => ChooseNewProc(self, null))); } @@ -211,8 +216,10 @@ namespace OpenRA.Mods.Common.Traits public void AcceptResource(ResourceType type) { - if (!contents.ContainsKey(type.Info)) contents[type.Info] = 1; - else contents[type.Info]++; + if (!contents.ContainsKey(type.Info)) + contents[type.Info] = 1; + else + contents[type.Info]++; } public void UnblockRefinery(Actor self) @@ -309,6 +316,20 @@ namespace OpenRA.Mods.Common.Traits return contents.Count == 0; } + public bool CanHarvestCell(Actor self, CPos cell) + { + // Resources only exist in the ground layer + if (cell.Layer != 0) + return false; + + var resType = resLayer.GetResource(cell); + if (resType == null) + return false; + + // Can the harvester collect this kind of resource? + return Info.Resources.Contains(resType.Info.Type); + } + public IEnumerable Orders { get @@ -355,20 +376,8 @@ namespace OpenRA.Mods.Common.Traits CPos? loc; if (order.TargetLocation != CPos.Zero) { - loc = order.TargetLocation; - - var territory = self.World.WorldActor.TraitOrDefault(); - if (territory != null) - { - // Find the nearest claimable cell to the order location (useful for group-select harvest): - 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(); - loc = mobile.NearestCell(loc.Value, p => mobile.CanEnterCell(p) && taken.Add(p), 1, 6); - } + // Find the nearest claimable cell to the order location (useful for group-select harvest): + loc = mobile.NearestCell(order.TargetLocation, p => mobile.CanEnterCell(p) && claimLayer.TryClaimCell(self, p), 1, 6); } else { @@ -423,15 +432,6 @@ namespace OpenRA.Mods.Common.Traits } } - public void OnNotifyResourceClaimLost(Actor self, ResourceClaim claim, Actor claimer) - { - if (self == claimer) return; - - // Our claim on a resource was stolen, find more unclaimed resources: - self.CancelActivity(); - self.QueueActivity(new FindResources(self)); - } - PipType GetPipAt(int i) { var n = i * Info.Capacity / Info.PipCount; diff --git a/OpenRA.Mods.Common/Traits/World/ResourceClaim.cs b/OpenRA.Mods.Common/Traits/World/ResourceClaim.cs deleted file mode 100644 index 69c4d31bb5..0000000000 --- a/OpenRA.Mods.Common/Traits/World/ResourceClaim.cs +++ /dev/null @@ -1,21 +0,0 @@ -#region Copyright & License Information -/* - * Copyright 2007-2017 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 - -namespace OpenRA.Mods.Common.Traits -{ - public sealed class ResourceClaim - { - public readonly Actor Claimer; - public CPos Cell; - - public ResourceClaim(Actor claimer, CPos cell) { Claimer = claimer; Cell = cell; } - } -} diff --git a/OpenRA.Mods.Common/Traits/World/ResourceClaimLayer.cs b/OpenRA.Mods.Common/Traits/World/ResourceClaimLayer.cs index 2fffc0047e..678fedc8f7 100644 --- a/OpenRA.Mods.Common/Traits/World/ResourceClaimLayer.cs +++ b/OpenRA.Mods.Common/Traits/World/ResourceClaimLayer.cs @@ -10,6 +10,7 @@ #endregion using System.Collections.Generic; +using System.Linq; using OpenRA.Graphics; using OpenRA.Traits; @@ -18,112 +19,55 @@ namespace OpenRA.Mods.Common.Traits [Desc("Allows harvesters to coordinate their operations. Attach this to the world actor.")] public sealed class ResourceClaimLayerInfo : TraitInfo { } - public sealed class ResourceClaimLayer : IWorldLoaded + public sealed class ResourceClaimLayer { - Dictionary claimByCell; - Dictionary claimByActor; - - private void MakeClaim(Actor claimer, CPos cell) - { - UnclaimByActor(claimer); - UnclaimByCell(cell, claimer); - claimByActor[claimer] = claimByCell[cell] = new ResourceClaim(claimer, cell); - } - - private void Unclaim(ResourceClaim claim, Actor claimer) - { - if (claimByActor.Remove(claim.Claimer) & claimByCell.Remove(claim.Cell)) - { - if (claim.Claimer.Disposed) return; - if (!claim.Claimer.IsInWorld) return; - if (claim.Claimer.IsDead) return; - - claim.Claimer.Trait().OnNotifyResourceClaimLost(claim.Claimer, claim, claimer); - } - } - - public void WorldLoaded(World w, WorldRenderer wr) - { - // 32 seems a sane default initial capacity for the total # of harvesters in a game. Purely a guesstimate. - claimByCell = new Dictionary(32); - claimByActor = new Dictionary(32); - } + readonly Dictionary> claimByCell = new Dictionary>(32); + readonly Dictionary claimByActor = new Dictionary(32); /// - /// Attempt to claim the resource at the cell for the given actor. + /// Attempt to reserve the resource in a cell for the given actor. /// - /// - /// - /// - public bool ClaimResource(Actor claimer, CPos cell) + public bool TryClaimCell(Actor claimer, CPos cell) { - // Has anyone else claimed this point? - ResourceClaim claim; - if (claimByCell.TryGetValue(cell, out claim)) - { - // Same claimer: - if (claim.Claimer == claimer) return true; + var claimers = claimByCell.GetOrAdd(cell); - // This is to prevent in-fighting amongst friendly harvesters: - if (claimer.Owner == claim.Claimer.Owner) return false; - if (claimer.Owner.Stances[claim.Claimer.Owner] == Stance.Ally) return false; + // Clean up any stale claims + claimers.RemoveAll(a => a.IsDead); - // If an enemy/neutral claimed this, don't respect that claim: - } + // Prevent harvesters from the player or their allies fighting over the same cell + if (claimers.Any(c => c != claimer && claimer.Owner.IsAlliedWith(c.Owner))) + return false; - // Either nobody else claims this point or an enemy/neutral claims it: - MakeClaim(claimer, cell); + // Remove the actor's last claim, if it has one + CPos lastClaim; + if (claimByActor.TryGetValue(claimer, out lastClaim)) + claimByCell.GetOrAdd(lastClaim).Remove(claimer); + + claimers.Add(claimer); + claimByActor[claimer] = cell; return true; } /// - /// Release the last resource claim made on this cell. + /// Returns false if the cell is already reserved by an allied actor. /// - /// - public void UnclaimByCell(CPos cell, Actor claimer) + public bool CanClaimCell(Actor claimer, CPos cell) { - ResourceClaim claim; - if (claimByCell.TryGetValue(cell, out claim)) - Unclaim(claim, claimer); + return !claimByCell.GetOrAdd(cell) + .Any(c => c != claimer && !c.IsDead && claimer.Owner.IsAlliedWith(c.Owner)); } /// /// Release the last resource claim made by this actor. /// /// - public void UnclaimByActor(Actor claimer) + public void RemoveClaim(Actor claimer) { - ResourceClaim claim; - if (claimByActor.TryGetValue(claimer, out claim)) - Unclaim(claim, claimer); - } + CPos lastClaim; + if (claimByActor.TryGetValue(claimer, out lastClaim)) + claimByCell.GetOrAdd(lastClaim).Remove(claimer); - /// - /// Is the cell location claimed for harvesting by any other actor? - /// - /// - /// - /// true if already claimed by an ally that isn't ; false otherwise. - public bool IsClaimedByAnyoneElse(Actor self, CPos cell, out ResourceClaim claim) - { - if (claimByCell.TryGetValue(cell, out claim)) - { - // Same claimer: - if (claim.Claimer == self) return false; - - // This is to prevent in-fighting amongst friendly harvesters: - if (self.Owner == claim.Claimer.Owner) return true; - if (self.Owner.Stances[claim.Claimer.Owner] == Stance.Ally) return true; - - // If an enemy/neutral claimed this, don't respect that claim and fall through: - } - else - { - // No claim. - claim = null; - } - - return false; + claimByActor.Remove(claimer); } } } diff --git a/OpenRA.Mods.Common/TraitsInterfaces.cs b/OpenRA.Mods.Common/TraitsInterfaces.cs index e15f00da4f..e8e1a60378 100644 --- a/OpenRA.Mods.Common/TraitsInterfaces.cs +++ b/OpenRA.Mods.Common/TraitsInterfaces.cs @@ -28,11 +28,6 @@ namespace OpenRA.Mods.Common.Traits int QuantizedBodyFacings(ActorInfo ai, SequenceProvider sequenceProvider, string race); } - public interface INotifyResourceClaimLost - { - void OnNotifyResourceClaimLost(Actor self, ResourceClaim claim, Actor claimer); - } - public interface IPlaceBuildingDecorationInfo : ITraitInfo { IEnumerable Render(WorldRenderer wr, World w, ActorInfo ai, WPos centerPosition);