From 0bfc487999b2f37eab821cc6f1d6e265a9abd46f Mon Sep 17 00:00:00 2001 From: Paul Chote Date: Fri, 25 Jan 2019 22:14:02 +0000 Subject: [PATCH] Fix target invalidation and reacquisition in AttackFollow. --- .../Traits/Attack/AttackTDGunboatTurreted.cs | 4 +- .../Traits/Attack/AttackFollow.cs | 197 +++++++++++++----- OpenRA.Mods.Common/Traits/AutoTarget.cs | 2 +- OpenRA.Mods.Common/Traits/Turreted.cs | 6 - 4 files changed, 145 insertions(+), 64 deletions(-) diff --git a/OpenRA.Mods.Cnc/Traits/Attack/AttackTDGunboatTurreted.cs b/OpenRA.Mods.Cnc/Traits/Attack/AttackTDGunboatTurreted.cs index ec8fa817448d..89aa71ef8e16 100644 --- a/OpenRA.Mods.Cnc/Traits/Attack/AttackTDGunboatTurreted.cs +++ b/OpenRA.Mods.Cnc/Traits/Attack/AttackTDGunboatTurreted.cs @@ -59,10 +59,10 @@ public override Activity Tick(Actor self) { // Check that AttackTDGunboatTurreted hasn't cancelled the target by modifying attack.Target // Having both this and AttackTDGunboatTurreted modify that field is a horrible hack. - if (hasTicked && attack.Target.Type == TargetType.Invalid) + if (hasTicked && attack.requestedTarget.Type == TargetType.Invalid) return NextActivity; - attack.Target = target; + attack.requestedTarget = target; hasTicked = true; } diff --git a/OpenRA.Mods.Common/Traits/Attack/AttackFollow.cs b/OpenRA.Mods.Common/Traits/Attack/AttackFollow.cs index 80f8cb79f82a..f87b6b94dc8a 100644 --- a/OpenRA.Mods.Common/Traits/Attack/AttackFollow.cs +++ b/OpenRA.Mods.Common/Traits/Attack/AttackFollow.cs @@ -13,7 +13,6 @@ using System.Drawing; using System.Linq; using OpenRA.Activities; -using OpenRA.Mods.Common.Activities; using OpenRA.Traits; namespace OpenRA.Mods.Common.Traits @@ -26,24 +25,53 @@ public class AttackFollowInfo : AttackBaseInfo public class AttackFollow : AttackBase, INotifyOwnerChanged { - public Target Target { get; protected set; } + protected Target requestedTarget; + protected bool requestedForceAttack; + Mobile mobile; public AttackFollow(Actor self, AttackFollowInfo info) : base(self, info) { } + protected override void Created(Actor self) + { + mobile = self.TraitOrDefault(); + base.Created(self); + } + + protected bool CanAimAtTarget(Actor self, Target target, bool forceAttack) + { + if (target.Type == TargetType.Actor && !target.Actor.CanBeViewedByPlayer(self.Owner)) + return false; + + if (target.Type == TargetType.FrozenActor && !target.FrozenActor.IsValid) + return false; + + var pos = self.CenterPosition; + var armaments = ChooseArmamentsForTarget(target, forceAttack); + foreach (var a in armaments) + if (target.IsInRange(pos, a.MaxRange()) && (a.Weapon.MinRange == WDist.Zero || !target.IsInRange(pos, a.Weapon.MinRange))) + return true; + + return false; + } + protected override void Tick(Actor self) { - // We can safely ignore target visibility here - the armament will handle this for us. - bool targetIsHiddenActor; - Target = Target.Recalculate(self.Owner, out targetIsHiddenActor); if (IsTraitDisabled) - { - Target = Target.Invalid; + requestedTarget = Target.Invalid; + + // Can't fire on anything + if (mobile != null && !mobile.CanInteractWithGroundLayer(self)) return; - } - DoAttack(self, Target); - IsAiming = Target.IsValidFor(self); + if (requestedTarget.Type != TargetType.Invalid) + { + IsAiming = CanAimAtTarget(self, requestedTarget, requestedForceAttack); + if (IsAiming) + DoAttack(self, requestedTarget); + } + else + IsAiming = false; base.Tick(self); } @@ -55,13 +83,18 @@ public override Activity GetAttackActivity(Actor self, Target newTarget, bool al public override void OnStopOrder(Actor self) { - Target = Target.Invalid; + requestedTarget = Target.Invalid; base.OnStopOrder(self); } - public void OnOwnerChanged(Actor self, Player oldOwner, Player newOwner) + public bool HasReachableTarget(bool allowMove) + { + return IsReachableTarget(requestedTarget, allowMove); + } + + void INotifyOwnerChanged.OnOwnerChanged(Actor self, Player oldOwner, Player newOwner) { - Target = Target.Invalid; + requestedTarget = Target.Invalid; } class AttackActivity : Activity @@ -70,7 +103,13 @@ class AttackActivity : Activity readonly RevealsShroud[] revealsShroud; readonly IMove move; readonly bool forceAttack; + Target target; + Target lastVisibleTarget; + bool useLastVisibleTarget; + WDist lastVisibleMaximumRange; + WDist lastVisibleMinimumRange; + bool wasMovingWithinRange; bool hasTicked; public AttackActivity(Actor self, Target target, bool allowMove, bool forceAttack) @@ -81,67 +120,115 @@ public AttackActivity(Actor self, Target target, bool allowMove, bool forceAttac this.target = target; this.forceAttack = forceAttack; + + // 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)) + || target.Type == TargetType.FrozenActor || target.Type == TargetType.Terrain) + { + lastVisibleTarget = Target.FromPos(target.CenterPosition); + lastVisibleMaximumRange = attack.GetMaximumRangeVersusTarget(target); + lastVisibleMinimumRange = attack.GetMinimumRangeVersusTarget(target); + } } public override Activity Tick(Actor self) { - // All of the interesting behaviour to move to the last known target position if it becomes hidden - // and to reacquire the target if it is revealed enroute is handled inside MoveWithinRange. - // At this point in the activity chain we are either ticking against the target for the first time - // (and so don't know where it is), or after MoveWithinRange has lost the target and given up. - // We can therefore treat a hidden targets as invalid and give up if we can't currently see it. - target = target.RecalculateInvalidatingHiddenTargets(self.Owner); - if (IsCanceled || !target.IsValidFor(self)) + if (IsCanceled) + return NextActivity; + + // Check that AttackFollow hasn't cancelled the target by modifying attack.Target + // Having both this and AttackFollow modify that field is a horrible hack. + if (hasTicked && attack.requestedTarget.Type == TargetType.Invalid) return NextActivity; if (attack.IsTraitPaused) return this; - var weapon = attack.ChooseArmamentsForTarget(target, forceAttack).FirstOrDefault(); - if (weapon != null) - { - // Check that AttackFollow hasn't cancelled the target by modifying attack.Target - // Having both this and AttackFollow modify that field is a horrible hack. - if (hasTicked && attack.Target.Type == TargetType.Invalid) - return NextActivity; + bool targetIsHiddenActor; + attack.requestedForceAttack = forceAttack; + attack.requestedTarget = target = target.Recalculate(self.Owner, out targetIsHiddenActor); + hasTicked = true; - var targetIsMobile = (target.Type == TargetType.Actor && target.Actor.Info.HasTraitInfo()) - || (target.Type == TargetType.FrozenActor && target.FrozenActor.Info.HasTraitInfo()); - - // Try and sit at least one cell closer than the max range to give some leeway if the target starts moving. - var modifiedRange = weapon.MaxRange(); - var maxRange = targetIsMobile ? new WDist(Math.Max(weapon.Weapon.MinRange.Length, modifiedRange.Length - 1024)) - : modifiedRange; + if (!targetIsHiddenActor && target.Type == TargetType.Actor) + { + lastVisibleTarget = Target.FromTargetPositions(target); + lastVisibleMaximumRange = attack.GetMaximumRangeVersusTarget(target); + lastVisibleMinimumRange = attack.GetMinimumRange(); - // Most actors want to be able to see their target before shooting - if (!attack.Info.TargetFrozenActors && !forceAttack && target.Type == TargetType.FrozenActor) + // Try and sit at least one cell away from the min or max ranges to give some leeway if the target starts moving. + if (target.Actor.Info.HasTraitInfo()) { - var rs = revealsShroud - .Where(Exts.IsTraitEnabled) - .MaxByOrDefault(s => s.Range); - - // Default to 2 cells if there are no active traits - var sightRange = rs != null ? rs.Range : WDist.FromCells(2); - if (sightRange < maxRange) - maxRange = sightRange; + var preferMinRange = Math.Min(lastVisibleMinimumRange.Length + 1024, lastVisibleMaximumRange.Length); + var preferMaxRange = Math.Max(lastVisibleMaximumRange.Length - 1024, lastVisibleMinimumRange.Length); + lastVisibleMaximumRange = new WDist((lastVisibleMaximumRange.Length - 1024).Clamp(preferMinRange, preferMaxRange)); } + } + + var oldUseLastVisibleTarget = useLastVisibleTarget; + var maxRange = lastVisibleMaximumRange; + var minRange = lastVisibleMinimumRange; + useLastVisibleTarget = targetIsHiddenActor || !target.IsValidFor(self); + + // Most actors want to be able to see their target before shooting + if (target.Type == TargetType.FrozenActor && !attack.Info.TargetFrozenActors && !forceAttack) + { + var rs = revealsShroud + .Where(Exts.IsTraitEnabled) + .MaxByOrDefault(s => s.Range); + + // Default to 2 cells if there are no active traits + var sightRange = rs != null ? rs.Range : WDist.FromCells(2); + if (sightRange < maxRange) + 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) + { + attack.requestedTarget = Target.Invalid; + return NextActivity; + } + + // Update target lines if required + if (useLastVisibleTarget != oldUseLastVisibleTarget) + self.SetTargetLine(useLastVisibleTarget ? lastVisibleTarget : target, Color.Red, false); - attack.Target = target; - hasTicked = true; + // Target is hidden or dead, and we don't have a fallback position to move towards + if (useLastVisibleTarget && !lastVisibleTarget.IsValidFor(self)) + { + attack.requestedTarget = Target.Invalid; + return NextActivity; + } - if (move != null) - return ActivityUtils.SequenceActivities( - move.MoveFollow(self, target, weapon.Weapon.MinRange, maxRange, targetLineColor: Color.Red), - this); + var pos = self.CenterPosition; + var checkTarget = useLastVisibleTarget ? lastVisibleTarget : target; + + // We've reached the required range - if the target is visible and valid then we wait + // otherwise if it is hidden or dead we give up + if (checkTarget.IsInRange(pos, maxRange) && !checkTarget.IsInRange(pos, minRange)) + { + if (useLastVisibleTarget) + { + attack.requestedTarget = Target.Invalid; + return NextActivity; + } - if (target.IsInRange(self.CenterPosition, weapon.MaxRange()) && - !target.IsInRange(self.CenterPosition, weapon.Weapon.MinRange)) - return this; + return this; } - attack.Target = Target.Invalid; + // We can't move into range, so give up + if (move == null) + { + attack.requestedTarget = Target.Invalid; + return NextActivity; + } - return NextActivity; + wasMovingWithinRange = true; + return ActivityUtils.SequenceActivities( + move.MoveWithinRange(target, minRange, maxRange, checkTarget.CenterPosition, Color.Red), + this); } } } diff --git a/OpenRA.Mods.Common/Traits/AutoTarget.cs b/OpenRA.Mods.Common/Traits/AutoTarget.cs index 1ea232a4da9a..01321ed22801 100644 --- a/OpenRA.Mods.Common/Traits/AutoTarget.cs +++ b/OpenRA.Mods.Common/Traits/AutoTarget.cs @@ -236,7 +236,7 @@ bool ShouldAttack(out bool allowMove) // PERF: Avoid LINQ. foreach (var attackFollow in attackFollows) - if (!attackFollow.IsTraitDisabled && attackFollow.IsReachableTarget(attackFollow.Target, allowMove)) + if (!attackFollow.IsTraitDisabled && attackFollow.HasReachableTarget(allowMove)) return false; return true; diff --git a/OpenRA.Mods.Common/Traits/Turreted.cs b/OpenRA.Mods.Common/Traits/Turreted.cs index b7a65c1d4c83..5e7d4d456317 100644 --- a/OpenRA.Mods.Common/Traits/Turreted.cs +++ b/OpenRA.Mods.Common/Traits/Turreted.cs @@ -255,12 +255,6 @@ protected override void TraitDisabled(Actor self) if (attack != null && attack.IsAiming) attack.OnStopOrder(self); } - - protected override void TraitResumed(Actor self) - { - if (attack != null) - FaceTarget(self, attack.Target); - } } public class TurretFacingInit : IActorInit