Allow Attack activities to target FrozenActors directly.

Removing the legacy FrozenActor to Actor workaround
fixes a number of long-standing bugs.

This also prevents units from losing their target when
it transforms into a different actor type.

OpenRA.Mods.Common/Activities/Air/FlyAttack.cs

@@ -18,10 +18,10 @@ namespace OpenRA.Mods.Common.Activities
 {
 	public class FlyAttack : Activity
 	{
-		readonly Target target;
 		readonly Aircraft aircraft;
 		readonly AttackAircraft attackAircraft;
 		readonly Rearmable rearmable;
+		Target target;
 
 		int ticksUntilTurn;
 
@@ -43,6 +43,8 @@ namespace OpenRA.Mods.Common.Activities
 				return NextActivity;
 			}
 
+			target = target.Recalculate(self.Owner);
+
 			if (!target.IsValidFor(self))
 				return NextActivity;
 

OpenRA.Mods.Common/Activities/Attack.cs

@@ -23,13 +23,13 @@ namespace OpenRA.Mods.Common.Activities
 		[Flags]
 		protected enum AttackStatus { UnableToAttack, NeedsToTurn, NeedsToMove, Attacking }
 
-		protected readonly Target Target;
-		readonly AttackBase[] attackTraits;
+		readonly AttackFrontal[] attackTraits;
+		readonly RevealsShroud[] revealsShroud;
 		readonly IMove move;
 		readonly IFacing facing;
 		readonly IPositionable positionable;
 		readonly bool forceAttack;
-		readonly int facingTolerance;
+		protected Target target;
 
 		WDist minRange;
 		WDist maxRange;
@@ -37,22 +37,27 @@ namespace OpenRA.Mods.Common.Activities
 		Activity moveActivity;
 		AttackStatus attackStatus = AttackStatus.UnableToAttack;
 
-		public Attack(Actor self, Target target, bool allowMovement, bool forceAttack, int facingTolerance)
+		public Attack(Actor self, Target target, bool allowMovement, bool forceAttack)
 		{
-			Target = target;
-
+			this.target = target;
 			this.forceAttack = forceAttack;
-			this.facingTolerance = facingTolerance;
 
-			attackTraits = self.TraitsImplementing<AttackBase>().ToArray();
+			attackTraits = self.TraitsImplementing<AttackFrontal>().ToArray();
+			revealsShroud = self.TraitsImplementing<RevealsShroud>().ToArray();
 			facing = self.Trait<IFacing>();
 			positionable = self.Trait<IPositionable>();
 
 			move = allowMovement ? self.TraitOrDefault<IMove>() : null;
 		}
 
+		protected virtual Target RecalculateTarget(Actor self)
+		{
+			return target.Recalculate(self.Owner);
+		}
+
 		public override Activity Tick(Actor self)
 		{
+			target = RecalculateTarget(self);
 			turnActivity = moveActivity = null;
 			attackStatus = AttackStatus.UnableToAttack;
 
@@ -74,31 +79,37 @@ namespace OpenRA.Mods.Common.Activities
 			return NextActivity;
 		}
 
-		protected virtual bool IgnoresVisibility { get { return false; } }
-
-		protected virtual AttackStatus TickAttack(Actor self, AttackBase attack)
+		protected virtual AttackStatus TickAttack(Actor self, AttackFrontal attack)
 		{
 			if (IsCanceled)
 				return AttackStatus.UnableToAttack;
 
-			var type = Target.Type;
-			if (!Target.IsValidFor(self) || type == TargetType.FrozenActor)
+			if (!target.IsValidFor(self))
 				return AttackStatus.UnableToAttack;
 
-			if (attack.Info.AttackRequiresEnteringCell && !positionable.CanEnterCell(Target.Actor.Location, null, false))
+			if (attack.Info.AttackRequiresEnteringCell && !positionable.CanEnterCell(target.Actor.Location, null, false))
 				return AttackStatus.UnableToAttack;
 
-			// Drop the target if it moves under the shroud / fog.
-			// HACK: This would otherwise break targeting frozen actors
-			// The problem is that Shroud.IsTargetable returns false (as it should) for
-			// frozen actors, but we do want to explicitly target the underlying actor here.
-			if (!IgnoresVisibility && type == TargetType.Actor
-					&& !Target.Actor.Info.HasTraitInfo<FrozenUnderFogInfo>()
-					&& !Target.Actor.CanBeViewedByPlayer(self.Owner))
-				return AttackStatus.UnableToAttack;
+			if (!attack.Info.TargetFrozenActors && !forceAttack && target.Type == TargetType.FrozenActor)
+			{
+				// Try to move within range, drop the target otherwise
+				if (move == null)
+					return AttackStatus.UnableToAttack;
+
+				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);
+
+				attackStatus |= AttackStatus.NeedsToMove;
+				moveActivity = ActivityUtils.SequenceActivities(move.MoveWithinRange(target, sightRange), this);
+				return AttackStatus.NeedsToMove;
+			}
 
 			// Drop the target once none of the weapons are effective against it
-			var armaments = attack.ChooseArmamentsForTarget(Target, forceAttack).ToList();
+			var armaments = attack.ChooseArmamentsForTarget(target, forceAttack).ToList();
 			if (armaments.Count == 0)
 				return AttackStatus.UnableToAttack;
 
@@ -108,8 +119,8 @@ namespace OpenRA.Mods.Common.Activities
 
 			var pos = self.CenterPosition;
 			var mobile = move as Mobile;
-			if (!Target.IsInRange(pos, maxRange)
-				|| (minRange.Length != 0 && Target.IsInRange(pos, minRange))
+			if (!target.IsInRange(pos, maxRange)
+				|| (minRange.Length != 0 && target.IsInRange(pos, minRange))
 				|| (mobile != null && !mobile.CanInteractWithGroundLayer(self)))
 			{
 				// Try to move within range, drop the target otherwise
@@ -117,13 +128,13 @@ namespace OpenRA.Mods.Common.Activities
 					return AttackStatus.UnableToAttack;
 
 				attackStatus |= AttackStatus.NeedsToMove;
-				moveActivity = ActivityUtils.SequenceActivities(move.MoveWithinRange(Target, minRange, maxRange), this);
+				moveActivity = ActivityUtils.SequenceActivities(move.MoveWithinRange(target, minRange, maxRange), this);
 				return AttackStatus.NeedsToMove;
 			}
 
-			var targetedPosition = attack.GetTargetPosition(pos, Target);
+			var targetedPosition = attack.GetTargetPosition(pos, target);
 			var desiredFacing = (targetedPosition - pos).Yaw.Facing;
-			if (!Util.FacingWithinTolerance(facing.Facing, desiredFacing, facingTolerance))
+			if (!Util.FacingWithinTolerance(facing.Facing, desiredFacing, ((AttackFrontalInfo)attack.Info).FacingTolerance))
 			{
 				attackStatus |= AttackStatus.NeedsToTurn;
 				turnActivity = ActivityUtils.SequenceActivities(new Turn(self, desiredFacing), this);
@@ -131,9 +142,48 @@ namespace OpenRA.Mods.Common.Activities
 			}
 
 			attackStatus |= AttackStatus.Attacking;
-			attack.DoAttack(self, Target, armaments);
+			attack.DoAttack(self, target, armaments);
 
 			return AttackStatus.Attacking;
 		}
 	}
+
+	public static class TargetExts
+	{
+		/// <summary>Update (Frozen)Actor targets to account for visibility changes or actor replacement</summary>
+		public static Target Recalculate(this Target t, Player viewer)
+		{
+			// Check whether the target has transformed into something else
+			// HACK: This relies on knowing the internal implementation details of Target
+			if (t.Type == TargetType.Invalid && t.Actor != null && t.Actor.ReplacedByActor != null)
+				t = Target.FromActor(t.Actor.ReplacedByActor);
+
+			if (t.Type == TargetType.Actor)
+			{
+				// Actor has been hidden under the fog
+				if (!t.Actor.CanBeViewedByPlayer(viewer))
+				{
+					// Replace with FrozenActor if applicable, otherwise drop the target
+					var frozen = viewer.FrozenActorLayer.FromID(t.Actor.ActorID);
+					return frozen != null ? Target.FromFrozenActor(frozen) : Target.Invalid;
+				}
+			}
+			else if (t.Type == TargetType.FrozenActor)
+			{
+				// Frozen actor has been revealed
+				if (!t.FrozenActor.Visible || !t.FrozenActor.IsValid)
+				{
+					// Original actor is still alive
+					if (t.FrozenActor.Actor != null && t.FrozenActor.Actor.CanBeViewedByPlayer(viewer))
+						return Target.FromActor(t.FrozenActor.Actor);
+
+					// Original actor was killed while hidden
+					if (t.Actor == null)
+						return Target.Invalid;
+				}
+			}
+
+			return t;
+		}
+	}
 }

OpenRA.Mods.Common/Traits/Attack/AttackBase.cs

@@ -31,6 +31,9 @@ namespace OpenRA.Mods.Common.Traits
 		[Desc("Does the attack type require the attacker to enter the target's cell?")]
 		public readonly bool AttackRequiresEnteringCell = false;
 
+		[Desc("Allow firing into the fog to target frozen actors without requiring force-fire.")]
+		public readonly bool TargetFrozenActors = false;
+
 		[VoiceReference] public readonly string Voice = "Action";
 
 		public override abstract object Create(ActorInitializer init);
@@ -154,12 +157,11 @@ namespace OpenRA.Mods.Common.Traits
 			var forceAttack = order.OrderString == forceAttackOrderName;
 			if (forceAttack || order.OrderString == attackOrderName)
 			{
-				var target = self.ResolveFrozenActorOrder(order, Color.Red);
-				if (!target.IsValidFor(self))
+				if (!order.Target.IsValidFor(self))
 					return;
 
-				self.SetTargetLine(target, Color.Red);
-				AttackTarget(target, order.Queued, true, forceAttack);
+				self.SetTargetLine(order.Target, Color.Red);
+				AttackTarget(order.Target, order.Queued, true, forceAttack);
 			}
 
 			if (order.OrderString == "Stop")
@@ -417,7 +419,8 @@ namespace OpenRA.Mods.Common.Traits
 				if (a == null)
 					a = armaments.First();
 
-				cursor = !target.IsInRange(self.CenterPosition, a.MaxRange())
+				cursor = !target.IsInRange(self.CenterPosition, a.MaxRange()) ||
+				         (!forceAttack && target.Type == TargetType.FrozenActor && !ab.Info.TargetFrozenActors)
 					? ab.Info.OutsideRangeCursor ?? a.Info.OutsideRangeCursor
 					: ab.Info.Cursor ?? a.Info.Cursor;
 

OpenRA.Mods.Common/Traits/Attack/AttackFollow.cs

@@ -12,6 +12,7 @@
 using System;
 using System.Linq;
 using OpenRA.Activities;
+using OpenRA.Mods.Common.Activities;
 using OpenRA.Traits;
 
 namespace OpenRA.Mods.Common.Traits
@@ -31,6 +32,7 @@ namespace OpenRA.Mods.Common.Traits
 
 		protected override void Tick(Actor self)
 		{
+			Target = Target.Recalculate(self.Owner);
 			if (IsTraitDisabled)
 			{
 				Target = Target.Invalid;
@@ -62,15 +64,17 @@ namespace OpenRA.Mods.Common.Traits
 		class AttackActivity : Activity
 		{
 			readonly AttackFollow attack;
+			readonly RevealsShroud[] revealsShroud;
 			readonly IMove move;
-			readonly Target target;
 			readonly bool forceAttack;
+			Target target;
 			bool hasTicked;
 
 			public AttackActivity(Actor self, Target target, bool allowMove, bool forceAttack)
 			{
 				attack = self.Trait<AttackFollow>();
 				move = allowMove ? self.TraitOrDefault<IMove>() : null;
+				revealsShroud = self.TraitsImplementing<RevealsShroud>().ToArray();
 
 				this.target = target;
 				this.forceAttack = forceAttack;
@@ -78,6 +82,7 @@ namespace OpenRA.Mods.Common.Traits
 
 			public override Activity Tick(Actor self)
 			{
+				target = target.Recalculate(self.Owner);
 				if (IsCanceled || !target.IsValidFor(self))
 					return NextActivity;
 
@@ -87,6 +92,11 @@ namespace OpenRA.Mods.Common.Traits
 				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;
+
 					var targetIsMobile = (target.Type == TargetType.Actor && target.Actor.Info.HasTraitInfo<IMoveInfo>())
 						|| (target.Type == TargetType.FrozenActor && target.FrozenActor.Info.HasTraitInfo<IMoveInfo>());
 
@@ -95,10 +105,18 @@ namespace OpenRA.Mods.Common.Traits
 					var maxRange = targetIsMobile ? new WDist(Math.Max(weapon.Weapon.MinRange.Length, modifiedRange.Length - 1024))
 						: modifiedRange;
 
-					// 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;
+					// Most actors want to be able to see their target before shooting
+					if (!attack.Info.TargetFrozenActors && !forceAttack && target.Type == TargetType.FrozenActor)
+					{
+						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;
+					}
 
 					attack.Target = target;
 					hasTicked = true;

OpenRA.Mods.Common/Traits/Attack/AttackFrontal.cs

@@ -57,7 +57,7 @@ namespace OpenRA.Mods.Common.Traits
 
 		public override Activity GetAttackActivity(Actor self, Target newTarget, bool allowMove, bool forceAttack)
 		{
-			return new Activities.Attack(self, newTarget, allowMove, forceAttack, info.FacingTolerance);
+			return new Activities.Attack(self, newTarget, allowMove, forceAttack);
 		}
 	}
 }

OpenRA.Mods.Common/Traits/Attack/AttackOmni.cs

@@ -10,6 +10,7 @@
 #endregion
 
 using OpenRA.Activities;
+using OpenRA.Mods.Common.Activities;
 using OpenRA.Traits;
 
 namespace OpenRA.Mods.Common.Traits
@@ -32,9 +33,9 @@ namespace OpenRA.Mods.Common.Traits
 		// Some 3rd-party mods rely on this being public
 		public class SetTarget : Activity
 		{
-			readonly Target target;
 			readonly AttackOmni attack;
 			readonly bool allowMove;
+			Target target;
 
 			public SetTarget(AttackOmni attack, Target target, bool allowMove)
 			{
@@ -45,6 +46,7 @@ namespace OpenRA.Mods.Common.Traits
 
 			public override Activity Tick(Actor self)
 			{
+				target = target.Recalculate(self.Owner);
 				if (IsCanceled || !target.IsValidFor(self) || !attack.IsReachableTarget(target, allowMove))
 					return NextActivity;
 

OpenRA.Mods.D2k/Traits/AttackSwallow.cs

@@ -72,16 +72,19 @@ namespace OpenRA.Mods.D2k.Traits
 
 		public override Activity GetAttackActivity(Actor self, Target newTarget, bool allowMove, bool forceAttack)
 		{
-			return new SwallowTarget(self, newTarget, allowMove, forceAttack, Info.FacingTolerance);
+			return new SwallowTarget(self, newTarget, allowMove, forceAttack);
 		}
 
 		public class SwallowTarget : Attack
 		{
-			public SwallowTarget(Actor self, Target target, bool allowMovement, bool forceAttack, int facingTolerance)
-				: base(self, target, allowMovement, forceAttack, facingTolerance) { }
+			public SwallowTarget(Actor self, Target target, bool allowMovement, bool forceAttack)
+				: base(self, target, allowMovement, forceAttack) { }
 
-			// Worms ignore visibility, so don't need to recalculate targets
-			protected override bool IgnoresVisibility { get { return true; } }
+			protected override Target RecalculateTarget(Actor self)
+			{
+				// Worms ignore visibility, so don't need to recalculate targets
+				return target;
+			}
 		}
 	}
 }

mods/cnc/rules/vehicles.yaml

@@ -174,6 +174,7 @@ ARTY:
 		LocalOffset: 624,0,208
 		MuzzleSequence: muzzle
 	AttackFrontal:
+		TargetFrozenActors: True
 	WithMuzzleOverlay:
 	AutoTarget:
 		InitialStanceAI: Defend
@@ -520,6 +521,7 @@ MSAM:
 		Weapon: 227mm
 		LocalOffset: 213,-128,0, 213,128,0
 	AttackFrontal:
+		TargetFrozenActors: True
 	WithSpriteTurret:
 	SpawnActorOnDeath:
 		Actor: MSAM.Husk

mods/ra/rules/ships.yaml

@@ -118,6 +118,7 @@ MSUB:
 		LocalOffset: 0,-171,0, 0,171,0
 		FireDelay: 2
 	AttackFrontal:
+		TargetFrozenActors: True
 	SelectionDecorations:
 	AutoTarget:
 		InitialStance: HoldFire
@@ -234,6 +235,7 @@ CA:
 		MuzzleSequence: muzzle
 	AttackTurreted:
 		Turrets: primary, secondary
+		TargetFrozenActors: True
 	WithMuzzleOverlay:
 	SelectionDecorations:
 	WithSpriteTurret@PRIMARY:

mods/ra/rules/vehicles.yaml

@@ -28,6 +28,7 @@ V2RL:
 	AutoTarget:
 		ScanRadius: 10
 	AttackFrontal:
+		TargetFrozenActors: True
 	WithFacingSpriteBody:
 		RequiresCondition: !reloading
 		Name: loaded
@@ -263,6 +264,7 @@ ARTY:
 		LocalOffset: 624,0,208
 		MuzzleSequence: muzzle
 	AttackFrontal:
+		TargetFrozenActors: True
 	WithMuzzleOverlay:
 	Explodes:
 		Weapon: ArtilleryExplode

mods/ts/rules/gdi-vehicles.yaml

@@ -363,6 +363,7 @@ JUGG:
 		Turrets: deployed
 		RequiresCondition: deployed
 		PauseOnCondition: empdisable
+		TargetFrozenActors: True
 	Armament@deployed:
 		Name: deployed
 		Turret: deployed

mods/ts/rules/nod-vehicles.yaml

@@ -258,6 +258,7 @@ ART2:
 		Turrets: deployed
 		RequiresCondition: deployed
 		PauseOnCondition: empdisable
+		TargetFrozenActors: True
 	Armament@deployed:
 		Name: deployed
 		Turret: deployed