From 2fe7e1bff9ee60f53cf8bfec765650a824694d01 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Sun, 30 Apr 2023 12:24:49 +0100 Subject: [PATCH] Fix HierarchicalPathFinder pathing from inaccessible source locations. When a search is initiated from an inaccessible source location, a path is still allowed if there is an adjacent, accessible location the unit can move into. The local pathfinder and HierarchicalPathFinder already account for this logic, but HPF has some bugs. Firstly, when the HierarchicalPathFinder heuristic is being evaluated, it assumes all cells being explored are accessible - this is important for performance as it avoids constantly rechecking the accessibility of cells. Although this fact holds true for cells explored by the path search, it does not hold true for cells being added as the initial starting points of the search. Secondly, when checking for adjacent locations to an inaccessible source cell, we checked only if these were still on the map. This is insufficient - we need to know if movement between the source cell and the adjacent cell is possible. The fixes resolve this by: - Teaching the heuristic an extra parameter to know if the location is known to be accessible. This allow an accessibility check to be performed for starting locations which stops HPF mistakenly assuming the abstract node for that location is the one we need to consider, and to correctly check the adjacent locations and their abstract nodes instead. The parameter means will can still skip the accessibility check in the typical case where the path search is being expanded, preserving performance. - When adjacent cells are considered we now check if movement to them is possible. This stops HPF from allowing jumps over height discontinuities (i.e. no magically jumping up or down cliffs) and thinking a path is possible when it is not. --- .../Pathfinder/HierarchicalPathFinder.cs | 31 +++++++++++-------- OpenRA.Mods.Common/Pathfinder/PathSearch.cs | 21 +++++++------ 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/OpenRA.Mods.Common/Pathfinder/HierarchicalPathFinder.cs b/OpenRA.Mods.Common/Pathfinder/HierarchicalPathFinder.cs index 71e18f55c1..e67fd9ecd3 100644 --- a/OpenRA.Mods.Common/Pathfinder/HierarchicalPathFinder.cs +++ b/OpenRA.Mods.Common/Pathfinder/HierarchicalPathFinder.cs @@ -763,7 +763,7 @@ namespace OpenRA.Mods.Common.Pathfinder foreach (var dir in CVec.Directions) { var adjacentSource = source + dir; - if (!world.Map.Contains(adjacentSource)) + if (!MovementAllowedBetweenCells(source, adjacentSource)) continue; var adjacentSourceAbstractCell = AbstractCellForLocalCell(adjacentSource); @@ -959,7 +959,7 @@ namespace OpenRA.Mods.Common.Pathfinder foreach (var dir in CVec.Directions) { var adjacentSource = source + dir; - if (!world.Map.Contains(adjacentSource)) + if (!MovementAllowedBetweenCells(source, adjacentSource)) continue; var abstractAdjacentSource = AbstractCellForLocalCell(adjacentSource); @@ -1118,12 +1118,12 @@ namespace OpenRA.Mods.Common.Pathfinder /// (the heuristic) for a local path search. The abstract search must run in the opposite direction to the /// local search. So when searching from source to target, the abstract search must be from target to source. /// - Func Heuristic(PathSearch abstractSearch, int estimatedSearchSize, + Func Heuristic(PathSearch abstractSearch, int estimatedSearchSize, HashSet sources, List unpathableNodes) { var nodeForCostLookup = new Dictionary(estimatedSearchSize); var graph = (SparsePathGraph)abstractSearch.Graph; - return cell => + return (cell, knownAccessible) => { // When dealing with an unreachable source cell, the path search will check adjacent locations. // These cells may be reachable, but may represent jumping into an area cut off from the target. @@ -1131,14 +1131,19 @@ namespace OpenRA.Mods.Common.Pathfinder if (unpathableNodes != null && unpathableNodes.Contains(cell)) return PathGraph.PathCostForInvalidPath; - // All other cells searched by the heuristic are guaranteed to be reachable. + // During a search, all other cells searched by the heuristic are guaranteed to be reachable. // So we don't need to handle an abstract cell lookup failing, or the search failing to expand. - // Cells added as initial starting points for the search are filtered out if they aren't reachable. - // The search only explores accessible cells from then on. + // Cells added as initial starting points for the search might not be reachable, + // so for those we need to perform accessibility checks. // If the exceptions here do fire, they indicate a bug. The abstract graph is considering a cell to be // unreachable, but the local pathfinder thinks it is reachable. We must fix the abstract graph to also // consider the cell to be reachable. - var maybeAbstractCell = AbstractCellForLocalCellNoAccessibleCheck(cell); + CPos? maybeAbstractCell; + if (knownAccessible) + maybeAbstractCell = AbstractCellForLocalCellNoAccessibleCheck(cell); + else + maybeAbstractCell = AbstractCellForLocalCell(cell); + if (maybeAbstractCell == null) { // If the source cell is unreachable, use one of the adjacent reachable cells instead. @@ -1146,14 +1151,14 @@ namespace OpenRA.Mods.Common.Pathfinder { foreach (var dir in CVec.Directions) { - var adjacentSource = cell + dir; - if (!world.Map.Contains(adjacentSource) || - (unpathableNodes != null && unpathableNodes.Contains(adjacentSource))) + var adjacentCell = cell + dir; + if (!MovementAllowedBetweenCells(cell, adjacentCell) || + (unpathableNodes != null && unpathableNodes.Contains(adjacentCell))) continue; // Ideally we'd choose the cheapest cell rather than just any one of them, // but we're lazy and this is an edge case. - maybeAbstractCell = AbstractCellForLocalCell(adjacentSource); + maybeAbstractCell = AbstractCellForLocalCell(adjacentCell); if (maybeAbstractCell != null) break; } @@ -1258,7 +1263,7 @@ namespace OpenRA.Mods.Common.Pathfinder PathSearch GetLocalPathSearch( Actor self, IEnumerable srcs, CPos dst, Func customCost, Actor ignoreActor, BlockedByActor check, bool laneBias, Grid? grid, int heuristicWeightPercentage, - Func heuristic = null, + Func heuristic = null, bool inReverse = false, PathSearch.IRecorder recorder = null) { diff --git a/OpenRA.Mods.Common/Pathfinder/PathSearch.cs b/OpenRA.Mods.Common/Pathfinder/PathSearch.cs index dd0a97f937..c514a4d1ef 100644 --- a/OpenRA.Mods.Common/Pathfinder/PathSearch.cs +++ b/OpenRA.Mods.Common/Pathfinder/PathSearch.cs @@ -44,7 +44,7 @@ namespace OpenRA.Mods.Common.Pathfinder IRecorder recorder = null) { var graph = new MapPathGraph(LayerPoolForWorld(world), locomotor, self, world, check, customCost, ignoreActor, laneBias, false); - var search = new PathSearch(graph, loc => 0, 0, targetPredicate, recorder); + var search = new PathSearch(graph, (_, _) => 0, 0, targetPredicate, recorder); AddInitialCells(world, locomotor, self, froms, check, customCost, ignoreActor, false, search); @@ -58,7 +58,7 @@ namespace OpenRA.Mods.Common.Pathfinder Actor ignoreActor = null, bool laneBias = true, bool inReverse = false, - Func heuristic = null, + Func heuristic = null, Grid? grid = null, IRecorder recorder = null) { @@ -129,10 +129,10 @@ namespace OpenRA.Mods.Common.Pathfinder /// Locomotor used to provide terrain costs. /// The cell for which costs are to be given by the estimation function. /// A delegate that calculates the cost estimation between the and the given cell. - public static Func DefaultCostEstimator(Locomotor locomotor, CPos destination) + public static Func DefaultCostEstimator(Locomotor locomotor, CPos destination) { var estimator = DefaultCostEstimator(locomotor); - return here => estimator(here, destination); + return (here, _) => estimator(here, destination); } /// @@ -162,7 +162,7 @@ namespace OpenRA.Mods.Common.Pathfinder public IPathGraph Graph { get; } public Func TargetPredicate { get; set; } - readonly Func heuristic; + readonly Func heuristic; readonly int heuristicWeightPercentage; readonly IRecorder recorder; readonly IPriorityQueue openQueue; @@ -171,7 +171,10 @@ namespace OpenRA.Mods.Common.Pathfinder /// Initialize a new search. /// /// Graph over which the search is conducted. - /// Provides an estimation of the distance between the given cell and the target. + /// Provides an estimation of the distance between the given cell and the target. + /// The Boolean parameter indicates if the cell is known to be accessible. + /// When true, it is known accessible as it is being explored by the search. + /// When false, the cell is being considered as a starting location and might not be accessible. /// /// The search will aim for the shortest path when given a weight of 100%. /// We can allow the search to find paths that aren't optimal by changing the weight. @@ -182,7 +185,7 @@ namespace OpenRA.Mods.Common.Pathfinder /// /// Determines if the given cell is the target. /// If provided, will record all nodes explored by searches performed. - PathSearch(IPathGraph graph, Func heuristic, int heuristicWeightPercentage, Func targetPredicate, IRecorder recorder) + PathSearch(IPathGraph graph, Func heuristic, int heuristicWeightPercentage, Func targetPredicate, IRecorder recorder) { Graph = graph; this.heuristic = heuristic; @@ -202,7 +205,7 @@ namespace OpenRA.Mods.Common.Pathfinder return; } - var heuristicCost = heuristic(location); + var heuristicCost = heuristic(location, false); if (heuristicCost == PathGraph.PathCostForInvalidPath) return; @@ -272,7 +275,7 @@ namespace OpenRA.Mods.Common.Pathfinder else { // If the heuristic reports the cell is unreachable, we won't consider it. - var heuristicCost = heuristic(neighbor); + var heuristicCost = heuristic(neighbor, true); if (heuristicCost == PathGraph.PathCostForInvalidPath) continue; estimatedRemainingCostToTarget = heuristicCost * heuristicWeightPercentage / 100;