erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases 6 Packages Wiki Activity Actions

fix #379: chat opacity fade was scoped to every window, not just chat

Browse source 
Root cause: RetailWindowOpacityController applied the Default/Active
opacity fade to EVERY registered UiRoot window (vitals, toolbar,
inventory, spellbook, radar, even the Options panel itself), but
retail's ChatInterface::SetDefaultOpacity/SetActiveOpacity are only
ever called by gmMainChatUI/gmFloatyChatUI — the mechanism is chat-only
in retail, not a global window-opacity feature.

Fix: scope the controller's catch-up loop, OnWindowRegistered,
ReapplyAll, and Dispose to WindowNames.Chat/ChatWindow1-4 only; every
other registered window now stays fully opaque regardless of slider
position, matching retail's own scope. Regressed by
RetailWindowOpacityControllerTests.
OpacityFade_AppliesOnlyToChatWindows_NeverOtherPanels (registers
vitals/toolbar/chat/a floating chat window and asserts the non-chat
windows never move off 1.0 while chat windows still track Default/
Active correctly).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-11 23:00:57 +02:00
parent c121842664
commit c0b3d8f233
4 changed files with 177 additions and 56 deletions
Download patch file Download diff file Expand all files Collapse all files

80
src/AcDream.App/UI/RetailWindowOpacityController.cs
View file

@ -13,14 +13,24 @@ namespace AcDream.App.UI;
/// (<c>docs/research/2026-08-09-chat-retail-window-shell.md</c> §3).
///
/// <para>
/// Retail applies this ONLY to <c>ChatInterface</c>-derived windows (the main chat
/// window + the four floaties). acdream applies it to every window
/// <see cref="RetailWindowManager"/> registers — the single Settings transparency
/// slider therefore affects the whole retained UI, not just chat (register row
/// AP-190). The retail focus test is "does <c>m_chatEntry</c> specifically have
/// focus"; the generalization here is "does ANY descendant of this window have
/// keyboard focus", which <see cref="RetailWindowManager"/> already computes for
/// every window via <see cref="RetailWindowHandle.DescendantFocusChanged"/>.
/// <b>#379 fix (2026-08-11, gate 4): scoped to <c>ChatInterface</c>-derived
/// windows only.</b> Retail's ONLY two callers of
/// <c>ChatInterface::SetDefaultOpacity</c>/<c>SetActiveOpacity</c> are
/// <c>gmFloatyMainChatUI::UpdateFromPlayerModule</c>/
/// <c>RecvNotice_GameplayOptionChanged</c> (<c>0x004ce3f0</c>/<c>0x004d25a0</c> —
/// the MAIN chat window) and <c>gmFloatyChatUI::UpdateFromPlayerModule</c>
/// (<c>0x004ce3f0</c> — the four floaty windows), each calling the method on
/// itself (<c>this</c>). No other <c>gmPanelUI</c> sibling derives from
/// <c>ChatInterface</c>, so no other window class even HAS these methods in its
/// vtable — the scope is structural in retail, not a runtime choice. The
/// PRE-#379 code attached to every window <see cref="RetailWindowManager"/>
/// registers (<see cref="WindowScopeNames"/> below narrows that to the five
/// named chat windows). The retail focus test is "does <c>m_chatEntry</c>
/// specifically have focus"; the generalization here is "does ANY descendant of
/// this window have keyboard focus", which <see cref="RetailWindowManager"/>
/// already computes for every window via
/// <see cref="RetailWindowHandle.DescendantFocusChanged"/> (AP-190 residual,
/// unaffected by this fix).
/// </para>
///
/// <para>
@ -31,13 +41,13 @@ namespace AcDream.App.UI;
/// <c>ChatInterface::ChatInterface</c> directly with no override, so the four
/// floating windows keep the base 0.5/1.0. <b>CH6c review fix:</b> acdream ships ONE
/// shared default — <c>gmMainChatUI</c>'s 1.0/1.0 override, not the base
/// ChatInterface value — applied uniformly to every window including the four
/// floating chat windows. Shipping the base 0.5/1.0 pair globally (the original
/// CH6c behavior) faded the WHOLE registered UI to 50% opacity out of the box,
/// including windows that can never take keyboard focus and so were stuck at 0.5
/// permanently; 1.0/1.0 is retail-identical for the 11 non-chat windows and the
/// main chat window, and only the four floaties diverge from retail's
/// 0.5-while-idle default now — user-settable via the same slider (register row
/// ChatInterface value — applied to the main chat window AND the four floating
/// chat windows. Shipping the base 0.5/1.0 pair globally (the original CH6c
/// behavior) faded the whole registered UI to 50% opacity out of the box; with
/// #379's scoping fix the other eleven <c>gmPanelUI</c> siblings are never
/// touched by this controller at all (retail-correct — they have no opacity
/// pair to begin with), so only the four floaties diverge from retail's
/// 0.5-while-idle default — user-settable via the same slider (register row
/// AP-190).
/// </para>
///
@ -51,6 +61,26 @@ namespace AcDream.App.UI;
/// </summary>
public sealed class RetailWindowOpacityController : IDisposable
{
/// <summary>
/// #379 fix: the exact five <c>ChatInterface</c>-derived windows —
/// <c>gmFloatyMainChatUI</c> (<see cref="WindowNames.Chat"/>) and the four
/// <c>gmFloatyChatUI</c> instances (<see cref="WindowNames.ChatWindow1"/>..
/// <see cref="WindowNames.ChatWindow4"/>) — per this class's own doc
/// comment. Every other registered window name (vitals, toolbar,
/// inventory, spellbook, combat, radar, options, ...) is out of scope by
/// construction: those classes never derive from <c>ChatInterface</c> and
/// so never call <c>SetDefaultOpacity</c>/<c>SetActiveOpacity</c> in
/// retail.
/// </summary>
private static readonly HashSet<string> ChatWindowNames = new(StringComparer.Ordinal)
{
WindowNames.Chat,
WindowNames.ChatWindow1,
WindowNames.ChatWindow2,
WindowNames.ChatWindow3,
WindowNames.ChatWindow4,
};
private readonly RetailWindowManager _manager;
private readonly HashSet<RetailWindowHandle> _focused = new();
private bool _disposed;
@ -72,7 +102,8 @@ public sealed class RetailWindowOpacityController : IDisposable
_manager.WindowRegistered += OnWindowRegistered;
_manager.WindowUnregistered += OnWindowUnregistered;
foreach (RetailWindowHandle handle in _manager.Windows)
Attach(handle);
if (ChatWindowNames.Contains(handle.Name))
Attach(handle);
}
public float DefaultOpacity { get; private set; }
@ -118,7 +149,13 @@ public sealed class RetailWindowOpacityController : IDisposable
ReapplyAll();
}
private void OnWindowRegistered(RetailWindowHandle handle) => Attach(handle);
private void OnWindowRegistered(RetailWindowHandle handle)
{
// #379: only the five chat windows are ChatInterface-derived in
// retail — see ChatWindowNames' own doc comment.
if (ChatWindowNames.Contains(handle.Name))
Attach(handle);
}
/// <summary>
/// CH6c review NIT: without this, a window unregistered while it held
@ -154,8 +191,12 @@ public sealed class RetailWindowOpacityController : IDisposable
private void ReapplyAll()
{
// #379: scoped to the same five chat windows Attach/OnWindowRegistered
// subscribed — iterating every registered window here (the pre-#379
// shape) is what leaked the slider onto vitals/inventory/spellbook/etc.
foreach (RetailWindowHandle handle in _manager.Windows)
Apply(handle, _focused.Contains(handle));
if (ChatWindowNames.Contains(handle.Name))
Apply(handle, _focused.Contains(handle));
}
public void Dispose()
@ -167,7 +208,8 @@ public sealed class RetailWindowOpacityController : IDisposable
_manager.WindowRegistered -= OnWindowRegistered;
_manager.WindowUnregistered -= OnWindowUnregistered;
foreach (RetailWindowHandle handle in _manager.Windows)
handle.DescendantFocusChanged -= OnDescendantFocusChanged;
if (ChatWindowNames.Contains(handle.Name))
handle.DescendantFocusChanged -= OnDescendantFocusChanged;
_focused.Clear();
}
}