Split Target.Recalculate into methods with and without invalidation.

TargetExtensions is moved into its own file.

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

@@ -43,7 +43,7 @@ namespace OpenRA.Mods.Common.Activities
 				return NextActivity;
 			}
 
-			target = target.Recalculate(self.Owner);
+			target = target.RecalculateInvalidatingHiddenTargets(self.Owner);
 
 			if (!target.IsValidFor(self))
 				return NextActivity;

OpenRA.Mods.Common/Activities/Attack.cs

@@ -53,7 +53,7 @@ namespace OpenRA.Mods.Common.Activities
 
 		protected virtual Target RecalculateTarget(Actor self)
 		{
-			return target.Recalculate(self.Owner);
+			return target.RecalculateInvalidatingHiddenTargets(self.Owner);
 		}
 
 		public override Activity Tick(Actor self)
@@ -148,47 +148,4 @@ namespace OpenRA.Mods.Common.Activities
 			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);
-
-			// Bot-controlled units aren't yet capable of understanding visibility changes
-			if (viewer.IsBot)
-				return t;
-
-			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/OpenRA.Mods.Common.csproj

@@ -120,6 +120,7 @@
     <Compile Include="Activities\WaitForTransport.cs" />
     <Compile Include="ActorExts.cs" />
     <Compile Include="AIUtils.cs" />
+    <Compile Include="TargetExtensions.cs" />
     <Compile Include="Traits\BotModules\Squads\AttackOrFleeFuzzy.cs" />
     <Compile Include="Traits\BotModules\BotModuleLogic\BaseBuilderQueueManager.cs" />
     <Compile Include="Traits\Player\ModularBot.cs" />

OpenRA.Mods.Common/TargetExtensions.cs

@@ -0,0 +1,79 @@
+#region Copyright & License Information
+/*
+ * Copyright 2007-2019 The OpenRA Developers (see AUTHORS)
+ * This file is part of OpenRA, which is free software. It is made
+ * available to you under the terms of the GNU General Public License
+ * as published by the Free Software Foundation, either version 3 of
+ * the License, or (at your option) any later version. For more
+ * information, see COPYING.
+ */
+#endregion
+
+using OpenRA.Traits;
+
+namespace OpenRA.Mods.Common
+{
+	public static class TargetExtensions
+	{
+		/// <summary>
+		/// Update (Frozen)Actor targets to account for visibility changes or actor replacement.
+		/// If the target actor becomes hidden without a FrozenActor, the target is invalidated.
+		/// /// </summary>
+		public static Target RecalculateInvalidatingHiddenTargets(this Target t, Player viewer)
+		{
+			bool targetIsHiddenActor;
+			var updated = t.Recalculate(viewer, out targetIsHiddenActor);
+			return targetIsHiddenActor ? Target.Invalid : updated;
+		}
+
+		/// <summary>
+		/// Update (Frozen)Actor targets to account for visibility changes or actor replacement.
+		/// If the target actor becomes hidden without a FrozenActor, the target actor is kept
+		/// and the actorHidden flag is set to true.
+		/// </summary>
+		public static Target Recalculate(this Target t, Player viewer, out bool targetIsHiddenActor)
+		{
+			targetIsHiddenActor = false;
+
+			// 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);
+
+			// Bot-controlled units aren't yet capable of understanding visibility changes
+			if (viewer.IsBot)
+				return t;
+
+			if (t.Type == TargetType.Actor)
+			{
+				// Actor has been hidden under the fog
+				if (!t.Actor.CanBeViewedByPlayer(viewer))
+				{
+					// Replace with FrozenActor if applicable, otherwise return target unmodified
+					var frozen = viewer.FrozenActorLayer.FromID(t.Actor.ActorID);
+					if (frozen != null)
+						return Target.FromFrozenActor(frozen);
+
+					targetIsHiddenActor = true;
+					return t;
+				}
+			}
+			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/AttackFollow.cs

@@ -33,7 +33,9 @@ namespace OpenRA.Mods.Common.Traits
 
 		protected override void Tick(Actor self)
 		{
-			Target = Target.Recalculate(self.Owner);
+			// 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;
@@ -83,7 +85,12 @@ namespace OpenRA.Mods.Common.Traits
 
 			public override Activity Tick(Actor self)
 			{
-				target = target.Recalculate(self.Owner);
+				// 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))
 					return NextActivity;
 

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

@@ -46,7 +46,9 @@ namespace OpenRA.Mods.Common.Traits
 
 			public override Activity Tick(Actor self)
 			{
-				target = target.Recalculate(self.Owner);
+				// This activity can't move to reacquire hidden targets, so use the
+				// Recalculate overload that invalidates hidden targets.
+				target = target.RecalculateInvalidatingHiddenTargets(self.Owner);
 				if (IsCanceled || !target.IsValidFor(self) || !attack.IsReachableTarget(target, allowMove))
 					return NextActivity;