Skip to content

Commit

Permalink
Fix moves being reported as blocked when already at the destination.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@RoosterDragon @PunkPun
RoosterDragon authored and PunkPun committed Aug 19, 2024
1 parent 8101c70 commit 0c4dff7
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 25 deletions.
44 changes: 29 additions & 15 deletions OpenRA.Mods.Common/Activities/Move/Move.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public class Move : Activity
public WAngle ActorFacingModifier { get; private set; }
readonly Mobile mobile;
readonly WDist nearEnough;
readonly Func<BlockedByActor, List<CPos>> getPath;
readonly Func<BlockedByActor, (bool AlreadyAtDestination, List<CPos> Path)> getPath;
readonly Actor ignoreActor;
readonly Color? targetLineColor;

Expand All @@ -39,6 +39,7 @@ public class Move : Activity
int carryoverProgress;
int lastMovePartCompletedTick;

bool alreadyAtDestination;
List<CPos> path;
CPos? destination;
int startTicks;
Expand All @@ -60,8 +61,11 @@ public Move(Actor self, CPos destination, Color? targetLineColor = null)

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;
Expand All @@ -78,10 +82,13 @@ public Move(Actor self, CPos destination, WDist nearEnough, Actor ignoreActor =
getPath = check =>
{
if (!this.destination.HasValue)
return PathFinder.NoPath;
return (false, PathFinder.NoPath);
if (mobile.ToCell == this.destination.Value)
return (true, PathFinder.NoPath);
return mobile.PathFinder.FindPathToTargetCell(
self, new[] { mobile.ToCell }, this.destination.Value, check, ignoreActor: ignoreActor);
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
Expand All @@ -93,7 +100,7 @@ public Move(Actor self, CPos destination, WDist nearEnough, Actor ignoreActor =
this.targetLineColor = targetLineColor;
}

public Move(Actor self, Func<BlockedByActor, List<CPos>> getPath, Color? targetLineColor = null)
public Move(Actor self, Func<BlockedByActor, (bool AlreadyAtDestination, List<CPos> 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;
Expand All @@ -105,10 +112,11 @@ public Move(Actor self, Func<BlockedByActor, List<CPos>> getPath, Color? targetL
this.targetLineColor = targetLineColor;
}

List<CPos> EvalPath(BlockedByActor check)
(bool AlreadyAtDestination, List<CPos> 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)
Expand All @@ -125,8 +133,8 @@ protected override void OnFirstRun(Actor self)
// 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;
}
}
Expand All @@ -146,6 +154,12 @@ public override bool Tick(Actor self)
if (mobile.IsTraitDisabled || mobile.IsTraitPaused)
return false;

if (alreadyAtDestination)
{
mobile.MoveResult = MoveResult.CompleteDestinationReached;
return true;
}

if (destination == mobile.ToCell)
{
if (hadNoPath)
Expand Down Expand Up @@ -229,7 +243,7 @@ public override bool Tick(Actor self)
// 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);
}

Expand Down Expand Up @@ -270,7 +284,7 @@ public override bool Tick(Actor self)
// 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);
}

Expand All @@ -296,7 +310,7 @@ public override bool Tick(Actor self)

// Calculate a new path
mobile.RemoveInfluence();
var newPath = EvalPath(BlockedByActor.All);
var (_, newPath) = EvalPath(BlockedByActor.All);
mobile.AddInfluence();

if (newPath.Count != 0)
Expand Down
13 changes: 10 additions & 3 deletions OpenRA.Mods.Common/Activities/Move/MoveAdjacentTo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public override bool Tick(Actor self)

protected int searchCellsTick = -1;

protected virtual List<CPos> CalculatePathToTarget(Actor self, BlockedByActor check)
protected virtual (bool AlreadyAtDestination, List<CPos> 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.
Expand All @@ -125,14 +125,21 @@ protected virtual List<CPos> CalculatePathToTarget(Actor self, BlockedByActor ch
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<Target> GetTargets(Actor self)
Expand Down
9 changes: 6 additions & 3 deletions OpenRA.Mods.Common/Activities/Move/MoveOnto.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,14 @@ protected override bool ShouldStop(Actor self)
return Target.Type == TargetType.Terrain;
}

protected override List<CPos> CalculatePathToTarget(Actor self, BlockedByActor check)
protected override (bool AlreadyAtDestination, List<CPos> 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.
Expand All @@ -51,7 +54,7 @@ protected override List<CPos> CalculatePathToTarget(Actor self, BlockedByActor c
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));
}
}
}
9 changes: 6 additions & 3 deletions OpenRA.Mods.Common/Activities/Move/MoveWithinRange.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,11 @@ protected override bool ShouldRepath(Actor self, CPos targetLocation)
|| !Mobile.CanInteractWithGroundLayer(self) || !Mobile.CanStayInCell(self.Location));
}

protected override List<CPos> CalculatePathToTarget(Actor self, BlockedByActor check)
protected override (bool AlreadyAtDestination, List<CPos> 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)
Expand All @@ -62,9 +65,9 @@ protected override List<CPos> CalculatePathToTarget(Actor self, BlockedByActor c
}

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)
Expand Down
2 changes: 1 addition & 1 deletion OpenRA.Mods.Common/Traits/Mobile.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ public void EnteringCell(Actor self)
CrushAction(self, (notifyCrushed) => notifyCrushed.WarnCrush);
}

public Activity MoveTo(Func<BlockedByActor, List<CPos>> pathFunc) { return new Move(self, pathFunc); }
public Activity MoveTo(Func<BlockedByActor, (bool AlreadyAtDestination, List<CPos> Path)> pathFunc) { return new Move(self, pathFunc); }

Activity LocalMove(Actor self, WPos fromPos, WPos toPos, CPos cell)
{
Expand Down

0 comments on commit 0c4dff7

Please sign in to comment.