From 0c4dff77c9da44601c871ebb72e393e2c85c6a70 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Thu, 15 Aug 2024 21:42:35 +0100 Subject: [PATCH] Fix moves being reported as blocked when already at the destination. When a move is made where the source and target locations are the same and no actual moving is required, a path of length 0 is returned. When a move cannot be made as there is no valid route, a path of length 0 is also returned. This means Move is unable to tell the difference between no movement required, and no path is possible. Currently it will hit the `hadNoPath` case and report CompleteDestinationBlocked. To fix the scenario where the source and target location match, track a alreadyAtDestination field. When this scenario triggers, report CompleteDestinationReached instead. This fixes activities that were using this result to inform their next actions. e.g. MoveOntoAndTurn would previously cancel the Turn portion of the activity, believing that the destination could not be reached. Now, it knows the destination was reached (since we are already there!) and will perform the turn. --- OpenRA.Mods.Common/Activities/Move/Move.cs | 44 ++++++++++++------- .../Activities/Move/MoveAdjacentTo.cs | 13 ++++-- .../Activities/Move/MoveOnto.cs | 9 ++-- .../Activities/Move/MoveWithinRange.cs | 9 ++-- OpenRA.Mods.Common/Traits/Mobile.cs | 2 +- 5 files changed, 52 insertions(+), 25 deletions(-) diff --git a/OpenRA.Mods.Common/Activities/Move/Move.cs b/OpenRA.Mods.Common/Activities/Move/Move.cs index b3e52e92d6..f3f5a5f8aa 100644 --- a/OpenRA.Mods.Common/Activities/Move/Move.cs +++ b/OpenRA.Mods.Common/Activities/Move/Move.cs @@ -24,7 +24,7 @@ namespace OpenRA.Mods.Common.Activities public WAngle ActorFacingModifier { get; private set; } readonly Mobile mobile; readonly WDist nearEnough; - readonly Func> getPath; + readonly Func Path)> getPath; readonly Actor ignoreActor; readonly Color? targetLineColor; @@ -39,6 +39,7 @@ namespace OpenRA.Mods.Common.Activities int carryoverProgress; int lastMovePartCompletedTick; + bool alreadyAtDestination; List path; CPos? destination; int startTicks; @@ -60,8 +61,11 @@ namespace OpenRA.Mods.Common.Activities getPath = check => { - return mobile.PathFinder.FindPathToTargetCell( - self, new[] { mobile.ToCell }, destination, check, laneBias: false); + if (mobile.ToCell == destination) + return (true, PathFinder.NoPath); + + return (false, mobile.PathFinder.FindPathToTargetCell( + self, new[] { mobile.ToCell }, destination, check, laneBias: false)); }; this.destination = destination; @@ -78,10 +82,13 @@ namespace OpenRA.Mods.Common.Activities getPath = check => { if (!this.destination.HasValue) - return PathFinder.NoPath; + return (false, PathFinder.NoPath); - return mobile.PathFinder.FindPathToTargetCell( - self, new[] { mobile.ToCell }, this.destination.Value, check, ignoreActor: ignoreActor); + if (mobile.ToCell == this.destination.Value) + return (true, PathFinder.NoPath); + + return (false, mobile.PathFinder.FindPathToTargetCell( + self, new[] { mobile.ToCell }, this.destination.Value, check, ignoreActor: ignoreActor)); }; // Note: Will be recalculated from OnFirstRun if evaluateNearestMovableCell is true @@ -93,7 +100,7 @@ namespace OpenRA.Mods.Common.Activities this.targetLineColor = targetLineColor; } - public Move(Actor self, Func> getPath, Color? targetLineColor = null) + public Move(Actor self, Func Path)> getPath, Color? targetLineColor = null) { // PERF: Because we can be sure that OccupiesSpace is Mobile here, we can save some performance by avoiding querying for the trait. mobile = (Mobile)self.OccupiesSpace; @@ -105,10 +112,11 @@ namespace OpenRA.Mods.Common.Activities this.targetLineColor = targetLineColor; } - List EvalPath(BlockedByActor check) + (bool AlreadyAtDestination, List Path) EvalPath(BlockedByActor check) { - var path = getPath(check).TakeWhile(a => a != mobile.ToCell).ToList(); - return path; + var (alreadyAtDestination, path) = getPath(check); + path = path.TakeWhile(a => a != mobile.ToCell).ToList(); + return (alreadyAtDestination, path); } protected override void OnFirstRun(Actor self) @@ -125,8 +133,8 @@ namespace OpenRA.Mods.Common.Activities // TODO: Change this to BlockedByActor.Stationary after improving the local avoidance behaviour foreach (var check in PathSearchOrder) { - path = EvalPath(check); - if (path.Count > 0) + (alreadyAtDestination, path) = EvalPath(check); + if (alreadyAtDestination || path.Count > 0) return; } } @@ -146,6 +154,12 @@ namespace OpenRA.Mods.Common.Activities if (mobile.IsTraitDisabled || mobile.IsTraitPaused) return false; + if (alreadyAtDestination) + { + mobile.MoveResult = MoveResult.CompleteDestinationReached; + return true; + } + if (destination == mobile.ToCell) { if (hadNoPath) @@ -229,7 +243,7 @@ namespace OpenRA.Mods.Common.Activities // Something else might have moved us, so the path is no longer valid. if (!Util.AreAdjacentCells(mobile.ToCell, nextCell)) { - path = EvalPath(BlockedByActor.Immovable); + (alreadyAtDestination, path) = EvalPath(BlockedByActor.Immovable); return (null, false); } @@ -270,7 +284,7 @@ namespace OpenRA.Mods.Common.Activities // There is no point in waiting for the other actor to move if it is incapable of moving. if (!mobile.CanEnterCell(nextCell, ignoreActor, BlockedByActor.Immovable)) { - path = EvalPath(BlockedByActor.Immovable); + (alreadyAtDestination, path) = EvalPath(BlockedByActor.Immovable); return (null, false); } @@ -296,7 +310,7 @@ namespace OpenRA.Mods.Common.Activities // Calculate a new path mobile.RemoveInfluence(); - var newPath = EvalPath(BlockedByActor.All); + var (_, newPath) = EvalPath(BlockedByActor.All); mobile.AddInfluence(); if (newPath.Count != 0) diff --git a/OpenRA.Mods.Common/Activities/Move/MoveAdjacentTo.cs b/OpenRA.Mods.Common/Activities/Move/MoveAdjacentTo.cs index f8756518e3..b43f6c7146 100644 --- a/OpenRA.Mods.Common/Activities/Move/MoveAdjacentTo.cs +++ b/OpenRA.Mods.Common/Activities/Move/MoveAdjacentTo.cs @@ -116,7 +116,7 @@ namespace OpenRA.Mods.Common.Activities protected int searchCellsTick = -1; - protected virtual List CalculatePathToTarget(Actor self, BlockedByActor check) + protected virtual (bool AlreadyAtDestination, List Path) CalculatePathToTarget(Actor self, BlockedByActor check) { // PERF: Assume that candidate cells don't change within a tick to avoid repeated queries // when Move enumerates different BlockedByActor values. @@ -125,14 +125,21 @@ namespace OpenRA.Mods.Common.Activities SearchCells.Clear(); searchCellsTick = self.World.WorldTick; foreach (var cell in Util.AdjacentCells(self.World, Target)) + { if (Mobile.CanStayInCell(cell) && Mobile.CanEnterCell(cell)) + { + if (cell == self.Location) + return (true, PathFinder.NoPath); + SearchCells.Add(cell); + } + } } if (SearchCells.Count == 0) - return PathFinder.NoPath; + return (false, PathFinder.NoPath); - return Mobile.PathFinder.FindPathToTargetCells(self, self.Location, SearchCells, check); + return (false, Mobile.PathFinder.FindPathToTargetCells(self, self.Location, SearchCells, check)); } public override IEnumerable GetTargets(Actor self) diff --git a/OpenRA.Mods.Common/Activities/Move/MoveOnto.cs b/OpenRA.Mods.Common/Activities/Move/MoveOnto.cs index ec8bb6d86d..f0f5e7ce1f 100644 --- a/OpenRA.Mods.Common/Activities/Move/MoveOnto.cs +++ b/OpenRA.Mods.Common/Activities/Move/MoveOnto.cs @@ -38,11 +38,14 @@ namespace OpenRA.Mods.Common.Activities return Target.Type == TargetType.Terrain; } - protected override List CalculatePathToTarget(Actor self, BlockedByActor check) + protected override (bool AlreadyAtDestination, List Path) CalculatePathToTarget(Actor self, BlockedByActor check) { + if (lastVisibleTargetLocation == self.Location) + return (true, PathFinder.NoPath); + // If we are close to the target but can't enter, we wait. if (!Mobile.CanEnterCell(lastVisibleTargetLocation) && Util.AreAdjacentCells(lastVisibleTargetLocation, self.Location)) - return PathFinder.NoPath; + return (false, PathFinder.NoPath); // PERF: Don't create a new list every run. // PERF: Also reuse the already created list in the base class. @@ -51,7 +54,7 @@ namespace OpenRA.Mods.Common.Activities else if (SearchCells[0] != lastVisibleTargetLocation) SearchCells[0] = lastVisibleTargetLocation; - return Mobile.PathFinder.FindPathToTargetCells(self, self.Location, SearchCells, check); + return (false, Mobile.PathFinder.FindPathToTargetCells(self, self.Location, SearchCells, check)); } } } diff --git a/OpenRA.Mods.Common/Activities/Move/MoveWithinRange.cs b/OpenRA.Mods.Common/Activities/Move/MoveWithinRange.cs index 6884e9413a..88cd2971ce 100644 --- a/OpenRA.Mods.Common/Activities/Move/MoveWithinRange.cs +++ b/OpenRA.Mods.Common/Activities/Move/MoveWithinRange.cs @@ -48,8 +48,11 @@ namespace OpenRA.Mods.Common.Activities || !Mobile.CanInteractWithGroundLayer(self) || !Mobile.CanStayInCell(self.Location)); } - protected override List CalculatePathToTarget(Actor self, BlockedByActor check) + protected override (bool AlreadyAtDestination, List Path) CalculatePathToTarget(Actor self, BlockedByActor check) { + if (lastVisibleTargetLocation == self.Location) + return (true, PathFinder.NoPath); + // PERF: Assume that candidate cells don't change within a tick to avoid repeated queries // when Move enumerates different BlockedByActor values. if (searchCellsTick != self.World.WorldTick) @@ -62,9 +65,9 @@ namespace OpenRA.Mods.Common.Activities } if (SearchCells.Count == 0) - return PathFinder.NoPath; + return (false, PathFinder.NoPath); - return Mobile.PathFinder.FindPathToTargetCells(self, self.Location, SearchCells, check); + return (false, Mobile.PathFinder.FindPathToTargetCells(self, self.Location, SearchCells, check)); } bool AtCorrectRange(WPos origin) diff --git a/OpenRA.Mods.Common/Traits/Mobile.cs b/OpenRA.Mods.Common/Traits/Mobile.cs index b1a4e057bd..17b51a1fc0 100644 --- a/OpenRA.Mods.Common/Traits/Mobile.cs +++ b/OpenRA.Mods.Common/Traits/Mobile.cs @@ -798,7 +798,7 @@ namespace OpenRA.Mods.Common.Traits CrushAction(self, (notifyCrushed) => notifyCrushed.WarnCrush); } - public Activity MoveTo(Func> pathFunc) { return new Move(self, pathFunc); } + public Activity MoveTo(Func Path)> pathFunc) { return new Move(self, pathFunc); } Activity LocalMove(Actor self, WPos fromPos, WPos toPos, CPos cell) {