From 31267aa22d5e6f11e4e8e64a53c849c48aea9b5a Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Sat, 25 Sep 2021 13:40:59 +0100 Subject: [PATCH] Fix some incorrect logic in PathGraph.GetConnections.PathGraph Firstly, when dealing with maps with height discontinuities, the neighbouring cells we need to search are more that the set we need to search on flat maps. We ensure that as we traverse a map with varying height, we now consider cells "behind" us that may have become accessible due to a height change. Secondly, when considering connections available via Custom Movement Layers, make sure the target cell on the new layer is actually enterable. Previously this cell would be reported as a valid connection, even if it wasn't actually possible to enter the cell as it was blocked. We also apply the same optimization of ignoring already closed cells. --- OpenRA.Game/Exts.cs | 5 ++ OpenRA.Mods.Common/Pathfinder/PathGraph.cs | 64 +++++++++++++++++----- 2 files changed, 56 insertions(+), 13 deletions(-) diff --git a/OpenRA.Game/Exts.cs b/OpenRA.Game/Exts.cs index 66dde71c4c..6d3fd1681e 100644 --- a/OpenRA.Game/Exts.cs +++ b/OpenRA.Game/Exts.cs @@ -375,6 +375,11 @@ namespace OpenRA return ts.Concat(moreTs); } + public static IEnumerable Exclude(this IEnumerable ts, params T[] exclusions) + { + return ts.Except(exclusions); + } + public static HashSet ToHashSet(this IEnumerable source) { return new HashSet(source); diff --git a/OpenRA.Mods.Common/Pathfinder/PathGraph.cs b/OpenRA.Mods.Common/Pathfinder/PathGraph.cs index 200297be7c..90db2fdc96 100644 --- a/OpenRA.Mods.Common/Pathfinder/PathGraph.cs +++ b/OpenRA.Mods.Common/Pathfinder/PathGraph.cs @@ -11,6 +11,7 @@ using System; using System.Collections.Generic; +using System.Linq; using OpenRA.Mods.Common.Traits; using OpenRA.Traits; @@ -116,19 +117,40 @@ namespace OpenRA.Mods.Common.Pathfinder // Sets of neighbors for each incoming direction. These exclude the neighbors which are guaranteed // to be reached more cheaply by a path through our parent cell which does not include the current cell. // For horizontal/vertical directions, the set is the three cells 'ahead'. For diagonal directions, the set - // is the three cells ahead, plus the two cells to the side, which we cannot exclude without knowing if - // the cell directly between them and our parent is passable. + // is the three cells ahead, plus the two cells to the side. Effectively, these are the cells left over + // if you ignore the ones reachable from the parent cell. + // We can do this because for any cell in range of both the current and parent location, + // if we can reach it from one we are guaranteed to be able to reach it from the other. static readonly CVec[][] DirectedNeighbors = { - new[] { new CVec(-1, -1), new CVec(0, -1), new CVec(1, -1), new CVec(-1, 0), new CVec(-1, 1) }, - new[] { new CVec(-1, -1), new CVec(0, -1), new CVec(1, -1) }, - new[] { new CVec(-1, -1), new CVec(0, -1), new CVec(1, -1), new CVec(1, 0), new CVec(1, 1) }, - new[] { new CVec(-1, -1), new CVec(-1, 0), new CVec(-1, 1) }, + new[] { new CVec(-1, -1), new CVec(0, -1), new CVec(1, -1), new CVec(-1, 0), new CVec(-1, 1) }, // TL + new[] { new CVec(-1, -1), new CVec(0, -1), new CVec(1, -1) }, // T + new[] { new CVec(-1, -1), new CVec(0, -1), new CVec(1, -1), new CVec(1, 0), new CVec(1, 1) }, // TR + new[] { new CVec(-1, -1), new CVec(-1, 0), new CVec(-1, 1) }, // L CVec.Directions, - new[] { new CVec(1, -1), new CVec(1, 0), new CVec(1, 1) }, - new[] { new CVec(-1, -1), new CVec(-1, 0), new CVec(-1, 1), new CVec(0, 1), new CVec(1, 1) }, - new[] { new CVec(-1, 1), new CVec(0, 1), new CVec(1, 1) }, - new[] { new CVec(1, -1), new CVec(1, 0), new CVec(-1, 1), new CVec(0, 1), new CVec(1, 1) }, + new[] { new CVec(1, -1), new CVec(1, 0), new CVec(1, 1) }, // R + new[] { new CVec(-1, -1), new CVec(-1, 0), new CVec(-1, 1), new CVec(0, 1), new CVec(1, 1) }, // BL + new[] { new CVec(-1, 1), new CVec(0, 1), new CVec(1, 1) }, // B + new[] { new CVec(1, -1), new CVec(1, 0), new CVec(-1, 1), new CVec(0, 1), new CVec(1, 1) }, // BR + }; + + // With height discontinuities between the parent and current cell, we cannot optimize the possible neighbors. + // It is no longer true that for any cell in range of both the current and parent location, + // if we can reach it from one we are guaranteed to be able to reach it from the other. + // This is because a height discontinuity may have prevented the parent location from reaching, + // but our current cell on a new height may be able to reach as the height difference may be small enough. + // Therefore, we can only exclude the parent cell in each set of directions. + static readonly CVec[][] DirectedNeighborsConservative = + { + CVec.Directions.Exclude(new CVec(1, 1)).ToArray(), // TL + CVec.Directions.Exclude(new CVec(0, 1)).ToArray(), // T + CVec.Directions.Exclude(new CVec(-1, 1)).ToArray(), // TR + CVec.Directions.Exclude(new CVec(1, 0)).ToArray(), // L + CVec.Directions, + CVec.Directions.Exclude(new CVec(-1, 0)).ToArray(), // R + CVec.Directions.Exclude(new CVec(1, -1)).ToArray(), // BL + CVec.Directions.Exclude(new CVec(0, -1)).ToArray(), // B + CVec.Directions.Exclude(new CVec(-1, -1)).ToArray(), // BR }; public List GetConnections(CPos position) @@ -141,7 +163,12 @@ namespace OpenRA.Mods.Common.Pathfinder var dy = position.Y - previousPos.Y; var index = dy * 3 + dx + 4; - var directions = DirectedNeighbors[index]; + var heightLayer = World.Map.Height; + var directions = + (checkTerrainHeight && layer == 0 && previousPos.Layer == 0 && heightLayer[position] != heightLayer[previousPos] + ? DirectedNeighborsConservative + : DirectedNeighbors)[index]; + var validNeighbors = new List(directions.Length + (layer == 0 ? cellInfoForLayer.Length : 1)); for (var i = 0; i < directions.Length; i++) { @@ -164,7 +191,9 @@ namespace OpenRA.Mods.Common.Pathfinder var layerPosition = new CPos(position.X, position.Y, cml.Index); var entryCost = cml.EntryMovementCost(locomotor.Info, layerPosition); - if (entryCost != MovementCostForUnreachableCell) + if (entryCost != MovementCostForUnreachableCell && + CanEnterNode(layerPosition) && + this[layerPosition].Status != CellStatus.Closed) validNeighbors.Add(new GraphConnection(layerPosition, entryCost)); } } @@ -172,13 +201,22 @@ namespace OpenRA.Mods.Common.Pathfinder { var layerPosition = new CPos(position.X, position.Y, 0); var exitCost = cmls[layer].ExitMovementCost(locomotor.Info, layerPosition); - if (exitCost != MovementCostForUnreachableCell) + if (exitCost != MovementCostForUnreachableCell && + CanEnterNode(layerPosition) && + this[layerPosition].Status != CellStatus.Closed) validNeighbors.Add(new GraphConnection(layerPosition, exitCost)); } return validNeighbors; } + bool CanEnterNode(CPos destNode) + { + return + locomotor.MovementCostToEnterCell(Actor, destNode, checkConditions, IgnoreActor) + != MovementCostForUnreachableCell; + } + int GetPathCostToNode(CPos destNode, CVec direction) { var movementCost = locomotor.MovementCostToEnterCell(Actor, destNode, checkConditions, IgnoreActor);