From 19760b04bd31f3c3275c02931c32a278f5a2fe82 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Sat, 25 Sep 2021 13:08:26 +0100 Subject: [PATCH] Allow the default value of a CellInfo to be an Unvisited location. In CellInfoLayerPool, instead of having to store a layer with the default values, we know we can just clear the pooled layer in order to reset it. This saves on memory, and also makes resetting marginally faster. In PathSearch, we need to amend a check to ensure a cell info is not Unvisited before we check on its cost. --- OpenRA.Game/Map/CellLayer.cs | 24 +++++-------- OpenRA.Game/Map/CellLayerBase.cs | 5 +++ OpenRA.Mods.Common/Pathfinder/CellInfo.cs | 34 ++++++++++++++----- .../Pathfinder/CellInfoLayerPool.cs | 18 +++++----- OpenRA.Mods.Common/Pathfinder/PathSearch.cs | 3 +- 5 files changed, 51 insertions(+), 33 deletions(-) diff --git a/OpenRA.Game/Map/CellLayer.cs b/OpenRA.Game/Map/CellLayer.cs index 1680d8f33f..b1e975a3b3 100644 --- a/OpenRA.Game/Map/CellLayer.cs +++ b/OpenRA.Game/Map/CellLayer.cs @@ -25,6 +25,15 @@ namespace OpenRA public CellLayer(MapGridType gridType, Size size) : base(gridType, size) { } + public override void Clear() + { + if (CellEntryChanged != null) + throw new InvalidOperationException( + "Cannot clear values when there are listeners attached to the CellEntryChanged event."); + + base.Clear(); + } + public override void CopyValuesFrom(CellLayerBase anotherLayer) { if (CellEntryChanged != null) @@ -34,21 +43,6 @@ namespace OpenRA base.CopyValuesFrom(anotherLayer); } - public static CellLayer CreateInstance(Func initialCellValueFactory, Size size, MapGridType mapGridType) - { - var cellLayer = new CellLayer(mapGridType, size); - for (var v = 0; v < size.Height; v++) - { - for (var u = 0; u < size.Width; u++) - { - var mpos = new MPos(u, v); - cellLayer[mpos] = initialCellValueFactory(mpos); - } - } - - return cellLayer; - } - // Resolve an array index from cell coordinates int Index(CPos cell) { diff --git a/OpenRA.Game/Map/CellLayerBase.cs b/OpenRA.Game/Map/CellLayerBase.cs index 42739bd9ca..9d09322f80 100644 --- a/OpenRA.Game/Map/CellLayerBase.cs +++ b/OpenRA.Game/Map/CellLayerBase.cs @@ -35,6 +35,11 @@ namespace OpenRA entries = new T[size.Width * size.Height]; } + public virtual void Clear() + { + Array.Clear(entries, 0, entries.Length); + } + public virtual void CopyValuesFrom(CellLayerBase anotherLayer) { if (Size != anotherLayer.Size || GridType != anotherLayer.GridType) diff --git a/OpenRA.Mods.Common/Pathfinder/CellInfo.cs b/OpenRA.Mods.Common/Pathfinder/CellInfo.cs index d0e113b5fd..c336eec65b 100644 --- a/OpenRA.Mods.Common/Pathfinder/CellInfo.cs +++ b/OpenRA.Mods.Common/Pathfinder/CellInfo.cs @@ -9,13 +9,15 @@ */ #endregion +using System; + namespace OpenRA.Mods.Common.Pathfinder { /// /// Describes the three states that a node in the graph can have. /// Based on A* algorithm specification /// - public enum CellStatus + public enum CellStatus : byte { Unvisited, Open, @@ -23,36 +25,52 @@ namespace OpenRA.Mods.Common.Pathfinder } /// - /// Stores information about nodes in the pathfinding graph + /// Stores information about nodes in the pathfinding graph. + /// The default value of this struct represents an location. /// public readonly struct CellInfo { /// - /// The cost to move from the start up to this node + /// The cost to move from the start up to this node. /// public readonly int CostSoFar; /// - /// The estimation of how far is the node from our goal + /// The estimation of how far this node is from our target. /// public readonly int EstimatedTotal; /// - /// The previous node of this one that follows the shortest path + /// The previous node of this one that follows the shortest path. /// public readonly CPos PreviousPos; /// - /// The status of this node + /// The status of this node. Accessing other fields is only valid when the status is not . /// public readonly CellStatus Status; public CellInfo(int costSoFar, int estimatedTotal, CPos previousPos, CellStatus status) { - CostSoFar = costSoFar; - PreviousPos = previousPos; + if (status == CellStatus.Unvisited) + throw new ArgumentException( + $"The default {nameof(CellInfo)} is the only such {nameof(CellInfo)} allowed for representing an {nameof(CellStatus.Unvisited)} location.", + nameof(status)); + Status = status; + CostSoFar = costSoFar; EstimatedTotal = estimatedTotal; + PreviousPos = previousPos; + } + + public override string ToString() + { + if (Status == CellStatus.Unvisited) + return Status.ToString(); + + return + $"{Status} {nameof(CostSoFar)}={CostSoFar} " + + $"{nameof(EstimatedTotal)}={EstimatedTotal} {nameof(PreviousPos)}={PreviousPos}"; } } } diff --git a/OpenRA.Mods.Common/Pathfinder/CellInfoLayerPool.cs b/OpenRA.Mods.Common/Pathfinder/CellInfoLayerPool.cs index 440a1ad3af..b76554747a 100644 --- a/OpenRA.Mods.Common/Pathfinder/CellInfoLayerPool.cs +++ b/OpenRA.Mods.Common/Pathfinder/CellInfoLayerPool.cs @@ -11,7 +11,6 @@ using System; using System.Collections.Generic; -using OpenRA.Primitives; namespace OpenRA.Mods.Common.Pathfinder { @@ -19,15 +18,11 @@ namespace OpenRA.Mods.Common.Pathfinder { const int MaxPoolSize = 4; readonly Stack> pool = new Stack>(MaxPoolSize); - readonly CellLayer defaultLayer; + readonly Map map; public CellInfoLayerPool(Map map) { - defaultLayer = - CellLayer.CreateInstance( - mpos => new CellInfo(PathGraph.PathCostForInvalidPath, PathGraph.PathCostForInvalidPath, mpos.ToCPos(map), CellStatus.Unvisited), - new Size(map.MapSize.X, map.MapSize.Y), - map.Grid.Type); + this.map = map; } public PooledCellInfoLayer Get() @@ -42,9 +37,14 @@ namespace OpenRA.Mods.Common.Pathfinder if (pool.Count > 0) layer = pool.Pop(); + // As the default value of CellInfo represents an Unvisited location, + // we don't need to initialize the values in the layer, + // we can just clear them to the defaults. if (layer == null) - layer = new CellLayer(defaultLayer.GridType, defaultLayer.Size); - layer.CopyValuesFrom(defaultLayer); + layer = new CellLayer(map); + else + layer.Clear(); + return layer; } diff --git a/OpenRA.Mods.Common/Pathfinder/PathSearch.cs b/OpenRA.Mods.Common/Pathfinder/PathSearch.cs index b8f34f903f..08f69a44b8 100644 --- a/OpenRA.Mods.Common/Pathfinder/PathSearch.cs +++ b/OpenRA.Mods.Common/Pathfinder/PathSearch.cs @@ -119,7 +119,8 @@ namespace OpenRA.Mods.Common.Pathfinder var neighborCell = Graph[neighborCPos]; // Cost is even higher; next direction: - if (neighborCell.Status == CellStatus.Closed || gCost >= neighborCell.CostSoFar) + if (neighborCell.Status == CellStatus.Closed || + (neighborCell.Status == CellStatus.Open && gCost >= neighborCell.CostSoFar)) continue; // Now we may seriously consider this direction using heuristics. If the cell has