diff --git a/docs/architecture/acdream-architecture.md b/docs/architecture/acdream-architecture.md index ade10126..6452fd24 100644 --- a/docs/architecture/acdream-architecture.md +++ b/docs/architecture/acdream-architecture.md @@ -104,6 +104,13 @@ Plugins register retained gameplay markup through the BCL-only `AcDream.Plugin.Abstractions.IUiRegistry`; they do not import App or ImGui assemblies. +Every retained gameplay window has a typed `RetailWindowHandle` and at most one +`IRetainedPanelController` lifecycle owner. Multi-controller windows use +`RetainedPanelControllerGroup`; the manager disposes controllers exactly once in +reverse ownership order. `UiHost` removes Silk device subscriptions first, then +disposes the manager/controllers, then its GL renderer. `GameWindow.OnClosing` +tears this runtime down before session and game-state sources. + --- ## Project Structure (current + target) diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 3be79c62..d916c42e 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -54,7 +54,7 @@ accepted-divergence entries (#96, #49, #50). | IA-12 | UI toolkit mirrors retail behavior from research docs, not a byte-port — keystone.dll is outside decomp coverage; observed constants embedded (drag 3 px, tooltip 1000 ms) | `src/AcDream.App/UI/README.md:3` (window mgr: `UiRoot.cs` RegisterWindow/Show/Hide/Toggle/BringToFront — F12 inventory toggle IS retail-faithful) | keystone.dll has no PDB/decomp; semantics reconstructed from the six `docs/research/retail-ui/` deep-dives, keeping retail's event-type constants so panel switch-cases transplant cleanly | Edge-case input semantics the research under-specified (drag threshold, tooltip timing, focus hand-off, capture corners) differ silently with no oracle to diff against | keystone.dll Device DAT_00837ff4; docs/research/retail-ui/04-input-events.md | | IA-13 | GameEventType registry deliberately omits event types retail ignores; unknown events fall through unhandled | `src/AcDream.Core.Net/Messages/GameEventType.cs:11` | Retail also ignores them — dropping matches retail by construction | If the "retail ignores X" judgment is wrong for any opcode (or a server mod uses one), the event is silently dropped with no diagnostic pointing at the omission | retail GameEvent dispatch (ignored-event set) | | IA-14 | Rendering + dat-handling base is WorldBuilder's tested port, not a fresh retail-decomp port (Phase N.4/O design stance) | `docs/architecture/worldbuilder-inventory.md` (code at `src/AcDream.{Core,App}/Rendering/Wb/`) | WB visually verified on the AC world, MIT, same stack; known WB↔retail deltas resolved case-by-case — terrain split kept retail `FSplitNESW` (**#51**, pinned by `SplitFormulaDivergenceTest`), scenery drift accepted (AP-31) | A WB-upstream divergence not yet caught ships silently as "our" behavior; guard = the inventory doc's 🟢/🔴 split + per-formula divergence tests | retail decomp per algorithm; `tests/.../SplitFormulaDivergenceTest.cs` | -| IA-15 | D.2b gameplay UI is our own `UiHost`/`UiRoot` retained tree, not a byte-port of Keystone. `RetailWindowManager`/typed handles centralize registry, raise, focus/capture cleanup, lifecycle events, controller teardown, and schema-v2 per-character/per-resolution layout persistence; `RetailWindowFrame` is the single production/Studio mount contract for imported-chrome and shared-wrapper windows. Production LayoutDesc imports include vitals `0x2100006C`, chat `0x21000006`, toolbar `0x21000016`, character `0x2100002E`, inventory `0x21000023` plus mounted `0x21000024/22/21`, and radar `0x21000074`. | `src/AcDream.App/UI/RetailWindowManager.cs`; `src/AcDream.App/UI/RetailWindowLayoutPersistence.cs`; `src/AcDream.App/UI/Layout/RetailWindowFrame.cs`; `src/AcDream.App/UI/Layout/LayoutImporter.cs`; production mounts in `GameWindow.cs` | Keystone has no matching PDB/decomp, so we preserve its observable ElementDesc/state/input behavior from DAT, named client call sites, and live evidence while using modern retained ownership. Real RenderSurfaces and imported element geometry remain the visual oracle. | Persistence is behaviorally reconstructed from `saveui/loadui` semantics rather than Keystone internals; lifecycle edge cases remain constrained by conformance tests instead of a byte-port | Production LayoutDesc objects; `docs/research/2026-06-15-layoutdesc-format.md`; Keystone behavior notes in `docs/research/retail-ui/` | +| IA-15 | D.2b gameplay UI is our own `UiHost`/`UiRoot` retained tree, not a byte-port of Keystone. `RetailWindowManager`/typed handles centralize registry, raise, focus/capture cleanup, lifecycle events, reverse-order grouped controller teardown, removable Silk input subscriptions, and schema-v2 per-character/per-resolution layout persistence; `RetailWindowFrame` is the single production/Studio mount contract for imported-chrome and shared-wrapper windows. Production LayoutDesc imports include vitals `0x2100006C`, chat `0x21000006`, toolbar `0x21000016`, character `0x2100002E`, inventory `0x21000023` plus mounted `0x21000024/22/21`, and radar `0x21000074`. | `src/AcDream.App/UI/RetailWindowManager.cs`; `src/AcDream.App/UI/RetainedPanelControllerGroup.cs`; `src/AcDream.App/UI/UiHost.cs`; `src/AcDream.App/UI/RetailWindowLayoutPersistence.cs`; `src/AcDream.App/UI/Layout/RetailWindowFrame.cs`; `src/AcDream.App/UI/Layout/LayoutImporter.cs`; production mounts in `GameWindow.cs` | Keystone has no matching PDB/decomp, so we preserve its observable ElementDesc/state/input behavior from DAT, named client call sites, and live evidence while using modern retained ownership. Real RenderSurfaces and imported element geometry remain the visual oracle. | Persistence is behaviorally reconstructed from `saveui/loadui` semantics rather than Keystone internals; lifecycle edge cases remain constrained by conformance tests instead of a byte-port | Production LayoutDesc objects; `docs/research/2026-06-15-layoutdesc-format.md`; Keystone behavior notes in `docs/research/retail-ui/` | | IA-17 | Toolbar chrome is toolkit-supplied through the central `RetailWindowFrame` mount (`UiCollapsibleFrame` 8-piece bevel) because LayoutDesc `0x21000016` carries no baked frame. It also supports a toolkit-defined collapse-to-one-row (bottom-edge resize snapping between a row-1-only and a two-row height, row-2 visibility tied to the stop) — retail's real collapse is keystone.dll (no decomp) and the DAT stacks both rows always. | `src/AcDream.App/UI/Layout/RetailWindowFrame.cs`; `src/AcDream.App/UI/UiCollapsibleFrame.cs`; toolbar policy in `GameWindow.cs`; spec: `docs/superpowers/specs/2026-06-20-d2b-toolbar-collapse-design.md` | The central mount now owns wrapper geometry/registration uniformly; border-over-content prevents the row-2 right cap from poking through | The collapse stops remain a toolkit reconstruction rather than a byte-port of Keystone behavior | gmToolbarUI WM chrome (keystone.dll, no PDB); no bevel ids in LayoutDesc 0x21000016 (toolbar dump) | | IA-18 | Effect overlay tile (enum 0x10000005) is a `ReplaceColor` SURFACE SOURCE — pure-white pixels in the composited drag icon are replaced PER-PIXEL with the same (x,y) pixel of the effect tile (the SURFACE overload `SurfaceWindow::ReplaceColor` 0x004415b0), preserving the tile's texture/gradient; the tile itself is NOT blitted as an additional layer. This IS faithful retail behavior. **Anti-regression: do NOT re-implement this as a blit layer NOR as a flat-color replace (it is a per-pixel surface copy).** | `src/AcDream.App/UI/IconComposer.cs` (`ReplaceWhiteFromSurface`) | Faithful port of `IconData::RenderIcons` @407614 → the SURFACE overload `ReplaceColor` 0x004415b0 (`dst[x,y]=src[x,y]` where `dst==white`); confirmed via clean Ghidra decompile + named decomp + visual (the Energy Crystal's blue is a gradient, 2026-06-17). | A blit-layer or flat-color re-implementation would show the wrong effect look (no gradient) — the visual-verification regression that retired the mean-color approximation | `IconData::RenderIcons` acclient_2013_pseudo_c.txt:407524; `ReplaceColor` SURFACE overload 0x004415b0:71656; `docs/research/2026-06-17-stateful-icon-RESOLVED.md` | diff --git a/docs/superpowers/plans/2026-07-10-retail-ui-fidelity-completion.md b/docs/superpowers/plans/2026-07-10-retail-ui-fidelity-completion.md index 6563f2d3..8b081e96 100644 --- a/docs/superpowers/plans/2026-07-10-retail-ui-fidelity-completion.md +++ b/docs/superpowers/plans/2026-07-10-retail-ui-fidelity-completion.md @@ -277,14 +277,20 @@ passed, 5 skipped. ### 2.4 Enforce controller teardown -- [ ] Apply the same `IRetainedPanelController : IDisposable` contract; allow plain +- [x] Apply the same `IRetainedPanelController : IDisposable` contract; allow plain `IDisposable` only for controllers that genuinely have no visibility/focus reaction. -- [ ] Convert inline event lambdas to removable handlers/subscription tokens. -- [ ] Dispose controllers on runtime shutdown/rebind and test that relog/recreate +- [x] Convert inline event lambdas to removable handlers/subscription tokens. +- [x] Dispose controllers on runtime shutdown/rebind and test that relog/recreate does not double-subscribe. -- [ ] Dispose controller subscriptions before session/data sources and before the +- [x] Dispose controller subscriptions before session/data sources and before the UI host; make every disposal path idempotent. +**Acceptance (2026-07-10):** every subscribing retained controller is owned by its +typed window handle (toolbar/inventory use reverse-order controller groups), late +attachment is lifecycle-safe, Silk input handlers are removable, and shutdown +disposes the retained runtime before session sources. Recreate/disposal regression +tests pass; full suite green: 4,549 passed, 5 skipped. + ### 2.5 Extract the composition block - [ ] Create `RetailUiRuntime` and focused binding records. diff --git a/memory/project_ui_architecture.md b/memory/project_ui_architecture.md index dc735609..79f7cecd 100644 --- a/memory/project_ui_architecture.md +++ b/memory/project_ui_architecture.md @@ -93,3 +93,19 @@ compass/coordinates, `RadarSnapshotProvider` joins the existing `WorldEntity` + and `UiRadar` only draws/handles retained interaction. `GameWindow` contains mount and delegate wiring, not the feature body. See `docs/research/2026-07-10-retail-radar-pseudocode.md`. + +## 2026-07-10 retained window ownership + +Every production retained window is registered through `RetailWindowManager` and +owned by a typed `RetailWindowHandle`. Controllers implement +`IRetainedPanelController`; windows with multiple focused controllers use +`RetainedPanelControllerGroup`, which forwards lifecycle in construction order and +disposes in reverse order exactly once. A controller created after a hidden mount +(inventory) is attached through `RetailWindowManager.AttachController`. + +Controller event subscriptions must use removable named handlers or paired +subscribe/unsubscribe delegates. `UiHost.Dispose` first removes Silk mouse/keyboard +subscriptions, then disposes the manager/controllers, then the GL text renderer. +`GameWindow.OnClosing` disposes layout persistence and `UiHost` before live session +or game-state sources. This ordering is the recreate/relogin double-subscription +guard; do not reintroduce anonymous lifetime-bound event lambdas. diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index f2940b29..284743ff 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -2158,6 +2158,7 @@ public sealed class GameWindow : IDisposable ResizeY = false, ConstrainDragToParent = true, ContentClickThrough = false, + Controller = _radarController, }); Console.WriteLine("[D.6] retail radar/compass from LayoutDesc 0x21000074."); } @@ -2219,6 +2220,7 @@ public sealed class GameWindow : IDisposable ResizeX = true, ResizeY = true, Opacity = 0.75f, + Controller = chatController, StateController = chatController, }); chatController.AttachWindow(chatHandle); @@ -2346,7 +2348,9 @@ public sealed class GameWindow : IDisposable _selectedObjectController = AcDream.App.UI.Layout.SelectedObjectController.Bind( toolbarLayout, subscribeSelectionChanged: h => SelectionChanged += h, + unsubscribeSelectionChanged: h => SelectionChanged -= h, subscribeHealthChanged: h => Combat.HealthChanged += h, + unsubscribeHealthChanged: h => Combat.HealthChanged -= h, isHealthTarget: IsHealthBarTarget, name: g => Objects.Get(g)?.Name, healthPercent: g => Combat.GetHealthPercent(g), @@ -2380,6 +2384,9 @@ public sealed class GameWindow : IDisposable | AcDream.App.UI.AnchorEdges.Top | AcDream.App.UI.AnchorEdges.Right, ContentClickThrough = true, + Controller = new AcDream.App.UI.RetainedPanelControllerGroup( + _toolbarController, + _selectedObjectController), }); var toolbarFrame = (AcDream.App.UI.UiCollapsibleFrame)toolbarHandle.OuterFrame; @@ -2578,6 +2585,12 @@ public sealed class GameWindow : IDisposable datFont: vitalsDatFont, itemInteraction: _itemInteractionController); // Slice 2: caption the "Slots" toggle button + _uiHost.WindowManager.AttachController( + AcDream.App.UI.WindowNames.Inventory, + new AcDream.App.UI.RetainedPanelControllerGroup( + _inventoryController, + _paperdollController)); + // Slice 2: capture the doll viewport widget (Type 0xD, element 0x100001D5) + the inventory // frame so the per-frame pre-UI hook can render the 3-D doll into the widget's texture only // when the window is open and in doll-view. The renderer itself is built after the @@ -14024,6 +14037,14 @@ public sealed class GameWindow : IDisposable private void OnClosing() { + // Retained controllers subscribe to session-owned object/selection/combat sources. + // Tear their window-manager ownership down first, while every source and GL-backed + // UI resource is still alive. UiHost disposes controllers before its renderer. + _windowLayoutPersistence?.Dispose(); + _windowLayoutPersistence = null; + _uiHost?.Dispose(); + _uiHost = null; + // Phase A.1: join the streamer worker thread before tearing down GL // state. The worker may still be processing a load job that references // _dats; Dispose cancels the token and waits up to 2s for the thread. @@ -14051,9 +14072,6 @@ public sealed class GameWindow : IDisposable _paperdollViewportRenderer?.Dispose(); // Slice 2: the doll's off-screen FBO + textures _particleRenderer?.Dispose(); _debugLines?.Dispose(); - _windowLayoutPersistence?.Dispose(); - _windowLayoutPersistence = null; - _uiHost?.Dispose(); _textRenderer?.Dispose(); _debugFont?.Dispose(); _frameProfiler.Dispose(); // MP0: releases the GpuFrameTimer query ring diff --git a/src/AcDream.App/UI/IRetainedPanelController.cs b/src/AcDream.App/UI/IRetainedPanelController.cs index 34d9ff2b..4ea2ae40 100644 --- a/src/AcDream.App/UI/IRetainedPanelController.cs +++ b/src/AcDream.App/UI/IRetainedPanelController.cs @@ -10,10 +10,10 @@ namespace AcDream.App.UI; public interface IRetainedPanelController : IDisposable { /// Called once for each hidden-to-shown transition. - void OnShown(); + void OnShown() { } /// Called once for each shown-to-hidden transition. - void OnHidden(); + void OnHidden() { } /// /// Called when keyboard focus enters, leaves, or moves within this window. diff --git a/src/AcDream.App/UI/Layout/ChatWindowController.cs b/src/AcDream.App/UI/Layout/ChatWindowController.cs index a944988c..d38f29dd 100644 --- a/src/AcDream.App/UI/Layout/ChatWindowController.cs +++ b/src/AcDream.App/UI/Layout/ChatWindowController.cs @@ -24,9 +24,10 @@ namespace AcDream.App.UI.Layout; /// and bound in place. /// /// -public sealed class ChatWindowController : IRetainedWindowStateController +public sealed class ChatWindowController : IRetainedWindowStateController, IRetainedPanelController { public const uint LayoutId = 0x21000006u; + private bool _disposed; // Element ids from chat LayoutDesc 0x21000006 (confirmed in Task D/G1). private const uint RootId = 0x1000000Eu; @@ -509,4 +510,10 @@ public sealed class ChatWindowController : IRetainedWindowStateController ChatKind.Combat => new(0.96f, 0.459f, 0.447f, 1f), // colorLightRed _ => new(0.824f, 0.824f, 0.784f, 1f), // colorGrey (fallback) }; + + public void Dispose() + { + if (_disposed) return; + _disposed = true; + } } diff --git a/src/AcDream.App/UI/Layout/InventoryController.cs b/src/AcDream.App/UI/Layout/InventoryController.cs index b7ba9379..3aab285b 100644 --- a/src/AcDream.App/UI/Layout/InventoryController.cs +++ b/src/AcDream.App/UI/Layout/InventoryController.cs @@ -12,7 +12,7 @@ namespace AcDream.App.UI.Layout; /// Container-switching is live (click a side bag → Use 0x0036 → ViewContents 0x0196 full-replace); /// drag-into-bag / wield-drop wire are later sub-phases. /// -public sealed class InventoryController : IItemListDragHandler +public sealed class InventoryController : IItemListDragHandler, IRetainedPanelController { // Element ids — spec §1 (dat dump of 0x21000022 / 0x21000021 + the *::PostInit binds). public const uint ContentsGridId = 0x100001C6u; // gm3DItemsUI m_itemList ("Contents of Backpack") @@ -58,6 +58,7 @@ public sealed class InventoryController : IItemListDragHandler private readonly Action? _sendNoLongerViewing; private readonly Action? _sendPutItemInContainer; // (item, container, placement) private readonly ItemInteractionController? _itemInteraction; + private bool _disposed; // ACE PropertyInt ids read by retail InqLoad (decomp 0x0058f130: InqInt(5)/InqInt(0xe6)). private const uint EncumbranceValProperty = 5u; // total carried burden @@ -528,6 +529,8 @@ public sealed class InventoryController : IItemListDragHandler /// Detach event handlers (idempotent). public void Dispose() { + if (_disposed) return; + _disposed = true; _objects.ObjectAdded -= OnObjectChanged; _objects.ObjectMoved -= OnObjectMoved; _objects.ObjectRemoved -= OnObjectChanged; diff --git a/src/AcDream.App/UI/Layout/PaperdollController.cs b/src/AcDream.App/UI/Layout/PaperdollController.cs index c967f9a1..bc66c1b2 100644 --- a/src/AcDream.App/UI/Layout/PaperdollController.cs +++ b/src/AcDream.App/UI/Layout/PaperdollController.cs @@ -12,7 +12,7 @@ namespace AcDream.App.UI.Layout; /// 175480 / 173620). Slice 1: equip slots only — no 3D doll viewport (that's Slice 2). /// Unwield is handled by InventoryController (dragging an equipped item onto the pack grid). /// -public sealed class PaperdollController : IItemListDragHandler +public sealed class PaperdollController : IItemListDragHandler, IRetainedPanelController { // ── Slots-toggle public surface ─────────────────────────────────────────────────────────────── /// @@ -82,6 +82,7 @@ public sealed class PaperdollController : IItemListDragHandler private readonly PaperdollViewState _viewState = new(); private readonly List _armorSlots = new(); private UiElement? _dollViewport; // UiViewport wired in Slice 3; UiElement? keeps this task independent + private bool _disposed; private PaperdollController( ImportedLayout layout, ClientObjectTable objects, Func playerGuid, @@ -244,6 +245,8 @@ public sealed class PaperdollController : IItemListDragHandler /// Detach event handlers (idempotent). public void Dispose() { + if (_disposed) return; + _disposed = true; _objects.ObjectAdded -= OnObjectChanged; _objects.ObjectMoved -= OnObjectMoved; _objects.ObjectRemoved -= OnObjectChanged; diff --git a/src/AcDream.App/UI/Layout/RadarController.cs b/src/AcDream.App/UI/Layout/RadarController.cs index 305f09c2..1097691b 100644 --- a/src/AcDream.App/UI/Layout/RadarController.cs +++ b/src/AcDream.App/UI/Layout/RadarController.cs @@ -11,7 +11,7 @@ namespace AcDream.App.UI.Layout; /// LayoutDesc 0x21000074 tree. Like the other gm* controllers, this class only /// finds children by retail id and attaches live providers; it does not recreate DAT chrome. /// -public sealed class RadarController +public sealed class RadarController : IRetainedPanelController { public const uint LayoutId = 0x21000074u; /// Production layout property 0x1000002D, recovered directly from the retail DAT. @@ -39,6 +39,7 @@ public sealed class RadarController private string? _lastCoordinates; private bool _coordinatesInitialized; private bool? _lastUiLocked; + private bool _disposed; private RadarController( ImportedLayout layout, @@ -235,6 +236,17 @@ public sealed class RadarController ? Array.Empty() : new[] { new UiText.Line(_lastCoordinates, CoordinateColor) }; + public void Dispose() + { + if (_disposed) return; + _disposed = true; + _radar.SnapshotProvider = null; + _radar.SelectObject = null; + _radar.HoveredObjectChanged = null; + if (_lockButton is not null) + _lockButton.OnClick = null; + } + private readonly record struct CompassToken( UiElement? Element, float Magnitude, diff --git a/src/AcDream.App/UI/Layout/SelectedObjectController.cs b/src/AcDream.App/UI/Layout/SelectedObjectController.cs index 381e2124..f7ba9169 100644 --- a/src/AcDream.App/UI/Layout/SelectedObjectController.cs +++ b/src/AcDream.App/UI/Layout/SelectedObjectController.cs @@ -41,7 +41,7 @@ namespace AcDream.App.UI.Layout; /// the gate only affects whether we proactively query; recorded in the divergence register. /// /// -public sealed class SelectedObjectController +public sealed class SelectedObjectController : IRetainedPanelController { // ── Element ids (toolbar LayoutDesc 0x21000016) ───────────────────────── /// Selected-object container / field element id (retail m_pSelObjectField). @@ -82,11 +82,14 @@ public sealed class SelectedObjectController private readonly Func _hasHealth; private readonly Func _stackSize; private readonly Action _sendQueryHealth; + private readonly Action> _unsubscribeSelectionChanged; + private readonly Action> _unsubscribeHealthChanged; // ── Live state (read by closures on the per-frame draw path) ──────────── private uint? _current; private string? _currentName; private double _flashRemaining; // > 0 while the selection overlay is flashing + private bool _disposed; /// White label color for the name line. private static readonly Vector4 NameColor = new(1f, 1f, 1f, 1f); @@ -94,7 +97,9 @@ public sealed class SelectedObjectController private SelectedObjectController( ImportedLayout layout, Action> subscribeSelectionChanged, + Action> unsubscribeSelectionChanged, Action> subscribeHealthChanged, + Action> unsubscribeHealthChanged, Func isHealthTarget, Func name, Func healthPercent, @@ -109,6 +114,8 @@ public sealed class SelectedObjectController _hasHealth = hasHealth; _stackSize = stackSize; _sendQueryHealth = sendQueryHealth; + _unsubscribeSelectionChanged = unsubscribeSelectionChanged; + _unsubscribeHealthChanged = unsubscribeHealthChanged; // Find elements — silently skip absent ones (partial/test layouts). _name = layout.FindElement(NameId); @@ -189,7 +196,9 @@ public sealed class SelectedObjectController public static SelectedObjectController Bind( ImportedLayout layout, Action> subscribeSelectionChanged, + Action> unsubscribeSelectionChanged, Action> subscribeHealthChanged, + Action> unsubscribeHealthChanged, Func isHealthTarget, Func name, Func healthPercent, @@ -198,7 +207,8 @@ public sealed class SelectedObjectController Action sendQueryHealth, UiDatFont? datFont) => new SelectedObjectController( - layout, subscribeSelectionChanged, subscribeHealthChanged, + layout, subscribeSelectionChanged, unsubscribeSelectionChanged, + subscribeHealthChanged, unsubscribeHealthChanged, isHealthTarget, name, healthPercent, hasHealth, stackSize, sendQueryHealth, datFont); /// @@ -268,4 +278,12 @@ public sealed class SelectedObjectController { _overlay?.TrySetRetailState(state); } + + public void Dispose() + { + if (_disposed) return; + _disposed = true; + _unsubscribeSelectionChanged(OnSelectionChanged); + _unsubscribeHealthChanged(OnHealthChanged); + } } diff --git a/src/AcDream.App/UI/Layout/ToolbarController.cs b/src/AcDream.App/UI/Layout/ToolbarController.cs index 36670fbd..a7f29268 100644 --- a/src/AcDream.App/UI/Layout/ToolbarController.cs +++ b/src/AcDream.App/UI/Layout/ToolbarController.cs @@ -23,7 +23,7 @@ namespace AcDream.App.UI.Layout; /// CreateObject resolves a formerly-unknown guid. /// /// -public sealed class ToolbarController : IItemListDragHandler +public sealed class ToolbarController : IItemListDragHandler, IRetainedPanelController { // Slot element ids in slot-index order (toolbar LayoutDesc 0x21000016, pre-dump). // Row 1 = slots 0-8 (0x100001A7..0x100001AF), Row 2 = slots 9-17 (0x100006B7..0x100006BF). @@ -62,6 +62,7 @@ public sealed class ToolbarController : IItemListDragHandler private readonly UiButton? _characterButton; private readonly UiButton? _inventoryButton; private readonly ClientObjectTable _repo; + private readonly CombatState? _combatState; private readonly Func> _shortcuts; private readonly Func _iconIds; // (itemType, icon, underlay, overlay, effects) → GL tex private readonly Action _useItem; // guid → fire UseObject @@ -70,6 +71,7 @@ public sealed class ToolbarController : IItemListDragHandler private readonly Action? _sendAddShortcut; // (index, objectGuid) private readonly Action? _sendRemoveShortcut; // (index) private readonly ItemInteractionController? _itemInteraction; + private bool _disposed; // Digit sprite DID arrays for slot labels (top row, numbers 1-9). // Read from LayoutDesc 0x21000037, element 0x1000034A under composite 0x10000346. @@ -98,6 +100,7 @@ public sealed class ToolbarController : IItemListDragHandler Action? sendRemoveShortcut = null) { _repo = repo; + _combatState = combatState; _shortcuts = shortcuts; _iconIds = iconIds; _useItem = useItem; @@ -141,15 +144,21 @@ public sealed class ToolbarController : IItemListDragHandler SetCombatMode(CombatMode.NonCombat); // Wire live combat-mode changes if a CombatState was provided. - if (combatState is not null) - combatState.CombatModeChanged += SetCombatMode; + if (_combatState is not null) + _combatState.CombatModeChanged += SetCombatMode; // D.5.4: the table now holds ALL objects (creatures, NPCs, etc.), so filter // to our 18 shortcut guids — else every creature spawn in a busy zone // needlessly re-populates the bar (gmToolbarUI::SetDelayedShortcutNum pattern). - repo.ObjectAdded += o => { if (IsShortcutGuid(o.ObjectId)) Populate(); }; - repo.ObjectUpdated += o => { if (IsShortcutGuid(o.ObjectId)) Populate(); }; - repo.ObjectRemoved += o => { if (IsShortcutGuid(o.ObjectId)) Populate(); }; + repo.ObjectAdded += OnRepositoryObjectChanged; + repo.ObjectUpdated += OnRepositoryObjectChanged; + repo.ObjectRemoved += OnRepositoryObjectChanged; + } + + private void OnRepositoryObjectChanged(ClientObject obj) + { + if (IsShortcutGuid(obj.ObjectId)) + Populate(); } /// @@ -409,4 +418,16 @@ public sealed class ToolbarController : IItemListDragHandler } Populate(); } + + public void Dispose() + { + if (_disposed) return; + _disposed = true; + + if (_combatState is not null) + _combatState.CombatModeChanged -= SetCombatMode; + _repo.ObjectAdded -= OnRepositoryObjectChanged; + _repo.ObjectUpdated -= OnRepositoryObjectChanged; + _repo.ObjectRemoved -= OnRepositoryObjectChanged; + } } diff --git a/src/AcDream.App/UI/README.md b/src/AcDream.App/UI/README.md index 45a79081..adef5ebf 100644 --- a/src/AcDream.App/UI/README.md +++ b/src/AcDream.App/UI/README.md @@ -38,6 +38,9 @@ synthesis + six deep-dive docs under geometry events, lock propagation, and controller teardown. - `IRetainedPanelController.cs` — disposable lifecycle contract for panel-specific show/hide and descendant-focus behavior. +- `RetainedPanelControllerGroup.cs` — one typed lifecycle owner for windows + whose imported subtree is bound by multiple focused controllers; teardown is + idempotent and runs in reverse construction order. - `RetailWindowLayoutPersistence.cs` / `IRetainedWindowStateController.cs` — per-character/per-resolution bounds, visibility, collapse/maximize, legacy migration, and safe resolution-clamped restore. @@ -45,7 +48,8 @@ synthesis + six deep-dive docs under contract for imported-chrome and shared-wrapper windows. - `UiHost.cs` — one-shot wrapper: owns the `UiRoot`, a `TextRenderer`, and a default `BitmapFont`. Provides `WireMouse` / `WireKeyboard` - helpers for Silk.NET plumbing. + helpers for Silk.NET plumbing and removes those device subscriptions before + disposing the window manager/controllers and renderer. ## Integration pattern diff --git a/src/AcDream.App/UI/RetailWindowHandle.cs b/src/AcDream.App/UI/RetailWindowHandle.cs index c200f30c..15ad14db 100644 --- a/src/AcDream.App/UI/RetailWindowHandle.cs +++ b/src/AcDream.App/UI/RetailWindowHandle.cs @@ -34,7 +34,7 @@ public sealed class RetailWindowHandle public string Name { get; } public UiElement OuterFrame { get; } public UiElement ContentRoot { get; } - public IRetainedPanelController? Controller { get; } + public IRetainedPanelController? Controller { get; private set; } public IRetainedWindowStateController? StateController { get; } public bool IsRegistered => !_disposed; public bool IsVisible => OuterFrame.Visible; @@ -70,6 +70,19 @@ public sealed class RetailWindowHandle Controller?.OnShown(); } + internal void AttachController(IRetainedPanelController controller) + { + ObjectDisposedException.ThrowIf(_disposed, this); + ArgumentNullException.ThrowIfNull(controller); + if (ReferenceEquals(Controller, controller)) return; + if (Controller is not null) + throw new InvalidOperationException($"Retained window '{Name}' already owns a controller."); + + Controller = controller; + if (_notifiedVisible) + Controller.OnShown(); + } + internal void NotifyVisibility(bool visible) { if (_notifiedVisible == visible) return; diff --git a/src/AcDream.App/UI/RetailWindowManager.cs b/src/AcDream.App/UI/RetailWindowManager.cs index 26dd05d5..15b6e650 100644 --- a/src/AcDream.App/UI/RetailWindowManager.cs +++ b/src/AcDream.App/UI/RetailWindowManager.cs @@ -80,6 +80,16 @@ public sealed class RetailWindowManager : IDisposable public bool TryGet(string name, out RetailWindowHandle handle) => _byName.TryGetValue(name, out handle!); + public void AttachController(string name, IRetainedPanelController controller) + { + ObjectDisposedException.ThrowIf(_disposed, this); + ArgumentException.ThrowIfNullOrWhiteSpace(name); + ArgumentNullException.ThrowIfNull(controller); + if (!_byName.TryGetValue(name, out var handle)) + throw new KeyNotFoundException($"No retained window named '{name}' is registered."); + handle.AttachController(controller); + } + internal bool TryGet(UiElement outerFrame, out RetailWindowHandle handle) => _byFrame.TryGetValue(outerFrame, out handle!); diff --git a/src/AcDream.App/UI/RetainedPanelControllerGroup.cs b/src/AcDream.App/UI/RetainedPanelControllerGroup.cs new file mode 100644 index 00000000..890fdba9 --- /dev/null +++ b/src/AcDream.App/UI/RetainedPanelControllerGroup.cs @@ -0,0 +1,52 @@ +using System; + +namespace AcDream.App.UI; + +/// +/// Gives one retained window a single lifecycle owner when its imported subtree +/// is bound by multiple focused controllers (for example toolbar+selection or +/// inventory+paperdoll). Teardown runs in reverse construction order and is +/// idempotent. +/// +public sealed class RetainedPanelControllerGroup : IRetainedPanelController +{ + private readonly IRetainedPanelController[] _controllers; + private bool _disposed; + + public RetainedPanelControllerGroup(params IRetainedPanelController[] controllers) + { + ArgumentNullException.ThrowIfNull(controllers); + _controllers = (IRetainedPanelController[])controllers.Clone(); + for (int i = 0; i < _controllers.Length; i++) + ArgumentNullException.ThrowIfNull(_controllers[i]); + } + + public void OnShown() + { + if (_disposed) return; + for (int i = 0; i < _controllers.Length; i++) + _controllers[i].OnShown(); + } + + public void OnHidden() + { + if (_disposed) return; + for (int i = _controllers.Length - 1; i >= 0; i--) + _controllers[i].OnHidden(); + } + + public void OnDescendantFocusChanged(UiElement? focusedDescendant) + { + if (_disposed) return; + for (int i = 0; i < _controllers.Length; i++) + _controllers[i].OnDescendantFocusChanged(focusedDescendant); + } + + public void Dispose() + { + if (_disposed) return; + _disposed = true; + for (int i = _controllers.Length - 1; i >= 0; i--) + _controllers[i].Dispose(); + } +} diff --git a/src/AcDream.App/UI/UiHost.cs b/src/AcDream.App/UI/UiHost.cs index 409c4613..04cc9be6 100644 --- a/src/AcDream.App/UI/UiHost.cs +++ b/src/AcDream.App/UI/UiHost.cs @@ -1,3 +1,4 @@ +using System.Collections.Generic; using System.Numerics; using AcDream.App.Rendering; using Silk.NET.Input; @@ -48,6 +49,8 @@ public sealed class UiHost : System.IDisposable public IKeyboard? Keyboard { get; private set; } private long _startTicks = System.Environment.TickCount64; + private readonly List _inputUnsubscribers = new(); + private bool _disposed; public UiHost(GL gl, string shaderDir, BitmapFont? defaultFont = null) { @@ -78,22 +81,48 @@ public sealed class UiHost : System.IDisposable public void WireMouse(IMouse mouse) { - mouse.MouseDown += (_, b) => - Root.OnMouseDown(MapButton(b), (int)mouse.Position.X, (int)mouse.Position.Y); - mouse.MouseUp += (_, b) => - Root.OnMouseUp(MapButton(b), (int)mouse.Position.X, (int)mouse.Position.Y); - mouse.MouseMove += (_, p) => - Root.OnMouseMove((int)p.X, (int)p.Y); - mouse.Scroll += (_, s) => - Root.OnScroll((int)s.Y); + System.ObjectDisposedException.ThrowIf(_disposed, this); + System.ArgumentNullException.ThrowIfNull(mouse); + + void OnMouseDown(IMouse sender, MouseButton button) => + Root.OnMouseDown(MapButton(button), (int)sender.Position.X, (int)sender.Position.Y); + void OnMouseUp(IMouse sender, MouseButton button) => + Root.OnMouseUp(MapButton(button), (int)sender.Position.X, (int)sender.Position.Y); + void OnMouseMove(IMouse sender, Vector2 position) => + Root.OnMouseMove((int)position.X, (int)position.Y); + void OnScroll(IMouse sender, ScrollWheel scroll) => Root.OnScroll((int)scroll.Y); + + mouse.MouseDown += OnMouseDown; + mouse.MouseUp += OnMouseUp; + mouse.MouseMove += OnMouseMove; + mouse.Scroll += OnScroll; + _inputUnsubscribers.Add(() => + { + mouse.MouseDown -= OnMouseDown; + mouse.MouseUp -= OnMouseUp; + mouse.MouseMove -= OnMouseMove; + mouse.Scroll -= OnScroll; + }); } public void WireKeyboard(IKeyboard kb) { + System.ObjectDisposedException.ThrowIf(_disposed, this); + System.ArgumentNullException.ThrowIfNull(kb); Keyboard = kb; // last wired keyboard wins (one-keyboard desktop) - kb.KeyDown += (_, k, _) => Root.OnKeyDown((int)k); - kb.KeyUp += (_, k, _) => Root.OnKeyUp((int)k); - kb.KeyChar += (_, c) => Root.OnChar(c); + void OnKeyDown(IKeyboard sender, Key key, int scanCode) => Root.OnKeyDown((int)key); + void OnKeyUp(IKeyboard sender, Key key, int scanCode) => Root.OnKeyUp((int)key); + void OnKeyChar(IKeyboard sender, char value) => Root.OnChar(value); + + kb.KeyDown += OnKeyDown; + kb.KeyUp += OnKeyUp; + kb.KeyChar += OnKeyChar; + _inputUnsubscribers.Add(() => + { + kb.KeyDown -= OnKeyDown; + kb.KeyUp -= OnKeyUp; + kb.KeyChar -= OnKeyChar; + }); } private static UiMouseButton MapButton(MouseButton b) => b switch @@ -133,6 +162,12 @@ public sealed class UiHost : System.IDisposable public void Dispose() { + if (_disposed) return; + _disposed = true; + for (int i = _inputUnsubscribers.Count - 1; i >= 0; i--) + _inputUnsubscribers[i](); + _inputUnsubscribers.Clear(); + Keyboard = null; WindowManager.Dispose(); TextRenderer.Dispose(); } diff --git a/tests/AcDream.App.Tests/UI/Layout/SelectedObjectControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/SelectedObjectControllerTests.cs index cdefebc0..d5bc649f 100644 --- a/tests/AcDream.App.Tests/UI/Layout/SelectedObjectControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/SelectedObjectControllerTests.cs @@ -82,7 +82,15 @@ public class SelectedObjectControllerTests => SelectedObjectController.Bind( layout, subscribeSelectionChanged: h => SelectionHandler = h, + unsubscribeSelectionChanged: h => + { + if (SelectionHandler == h) SelectionHandler = null; + }, subscribeHealthChanged: h => HealthHandler = h, + unsubscribeHealthChanged: h => + { + if (HealthHandler == h) HealthHandler = null; + }, isHealthTarget: g => HealthTargetMap.TryGetValue(g, out var v) && v, name: g => NameMap.TryGetValue(g, out var v) ? v : null, healthPercent: g => HealthMap.TryGetValue(g, out var v) ? v : 1f, @@ -403,4 +411,23 @@ public class SelectedObjectControllerTests h.FireSelection(null); Assert.Equal(0f, healthMeterEl.Fill() ?? 0f); } + + [Fact] + public void Dispose_UnsubscribesBothHandlers_AndIsIdempotent() + { + var (layout, _, _, healthMeterEl) = FakeLayout(); + var h = new Harness(); + h.HealthTargetMap[0xAA09u] = true; + var controller = h.Bind(layout); + + controller.Dispose(); + controller.Dispose(); + h.FireSelection(0xAA09u); + h.FireHealth(0xAA09u, 0.5f); + + Assert.Null(h.SelectionHandler); + Assert.Null(h.HealthHandler); + Assert.False(healthMeterEl.Visible); + Assert.Empty(h.QueryHealthCalls); + } } diff --git a/tests/AcDream.App.Tests/UI/Layout/ToolbarControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/ToolbarControllerTests.cs index 20e1b27f..6afb53e7 100644 --- a/tests/AcDream.App.Tests/UI/Layout/ToolbarControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/ToolbarControllerTests.cs @@ -98,6 +98,28 @@ public class ToolbarControllerTests Assert.Equal(0x5002u, slots[Row1[2]].Cell.ItemId); // rebound on ItemAdded } + [Fact] + public void Dispose_UnsubscribesDeferredRebind_AndIsIdempotent() + { + var (layout, slots, _) = FakeToolbar(); + var repo = new ClientObjectTable(); + var shortcuts = new List + { new(Index: 2, ObjectGuid: 0x5002u, SpellId: 0, Layer: 0) }; + var controller = ToolbarController.Bind(layout, repo, () => shortcuts, + iconIds: (_,_,_,_,_) => 0x88u, useItem: _ => { }); + + controller.Dispose(); + controller.Dispose(); + repo.AddOrUpdate(new ClientObject + { + ObjectId = 0x5002u, + WeenieClassId = 1u, + IconId = 0x06005678u, + }); + + Assert.Equal(0u, slots[Row1[2]].Cell.ItemId); + } + [Fact] public void Click_emitsUseForBoundItem() { diff --git a/tests/AcDream.App.Tests/UI/RetailWindowManagerTests.cs b/tests/AcDream.App.Tests/UI/RetailWindowManagerTests.cs index 57118b59..96e96fb3 100644 --- a/tests/AcDream.App.Tests/UI/RetailWindowManagerTests.cs +++ b/tests/AcDream.App.Tests/UI/RetailWindowManagerTests.cs @@ -179,6 +179,28 @@ public sealed class RetailWindowManagerTests Assert.Equal(1, secondController.DisposeCount); } + [Fact] + public void AttachController_AfterRegistration_DeliversVisibilityAndOwnsDisposal() + { + var root = new UiRoot { Width = 800, Height = 600 }; + var frame = new UiPanel { Width = 100, Height = 100 }; + root.AddChild(frame); + RetailWindowHandle handle = root.RegisterWindow("inventory", frame); + var controller = new RecordingController(); + + root.WindowManager.AttachController("inventory", controller); + + Assert.Same(controller, handle.Controller); + Assert.Equal(1, controller.ShownCount); + Assert.Throws(() => + root.WindowManager.AttachController("inventory", new RecordingController())); + + root.WindowManager.Dispose(); + root.WindowManager.Dispose(); + Assert.Equal(1, controller.HiddenCount); + Assert.Equal(1, controller.DisposeCount); + } + private sealed class RecordingController : IRetainedPanelController { public int ShownCount { get; private set; } diff --git a/tests/AcDream.App.Tests/UI/RetainedPanelControllerGroupTests.cs b/tests/AcDream.App.Tests/UI/RetainedPanelControllerGroupTests.cs new file mode 100644 index 00000000..bb7105d2 --- /dev/null +++ b/tests/AcDream.App.Tests/UI/RetainedPanelControllerGroupTests.cs @@ -0,0 +1,42 @@ +using System.Collections.Generic; +using AcDream.App.UI; + +namespace AcDream.App.Tests.UI; + +public sealed class RetainedPanelControllerGroupTests +{ + [Fact] + public void Lifecycle_ForwardsInOwnershipOrder_AndDisposesInReverseExactlyOnce() + { + var calls = new List(); + var first = new RecordingController("first", calls); + var second = new RecordingController("second", calls); + var group = new RetainedPanelControllerGroup(first, second); + var focus = new UiPanel(); + + group.OnShown(); + group.OnDescendantFocusChanged(focus); + group.OnHidden(); + group.Dispose(); + group.Dispose(); + + Assert.Equal( + [ + "first:shown", "second:shown", + "first:focus", "second:focus", + "second:hidden", "first:hidden", + "second:dispose", "first:dispose", + ], calls); + } + + private sealed class RecordingController( + string name, + List calls) : IRetainedPanelController + { + public void OnShown() => calls.Add($"{name}:shown"); + public void OnHidden() => calls.Add($"{name}:hidden"); + public void OnDescendantFocusChanged(UiElement? focusedDescendant) + => calls.Add($"{name}:focus"); + public void Dispose() => calls.Add($"{name}:dispose"); + } +}