diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 14370c9f..36220375 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -62,14 +62,36 @@ missing from our template handling. ## #379 — Chat-window opacity applies to ALL retained windows/panels, not only the chat windows -**Status:** OPEN — filed 2026-08-11 at Campaign OP gate 4 (user report: -"When I change the opacity for the chat window only the chatwindows shall -change not the other panels"). Retail's ChatInterface::SetOpacity scopes -the transparency pair to the chat windows; acdream's -`RetailWindowOpacityController` (CH6c) is applying the pair to other -retained panels too. Determine the exact retail scope (main chat + the -four floating chat windows only?) from the decomp before narrowing, then -scope the controller's application set to it. +**Status:** ROOT-CAUSED + FIXED (this commit) — pending the user's +re-gate. Filed 2026-08-11 at Campaign OP gate 4 (user report: "When I +change the opacity for the chat window only the chatwindows shall change +not the other panels"). + +**ROOT CAUSE — confirmed structural, not just AP-190's known divergence.** +Grepped `acclient_2013_pseudo_c.txt` for every call site of +`ChatInterface::SetDefaultOpacity`/`SetActiveOpacity`: there are exactly +two, `gmFloatyMainChatUI::UpdateFromPlayerModule`/ +`RecvNotice_GameplayOptionChanged` (the main chat window) and +`gmFloatyChatUI::UpdateFromPlayerModule` (the four floating windows), +each calling the method on itself. No other `gmPanelUI` sibling derives +from `ChatInterface`, so no other window class even has these methods in +its vtable — retail's scope is structural, not a runtime choice. **Fix:** +`RetailWindowOpacityController` now scopes `Attach`/`OnWindowRegistered`/ +`ReapplyAll`/`Dispose` to exactly the five `ChatWindowNames` (main chat + +`ChatWindow1`-`4`) instead of every window `RetailWindowManager` +registers. Register row AP-190 updated in the same commit (the scope +divergence it recorded is now closed; the default-value/easing/focus- +predicate residuals it also recorded are unaffected). Regressed by +`tests/AcDream.App.Tests/UI/RetailWindowOpacityControllerTests.cs` +(`OpacityFade_AppliesOnlyToChatWindows_NeverOtherPanels` pins the exact +applied-window set; the pre-existing tests were updated to register +windows under their real `WindowNames` so the scope check is exercised +by name, matching production). + +**Re-gate (§OP5 step 4, rewritten in the gate script): use a chat window +(main or floating) as the "other window," not the toolbar/vitals/another +panel — a non-chat window should now stay fully opaque regardless of the +slider position.** ## #378 — Config-tab dropdown menus render bare (no button well, no arrow) and no popup opens on click diff --git a/docs/research/2026-08-11-campaign-op-test-script.md b/docs/research/2026-08-11-campaign-op-test-script.md index 3f05ac38..9403b3a7 100644 --- a/docs/research/2026-08-11-campaign-op-test-script.md +++ b/docs/research/2026-08-11-campaign-op-test-script.md @@ -427,10 +427,24 @@ chat windows already read when deciding which lines to show. ### Opacity sliders — live drag, linked, never clamping + +**Gate-4 re-test note (#379) — step 4 below is REWRITTEN:** the sliders +previously faded EVERY registered window (vitals, toolbar, inventory, +spellbook, radar, the Options panel itself, ...), not just chat windows — +matching retail's own bug-for-bug scope. They now affect ONLY the main +chat window and the four floating chat windows, exactly like retail's +`ChatInterface::SetDefaultOpacity`/`SetActiveOpacity` (which only +`gmMainChatUI`/`gmFloatyChatUI` ever call). **Use a chat window (main or +floating), not the toolbar/vitals/another panel, as your "other window" in +step 4** — a non-chat window's opacity should now stay FIXED (opaque) +regardless of the slider position. + 4. **Drag the FIRST slider (Default Opacity) most of the way to the right** while the Options panel itself is NOT focused/hovered by your mouse (so - you can see another registered window, e.g. the toolbar or a floating - chat window, at its UNFOCUSED opacity). Watch that OTHER window's + you can see another CHAT window — the main chat window or a floating + chat window 1-4 — at its UNFOCUSED opacity; a NON-chat window like the + toolbar or vitals bar must stay fully opaque throughout this whole + section, per the #379 note above). Watch that other CHAT window's transparency change LIVE, continuously, as you drag — not just on release. This is retail's `SetCurrentValue -> Apply(1)` immediate-apply semantic. diff --git a/src/AcDream.App/UI/RetailWindowOpacityController.cs b/src/AcDream.App/UI/RetailWindowOpacityController.cs index 46251781..15630e87 100644 --- a/src/AcDream.App/UI/RetailWindowOpacityController.cs +++ b/src/AcDream.App/UI/RetailWindowOpacityController.cs @@ -13,14 +13,24 @@ namespace AcDream.App.UI; /// (docs/research/2026-08-09-chat-retail-window-shell.md §3). /// /// -/// Retail applies this ONLY to ChatInterface-derived windows (the main chat -/// window + the four floaties). acdream applies it to every window -/// 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 m_chatEntry specifically have -/// focus"; the generalization here is "does ANY descendant of this window have -/// keyboard focus", which already computes for -/// every window via . +/// #379 fix (2026-08-11, gate 4): scoped to ChatInterface-derived +/// windows only. Retail's ONLY two callers of +/// ChatInterface::SetDefaultOpacity/SetActiveOpacity are +/// gmFloatyMainChatUI::UpdateFromPlayerModule/ +/// RecvNotice_GameplayOptionChanged (0x004ce3f0/0x004d25a0 — +/// the MAIN chat window) and gmFloatyChatUI::UpdateFromPlayerModule +/// (0x004ce3f0 — the four floaty windows), each calling the method on +/// itself (this). No other gmPanelUI sibling derives from +/// ChatInterface, 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 +/// registers ( below narrows that to the five +/// named chat windows). The retail focus test is "does m_chatEntry +/// specifically have focus"; the generalization here is "does ANY descendant of +/// this window have keyboard focus", which +/// already computes for every window via +/// (AP-190 residual, +/// unaffected by this fix). /// /// /// @@ -31,13 +41,13 @@ namespace AcDream.App.UI; /// ChatInterface::ChatInterface directly with no override, so the four /// floating windows keep the base 0.5/1.0. CH6c review fix: acdream ships ONE /// shared default — gmMainChatUI'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 gmPanelUI 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). /// /// @@ -51,6 +61,26 @@ namespace AcDream.App.UI; /// public sealed class RetailWindowOpacityController : IDisposable { + /// + /// #379 fix: the exact five ChatInterface-derived windows — + /// gmFloatyMainChatUI () and the four + /// gmFloatyChatUI instances (.. + /// ) — 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 ChatInterface and + /// so never call SetDefaultOpacity/SetActiveOpacity in + /// retail. + /// + private static readonly HashSet ChatWindowNames = new(StringComparer.Ordinal) + { + WindowNames.Chat, + WindowNames.ChatWindow1, + WindowNames.ChatWindow2, + WindowNames.ChatWindow3, + WindowNames.ChatWindow4, + }; + private readonly RetailWindowManager _manager; private readonly HashSet _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); + } /// /// 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(); } } diff --git a/tests/AcDream.App.Tests/UI/RetailWindowOpacityControllerTests.cs b/tests/AcDream.App.Tests/UI/RetailWindowOpacityControllerTests.cs index c85a72bc..5abbd3f7 100644 --- a/tests/AcDream.App.Tests/UI/RetailWindowOpacityControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/RetailWindowOpacityControllerTests.cs @@ -6,8 +6,22 @@ namespace AcDream.App.Tests.UI; /// Campaign CH slice CH6c: — the live /// per-window opacity mechanism ported from ChatInterface::SetOpacity/ /// SetDefaultOpacity/SetActiveOpacity (0x004F3120/0x004F3BC0/ -/// 0x004F3C40), extended to every -registered -/// window rather than retail's ChatInterface-only scope (register row AP-190). +/// 0x004F3C40). +/// +/// +/// #379 fix (2026-08-11, gate 4): the CH6c-era "extended to every +/// RetailWindowManager-registered window" scope (formerly register row AP-190) +/// was a bug, not a deliberate divergence — retail's ONLY two +/// ChatInterface::SetDefaultOpacity/SetActiveOpacity callers are +/// the main chat window and the four floaty chat windows (see the class's own +/// doc comment). These tests now register windows under their REAL +/// so the scope check is exercised by name, exactly +/// like the production wiring ( registers every +/// window under its constant, never an arbitrary +/// string) — the pre-fix tests used placeholder names ("Vitals", "A", "B", +/// ...) that happened to fall inside the (bugged) unscoped set and would now +/// silently fall outside the real scope for the wrong reason. +/// /// public sealed class RetailWindowOpacityControllerTests { @@ -30,7 +44,7 @@ public sealed class RetailWindowOpacityControllerTests public void Construction_AttachesToAlreadyRegisteredWindows_AtDefaultOpacity() { UiRoot root = NewRoot(); - (RetailWindowHandle handle, _) = RegisterWindow(root, "Vitals"); + (RetailWindowHandle handle, _) = RegisterWindow(root, WindowNames.Chat); var controller = new RetailWindowOpacityController( root.WindowManager, defaultOpacity: 0.5f, activeOpacity: 1.0f); @@ -51,9 +65,9 @@ public sealed class RetailWindowOpacityControllerTests // The window is mounted AFTER the controller exists — proves the // RetailWindowManager.WindowRegistered subscription (not just the ctor's - // catch-up loop over already-registered windows) is what applies retail's - // GLOBAL opacity scope to every future Mount* call too. - (RetailWindowHandle handle, _) = RegisterWindow(root, "Toolbar"); + // catch-up loop over already-registered windows) is what applies the + // chat-scoped opacity to every future Mount* call too. + (RetailWindowHandle handle, _) = RegisterWindow(root, WindowNames.ChatWindow1); Assert.Equal(0.3f, handle.Opacity); } @@ -62,7 +76,7 @@ public sealed class RetailWindowOpacityControllerTests public void FocusEnteringAWindow_SwitchesToActiveOpacity_LeavingSwitchesBack() { UiRoot root = NewRoot(); - (RetailWindowHandle handle, UiElement child) = RegisterWindow(root, "Chat"); + (RetailWindowHandle handle, UiElement child) = RegisterWindow(root, WindowNames.Chat); var controller = new RetailWindowOpacityController( root.WindowManager, defaultOpacity: 0.5f, activeOpacity: 1.0f); Assert.Equal(0.5f, handle.Opacity); @@ -75,26 +89,55 @@ public sealed class RetailWindowOpacityControllerTests GC.KeepAlive(controller); } + /// + /// #379 regression: pins the EXACT applied-window set to the five real + /// chat windows. Before the fix, this same setup + /// left vitals/toolbar at 0.4 (the DefaultOpacity) instead of + /// their own authored 1.0 — the user's literal complaint ("only the + /// chatwindows shall change not the other panels"). + /// [Fact] - public void OpacityFade_AppliesToEveryRegisteredWindow_NotJustChat() + public void OpacityFade_AppliesOnlyToChatWindows_NeverOtherPanels() { - // The CH6c scope extension: retail's ChatInterface::SetOpacity only ever - // runs on chat-derived windows. acdream applies the SAME mechanism to - // every RetailWindowManager window — vitals, toolbar, whatever else is - // mounted — matching the task's GLOBAL-option framing. UiRoot root = NewRoot(); - (RetailWindowHandle vitals, _) = RegisterWindow(root, "Vitals"); - (RetailWindowHandle toolbar, UiElement toolbarChild) = RegisterWindow(root, "Toolbar"); + (RetailWindowHandle vitals, _) = RegisterWindow(root, WindowNames.Vitals); + (RetailWindowHandle toolbar, UiElement toolbarChild) = RegisterWindow(root, WindowNames.Toolbar); + (RetailWindowHandle chat, _) = RegisterWindow(root, WindowNames.Chat); + (RetailWindowHandle floaty1, UiElement floaty1Child) = RegisterWindow(root, WindowNames.ChatWindow1); var controller = new RetailWindowOpacityController( root.WindowManager, defaultOpacity: 0.4f, activeOpacity: 1.0f); - Assert.Equal(0.4f, vitals.Opacity); - Assert.Equal(0.4f, toolbar.Opacity); + // Non-chat windows never had Apply called on them at all — they sit at + // UiElement.Opacity's own 1.0 ctor default, untouched by the slider. + Assert.Equal(1.0f, vitals.Opacity); + Assert.Equal(1.0f, toolbar.Opacity); + // The two chat windows DID get the DefaultOpacity applied. + Assert.Equal(0.4f, chat.Opacity); + Assert.Equal(0.4f, floaty1.Opacity); root.SetKeyboardFocus(toolbarChild); - Assert.Equal(0.4f, vitals.Opacity); // unrelated window: still unfocused - Assert.Equal(1.0f, toolbar.Opacity); // the focused one: active + // Focusing a NON-chat window's descendant still does not touch its own + // opacity (that window was never subscribed to focus changes either) + // NOR does it bleed the ACTIVE opacity onto the chat windows — they + // stay at DefaultOpacity since neither of THEM has focus. + Assert.Equal(1.0f, vitals.Opacity); + Assert.Equal(1.0f, toolbar.Opacity); + Assert.Equal(0.4f, chat.Opacity); + Assert.Equal(0.4f, floaty1.Opacity); + + root.SetKeyboardFocus(floaty1Child); + controller.SetActiveOpacity(0.6f); + + // A live re-apply (SetActiveOpacity -> ReapplyAll) is ALSO scoped — the + // two non-chat windows remain untouched throughout, proving ReapplyAll + // itself (not just the initial Attach) respects the scope. The FOCUSED + // chat window picks up the new active value; the unfocused one stays + // at the (unchanged) default. + Assert.Equal(1.0f, vitals.Opacity); + Assert.Equal(1.0f, toolbar.Opacity); + Assert.Equal(0.4f, chat.Opacity); + Assert.Equal(0.6f, floaty1.Opacity); } [Fact] @@ -104,8 +147,8 @@ public sealed class RetailWindowOpacityControllerTests // raising DEFAULT above the current ACTIVE value drags active UP to // match — it never clamps the default down instead. UiRoot root = NewRoot(); - (RetailWindowHandle unfocused, _) = RegisterWindow(root, "A"); - (RetailWindowHandle focused, UiElement focusedChild) = RegisterWindow(root, "B"); + (RetailWindowHandle unfocused, _) = RegisterWindow(root, WindowNames.Chat); + (RetailWindowHandle focused, UiElement focusedChild) = RegisterWindow(root, WindowNames.ChatWindow1); var controller = new RetailWindowOpacityController( root.WindowManager, defaultOpacity: 0.3f, activeOpacity: 0.5f); root.SetKeyboardFocus(focusedChild); @@ -125,8 +168,8 @@ public sealed class RetailWindowOpacityControllerTests { // Symmetric case (ChatInterface::SetActiveOpacity @0x004F3C40). UiRoot root = NewRoot(); - (RetailWindowHandle unfocused, _) = RegisterWindow(root, "A"); - (RetailWindowHandle focused, UiElement focusedChild) = RegisterWindow(root, "B"); + (RetailWindowHandle unfocused, _) = RegisterWindow(root, WindowNames.Chat); + (RetailWindowHandle focused, UiElement focusedChild) = RegisterWindow(root, WindowNames.ChatWindow1); var controller = new RetailWindowOpacityController( root.WindowManager, defaultOpacity: 0.5f, activeOpacity: 0.7f); root.SetKeyboardFocus(focusedChild); @@ -145,7 +188,7 @@ public sealed class RetailWindowOpacityControllerTests // UpdateFromPlayerModule (0x004CE3F0) reads/applies Default first, then // Active — the shape used to push a freshly loaded ChatSettings pair. UiRoot root = NewRoot(); - (RetailWindowHandle handle, _) = RegisterWindow(root, "Chat"); + (RetailWindowHandle handle, _) = RegisterWindow(root, WindowNames.Chat); var controller = new RetailWindowOpacityController( root.WindowManager, defaultOpacity: 0.5f, activeOpacity: 1.0f); @@ -174,7 +217,7 @@ public sealed class RetailWindowOpacityControllerTests public void Dispose_UnsubscribesFromFocusChanges() { UiRoot root = NewRoot(); - (RetailWindowHandle handle, UiElement child) = RegisterWindow(root, "Chat"); + (RetailWindowHandle handle, UiElement child) = RegisterWindow(root, WindowNames.Chat); var controller = new RetailWindowOpacityController( root.WindowManager, defaultOpacity: 0.5f, activeOpacity: 1.0f); @@ -198,14 +241,14 @@ public sealed class RetailWindowOpacityControllerTests // kind a lingering external reference to the handle could still fire) // must not reach the controller anymore. UiRoot root = NewRoot(); - (RetailWindowHandle handle, UiElement child) = RegisterWindow(root, "Chat"); + (RetailWindowHandle handle, UiElement child) = RegisterWindow(root, WindowNames.Chat); var controller = new RetailWindowOpacityController( root.WindowManager, defaultOpacity: 0.5f, activeOpacity: 1.0f); root.SetKeyboardFocus(child); Assert.Equal(1.0f, handle.Opacity); - root.WindowManager.Unregister("Chat"); + root.WindowManager.Unregister(WindowNames.Chat); // Unregister hides the outer frame, which drops keyboard focus off // the now-invisible child — the manager's (still-live at that point) @@ -228,7 +271,7 @@ public sealed class RetailWindowOpacityControllerTests public void SetMutators_AfterDispose_AreNoOps() { UiRoot root = NewRoot(); - (RetailWindowHandle handle, _) = RegisterWindow(root, "Chat"); + (RetailWindowHandle handle, _) = RegisterWindow(root, WindowNames.Chat); var controller = new RetailWindowOpacityController( root.WindowManager, defaultOpacity: 0.5f, activeOpacity: 1.0f);