From 216758dbc7b77458eb21ddd747f09c5785e5cbfc Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Sat, 28 Oct 2023 13:53:19 +0100 Subject: [PATCH] Fix Locomotor.CanMoveFreelyInto when using ignoreSelf. 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. --- OpenRA.Mods.Common/Traits/World/Locomotor.cs | 33 +++++++++++++++----- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/OpenRA.Mods.Common/Traits/World/Locomotor.cs b/OpenRA.Mods.Common/Traits/World/Locomotor.cs index 53c99f2849..8f4d051a49 100644 --- a/OpenRA.Mods.Common/Traits/World/Locomotor.cs +++ b/OpenRA.Mods.Common/Traits/World/Locomotor.cs @@ -283,11 +283,30 @@ namespace OpenRA.Mods.Common.Traits } 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) @@ -305,7 +324,7 @@ namespace OpenRA.Mods.Common.Traits 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; @@ -322,9 +341,9 @@ namespace OpenRA.Mods.Common.Traits /// This logic is replicated in and /// . If this method is updated please update those as /// well. - 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;