From a85ac26367c1053c538b77963edfeb157261a0db Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Tue, 25 Oct 2022 20:23:53 +0100 Subject: [PATCH] Pathing considers reachability of source cells consistently. Using the local pathfinder, you could not find a path to an unreachable destination cell, but it was possible to find a path from an unreachable source cell if there was a reachable cells adjacent to it. The hierarchical pathfinder did not have this behaviour and considering an unreachable source cell to block attempts to find a path. Now, we unify the pathfinders to use a consistent behaviour, allowing paths from unreachable source cells to be found. --- .../Pathfinder/HierarchicalPathFinder.cs | 151 +++++++++++++----- OpenRA.Mods.Common/Pathfinder/PathSearch.cs | 10 +- .../Traits/Buildings/TransformsIntoMobile.cs | 6 +- OpenRA.Mods.Common/Traits/Mobile.cs | 6 +- OpenRA.Mods.Common/Traits/World/Locomotor.cs | 20 ++- OpenRA.Mods.Common/Traits/World/PathFinder.cs | 12 +- 6 files changed, 145 insertions(+), 60 deletions(-) diff --git a/OpenRA.Mods.Common/Pathfinder/HierarchicalPathFinder.cs b/OpenRA.Mods.Common/Pathfinder/HierarchicalPathFinder.cs index 4521944d3f..44915832c1 100644 --- a/OpenRA.Mods.Common/Pathfinder/HierarchicalPathFinder.cs +++ b/OpenRA.Mods.Common/Pathfinder/HierarchicalPathFinder.cs @@ -73,13 +73,14 @@ namespace OpenRA.Mods.Common.Pathfinder /// nodes, but uses a heuristic informed from the previous level to guide the search in the right direction. /// /// This implementation is aware of movement costs over terrain given by - /// . It is aware of changes to the costs in terrain and able to - /// update the abstract graph when this occurs. It is able to search the abstract graph as if - /// had been specified. If is given in the - /// constructor, the abstract graph will additionally account for a subset of immovable actors using the same rules - /// as . It will be aware of changes - /// to actors on the map and update the abstract graph when this occurs. Other types of blocking actors will not be - /// accounted for in the heuristic. + /// . It is aware of + /// changes to the costs in terrain and able to update the abstract graph when this occurs. It is able to search + /// the abstract graph as if had been specified. If + /// is given in the constructor, the abstract graph will additionally + /// account for a subset of immovable actors using the same rules as + /// . It will be aware of + /// changes to actors on the map and update the abstract graph when this occurs. Other types of blocking actors + /// will not be accounted for in the heuristic. /// /// If the obstacle on the map is from terrain (e.g. a cliff or lake) the heuristic will work well. If the /// obstacle is from the subset of immovable actors (e.g. trees, walls, buildings) and @@ -620,14 +621,14 @@ namespace OpenRA.Mods.Common.Pathfinder /// /// defines immovability based on the mobile trait. The blocking rules - /// in allow units to pass these - /// immovable actors if they are temporary blockers (e.g. gates) or crushable by the locomotor. Since our - /// abstract graph must work for any actor, we have to be conservative and can only consider a subset of the - /// immovable actors in the graph - ones we know cannot be passed by some actors due to these rules. + /// in allow units to + /// pass these immovable actors if they are temporary blockers (e.g. gates) or crushable by the locomotor. + /// Since our abstract graph must work for any actor, we have to be conservative and can only consider a subset + /// of the immovable actors in the graph - ones we know cannot be passed by some actors due to these rules. /// Both this and must be true for a cell to be blocked. /// /// This method is dependant on the logic in - /// and + /// and /// . This method must be kept in sync with changes in the locomotor /// rules. /// @@ -718,24 +719,51 @@ namespace OpenRA.Mods.Common.Pathfinder pathFinderOverlay?.NewRecording(self, sources, target); + if (!world.Map.Contains(target)) + return PathFinder.NoPath; + RebuildDirtyGrids(); var targetAbstractCell = AbstractCellForLocalCell(target); if (targetAbstractCell == null) return PathFinder.NoPath; - var sourcesWithReachableNodes = new List(sources.Count); + // Unlike the target cell, the source cell is allowed to be an unreachable location. + // Instead, what matters is whether any cell adjacent to the source cell can be reached. + var sourcesWithReachableNodes = new List<(CPos Source, CPos AdjacentSource)>(sources.Count); var sourceEdges = new List(sources.Count); foreach (var source in sources) { - var sourceAbstractCell = AbstractCellForLocalCell(source); - if (sourceAbstractCell == null) + if (!world.Map.Contains(source)) continue; - sourcesWithReachableNodes.Add(source); - var sourceEdge = EdgeFromLocalToAbstract(source, sourceAbstractCell.Value); - if (sourceEdge != null) - sourceEdges.Add(sourceEdge.Value); + // The source cell is reachable, we can add an edge from there and have no need to check adjacent cells. + var sourceAbstractCell = AbstractCellForLocalCell(source); + if (sourceAbstractCell != null) + { + sourcesWithReachableNodes.Add((source, source)); + var sourceEdge = EdgeFromLocalToAbstract(source, sourceAbstractCell.Value); + if (sourceEdge != null) + sourceEdges.Add(sourceEdge.Value); + continue; + } + + // If the source cell is unreachable, we must add edges from any adjacent cells that are reachable instead. + foreach (var dir in CVec.Directions) + { + var adjacentSource = source + dir; + if (!world.Map.Contains(adjacentSource)) + continue; + + var adjacentSourceAbstractCell = AbstractCellForLocalCell(adjacentSource); + if (adjacentSourceAbstractCell == null) + continue; + + sourcesWithReachableNodes.Add((source, adjacentSource)); + var sourceEdge = EdgeFromLocalToAbstract(adjacentSource, adjacentSourceAbstractCell.Value); + if (sourceEdge != null) + sourceEdges.Add(sourceEdge.Value); + } } if (sourcesWithReachableNodes.Count == 0) @@ -751,11 +779,11 @@ namespace OpenRA.Mods.Common.Pathfinder using (var reverseAbstractSearch = PathSearch.ToTargetCellOverGraph( fullGraph.GetConnections, locomotor, target, target, estimatedSearchSize, pathFinderOverlay?.RecordAbstractEdges(self))) { - var sourcesWithPathableNodes = new List(sourcesWithReachableNodes.Count); - foreach (var source in sourcesWithReachableNodes) + var sourcesWithPathableNodes = new HashSet(sources.Count); + foreach (var (source, adjacentSource) in sourcesWithReachableNodes) { // Check if we have already found a route to this node before we attempt to expand the search. - var sourceStatus = reverseAbstractSearch.Graph[source]; + var sourceStatus = reverseAbstractSearch.Graph[adjacentSource]; if (sourceStatus.Status == CellStatus.Closed) { if (sourceStatus.CostSoFar != PathGraph.PathCostForInvalidPath) @@ -763,7 +791,7 @@ namespace OpenRA.Mods.Common.Pathfinder } else { - reverseAbstractSearch.TargetPredicate = cell => cell == source; + reverseAbstractSearch.TargetPredicate = cell => cell == adjacentSource; if (reverseAbstractSearch.ExpandToTarget()) sourcesWithPathableNodes.Add(source); } @@ -774,7 +802,7 @@ namespace OpenRA.Mods.Common.Pathfinder using (var fromSrc = GetLocalPathSearch( self, sourcesWithPathableNodes, target, customCost, ignoreActor, check, laneBias, null, heuristicWeightPercentage, - heuristic: Heuristic(reverseAbstractSearch, estimatedSearchSize), + heuristic: Heuristic(reverseAbstractSearch, estimatedSearchSize, sourcesWithPathableNodes), recorder: pathFinderOverlay?.RecordLocalEdges(self))) return fromSrc.FindPath(); } @@ -824,12 +852,18 @@ namespace OpenRA.Mods.Common.Pathfinder RebuildDirtyGrids(); + // If the target cell in unreachable, there is no path. var targetAbstractCell = AbstractCellForLocalCell(target); if (targetAbstractCell == null) return PathFinder.NoPath; + + // If the source cell in unreachable, there may still be a path. + // As long as one of the cells adjacent to the source is reachable, the path can be made. + // Call the other overload which can handle this scenario. var sourceAbstractCell = AbstractCellForLocalCell(source); if (sourceAbstractCell == null) - return PathFinder.NoPath; + return FindPath(self, new[] { source }, target, check, heuristicWeightPercentage, customCost, ignoreActor, laneBias, pathFinderOverlay); + var targetEdge = EdgeFromLocalToAbstract(target, targetAbstractCell.Value); var sourceEdge = EdgeFromLocalToAbstract(source, sourceAbstractCell.Value); @@ -852,11 +886,11 @@ namespace OpenRA.Mods.Common.Pathfinder using (var fromSrc = GetLocalPathSearch( self, new[] { source }, target, customCost, ignoreActor, check, laneBias, null, heuristicWeightPercentage, - heuristic: Heuristic(reverseAbstractSearch, estimatedSearchSize), + heuristic: Heuristic(reverseAbstractSearch, estimatedSearchSize, null), recorder: pathFinderOverlay?.RecordLocalEdges(self))) using (var fromDest = GetLocalPathSearch( self, new[] { target }, source, customCost, ignoreActor, check, laneBias, null, heuristicWeightPercentage, - heuristic: Heuristic(forwardAbstractSearch, estimatedSearchSize), + heuristic: Heuristic(forwardAbstractSearch, estimatedSearchSize, null), recorder: pathFinderOverlay?.RecordLocalEdges(self), inReverse: true)) return PathSearch.FindBidiPath(fromDest, fromSrc); @@ -884,15 +918,38 @@ namespace OpenRA.Mods.Common.Pathfinder RebuildDomains(); - var abstractSource = AbstractCellForLocalCell(source); - if (abstractSource == null) - return false; var abstractTarget = AbstractCellForLocalCell(target); if (abstractTarget == null) return false; - var sourceDomain = abstractDomains[abstractSource.Value]; var targetDomain = abstractDomains[abstractTarget.Value]; - return sourceDomain == targetDomain; + + // The source cell is reachable, we can compare the domains directly. + var abstractSource = AbstractCellForLocalCell(source); + if (abstractSource != null) + { + var sourceDomain = abstractDomains[abstractSource.Value]; + return sourceDomain == targetDomain; + } + + // Unlike the target cell, the source cell is allowed to be an unreachable location. + // Instead, what matters is whether any cell adjacent to the source cell can be reached. + // So we need to compare the domains of reachable cells adjacent to the source location. + foreach (var dir in CVec.Directions) + { + var adjacentSource = source + dir; + if (!world.Map.Contains(adjacentSource)) + continue; + + var abstractAdjacentSource = AbstractCellForLocalCell(adjacentSource); + if (abstractAdjacentSource == null) + continue; + + var adjacentSourceDomain = abstractDomains[abstractAdjacentSource.Value]; + if (adjacentSourceDomain == targetDomain) + return true; + } + + return false; } /// @@ -1005,6 +1062,7 @@ namespace OpenRA.Mods.Common.Pathfinder /// /// Maps a local cell to a abstract node in the graph. Returns null when the local cell is unreachable. + /// The cell must have been checked to be on the map with . /// CPos? AbstractCellForLocalCell(CPos localCell) { @@ -1038,7 +1096,7 @@ 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) { var nodeForCostLookup = new Dictionary(estimatedSearchSize); var graph = (SparsePathGraph)abstractSearch.Graph; @@ -1053,9 +1111,30 @@ namespace OpenRA.Mods.Common.Pathfinder // consider the cell to be reachable. var maybeAbstractCell = AbstractCellForLocalCellNoAccessibleCheck(cell); if (maybeAbstractCell == null) - throw new Exception( - "The abstract path should never be searched for an unreachable point. " + - "This is a bug. Failed lookup for an abstract cell."); + { + // If the source cell in unreachable, use one of the adjacent reachable cells instead. + if (sources != null && sources.Contains(cell)) + { + foreach (var dir in CVec.Directions) + { + var adjacentSource = cell + dir; + if (!world.Map.Contains(adjacentSource)) + 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); + if (maybeAbstractCell != null) + break; + } + } + + if (maybeAbstractCell == null) + throw new Exception( + "The abstract path should never be searched for an unreachable point. " + + "This is a bug. Failed lookup for an abstract cell."); + } + var abstractCell = maybeAbstractCell.Value; var info = graph[abstractCell]; diff --git a/OpenRA.Mods.Common/Pathfinder/PathSearch.cs b/OpenRA.Mods.Common/Pathfinder/PathSearch.cs index 84eee81d1b..96706b9b83 100644 --- a/OpenRA.Mods.Common/Pathfinder/PathSearch.cs +++ b/OpenRA.Mods.Common/Pathfinder/PathSearch.cs @@ -76,11 +76,17 @@ namespace OpenRA.Mods.Common.Pathfinder return search; } + public static bool CellAllowsMovement(World world, Locomotor locomotor, CPos cell, Func customCost) + { + return world.Map.Contains(cell) && + (cell.Layer == 0 || world.GetCustomMovementLayers()[cell.Layer].EnabledForLocomotor(locomotor.Info)) && + (customCost == null || customCost(cell) != PathGraph.PathCostForInvalidPath); + } + static void AddInitialCells(World world, Locomotor locomotor, IEnumerable froms, Func customCost, PathSearch search) { - var customMovementLayers = world.GetCustomMovementLayers(); foreach (var sl in froms) - if (world.Map.Contains(sl) && (sl.Layer == 0 || customMovementLayers[sl.Layer].EnabledForLocomotor(locomotor.Info))) + if (CellAllowsMovement(world, locomotor, sl, customCost)) search.AddInitialCell(sl, customCost); } diff --git a/OpenRA.Mods.Common/Traits/Buildings/TransformsIntoMobile.cs b/OpenRA.Mods.Common/Traits/Buildings/TransformsIntoMobile.cs index e8d9552af5..56ea57a4c7 100644 --- a/OpenRA.Mods.Common/Traits/Buildings/TransformsIntoMobile.cs +++ b/OpenRA.Mods.Common/Traits/Buildings/TransformsIntoMobile.cs @@ -211,10 +211,8 @@ namespace OpenRA.Mods.Common.Traits bool CanEnterCell(Actor self, CPos cell) { - if (mobile.locomotor.MovementCostForCell(cell) == PathGraph.MovementCostForUnreachableCell) - return false; - - return mobile.locomotor.CanMoveFreelyInto(self, cell, BlockedByActor.All, null); + return mobile.locomotor.MovementCostToEnterCell( + self, cell, BlockedByActor.All, null) != PathGraph.MovementCostForUnreachableCell; } } } diff --git a/OpenRA.Mods.Common/Traits/Mobile.cs b/OpenRA.Mods.Common/Traits/Mobile.cs index 2c08e7a819..228ff39004 100644 --- a/OpenRA.Mods.Common/Traits/Mobile.cs +++ b/OpenRA.Mods.Common/Traits/Mobile.cs @@ -124,10 +124,8 @@ namespace OpenRA.Mods.Common.Traits locomotor = world.WorldActor.TraitsImplementing() .SingleOrDefault(l => l.Info.Name == Locomotor); - if (locomotor.MovementCostForCell(cell) == PathGraph.MovementCostForUnreachableCell) - return false; - - return locomotor.CanMoveFreelyInto(self, cell, subCell, check, ignoreActor); + return locomotor.MovementCostToEnterCell( + self, cell, check, ignoreActor, subCell) != PathGraph.MovementCostForUnreachableCell; } public bool CanStayInCell(World world, CPos cell) diff --git a/OpenRA.Mods.Common/Traits/World/Locomotor.cs b/OpenRA.Mods.Common/Traits/World/Locomotor.cs index bfae88fa2a..2e5d46a43a 100644 --- a/OpenRA.Mods.Common/Traits/World/Locomotor.cs +++ b/OpenRA.Mods.Common/Traits/World/Locomotor.cs @@ -204,24 +204,30 @@ namespace OpenRA.Mods.Common.Traits return terrainInfos[index].Speed; } + public short MovementCostToEnterCell(Actor actor, CPos destNode, BlockedByActor check, Actor ignoreActor, SubCell subCell = SubCell.FullCell) + { + var cellCost = MovementCostForCell(destNode); + + if (cellCost == PathGraph.MovementCostForUnreachableCell || + !CanMoveFreelyInto(actor, destNode, subCell, check, ignoreActor)) + return PathGraph.MovementCostForUnreachableCell; + + return cellCost; + } + public short MovementCostToEnterCell(Actor actor, CPos srcNode, CPos destNode, BlockedByActor check, Actor ignoreActor) { var cellCost = MovementCostForCell(destNode, srcNode); if (cellCost == PathGraph.MovementCostForUnreachableCell || - !CanMoveFreelyInto(actor, destNode, check, ignoreActor)) + !CanMoveFreelyInto(actor, destNode, SubCell.FullCell, check, ignoreActor)) return PathGraph.MovementCostForUnreachableCell; return cellCost; } // Determines whether the actor is blocked by other Actors - public bool CanMoveFreelyInto(Actor actor, CPos cell, BlockedByActor check, Actor ignoreActor) - { - return CanMoveFreelyInto(actor, cell, SubCell.FullCell, check, ignoreActor); - } - - public bool CanMoveFreelyInto(Actor actor, CPos cell, SubCell subCell, BlockedByActor check, Actor ignoreActor) + bool CanMoveFreelyInto(Actor actor, CPos cell, SubCell subCell, BlockedByActor check, Actor ignoreActor) { // If the check allows: We are not blocked by other actors. if (check == BlockedByActor.None) diff --git a/OpenRA.Mods.Common/Traits/World/PathFinder.cs b/OpenRA.Mods.Common/Traits/World/PathFinder.cs index d902fadb4f..d163855da3 100644 --- a/OpenRA.Mods.Common/Traits/World/PathFinder.cs +++ b/OpenRA.Mods.Common/Traits/World/PathFinder.cs @@ -93,11 +93,9 @@ namespace OpenRA.Mods.Common.Traits var locomotor = GetActorLocomotor(self); // If the target cell is inaccessible, bail early. - var inaccessible = - !world.Map.Contains(target) || - !locomotor.CanMoveFreelyInto(self, target, check, ignoreActor) || - (customCost != null && customCost(target) == PathGraph.PathCostForInvalidPath); - if (inaccessible) + // The destination cell must allow movement and also have a reachable movement cost. + if (!PathSearch.CellAllowsMovement(self.World, locomotor, target, customCost) + || locomotor.MovementCostToEnterCell(self, target, check, ignoreActor) == PathGraph.MovementCostForUnreachableCell) return NoPath; // When searching from only one source cell, some optimizations are possible. @@ -109,8 +107,8 @@ namespace OpenRA.Mods.Common.Traits if (source.Layer == target.Layer && (source - target).LengthSquared < 3) { // If the source cell is inaccessible, there is no path. - if (!world.Map.Contains(source) || - (customCost != null && customCost(source) == PathGraph.PathCostForInvalidPath)) + // Unlike the destination cell, the source cell is allowed to have an unreachable movement cost. + if (!PathSearch.CellAllowsMovement(self.World, locomotor, source, customCost)) return NoPath; return new List(2) { target, source }; }