Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix HierarchicalPathFinder considering some unreachable cells as reachable #20239

Merged
merged 1 commit into from Sep 3, 2022
Merged

Fix HierarchicalPathFinder considering some unreachable cells as reachable #20239

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 30 additions & 15 deletions OpenRA.Mods.Common/Pathfinder/HierarchicalPathFinder.cs
View file
Open in desktop
Expand Up @@ -146,12 +146,21 @@ public GridInfo(CPos?[] singleAbstractCellForLayer, Dictionary<CPos, CPos> local
/// <summary>
/// Maps a local cell to a abstract node in the graph.
/// Returns null when the local cell is unreachable.
/// Pass a null <paramref name="hpf"/> to skip cost checks if the caller already checked.
/// </summary>
public CPos? AbstractCellForLocalCell(CPos localCell)
public CPos? AbstractCellForLocalCell(CPos localCell, HierarchicalPathFinder hpf)
{
var abstractCell = singleAbstractCellForLayer[localCell.Layer];
if (abstractCell != null)
{
// All reachable cells in the grid are joined together so only a single abstract cell was needed,
// but there may be unreachable cells in the grid which we must exclude.
if (hpf != null && !hpf.CellIsAccessible(localCell))
return null;
return abstractCell;
Mailaender marked this conversation as resolved.
Show resolved Hide resolved
}

// Only reachable cells would be populated in the lookup, so no need to check their cost.
if (localCellToAbstractCell.TryGetValue(localCell, out var abstractCellFromMap))
return abstractCellFromMap;
return null;
Expand Down Expand Up @@ -461,13 +470,11 @@ void AddEdgesIfMovementAllowedBetweenCells(CPos cell, CPos candidateCell)
if (!MovementAllowedBetweenCells(cell, candidateCell))
return;

var gridInfo = gridInfos[GridIndex(cell)];
var abstractCell = gridInfo.AbstractCellForLocalCell(cell);
var abstractCell = AbstractCellForLocalCellNoAccessibleCheck(cell);
if (abstractCell == null)
return;

var gridInfoAdjacent = gridInfos[GridIndex(candidateCell)];
var abstractCellAdjacent = gridInfoAdjacent.AbstractCellForLocalCell(candidateCell);
var abstractCellAdjacent = AbstractCellForLocalCellNoAccessibleCheck(candidateCell);
if (abstractCellAdjacent == null)
return;

Expand Down Expand Up @@ -877,12 +884,10 @@ public bool PathExists(CPos source, CPos target)

RebuildDomains();

var sourceGridInfo = gridInfos[GridIndex(source)];
var targetGridInfo = gridInfos[GridIndex(target)];
var abstractSource = sourceGridInfo.AbstractCellForLocalCell(source);
var abstractSource = AbstractCellForLocalCell(source);
if (abstractSource == null)
return false;
var abstractTarget = targetGridInfo.AbstractCellForLocalCell(target);
var abstractTarget = AbstractCellForLocalCell(target);
if (abstractTarget == null)
return false;
var sourceDomain = abstractDomains[abstractSource.Value];
Expand Down Expand Up @@ -999,11 +1004,22 @@ List<GraphConnection> AbstractEdge(CPos abstractCell)
}

/// <summary>
/// Maps a local cell to a abstract node in the graph.
/// Returns null when the local cell is unreachable.
/// Maps a local cell to a abstract node in the graph. Returns null when the local cell is unreachable.
/// </summary>
CPos? AbstractCellForLocalCell(CPos localCell)
{
return gridInfos[GridIndex(localCell)].AbstractCellForLocalCell(localCell, this);
}

/// <summary>
/// Maps a local cell to a abstract node in the graph. Returns null when the local cell is unreachable.
/// Skips the <see cref="CellIsAccessible"/> check, if it has already been performed.
/// If an accessible check has not been performed, call <see cref="AbstractCellForLocalCell"/> instead.
/// </summary>
CPos? AbstractCellForLocalCell(CPos localCell) =>
gridInfos[GridIndex(localCell)].AbstractCellForLocalCell(localCell);
CPos? AbstractCellForLocalCellNoAccessibleCheck(CPos localCell)
{
return gridInfos[GridIndex(localCell)].AbstractCellForLocalCell(localCell, null);
}

/// <summary>
/// Creates a <see cref="GraphEdge"/> from the <paramref name="localCell"/> to the <paramref name="abstractCell"/>.
Expand Down Expand Up @@ -1035,8 +1051,7 @@ List<GraphConnection> AbstractEdge(CPos abstractCell)
// 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 gridInfo = gridInfos[GridIndex(cell)];
var maybeAbstractCell = gridInfo.AbstractCellForLocalCell(cell);
var maybeAbstractCell = AbstractCellForLocalCellNoAccessibleCheck(cell);
if (maybeAbstractCell == null)
throw new Exception(
"The abstract path should never be searched for an unreachable point. " +
Expand Down