From 1fc1bdc849a6ef0fa353433140cd6f7812c5baad Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Fri, 19 Aug 2022 21:46:23 +0100 Subject: [PATCH] Fix frozen actors lacking tooltips if they have the cloak ability. Since bbf5970bc16a41ac5f7a7324c99dd0c3a917418d we update frozen actors only when required. In 8339c6843ed122fb093b2f22e17a2c26e483b64d 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 | 29 ++++++++++++++----- .../Traits/FrozenUnderFogUpdatedByGps.cs | 4 --- .../Traits/Modifiers/FrozenUnderFog.cs | 14 +++++---- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/OpenRA.Game/Traits/Player/FrozenActorLayer.cs b/OpenRA.Game/Traits/Player/FrozenActorLayer.cs index 1c4c8ee924..2420dd27e2 100644 --- a/OpenRA.Game/Traits/Player/FrozenActorLayer.cs +++ b/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 - // the actor is hidden under the fog would leak the actors position via the tooltips and - // AutoTargetability, and keeping Visible as false would cause the actor to be rendered - // under the fog. - public bool Visible = true; - public bool Hidden = false; + // but not actually visible because a visibility modifier hid the actor. Setting Visible to + // true when the actor is hidden under the fog would leak the actors position via the + // tooltips and AutoTargetability, and keeping Visible as false would cause the actor to be + // rendered under the fog. + public bool Visible { get; private set; } = true; + 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().ToArray(); health = actor.TraitOrDefault(); + visibilityModifiers = actor.TraitsImplementing().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) diff --git a/OpenRA.Mods.Cnc/Traits/FrozenUnderFogUpdatedByGps.cs b/OpenRA.Mods.Cnc/Traits/FrozenUnderFogUpdatedByGps.cs index 3f334dee22..4939406dfb 100644 --- a/OpenRA.Mods.Cnc/Traits/FrozenUnderFogUpdatedByGps.cs +++ b/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; } }; diff --git a/OpenRA.Mods.Common/Traits/Modifiers/FrozenUnderFog.cs b/OpenRA.Mods.Common/Traits/Modifiers/FrozenUnderFog.cs index 1b86e24e97..a162b04a4b 100644 --- a/OpenRA.Mods.Common/Traits/Modifiers/FrozenUnderFog.cs +++ b/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(state.FrozenActor, playerIndex); + UpdateFrozenActor(frozen, playerIndex); - // Needed so tooltips appear. - state.FrozenActor.Hidden = false; - } + frozen.RefreshHidden(); } }); @@ -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)