Fix frozen actors lacking tooltips if they have the cloak ability.

Since bbf5970bc1 we update frozen actors only when required.

In 8339c6843e a regression was fixed where actors created in line of sight would be invisible.

Here, we fix a related regression where cloaked units that are revealed, and then frozen when you move out of line of sight would lack tooltips.

The fix centers around the setting of the Hidden flag. In the old code this used CanBeViewedByPlayer which checks for visibility modifiers and then uses the default visibility. The bug with this code is that when a visibility modifier was not hiding the actor, then we would report the default visibility state instead. However the frozen visibility state applies here which means if the frozen actor is visible, then we consider the actor to be hidden and therefore tooltips will not appear. In the fixed version we only consider the modifiers. This means a visibility modifier such as Cloak can hide the frozen actor tooltips. But otherwise we do not consider the frozen actor to be hidden. This prevents a frozen actor from hiding its own tooltips in some unintended circular logic. Hidden now becomes just a flag to indicate if the visibility modifiers are overriding things, as intended.

OpenRA.Game/Traits/Player/FrozenActorLayer.cs

@@ -54,16 +54,18 @@ namespace OpenRA.Traits
 		public DamageState DamageState { get; private set; }
 		readonly IHealth health;
 
+		readonly IVisibilityModifier[] visibilityModifiers;
+
 		// The Visible flag is tied directly to the actor visibility under the fog.
 		// If Visible is true, the actor is made invisible (via FrozenUnderFog/IDefaultVisibility)
 		// and this FrozenActor is rendered instead.
 		// The Hidden flag covers the edge case that occurs when the backing actor was last "seen"
-		// to be cloaked or otherwise not CanBeViewedByPlayer()ed. Setting Visible to true when
+		// but not actually visible because a visibility modifier hid the actor. Setting Visible to
-		// the actor is hidden under the fog would leak the actors position via the tooltips and
+		// true when the actor is hidden under the fog would leak the actors position via the
-		// AutoTargetability, and keeping Visible as false would cause the actor to be rendered
+		// tooltips and AutoTargetability, and keeping Visible as false would cause the actor to be
-		// under the fog.
+		// rendered under the fog.
-		public bool Visible = true;
+		public bool Visible { get; private set; } = true;
-		public bool Hidden = false;
+		public bool Hidden { get; private set; } = false;
 
 		public bool Shrouded { get; private set; }
 		public bool NeedRenderables { get; set; }
@@ -108,6 +110,7 @@ namespace OpenRA.Traits
 
 			tooltips = actor.TraitsImplementing<ITooltip>().ToArray();
 			health = actor.TraitOrDefault<IHealth>();
+			visibilityModifiers = actor.TraitsImplementing<IVisibilityModifier>().ToArray();
 
 			UpdateVisibility();
 		}
@@ -124,7 +127,6 @@ namespace OpenRA.Traits
 			TargetTypes = actor.GetEnabledTargetTypes();
 			targetablePositions.Clear();
 			targetablePositions.AddRange(actor.GetTargetablePositions());
-			Hidden = !actor.CanBeViewedByPlayer(viewer);
 
 			if (health != null)
 			{
@@ -140,6 +142,19 @@ namespace OpenRA.Traits
 			}
 		}
 
+		public void RefreshHidden()
+		{
+			Hidden = false;
+			foreach (var visibilityModifier in visibilityModifiers)
+			{
+				if (!visibilityModifier.IsVisible(actor, viewer))
+				{
+					Hidden = true;
+					break;
+				}
+			}
+		}
+
 		public void Tick()
 		{
 			if (flashTicks > 0)

OpenRA.Mods.Cnc/Traits/FrozenUnderFogUpdatedByGps.cs

@@ -32,11 +32,7 @@ namespace OpenRA.Mods.Cnc.Traits
 			// This only makes sense if the frozen actor has already been revealed (i.e. has renderables)
 			if (fa.HasRenderables)
 			{
-				// HACK: RefreshState updated *all* actor state, not just the owner
-				// This is generally bogus, and specifically breaks cursors and tooltips by setting Hidden to false
-				var hidden = fa.Hidden;
 				fa.RefreshState();
-				fa.Hidden = hidden;
 				fa.NeedRenderables = true;
 			}
 		};

OpenRA.Mods.Common/Traits/Modifiers/FrozenUnderFog.cs

@@ -81,13 +81,11 @@ namespace OpenRA.Mods.Common.Traits
 				for (var playerIndex = 0; playerIndex < frozenStates.Count; playerIndex++)
 				{
 					var state = frozenStates[playerIndex];
+					var frozen = state.FrozenActor;
 					if (startsRevealed || state.IsVisible)
-					{
+						UpdateFrozenActor(frozen, playerIndex);
-						UpdateFrozenActor(state.FrozenActor, playerIndex);
 
-						// Needed so tooltips appear.
+					frozen.RefreshHidden();
-						state.FrozenActor.Hidden = false;
-					}
 				}
 			});
 
@@ -113,6 +111,8 @@ namespace OpenRA.Mods.Common.Traits
 
 			if (isVisible)
 				UpdateFrozenActor(frozen, frozen.Viewer.World.Players.IndexOf(frozen.Viewer));
+
+			frozen.RefreshHidden();
 		}
 
 		bool IsVisibleInner(Player byPlayer)
@@ -176,7 +176,9 @@ namespace OpenRA.Mods.Common.Traits
 		{
 			// Force a state update for the old owner so the tooltip etc doesn't show them as the owner
 			var oldOwnerIndex = self.World.Players.IndexOf(oldOwner);
-			UpdateFrozenActor(frozenStates[oldOwnerIndex].FrozenActor, oldOwnerIndex);
+			var frozen = frozenStates[oldOwnerIndex].FrozenActor;
+			UpdateFrozenActor(frozen, oldOwnerIndex);
+			frozen.RefreshHidden();
 		}
 
 		void INotifyActorDisposing.Disposing(Actor self)