Skip to content

Commit

Permalink
Fix Locomotor.CanMoveFreelyInto when using ignoreSelf.
Browse files Browse the repository at this point in the history
The ignoreSelf flag is intended to allow the current actor to be ignored when checking for blocking actors. This check worked correctly for cells occupied by a single actor. When a cell was occupied by multiple actors, the check was only working if the current actor happened to be the first actor. This is incorrect, if the current actor is anywhere in the cell then this flag should apply.

This flag failing to be as effective as intended meant that checks in methods such as PathFinder.FindPathToTargetCells would consider the source cell inaccessible, when it should have considered the cell accessible. This is a disaster for performance as an inaccessible cell requires a slow fallback path that performs a local path search. This means pathfinding was unexpectedly slow when this occurred. One scenario is force attacking with a group of infantry sharing the same cell. They should benefit from this check to do a fast path search, but failed to benefit from this check and the search would be slow instead.

Applying the flag correctly resolves the performance impact.
  • Loading branch information
RoosterDragon authored and PunkPun committed Oct 30, 2023
1 parent 96dc085 commit 216758d
Showing 1 changed file with 26 additions and 7 deletions.
33 changes: 26 additions & 7 deletions OpenRA.Mods.Common/Traits/World/Locomotor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -283,11 +283,30 @@ bool CanMoveFreelyInto(Actor actor, CPos cell, SubCell subCell, BlockedByActor c
}

var otherActors = subCell == SubCell.FullCell ? world.ActorMap.GetActorsAt(cell) : world.ActorMap.GetActorsAt(cell, subCell);
foreach (var otherActor in otherActors)
if (IsBlockedBy(actor, otherActor, ignoreActor, ignoreSelf, cell, check, cellFlag))
return false;

return true;
if (ignoreSelf)
{
// Any actor blocking us will prevent our movement, *unless* we are one of those actors.
var isBlocked = false;
foreach (var otherActor in otherActors)
{
if (actor == otherActor)
return true;

isBlocked = isBlocked || IsBlockedBy(actor, otherActor, ignoreActor, cell, check, cellFlag);
}

return !isBlocked;
}
else
{
// Any actor blocking us will prevent our movement.
foreach (var otherActor in otherActors)
if (IsBlockedBy(actor, otherActor, ignoreActor, cell, check, cellFlag))
return false;

return true;
}
}

public bool CanStayInCell(CPos cell)
Expand All @@ -305,7 +324,7 @@ public SubCell GetAvailableSubCell(Actor self, CPos cell, BlockedByActor check,

if (check > BlockedByActor.None)
{
bool CheckTransient(Actor otherActor) => IsBlockedBy(self, otherActor, ignoreActor, false, cell, check, GetCache(cell).CellFlag);
bool CheckTransient(Actor otherActor) => IsBlockedBy(self, otherActor, ignoreActor, cell, check, GetCache(cell).CellFlag);

if (!sharesCell)
return world.ActorMap.AnyActorsAt(cell, SubCell.FullCell, CheckTransient) ? SubCell.Invalid : SubCell.FullCell;
Expand All @@ -322,9 +341,9 @@ public SubCell GetAvailableSubCell(Actor self, CPos cell, BlockedByActor check,
/// <remarks>This logic is replicated in <see cref="HierarchicalPathFinder.ActorIsBlocking"/> and
/// <see cref="HierarchicalPathFinder.ActorCellIsBlocking"/>. If this method is updated please update those as
/// well.</remarks>
bool IsBlockedBy(Actor actor, Actor otherActor, Actor ignoreActor, bool ignoreSelf, CPos cell, BlockedByActor check, CellFlag cellFlag)
bool IsBlockedBy(Actor actor, Actor otherActor, Actor ignoreActor, CPos cell, BlockedByActor check, CellFlag cellFlag)
{
if (otherActor == ignoreActor || (ignoreSelf && otherActor == actor))
if (otherActor == ignoreActor)
return false;

var otherMobile = otherActor.OccupiesSpace as Mobile;
Expand Down

0 comments on commit 216758d

Please sign in to comment.