From 2ed0656d1b36d99d5a1e814ce66c6a6d084fb891 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Sun, 7 Apr 2024 15:02:53 +0100 Subject: [PATCH] Introduce MoveCooldownHelper to prevent lag spikes from failed pathfinding Several activities that queue child Move activities can get into a bad scenario where the actor is pathfinding and then gets stuck because the destination is unreachable. When the Move activity then completes, then parent activity sees it has yet to reach the destination and tries to move again. However, the actor is still blocked in the same spot as before and thus the movment finishes immediately. This causes a performance death spiral where the actor attempts to pathfind every tick. The pathfinding attempt can also be very expensive if it must exhaustively check the whole map to determine no route is possible. In order to prevent blocked actors from running into this scenario, we introduce MoveCooldownHelper. In its default setup it allows the parent activity to bail out if the actor was blocked during a pathfinding attempt. This means the activity will be dropped rather than trying to move endlessly. It also has an option to allow retrying if pathfinding was blocked, but applies a cooldown to avoid the performance penalty. For activities such as Enter, this means the actors will still try and enter their target if it is unreachable, but will only attempt once a second now rather than every tick. MoveAdjacentTo will now cancel if it fails to reach the destination. This fixes MoveOntoAndTurn to skip the Turn if the move didn't reach the intended destination. Any other derived classes will similarly benefit from skipping follow-up actions. --- OpenRA.Mods.Cnc/Activities/LeapAttack.cs | 7 ++ OpenRA.Mods.Common/Activities/Attack.cs | 19 ++-- OpenRA.Mods.Common/Activities/Enter.cs | 9 ++ .../Activities/FindAndDeliverResources.cs | 8 ++ .../Activities/HarvestResource.cs | 7 ++ OpenRA.Mods.Common/Activities/LayMines.cs | 8 ++ OpenRA.Mods.Common/Activities/Move/Follow.cs | 14 ++- OpenRA.Mods.Common/Activities/Move/Move.cs | 49 ++++++--- .../Activities/Move/MoveAdjacentTo.cs | 14 ++- .../Activities/Move/MoveCooldownHelper.cs | 100 ++++++++++++++++++ OpenRA.Mods.Common/Activities/MoveToDock.cs | 10 +- OpenRA.Mods.Common/Activities/Resupply.cs | 14 ++- .../Traits/Attack/AttackFollow.cs | 12 +-- OpenRA.Mods.Common/Traits/DockHost.cs | 3 +- OpenRA.Mods.Common/Traits/Mobile.cs | 1 + OpenRA.Mods.Common/TraitsInterfaces.cs | 11 +- 16 files changed, 239 insertions(+), 47 deletions(-) create mode 100644 OpenRA.Mods.Common/Activities/Move/MoveCooldownHelper.cs diff --git a/OpenRA.Mods.Cnc/Activities/LeapAttack.cs b/OpenRA.Mods.Cnc/Activities/LeapAttack.cs index 0df455f26f..ae611bf785 100644 --- a/OpenRA.Mods.Cnc/Activities/LeapAttack.cs +++ b/OpenRA.Mods.Cnc/Activities/LeapAttack.cs @@ -29,6 +29,7 @@ namespace OpenRA.Mods.Cnc.Activities readonly bool allowMovement; readonly bool forceAttack; readonly Color? targetLineColor; + readonly MoveCooldownHelper moveCooldownHelper; Target target; Target lastVisibleTarget; @@ -47,6 +48,7 @@ namespace OpenRA.Mods.Cnc.Activities this.allowMovement = allowMovement; this.forceAttack = forceAttack; mobile = self.Trait(); + moveCooldownHelper = new MoveCooldownHelper(self.World, mobile); // The target may become hidden between the initial order request and the first tick (e.g. if queued) // Moving to any position (even if quite stale) is still better than immediately giving up @@ -92,6 +94,10 @@ namespace OpenRA.Mods.Cnc.Activities useLastVisibleTarget = targetIsHiddenActor || !target.IsValidFor(self); + var result = moveCooldownHelper.Tick(targetIsHiddenActor); + if (result != null) + return result.Value; + // Target is hidden or dead, and we don't have a fallback position to move towards if (useLastVisibleTarget && !lastVisibleTarget.IsValidFor(self)) return true; @@ -104,6 +110,7 @@ namespace OpenRA.Mods.Cnc.Activities if (!allowMovement || lastVisibleMaxRange == WDist.Zero || lastVisibleMaxRange < lastVisibleMinRange) return true; + moveCooldownHelper.NotifyMoveQueued(); QueueChild(mobile.MoveWithinRange(target, lastVisibleMinRange, lastVisibleMaxRange, checkTarget.CenterPosition, Color.Red)); return false; } diff --git a/OpenRA.Mods.Common/Activities/Attack.cs b/OpenRA.Mods.Common/Activities/Attack.cs index e8968cd2f0..e8cb2cf743 100644 --- a/OpenRA.Mods.Common/Activities/Attack.cs +++ b/OpenRA.Mods.Common/Activities/Attack.cs @@ -33,6 +33,7 @@ namespace OpenRA.Mods.Common.Activities readonly IPositionable positionable; readonly bool forceAttack; readonly Color? targetLineColor; + readonly MoveCooldownHelper moveCooldownHelper; protected Target target; Target lastVisibleTarget; @@ -40,7 +41,6 @@ namespace OpenRA.Mods.Common.Activities BitSet lastVisibleTargetTypes; Player lastVisibleOwner; bool useLastVisibleTarget; - bool wasMovingWithinRange; WDist minRange; WDist maxRange; @@ -61,6 +61,8 @@ namespace OpenRA.Mods.Common.Activities mobile = iMove as Mobile; move = allowMovement ? iMove : null; + moveCooldownHelper = new MoveCooldownHelper(self.World, mobile); + // The target may become hidden between the initial order request and the first tick (e.g. if queued) // Moving to any position (even if quite stale) is still better than immediately giving up if ((target.Type == TargetType.Actor && target.Actor.CanBeViewedByPlayer(self.Owner)) @@ -114,16 +116,14 @@ namespace OpenRA.Mods.Common.Activities useLastVisibleTarget = targetIsHiddenActor || !target.IsValidFor(self); - // If we are ticking again after previously sequencing a MoveWithRange then that move must have completed - // Either we are in range and can see the target, or we've lost track of it and should give up - if (wasMovingWithinRange && targetIsHiddenActor) - return true; + var result = moveCooldownHelper.Tick(targetIsHiddenActor); + if (result != null) + return result.Value; // Target is hidden or dead, and we don't have a fallback position to move towards if (useLastVisibleTarget && !lastVisibleTarget.IsValidFor(self)) return true; - wasMovingWithinRange = false; var pos = self.CenterPosition; var checkTarget = useLastVisibleTarget ? lastVisibleTarget : target; @@ -135,7 +135,7 @@ namespace OpenRA.Mods.Common.Activities return true; // Move towards the last known position - wasMovingWithinRange = true; + moveCooldownHelper.NotifyMoveQueued(); QueueChild(move.MoveWithinRange(target, WDist.Zero, lastVisibleMaximumRange, checkTarget.CenterPosition, Color.Red)); return false; } @@ -148,9 +148,6 @@ namespace OpenRA.Mods.Common.Activities attack.IsAiming = status == AttackStatus.Attacking || status == AttackStatus.NeedsToTurn; } - if (attackStatus.HasFlag(AttackStatus.NeedsToMove)) - wasMovingWithinRange = true; - if (attackStatus >= AttackStatus.NeedsToTurn) return false; @@ -185,6 +182,7 @@ namespace OpenRA.Mods.Common.Activities var sightRange = rs != null ? rs.Range : WDist.FromCells(2); attackStatus |= AttackStatus.NeedsToMove; + moveCooldownHelper.NotifyMoveQueued(); QueueChild(move.MoveWithinRange(target, sightRange, target.CenterPosition, Color.Red)); return AttackStatus.NeedsToMove; } @@ -218,6 +216,7 @@ namespace OpenRA.Mods.Common.Activities return AttackStatus.UnableToAttack; attackStatus |= AttackStatus.NeedsToMove; + moveCooldownHelper.NotifyMoveQueued(); var checkTarget = useLastVisibleTarget ? lastVisibleTarget : target; QueueChild(move.MoveWithinRange(target, minRange, maxRange, checkTarget.CenterPosition, Color.Red)); return AttackStatus.NeedsToMove; diff --git a/OpenRA.Mods.Common/Activities/Enter.cs b/OpenRA.Mods.Common/Activities/Enter.cs index 4ee5237955..032cd3c7ef 100644 --- a/OpenRA.Mods.Common/Activities/Enter.cs +++ b/OpenRA.Mods.Common/Activities/Enter.cs @@ -25,6 +25,7 @@ namespace OpenRA.Mods.Common.Activities readonly IMove move; readonly Color? targetLineColor; + readonly MoveCooldownHelper moveCooldownHelper; Target target; Target lastVisibleTarget; @@ -37,6 +38,7 @@ namespace OpenRA.Mods.Common.Activities this.target = target; this.targetLineColor = targetLineColor; ChildHasPriority = false; + moveCooldownHelper = new MoveCooldownHelper(self.World, move as Mobile) { RetryIfDestinationBlocked = true }; } /// @@ -79,6 +81,10 @@ namespace OpenRA.Mods.Common.Activities if (!TickChild(self)) return false; + var result = moveCooldownHelper.Tick(targetIsHiddenActor); + if (result != null) + return result.Value; + // Note that lastState refers to what we have just *finished* doing switch (lastState) { @@ -97,6 +103,7 @@ namespace OpenRA.Mods.Common.Activities if (target.Type != TargetType.Invalid && !move.CanEnterTargetNow(self, target)) { // Target lines are managed by this trait, so we do not pass targetLineColor + moveCooldownHelper.NotifyMoveQueued(); var initialTargetPosition = (useLastVisibleTarget ? lastVisibleTarget : target).CenterPosition; QueueChild(move.MoveToTarget(self, target, initialTargetPosition)); return false; @@ -110,6 +117,7 @@ namespace OpenRA.Mods.Common.Activities // Are we ready to move into the target? if (TryStartEnter(self, target.Actor)) { + moveCooldownHelper.NotifyMoveQueued(); lastState = EnterState.Entering; QueueChild(move.MoveIntoTarget(self, target)); return false; @@ -136,6 +144,7 @@ namespace OpenRA.Mods.Common.Activities case EnterState.Exiting: { + moveCooldownHelper.NotifyMoveQueued(); QueueChild(move.ReturnToCell(self)); lastState = EnterState.Finished; return false; diff --git a/OpenRA.Mods.Common/Activities/FindAndDeliverResources.cs b/OpenRA.Mods.Common/Activities/FindAndDeliverResources.cs index 644d4abfd8..1347dff54e 100644 --- a/OpenRA.Mods.Common/Activities/FindAndDeliverResources.cs +++ b/OpenRA.Mods.Common/Activities/FindAndDeliverResources.cs @@ -24,6 +24,7 @@ namespace OpenRA.Mods.Common.Activities readonly HarvesterInfo harvInfo; readonly Mobile mobile; readonly ResourceClaimLayer claimLayer; + readonly MoveCooldownHelper moveCooldownHelper; CPos? orderLocation; CPos? lastHarvestedCell; bool hasDeliveredLoad; @@ -38,6 +39,7 @@ namespace OpenRA.Mods.Common.Activities harvInfo = self.Info.TraitInfo(); mobile = self.Trait(); claimLayer = self.World.WorldActor.Trait(); + moveCooldownHelper = new MoveCooldownHelper(self.World, mobile) { RetryIfDestinationBlocked = true }; if (orderLocation.HasValue) this.orderLocation = orderLocation.Value; } @@ -112,6 +114,10 @@ namespace OpenRA.Mods.Common.Activities else LastSearchFailed = false; + var result = moveCooldownHelper.Tick(false); + if (result != null) + return result.Value; + // If no harvestable position could be found and we are at the refinery, get out of the way // of the refinery entrance. if (LastSearchFailed) @@ -124,6 +130,7 @@ namespace OpenRA.Mods.Common.Activities { var unblockCell = deliveryLoc + harv.Info.UnblockCell; var moveTo = mobile.NearestMoveableCell(unblockCell, 1, 5); + moveCooldownHelper.NotifyMoveQueued(); QueueChild(mobile.MoveTo(moveTo, 1)); } } @@ -132,6 +139,7 @@ namespace OpenRA.Mods.Common.Activities } // If we get here, our search for resources was successful. Commence harvesting. + moveCooldownHelper.NotifyMoveQueued(); QueueChild(new HarvestResource(self, closestHarvestableCell.Value)); lastHarvestedCell = closestHarvestableCell.Value; hasHarvestedCell = true; diff --git a/OpenRA.Mods.Common/Activities/HarvestResource.cs b/OpenRA.Mods.Common/Activities/HarvestResource.cs index 85537bdb61..90185bfcf6 100644 --- a/OpenRA.Mods.Common/Activities/HarvestResource.cs +++ b/OpenRA.Mods.Common/Activities/HarvestResource.cs @@ -28,6 +28,7 @@ namespace OpenRA.Mods.Common.Activities readonly IMove move; readonly CPos targetCell; readonly INotifyHarvestAction[] notifyHarvestActions; + readonly MoveCooldownHelper moveCooldownHelper; public HarvestResource(Actor self, CPos targetCell) { @@ -40,6 +41,7 @@ namespace OpenRA.Mods.Common.Activities resourceLayer = self.World.WorldActor.Trait(); this.targetCell = targetCell; notifyHarvestActions = self.TraitsImplementing().ToArray(); + moveCooldownHelper = new MoveCooldownHelper(self.World, move as Mobile); } protected override void OnFirstRun(Actor self) @@ -58,12 +60,17 @@ namespace OpenRA.Mods.Common.Activities if (IsCanceling || harv.IsFull) return true; + var result = moveCooldownHelper.Tick(false); + if (result != null) + return result.Value; + // Move towards the target cell if (self.Location != targetCell) { foreach (var n in notifyHarvestActions) n.MovingToResources(self, targetCell); + moveCooldownHelper.NotifyMoveQueued(); QueueChild(move.MoveTo(targetCell, 0)); return false; } diff --git a/OpenRA.Mods.Common/Activities/LayMines.cs b/OpenRA.Mods.Common/Activities/LayMines.cs index fb5740af16..1f8f838c30 100644 --- a/OpenRA.Mods.Common/Activities/LayMines.cs +++ b/OpenRA.Mods.Common/Activities/LayMines.cs @@ -26,6 +26,7 @@ namespace OpenRA.Mods.Common.Activities readonly IMove movement; readonly IMoveInfo moveInfo; readonly RearmableInfo rearmableInfo; + readonly MoveCooldownHelper moveCooldownHelper; List minefield; bool returnToBase; @@ -39,6 +40,7 @@ namespace OpenRA.Mods.Common.Activities movement = self.Trait(); moveInfo = self.Info.TraitInfo(); rearmableInfo = self.Info.TraitInfoOrDefault(); + moveCooldownHelper = new MoveCooldownHelper(self.World, movement as Mobile) { RetryIfDestinationBlocked = true }; this.minefield = minefield; } @@ -83,6 +85,10 @@ namespace OpenRA.Mods.Common.Activities } } + var result = moveCooldownHelper.Tick(false); + if (result != null) + return result.Value; + if ((minefield == null || minefield.Contains(self.Location)) && CanLayMine(self, self.Location)) { if (rearmableInfo != null && ammoPools.Any(p => p.Info.Name == minelayer.Info.AmmoPoolName && !p.HasAmmo)) @@ -98,6 +104,7 @@ namespace OpenRA.Mods.Common.Activities return true; // Add a CloseEnough range of 512 to the Rearm/Repair activities in order to ensure that we're at the host actor + moveCooldownHelper.NotifyMoveQueued(); QueueChild(new MoveAdjacentTo(self, Target.FromActor(rearmTarget))); QueueChild(movement.MoveTo(self.World.Map.CellContaining(rearmTarget.CenterPosition), ignoreActor: rearmTarget)); QueueChild(new Resupply(self, rearmTarget, new WDist(512))); @@ -125,6 +132,7 @@ namespace OpenRA.Mods.Common.Activities var nextCell = NextValidCell(self); if (nextCell != null) { + moveCooldownHelper.NotifyMoveQueued(); QueueChild(movement.MoveTo(nextCell.Value, 0)); return false; } diff --git a/OpenRA.Mods.Common/Activities/Move/Follow.cs b/OpenRA.Mods.Common/Activities/Move/Follow.cs index c6d10c6d47..6d74fca571 100644 --- a/OpenRA.Mods.Common/Activities/Move/Follow.cs +++ b/OpenRA.Mods.Common/Activities/Move/Follow.cs @@ -23,10 +23,10 @@ namespace OpenRA.Mods.Common.Activities readonly WDist maxRange; readonly IMove move; readonly Color? targetLineColor; + readonly MoveCooldownHelper moveCooldownHelper; Target target; Target lastVisibleTarget; bool useLastVisibleTarget; - bool wasMovingWithinRange; public Follow(Actor self, in Target target, WDist minRange, WDist maxRange, WPos? initialTargetPosition, Color? targetLineColor = null) @@ -36,6 +36,7 @@ namespace OpenRA.Mods.Common.Activities this.maxRange = maxRange; this.targetLineColor = targetLineColor; move = self.Trait(); + moveCooldownHelper = new MoveCooldownHelper(self.World, move as Mobile) { RetryIfDestinationBlocked = true }; // The target may become hidden between the initial order request and the first tick (e.g. if queued) // Moving to any position (even if quite stale) is still better than immediately giving up @@ -57,12 +58,9 @@ namespace OpenRA.Mods.Common.Activities useLastVisibleTarget = targetIsHiddenActor || !target.IsValidFor(self); - // If we are ticking again after previously sequencing a MoveWithRange then that move must have completed - // Either we are in range and can see the target, or we've lost track of it and should give up - if (wasMovingWithinRange && targetIsHiddenActor) - return true; - - wasMovingWithinRange = false; + var result = moveCooldownHelper.Tick(targetIsHiddenActor); + if (result != null) + return result.Value; // Target is hidden or dead, and we don't have a fallback position to move towards if (useLastVisibleTarget && !lastVisibleTarget.IsValidFor(self)) @@ -77,7 +75,7 @@ namespace OpenRA.Mods.Common.Activities return useLastVisibleTarget; // Move into range - wasMovingWithinRange = true; + moveCooldownHelper.NotifyMoveQueued(); QueueChild(move.MoveWithinRange(target, minRange, maxRange, checkTarget.CenterPosition, targetLineColor)); return false; } diff --git a/OpenRA.Mods.Common/Activities/Move/Move.cs b/OpenRA.Mods.Common/Activities/Move/Move.cs index 6d63f05af8..86c72b4504 100644 --- a/OpenRA.Mods.Common/Activities/Move/Move.cs +++ b/OpenRA.Mods.Common/Activities/Move/Move.cs @@ -42,6 +42,7 @@ namespace OpenRA.Mods.Common.Activities List path; CPos? destination; int startTicks; + bool hadNoPath; // For dealing with blockers bool hasWaited; @@ -113,6 +114,7 @@ namespace OpenRA.Mods.Common.Activities protected override void OnFirstRun(Actor self) { startTicks = self.World.WorldTick; + mobile.MoveResult = MoveResult.InProgress; if (evaluateNearestMovableCell && destination.HasValue) { @@ -137,6 +139,7 @@ namespace OpenRA.Mods.Common.Activities { path?.Clear(); + mobile.MoveResult = MoveResult.CompleteCanceled; return true; } @@ -144,19 +147,35 @@ namespace OpenRA.Mods.Common.Activities return false; if (destination == mobile.ToCell) + { + if (hadNoPath) + mobile.MoveResult = MoveResult.CompleteDestinationBlocked; + else + mobile.MoveResult = MoveResult.CompleteDestinationReached; + return true; + } if (path.Count == 0) { + hadNoPath = true; destination = mobile.ToCell; return false; } destination = path[0]; - var nextCell = PopPath(self); + var (nextCell, shouldTryAgain) = PopPath(self); if (nextCell == null) + { + if (!shouldTryAgain) + { + mobile.MoveResult = MoveResult.CompleteDestinationBlocked; + return true; + } + return false; + } var firstFacing = self.World.Map.FacingBetween(mobile.FromCell, nextCell.Value.Cell, mobile.Facing); @@ -200,10 +219,10 @@ namespace OpenRA.Mods.Common.Activities return false; } - (CPos Cell, SubCell SubCell)? PopPath(Actor self) + ((CPos Cell, SubCell SubCell)? Next, bool ShouldTryAgain) PopPath(Actor self) { if (path.Count == 0) - return null; + return (null, false); var nextCell = path[^1]; @@ -211,7 +230,7 @@ namespace OpenRA.Mods.Common.Activities if (!Util.AreAdjacentCells(mobile.ToCell, nextCell)) { path = EvalPath(BlockedByActor.Immovable); - return null; + return (null, false); } var containsTemporaryBlocker = self.World.ContainsTemporaryBlocker(nextCell, self); @@ -230,7 +249,7 @@ namespace OpenRA.Mods.Common.Activities if (path.Count < 2) { path.Clear(); - return null; + return (null, false); } // We can reasonably assume that the blocker is friendly and has a similar locomotor type. @@ -244,7 +263,7 @@ namespace OpenRA.Mods.Common.Activities if (!nudgeOrRepath) { path.Clear(); - return null; + return (null, false); } } @@ -252,7 +271,7 @@ namespace OpenRA.Mods.Common.Activities if (!mobile.CanEnterCell(nextCell, ignoreActor, BlockedByActor.Immovable)) { path = EvalPath(BlockedByActor.Immovable); - return null; + return (null, false); } // See if they will move @@ -263,17 +282,17 @@ namespace OpenRA.Mods.Common.Activities { waitTicksRemaining = mobile.Info.LocomotorInfo.WaitAverage; hasWaited = true; - return null; + return (null, true); } if (--waitTicksRemaining >= 0) - return null; + return (null, true); hasWaited = false; // If the blocking actors are already leaving, wait a little longer instead of repathing if (CellIsEvacuating(self, nextCell)) - return null; + return (null, true); // Calculate a new path mobile.RemoveInfluence(); @@ -286,7 +305,7 @@ namespace OpenRA.Mods.Common.Activities var newCell = path[^1]; path.RemoveAt(path.Count - 1); - return (newCell, mobile.GetAvailableSubCell(nextCell, mobile.FromSubCell, ignoreActor)); + return ((newCell, mobile.GetAvailableSubCell(nextCell, mobile.FromSubCell, ignoreActor)), true); } else if (mobile.IsBlocking) { @@ -297,17 +316,17 @@ namespace OpenRA.Mods.Common.Activities if ((nextCell - newCell).Value.LengthSquared > 2) path.Add(mobile.ToCell); - return (newCell.Value, mobile.GetAvailableSubCell(newCell.Value, mobile.FromSubCell, ignoreActor)); + return ((newCell.Value, mobile.GetAvailableSubCell(newCell.Value, mobile.FromSubCell, ignoreActor)), true); } } - return null; + return (null, false); } hasWaited = false; path.RemoveAt(path.Count - 1); - return (nextCell, mobile.GetAvailableSubCell(nextCell, mobile.FromSubCell, ignoreActor)); + return ((nextCell, mobile.GetAvailableSubCell(nextCell, mobile.FromSubCell, ignoreActor)), true); } protected override void OnLastRun(Actor self) @@ -518,7 +537,7 @@ namespace OpenRA.Mods.Common.Activities var fromSubcellOffset = map.Grid.OffsetOfSubCell(mobile.FromSubCell); var toSubcellOffset = map.Grid.OffsetOfSubCell(mobile.ToSubCell); - var nextCell = parent.PopPath(self); + var (nextCell, _) = parent.PopPath(self); if (nextCell != null) { if (!mobile.IsTraitPaused && !mobile.IsTraitDisabled && IsTurn(self, mobile, nextCell.Value.Cell, map)) diff --git a/OpenRA.Mods.Common/Activities/Move/MoveAdjacentTo.cs b/OpenRA.Mods.Common/Activities/Move/MoveAdjacentTo.cs index 0204b6dbcd..d8129a0f79 100644 --- a/OpenRA.Mods.Common/Activities/Move/MoveAdjacentTo.cs +++ b/OpenRA.Mods.Common/Activities/Move/MoveAdjacentTo.cs @@ -99,9 +99,17 @@ namespace OpenRA.Mods.Common.Activities QueueChild(Mobile.MoveTo(check => CalculatePathToTarget(self, check))); } - // The last queued childactivity is guaranteed to be the inner move, so if the childactivity - // queue is empty it means we have reached our destination. - return TickChild(self); + // The last queued child activity is guaranteed to be the inner move, + // so if the child activity queue is empty it means the move completed. + if (!TickChild(self)) + return false; + + if (Mobile.MoveResult == MoveResult.CompleteDestinationReached) + return true; + + // The move completed but we didn't reach the destination, so Cancel. + Cancel(self); + return true; } protected readonly List SearchCells = new(); diff --git a/OpenRA.Mods.Common/Activities/Move/MoveCooldownHelper.cs b/OpenRA.Mods.Common/Activities/Move/MoveCooldownHelper.cs new file mode 100644 index 0000000000..d2e4e1766b --- /dev/null +++ b/OpenRA.Mods.Common/Activities/Move/MoveCooldownHelper.cs @@ -0,0 +1,100 @@ +using OpenRA.Activities; +using OpenRA.Mods.Common.Traits; + +namespace OpenRA.Mods.Common.Activities +{ + /// + /// Activities that queue move activities via can use this helper to decide + /// when moves with blocked destinations should be retried and to apply a cooldown between repeated moves. + /// + public sealed class MoveCooldownHelper + { + /// + /// If a move failed because the destination was blocked, indicates if we should try again. + /// When true, will return null when the destination is blocked, after the cooldown has been applied. + /// When false, will return true to indicate the activity should give up and complete. + /// Defaults to false. + /// + public bool RetryIfDestinationBlocked { get; set; } + + /// + /// The cooldown delay in ticks. After a move with a blocked destination, the cooldown will be started. + /// Whilst the cooldown is in effect, will return false. + /// After the cooldown finishes, will return null to allow activity logic to resume. + /// This cooldown is important to avoid lag spikes caused by pathfinding every tick because the destination is unreachable. + /// Defaults to (20, 31). + /// + public (int MinTicksInclusive, int MaxTicksExclusive) Cooldown { get; set; } = (20, 31); + + readonly World world; + readonly Mobile mobile; + bool wasMoving; + bool hasRunCooldown; + int cooldownTicks; + + public MoveCooldownHelper(World world, Mobile mobile) + { + this.world = world; + this.mobile = mobile; + } + + /// + /// Call this when queuing a move activity. + /// + public void NotifyMoveQueued() + { + wasMoving = true; + } + + /// + /// Call this method within the method. It will return a tick result. + /// + /// If the target is a hidden actor, forces the result to be true, once the move has completed. + /// A result that should be returned from the calling Tick method. + /// A non-null result should be returned immediately. + /// On a null result, the method should continue with it's usual logic and perform any desired moves. + public bool? Tick(bool targetIsHiddenActor) + { + // We haven't moved yet, or we did move and we've finished the cooldown, allow the caller to resume with their logic. + if (!wasMoving) + return null; + + if (!hasRunCooldown) + { + // The target is hidden, don't continue tracking it. + if (targetIsHiddenActor) + return true; + + // Movement was cancelled, or we reached our destination, return immediately to allow the caller to perform their next steps. + if (mobile == null || mobile.MoveResult == MoveResult.CompleteCanceled || mobile.MoveResult == MoveResult.CompleteDestinationReached) + { + wasMoving = false; + return null; + } + + // We couldn't reach the destination, don't try and keep going after the actor. + if (!RetryIfDestinationBlocked && mobile.MoveResult == MoveResult.CompleteDestinationBlocked) + return true; + + // To avoid excessive pathfinding when the destination is blocked, wait for the cooldown before trying to move again. + // Applying some jitter to the wait time helps avoid multiple units repathing on the same tick and creating a lag spike. + hasRunCooldown = true; + cooldownTicks = world.SharedRandom.Next(Cooldown.MinTicksInclusive, Cooldown.MaxTicksExclusive); + return false; + } + else + { + if (cooldownTicks > 0) + cooldownTicks--; + + if (cooldownTicks <= 0) + { + hasRunCooldown = false; + wasMoving = false; + } + + return false; + } + } + } +} diff --git a/OpenRA.Mods.Common/Activities/MoveToDock.cs b/OpenRA.Mods.Common/Activities/MoveToDock.cs index 46621adc80..db7b684502 100644 --- a/OpenRA.Mods.Common/Activities/MoveToDock.cs +++ b/OpenRA.Mods.Common/Activities/MoveToDock.cs @@ -23,6 +23,7 @@ namespace OpenRA.Mods.Common.Activities Actor dockHostActor; IDockHost dockHost; readonly INotifyDockClientMoving[] notifyDockClientMoving; + readonly MoveCooldownHelper moveCooldownHelper; public MoveToDock(Actor self, Actor dockHostActor = null, IDockHost dockHost = null) { @@ -30,6 +31,7 @@ namespace OpenRA.Mods.Common.Activities this.dockHostActor = dockHostActor; this.dockHost = dockHost; notifyDockClientMoving = self.TraitsImplementing().ToArray(); + moveCooldownHelper = new MoveCooldownHelper(self.World, self.Trait() as Mobile) { RetryIfDestinationBlocked = true }; } public override bool Tick(Actor self) @@ -60,9 +62,13 @@ namespace OpenRA.Mods.Common.Activities } } + var result = moveCooldownHelper.Tick(false); + if (result != null) + return result.Value; + if (dockClient.ReserveHost(dockHostActor, dockHost)) { - if (dockHost.QueueMoveActivity(this, dockHostActor, self, dockClient)) + if (dockHost.QueueMoveActivity(this, dockHostActor, self, dockClient, moveCooldownHelper)) { foreach (var ndcm in notifyDockClientMoving) ndcm.MovingToDock(self, dockHostActor, dockHost); @@ -75,7 +81,7 @@ namespace OpenRA.Mods.Common.Activities } else { - // The dock explicitely chosen by the user is currently occupied. Wait and check again. + // The dock explicitly chosen by the user is currently occupied. Wait and check again. QueueChild(new Wait(dockClient.Info.SearchForDockDelay)); return false; } diff --git a/OpenRA.Mods.Common/Activities/Resupply.cs b/OpenRA.Mods.Common/Activities/Resupply.cs index f8316f08ed..4b67b57da4 100644 --- a/OpenRA.Mods.Common/Activities/Resupply.cs +++ b/OpenRA.Mods.Common/Activities/Resupply.cs @@ -38,6 +38,7 @@ namespace OpenRA.Mods.Common.Activities readonly bool wasRepaired; readonly PlayerResources playerResources; readonly int unitCost; + readonly MoveCooldownHelper moveCooldownHelper; int remainingTicks; bool played; @@ -62,6 +63,7 @@ namespace OpenRA.Mods.Common.Activities aircraft = move as Aircraft; moveInfo = self.Info.TraitInfo(); playerResources = self.Owner.PlayerActor.Trait(); + moveCooldownHelper = new MoveCooldownHelper(self.World, move as Mobile) { RetryIfDestinationBlocked = true }; var valued = self.Info.TraitInfoOrDefault(); unitCost = valued != null ? valued.Cost : 0; @@ -129,12 +131,18 @@ namespace OpenRA.Mods.Common.Activities return true; } - else if (activeResupplyTypes != 0 && aircraft == null && !isCloseEnough) + + var result = moveCooldownHelper.Tick(false); + if (result != null) + return result.Value; + + if (activeResupplyTypes != 0 && aircraft == null && !isCloseEnough) { var targetCell = self.World.Map.CellContaining(host.Actor.CenterPosition); // HACK: Repairable needs the actor to move to host center. // TODO: Get rid of this or at least replace it with something less hacky. + moveCooldownHelper.NotifyMoveQueued(); if (repairableNear == null) QueueChild(move.MoveOntoTarget(self, host, WVec.Zero, null, moveInfo.GetTargetLineColor())); else @@ -217,8 +225,11 @@ namespace OpenRA.Mods.Common.Activities if (wasRepaired || isHostInvalid || (!stayOnResupplier && aircraft.Info.TakeOffOnResupply)) { if (self.CurrentActivity.NextActivity == null && rp != null && rp.Path.Count > 0) + { + moveCooldownHelper.NotifyMoveQueued(); foreach (var cell in rp.Path) QueueChild(new AttackMoveActivity(self, () => move.MoveTo(cell, 1, ignoreActor: repairableNear != null ? null : host.Actor, targetLineColor: aircraft.Info.TargetLineColor))); + } else QueueChild(new TakeOff(self)); @@ -235,6 +246,7 @@ namespace OpenRA.Mods.Common.Activities // If there's no next activity, move to rallypoint if available, else just leave host if Repairable. // Do nothing if RepairableNear (RepairableNear actors don't enter their host and will likely remain within closeEnough). // If there's a next activity and we're not RepairableNear, first leave host if the next activity is not a Move. + moveCooldownHelper.NotifyMoveQueued(); if (self.CurrentActivity.NextActivity == null) { if (rp != null && rp.Path.Count > 0) diff --git a/OpenRA.Mods.Common/Traits/Attack/AttackFollow.cs b/OpenRA.Mods.Common/Traits/Attack/AttackFollow.cs index 5c5980c000..7d9b49a404 100644 --- a/OpenRA.Mods.Common/Traits/Attack/AttackFollow.cs +++ b/OpenRA.Mods.Common/Traits/Attack/AttackFollow.cs @@ -237,6 +237,7 @@ namespace OpenRA.Mods.Common.Traits readonly Rearmable rearmable; readonly AttackSource source; readonly bool isAircraft; + readonly MoveCooldownHelper moveCooldownHelper; Target target; Target lastVisibleTarget; @@ -245,7 +246,6 @@ namespace OpenRA.Mods.Common.Traits WDist lastVisibleMinimumRange; BitSet lastVisibleTargetTypes; Player lastVisibleOwner; - bool wasMovingWithinRange; bool hasTicked; bool returnToBase = false; @@ -255,6 +255,7 @@ namespace OpenRA.Mods.Common.Traits move = allowMove ? self.TraitOrDefault() : null; revealsShroud = self.TraitsImplementing().ToArray(); rearmable = self.TraitOrDefault(); + moveCooldownHelper = new MoveCooldownHelper(self.World, move as Mobile) { RetryIfDestinationBlocked = true }; this.target = target; this.forceAttack = forceAttack; @@ -348,10 +349,9 @@ namespace OpenRA.Mods.Common.Traits maxRange = sightRange; } - // If we are ticking again after previously sequencing a MoveWithRange then that move must have completed - // Either we are in range and can see the target, or we've lost track of it and should give up - if (wasMovingWithinRange && targetIsHiddenActor) - return true; + var result = moveCooldownHelper.Tick(targetIsHiddenActor); + if (result != null) + return result.Value; // Target is hidden or dead, and we don't have a fallback position to move towards if (useLastVisibleTarget && !lastVisibleTarget.IsValidFor(self)) @@ -406,7 +406,7 @@ namespace OpenRA.Mods.Common.Traits if (move == null || maxRange == WDist.Zero || maxRange < minRange) return true; - wasMovingWithinRange = true; + moveCooldownHelper.NotifyMoveQueued(); QueueChild(move.MoveWithinRange(target, minRange, maxRange, checkTarget.CenterPosition)); return false; } diff --git a/OpenRA.Mods.Common/Traits/DockHost.cs b/OpenRA.Mods.Common/Traits/DockHost.cs index 24f0460aa6..a6e177a47f 100644 --- a/OpenRA.Mods.Common/Traits/DockHost.cs +++ b/OpenRA.Mods.Common/Traits/DockHost.cs @@ -132,7 +132,7 @@ namespace OpenRA.Mods.Common.Traits OnDockCompleted(self, dockedClientActor, dockedClient); } - public virtual bool QueueMoveActivity(Activity moveToDockActivity, Actor self, Actor clientActor, DockClientManager client) + public virtual bool QueueMoveActivity(Activity moveToDockActivity, Actor self, Actor clientActor, DockClientManager client, MoveCooldownHelper moveCooldownHelper) { var move = clientActor.Trait(); @@ -141,6 +141,7 @@ namespace OpenRA.Mods.Common.Traits if ((move is Mobile ? clientActor.Location != clientActor.World.Map.CellContaining(DockPosition) : clientActor.CenterPosition != DockPosition) || move is not IFacing facing || facing.Facing != DockAngle) { + moveCooldownHelper.NotifyMoveQueued(); moveToDockActivity.QueueChild(move.MoveOntoTarget(clientActor, Target.FromActor(self), DockPosition - self.CenterPosition, DockAngle)); return true; } diff --git a/OpenRA.Mods.Common/Traits/Mobile.cs b/OpenRA.Mods.Common/Traits/Mobile.cs index f359383717..bca9fa82a9 100644 --- a/OpenRA.Mods.Common/Traits/Mobile.cs +++ b/OpenRA.Mods.Common/Traits/Mobile.cs @@ -217,6 +217,7 @@ namespace OpenRA.Mods.Common.Traits public bool IsBlocking { get; private set; } public bool IsMovingBetweenCells => FromCell != ToCell; + public MoveResult MoveResult { get; set; } #region IFacing diff --git a/OpenRA.Mods.Common/TraitsInterfaces.cs b/OpenRA.Mods.Common/TraitsInterfaces.cs index 77f7138046..d52e56215e 100644 --- a/OpenRA.Mods.Common/TraitsInterfaces.cs +++ b/OpenRA.Mods.Common/TraitsInterfaces.cs @@ -13,6 +13,7 @@ using System; using System.Collections.Generic; using OpenRA.Activities; using OpenRA.Graphics; +using OpenRA.Mods.Common.Activities; using OpenRA.Mods.Common.Graphics; using OpenRA.Mods.Common.Terrain; using OpenRA.Mods.Common.Widgets; @@ -262,7 +263,7 @@ namespace OpenRA.Mods.Common.Traits void OnDockCompleted(Actor self, Actor clientActor, DockClientManager client); /// If is not in range of queues a child move activity and returns true. If in range returns false. - bool QueueMoveActivity(Activity moveToDockActivity, Actor self, Actor clientActor, DockClientManager client); + bool QueueMoveActivity(Activity moveToDockActivity, Actor self, Actor clientActor, DockClientManager client, MoveCooldownHelper moveCooldownHelper); /// Should be called when in range of . void QueueDockActivity(Activity moveToDockActivity, Actor self, Actor clientActor, DockClientManager client); @@ -728,6 +729,14 @@ namespace OpenRA.Mods.Common.Traits Turn = 4 } + public enum MoveResult + { + InProgress, + CompleteCanceled, + CompleteDestinationReached, + CompleteDestinationBlocked, + } + [RequireExplicitImplementation] public interface INotifyMoving {