diff --git a/src/AcDream.App/UI/IRetainedWindowStateController.cs b/src/AcDream.App/UI/IRetainedWindowStateController.cs index cdb129bcd..095a22942 100644 --- a/src/AcDream.App/UI/IRetainedWindowStateController.cs +++ b/src/AcDream.App/UI/IRetainedWindowStateController.cs @@ -1,11 +1,25 @@ namespace AcDream.App.UI; /// Panel state that is not completely described by outer-frame bounds. +/// +/// Review fix round (finding 3, docs/plans/2026-09-06-plugin-shelf-and-dat-icons.md +/// Slice A): the panel's own persisted show/hide INTENT, independent of the +/// outer frame's derived (which for a +/// controller like PluginSidePanel also folds in an availability gate — +/// see that class's ApplyVisibility). Null means "no override, fall back +/// to " — every existing state +/// controller that never had an intent distinct from derived visibility keeps +/// working unchanged. 's +/// Capture/Apply read and write this instead of the outer frame's +/// raw visibility so an availability-driven hide is never mistaken for (and +/// then persisted as) a user hide. +/// public readonly record struct RetainedWindowState( bool Collapsed = false, bool Maximized = false, float? PersistedTop = null, - float? PersistedHeight = null); + float? PersistedHeight = null, + bool? RequestedVisible = null); /// /// Optional state seam used by retained-window persistence. Bounds are restored diff --git a/src/AcDream.App/UI/PluginSidePanel.cs b/src/AcDream.App/UI/PluginSidePanel.cs index fa1defb76..2d0cfb912 100644 --- a/src/AcDream.App/UI/PluginSidePanel.cs +++ b/src/AcDream.App/UI/PluginSidePanel.cs @@ -10,11 +10,12 @@ namespace AcDream.App.UI; /// /// Slice A (2026-09-06, docs/plans/2026-09-06-plugin-shelf-and-dat-icons.md): /// the shelf is itself a retained window ( -/// registers it as "plugin-shelf") so it gets drag, the UI lock, and -/// position/visibility/collapsed -/// persistence for free, exactly like every other retained window. It implements -/// for the collapse flag and -/// to separate "has entries to show" +/// registers it as ) so it gets drag, the UI +/// lock, and position/visibility/ +/// collapsed persistence for free, exactly like every other retained window. It +/// implements for the collapse flag +/// (and now the persisted show/hide intent — see ) +/// and to separate "has entries to show" /// (availability) from the user's own show/hide choice — the same pattern /// PluginWindowVisibilityController uses for individual plugin windows, /// just applied synchronously (see ) so unit tests @@ -24,19 +25,47 @@ namespace AcDream.App.UI; /// /// /// -/// The grip/collapse-toggle band across the top is drawn, not a child element: -/// paints it and / -/// handle its input by inspecting the live pointer position against the shelf's -/// OWN bounds. This is deliberate — a real child element for the toggle would -/// have to sit outside any grip subtree -/// (nesting it inside would let the drag-handle promotion swallow its press -/// before the button ever sees a click; see UiRoot.OnMouseDown's -/// handleWindow is not null priority over HandlesClick), and a -/// grip element as a plain sibling would change 's -/// count/shape, which the existing single-button shelf tests assert directly. -/// Keeping the whole shelf and excluding just -/// the toggle's pixel rect from gets the same -/// grip-drags/buttons-and-toggle-don't behavior without adding any child. +/// Review fix round (2026-09-06): the grip and collapse toggle are now REAL +/// child elements — ( +/// ) and a plain toggle beside +/// it — exactly the mechanism every other retail window with a non-title-bar move +/// strip already uses (ChatLayoutConformanceTests.MountedChatWindow_TopStrip_IsAMoveHandleNotAGrip +/// pins the same pattern on the imported chat window's top strip). The original +/// Slice A implementation drew the grip/toggle and computed a live, cursor-position- +/// dependent override instead — a real design +/// smell (input handling required reading the CURRENT mouse position out of +/// from inside a hit-test-adjacent property getter) kept only +/// because a real toggle CHILD would have had to sit outside the grip's own +/// subtree — which is exactly where it +/// belongs anyway: only walks INTO a +/// ancestor chain looking for the flag: a +/// SIBLING toggle button never enters that walk, so nesting was never required. +/// +/// +/// +/// is on the shelf +/// itself (verified against , which does +/// not read a window's own at all — it walks +/// from the pressed element for a +/// ancestor-or-self, then climbs to the nearest child of . A +/// press on therefore resolves handleWindow == this and +/// drags regardless of ). Leaving the shelf +/// would ALSO satisfy the grip drag, but would +/// additionally arm UiRoot.OnMouseDown's whole-window-drag fallback +/// (window is {{ Draggable: true }}) for any press that lands on the +/// shelf's own padding — between entry buttons, or in the margin around them — +/// since neither the padding nor the shelf itself declares +/// . That is exactly the behavior Slice A's +/// review round asked NOT to have ("the GRIP drags, the padding between buttons +/// does NOT"), so stays false: only +/// 's subtree can +/// start a move. The one cost is that a press on padding or an entry/toggle +/// button no longer raises the shelf to front via UiRoot's +/// FindWindow(target) fallback (which requires +/// or on an ancestor-or-self) — only a grip +/// press does. This is an acceptable trade: a click can only ever land on the +/// shelf's exposed region in the first place (whatever already won the Z-order +/// hit-test), so there is nothing behind it left to reveal. /// /// public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowStateController, IRetainedPanelController @@ -50,8 +79,8 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState /// total height (only the grip remains when collapsed). private const float GripHeight = 12f; - /// Width of the collapse-toggle glyph's clickable rect, anchored - /// to the grip's right end. + /// Width of the collapse-toggle button, anchored to the grip's + /// right end. private const float ToggleWidth = 16f; /// Collapsed shelf width — just enough for the toggle glyph. @@ -63,6 +92,8 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState private readonly Func _resolve; private readonly UiDatFont? _font; private readonly Dictionary _entries = []; + private readonly ShelfGripPanel _grip; + private readonly UiSimpleButton _toggle; private bool _disposed; private float _lastLayoutHeight = -1f; @@ -74,17 +105,20 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState /// The user's/persisted show-hide intent, independent of whether /// there are currently any entries to show (see ). /// Shift+Ctrl+F1 (InputAction.TogglePluginManager) flips this via - /// /. + /// /. Persisted directly (review fix + /// round finding 3): see / + /// and . private bool _requestedVisible = true; - /// True once the shelf has been dragged, or a persisted layout - /// with a differing position has been restored onto it. While false, the - /// shelf is still in its default right-edge dock and - /// preserves the DOCKED (top-right) corner on growth; once true, it - /// preserves the top-left corner instead (see the class doc + Slice A - /// plan item 2). Tracked via on the - /// shelf's own registration, so unregistered (legacy unit-test) use never - /// sets this and behaves like the shelf has always been docked. + /// True once the shelf has been dragged away from its current + /// docked position (a real grip drag, or a persisted layout with a + /// differing position restored onto it — see ). + /// While false, the shelf is still docked and preserves + /// the DOCKED (top-right) corner on growth; once true, it preserves the + /// top-left corner instead (see the class doc + Slice A plan item 2). + /// Tracked via on the shelf's own + /// registration, so unregistered (legacy unit-test) use never sets this and + /// behaves like the shelf has always been docked. private bool _userPositioned; /// True once the one-time initial dock placement (right edge, top @@ -93,10 +127,27 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState /// position to preserve a corner of). private bool _initialDockApplied; + /// + /// The last position or the one-time dock placement + /// left the shelf at while it was still docked ( + /// is false). Review fix round finding 6: + /// fires unconditionally on EVERY window-drag release — including a + /// zero-movement grip click, and every + /// reachability re-clamp — not only a genuine drag. Comparing the handle's + /// CURRENT position against this recorded dock placement (rather than + /// treating any as "the user moved + /// it") is what lets a same-position click, or an unrelated clamp that + /// happens to leave the shelf exactly where the dock formula already had + /// it, pass through without flipping . + /// + private float _dockLeft; + private float _dockTop; + /// The shelf's own retained-window handle, captured the moment - /// RetailUiRuntime.MountPlugins registers it as "plugin-shelf" - /// (via ). Null when the shelf is used - /// unregistered, e.g. by tests that predate Slice A. + /// RetailUiRuntime.MountPlugins registers it as + /// (via ). + /// Null when the shelf is used unregistered, e.g. by tests that predate + /// Slice A. private RetailWindowHandle? _handle; public PluginSidePanel( @@ -112,14 +163,41 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState Height = GripHeight + OuterPadding * 2f; Top = DefaultTop; Anchors = AnchorEdges.None; - Draggable = true; + // See the class doc for why this stays false: FindDragHandleWindow does + // not require it for the grip to drag, and leaving it true would also + // arm the whole-window-drag fallback for padding clicks. + Draggable = false; ConstrainDragToParent = true; Resizable = false; + // Nit 10: the shelf's Width/Height are entirely derived (Reflow), so a + // restored layout's saved dimensions must never stomp them via ResizeTo. + ResizeX = false; + ResizeY = false; BackgroundColor = new Vector4(0f, 0f, 0f, 0.88f); BorderColor = new Vector4(0.62f, 0.48f, 0.16f, 1f); BorderThickness = 1f; Visible = false; + _grip = new ShelfGripPanel + { + WindowMoveHandle = true, + BackgroundColor = Vector4.Zero, + BorderColor = Vector4.Zero, + }; + _toggle = new UiSimpleButton + { + BackgroundColor = Vector4.Zero, + BorderColor = Vector4.Zero, + TextColor = ToggleGlyphColor, + DatFont = _font, + Outline = true, + TextSource = () => _collapsed ? "<" : ">", + }; + _toggle.Click += ToggleCollapsed; + AddChild(_grip); + AddChild(_toggle); + LayoutChrome(); + _windows.WindowUnregistered += OnWindowUnregistered; _windows.WindowRegistered += OnWindowRegistered; } @@ -197,6 +275,7 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState if (_collapsed) _collapsed = false; Reflow(); + _handle?.NotifyStateChanged(); } /// Hide the shelf. Preserves entries/positions; never disables a @@ -205,12 +284,20 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState { _requestedVisible = false; Reflow(); + _handle?.NotifyStateChanged(); } protected override void OnTick(double deltaSeconds) { base.OnTick(deltaSeconds); + // Nit 12: dim the grip under the global UI lock, the same visual cue + // every retail window gets from RetailWindowLockPresentationController — + // that controller only recognizes AUTHORED dat chrome ids, and the grip + // is a runtime-created element with no DatElementId, so it needs its + // own lock-driven dim here. + _grip.Opacity = _windows.IsLocked ? 0.5f : 1f; + if (Parent is { } parent) { // Row-wrap: re-flow into however many columns fit the live parent @@ -235,108 +322,33 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState { Left = MathF.Max(0f, parent.Width - Width - OuterPadding); _initialDockApplied = true; - } - - // Keep the shelf itself reachable across a display resize — the - // same reachability guarantee KeepWindowReachable below gives the - // individual plugin windows. A direct clamp (not handle.MoveTo) - // so it never reads as a user move. - if (parent.Width > 0f && parent.Height > 0f) - { - float clampedLeft = Math.Clamp(Left, 0f, MathF.Max(0f, parent.Width - Width)); - float clampedTop = Math.Clamp(Top, 0f, MathF.Max(0f, parent.Height - Height)); - if (clampedLeft != Left || clampedTop != Top) - { - Left = clampedLeft; - Top = clampedTop; - } + _dockLeft = Left; + _dockTop = Top; } } foreach (RetailWindowHandle handle in _entries.Keys) KeepWindowReachable(handle); - // The shelf remains reachable even after ordinary windows are raised. - if (Parent is { } root) - { - int highest = 0; - foreach (UiElement sibling in root.Children) - { - if (!ReferenceEquals(sibling, this)) - highest = Math.Max(highest, sibling.ZOrder); - } - if (ZOrder <= highest) - ZOrder = highest == int.MaxValue ? highest : highest + 1; - } + // Review fix round finding 2: the per-tick "always highest ZOrder" raise + // that used to live here is DELETED. Registration already gives the + // shelf press-to-raise (a grip press resolves as this window's own + // move-drag and calls UiRoot.BringToFront before the drag starts); + // forcing it back to the top of EVERY sibling on every tick fought + // RetailDialogFactory.Tick's own re-raise of open dialogs (it + // re-asserts each dialog's Z-order every tick specifically so the + // screen's opaque backdrop cannot bury it — see that method's own doc + // comment), so a dialog opened while the shelf was visible could never + // actually end up on top of it. Retail dialogs must outrank the shelf. } - /// - /// A press within the collapse-toggle's rect (top-right of the grip band) - /// is handled by this element instead of promoting to a whole-window drag - /// — see UiRoot.OnMouseDown's target.HandlesClick branch, - /// checked BEFORE its window is {{ Draggable: true }} fallback. - /// Computed from the live pointer position (not a cached hit-test side - /// effect) because this getter has exactly one caller in the whole - /// codebase (OnMouseDown), which sets UiRoot.MouseX/MouseY - /// to the press coordinates immediately before reading it. - /// - public override bool HandlesClick + private void ToggleCollapsed() { - get - { - if (FindRoot() is not { } root) return false; - Vector2 sp = ScreenPosition; - return IsWithinToggleRect(root.MouseX - sp.X, root.MouseY - sp.Y); - } + _collapsed = !_collapsed; + Reflow(); + _handle?.NotifyStateChanged(); } - public override bool OnEvent(in UiEvent e) - { - if (e.Type == UiEventType.Click && Enabled && IsWithinToggleRect(e.Data1, e.Data2)) - { - _collapsed = !_collapsed; - Reflow(); - return true; - } - return false; - } - - protected override void OnDraw(UiRenderContext ctx) - { - base.OnDraw(ctx); - - // Grip strip: three short dashes centered in the band (excluding the - // toggle's own rect) — a subtle drag affordance in the existing - // border color. - const float dashWidth = 5f; - const float dashGap = 4f; - float totalDashWidth = dashWidth * 3f + dashGap * 2f; - float dashX = MathF.Max(2f, (Width - ToggleWidth - totalDashWidth) * 0.5f); - float dashY = GripHeight * 0.5f - 1f; - for (int i = 0; i < 3; i++) - ctx.DrawFill(dashX + i * (dashWidth + dashGap), dashY, dashWidth, 2f, BorderColor); - - string glyph = _collapsed ? "«" : "»"; // « / » - if (_font is { } dat) - { - float glyphWidth = dat.MeasureWidth(glyph); - ctx.DrawStringDat( - dat, - glyph, - Width - ToggleWidth + (ToggleWidth - glyphWidth) * 0.5f, - (GripHeight - dat.LineHeight) * 0.5f, - ToggleGlyphColor, - outline: true); - } - } - - private static bool IsWithinToggleRect(float localX, float localY, float width) - => localY >= 0f && localY < GripHeight - && localX >= width - ToggleWidth && localX < width; - - private bool IsWithinToggleRect(float localX, float localY) - => IsWithinToggleRect(localX, localY, Width); - private void OnWindowRegistered(RetailWindowHandle handle) { if (!ReferenceEquals(handle.OuterFrame, this)) return; @@ -345,7 +357,11 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState _windows.WindowRegistered -= OnWindowRegistered; } - private void OnHandleMoved(RetailWindowHandle _) => _userPositioned = true; + private void OnHandleMoved(RetailWindowHandle _) + { + if (Left != _dockLeft || Top != _dockTop) + _userPositioned = true; + } // ── IRetainedPanelController: separates "has entries" (availability) from // the user's own show/hide request — see ApplyVisibility. ──────────────── @@ -360,13 +376,29 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState _requestedVisible = false; } - // ── IRetainedWindowStateController: collapse persistence ──────────────── + // ── IRetainedWindowStateController: collapse + requested-visible intent ── - public RetainedWindowState CaptureWindowState() => new(Collapsed: _collapsed); + public RetainedWindowState CaptureWindowState() => + new(Collapsed: _collapsed, RequestedVisible: _requestedVisible); + /// + /// Review fix round finding 3: restores BOTH the collapse flag and the + /// persisted show/hide INTENT directly onto , + /// then re-derives through the same synchronous + /// path every other state change uses — never + /// through /, so a login-time restore + /// never fires an extra BringToFront or a redundant state-changed + /// save. registers + /// as one of 's state-managed + /// visibility windows, so RetailWindowLayoutPersistence.Apply calls + /// this instead of / + /// for visibility. + /// public void RestoreWindowState(RetainedWindowState state) { _collapsed = state.Collapsed; + if (state.RequestedVisible is { } requestedVisible) + _requestedVisible = requestedVisible; Reflow(); } @@ -403,6 +435,23 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState handle.MoveTo(left, top); } + /// Repositions the grip/toggle children to span the current + /// Width — called whenever changes it (entries added/ + /// removed, collapse toggled, row-wrap) so the toggle always sits flush + /// against the shelf's right edge and the grip fills the rest of the band. + private void LayoutChrome() + { + _grip.Left = 0f; + _grip.Top = 0f; + _grip.Width = MathF.Max(0f, Width - ToggleWidth); + _grip.Height = GripHeight; + + _toggle.Left = Width - ToggleWidth; + _toggle.Top = 0f; + _toggle.Width = ToggleWidth; + _toggle.Height = GripHeight; + } + /// /// Recomputes button positions and the shelf's own Width/Height for the /// current entries + collapsed state, then (once the shelf has been @@ -411,17 +460,32 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState /// false), top-left once the user has moved it. Always reflows the full /// button set (even while collapsed, with Visible=false) so /// expanding is instant, per Slice A plan item 4. + /// + /// Review fix round finding 5: the parameterless default no longer + /// forces a single-column layout. Every OTHER call site (, + /// unregister, /, the collapse toggle, + /// ) calls this with no argument — only + /// 's row-wrap logic ever passes an explicit height. + /// Defaulting to "unbounded" on every OTHER call site collapsed a + /// multi-column wrapped layout back to one column for a single frame + /// (until the next row-wrap pass corrected it), + /// visible as a one-frame reflow flash on every collapse/expand. Reusing + /// (the last height + /// actually measured) when no explicit height is given keeps the same + /// column count these calls would already be using. /// - private void Reflow(float maximumHeight = float.PositiveInfinity) + private void Reflow(float? maximumHeight = null) { + float effectiveHeight = maximumHeight + ?? (_lastLayoutHeight >= 0f ? _lastLayoutHeight : float.PositiveInfinity); float oldWidth = Width; - int maximumRows = float.IsPositiveInfinity(maximumHeight) + int maximumRows = float.IsPositiveInfinity(effectiveHeight) ? Math.Max(1, _entries.Count) : Math.Max( 1, (int)MathF.Floor( - (maximumHeight - OuterPadding * 2f + ButtonGap) + (effectiveHeight - OuterPadding * 2f + ButtonGap) / (ButtonExtent + ButtonGap))); int index = 0; foreach (ShelfEntry entry in _entries.Values) @@ -458,7 +522,14 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState if (_initialDockApplied && !_userPositioned) Left += oldWidth - Width; + LayoutChrome(); ApplyVisibility(); + + if (_initialDockApplied && !_userPositioned) + { + _dockLeft = Left; + _dockTop = Top; + } } /// Availability (has entries) AND the user's requested-visible @@ -495,7 +566,30 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState PluginShelfButton Button, PluginMinimizeButton Minimize); - private sealed class PluginShelfButton : UiSimpleButton + /// The top drag-grip band — a real + /// child (review fix round finding 1), drawn as three short dashes centered + /// in its own bounds (which already exclude the toggle's rect — see + /// ). is dimmed by + /// under the global UI lock (nit 12). + private sealed class ShelfGripPanel : UiPanel + { + private static readonly Vector4 DashColor = new(0.62f, 0.48f, 0.16f, 1f); + + protected override void OnDraw(UiRenderContext ctx) + { + base.OnDraw(ctx); + + const float dashWidth = 5f; + const float dashGap = 4f; + float totalDashWidth = dashWidth * 3f + dashGap * 2f; + float dashX = MathF.Max(2f, (Width - totalDashWidth) * 0.5f); + float dashY = Height * 0.5f - 1f; + for (int i = 0; i < 3; i++) + ctx.DrawFill(dashX + i * (dashWidth + dashGap), dashY, dashWidth, 2f, DashColor); + } + } + + internal sealed class PluginShelfButton : UiSimpleButton { private static readonly Vector4 HiddenBackground = new(0.025f, 0.025f, 0.02f, 0.96f); diff --git a/src/AcDream.App/UI/RetailUiRuntime.cs b/src/AcDream.App/UI/RetailUiRuntime.cs index d765f1204..fc031471d 100644 --- a/src/AcDream.App/UI/RetailUiRuntime.cs +++ b/src/AcDream.App/UI/RetailUiRuntime.cs @@ -678,6 +678,13 @@ public sealed class RetailUiRuntime : IDisposable // session that ended mid-trade was restoring an empty // open window at every launch). WindowNames.SecureTrade, + // Review fix round finding 3 (2026-09-06): the shelf folds + // an availability gate (EntryCount > 0) on top of the + // user's own show/hide request — PluginSidePanel's + // IRetainedWindowStateController restores the saved INTENT + // itself (RestoreWindowState), so this layer must not also + // drive Show/Hide from the derived layout.Visible. + WindowNames.PluginShelf, ]); } @@ -1057,9 +1064,19 @@ public sealed class RetailUiRuntime : IDisposable return true; } if (shelf.Visible) + { shelf.Hide(); + // Finding 8: hiding has no other affordance to bring it + // back (no menu entry, no button) — tell the user the + // exact chord that reopens it. The show branch stays + // silent; showing something is its own feedback. + _bindings.Options.DisplaySystemMessage( + "Plugin shelf hidden. Press Shift+Ctrl+F1 to show it again."); + } else + { shelf.Show(); + } return true; case AcDream.UI.Abstractions.Input.InputAction.ToggleAbuseReportingPanel: _bindings.Options.DisplaySystemMessage(OptionsPanelText.ReportAbuseUnavailable); @@ -4709,13 +4726,14 @@ public sealed class RetailUiRuntime : IDisposable _bindings.Assets.DefaultFont); Host.Root.AddChild(_pluginSidePanel); // Slice A: the shelf is itself a retained window (stable - // key "plugin-shelf") so drag, the UI lock, and + // key WindowNames.PluginShelf) so drag, the UI lock, and // RetailWindowLayoutPersistence apply for free — the // panel implements both IRetainedPanelController (the // availability-vs-requested-visible split) and - // IRetainedWindowStateController (collapse) itself. + // IRetainedWindowStateController (collapse + persisted + // show/hide intent) itself. Host.WindowManager.Register( - "plugin-shelf", + WindowNames.PluginShelf, _pluginSidePanel, _pluginSidePanel, controller: _pluginSidePanel); diff --git a/src/AcDream.App/UI/RetailWindowHandle.cs b/src/AcDream.App/UI/RetailWindowHandle.cs index fd7d12e68..ee8328f27 100644 --- a/src/AcDream.App/UI/RetailWindowHandle.cs +++ b/src/AcDream.App/UI/RetailWindowHandle.cs @@ -57,6 +57,18 @@ public sealed class RetailWindowHandle public event Action? Moved; public event Action? Resized; public event Action? Closed; + + /// + /// Review fix round (finding 3, docs/plans/2026-09-06-plugin-shelf-and-dat-icons.md + /// Slice A): raised by a controller-owned state change that Moved/Resized/ + /// Shown/Hidden do not cover — e.g. PluginSidePanel's collapse toggle + /// or its own /, which route through + /// instead of the outer frame's + /// Visible property. + /// subscribes here (alongside Moved/Resized/Shown/Hidden) to trigger a save. + /// Internal: no consumer outside this assembly needs it today. + /// + internal event Action? StateChanged; public event Action? LockChanged; public event Action? DescendantFocusChanged; public event Action? DescendantCaptureChanged; @@ -111,6 +123,9 @@ public sealed class RetailWindowHandle internal void NotifyMoved() => Moved?.Invoke(this); internal void NotifyResized() => Resized?.Invoke(this); + /// Raises . See that event's own doc. + internal void NotifyStateChanged() => StateChanged?.Invoke(this); + internal void NotifyClosed() { if (_closedSinceShown) return; diff --git a/src/AcDream.App/UI/RetailWindowLayoutPersistence.cs b/src/AcDream.App/UI/RetailWindowLayoutPersistence.cs index de2ae0614..9a0353dd5 100644 --- a/src/AcDream.App/UI/RetailWindowLayoutPersistence.cs +++ b/src/AcDream.App/UI/RetailWindowLayoutPersistence.cs @@ -36,10 +36,22 @@ public sealed class RetailWindowLayoutPersistence : IDisposable ? new HashSet(StringComparer.Ordinal) : new HashSet(stateManagedVisibilityWindows, StringComparer.Ordinal); + // Review fix round finding 4: attach late registrations too. Before this + // fix, only windows already registered at CONSTRUCTION time ever got a + // save subscription — a plugin window (or the plugin shelf, on a path + // that somehow constructs persistence first) registered afterward was + // silently never persisted. WindowUnregistered detaches the mirror image + // so a stale handle is not held (and re-notified) forever. + _manager.WindowRegistered += OnWindowRegistered; + _manager.WindowUnregistered += OnWindowUnregistered; foreach (RetailWindowHandle handle in manager.Windows) Attach(handle); } + private void OnWindowRegistered(RetailWindowHandle handle) => Attach(handle); + + private void OnWindowUnregistered(RetailWindowHandle handle) => Detach(handle); + /// Restore all registered windows after character and screen are /// known. (#390): the login-time restore keeps /// its lazy schema-migration save; the LIVE display-change reload passes @@ -206,9 +218,12 @@ public sealed class RetailWindowLayoutPersistence : IDisposable private void Attach(RetailWindowHandle handle) { + if (_attached.Contains(handle)) + return; _attached.Add(handle); handle.Moved += OnChanged; handle.Resized += OnChanged; + handle.StateChanged += OnChanged; if (!_stateManagedVisibilityWindows.Contains(handle.Name)) { handle.Shown += OnChanged; @@ -216,6 +231,20 @@ public sealed class RetailWindowLayoutPersistence : IDisposable } } + private void Detach(RetailWindowHandle handle) + { + if (!_attached.Remove(handle)) + return; + handle.Moved -= OnChanged; + handle.Resized -= OnChanged; + handle.StateChanged -= OnChanged; + if (!_stateManagedVisibilityWindows.Contains(handle.Name)) + { + handle.Shown -= OnChanged; + handle.Hidden -= OnChanged; + } + } + private void OnChanged(RetailWindowHandle handle) { if (_restoring || _disposed) return; @@ -245,7 +274,12 @@ public sealed class RetailWindowLayoutPersistence : IDisposable state.PersistedTop ?? handle.Top, handle.Width, state.PersistedHeight ?? handle.Height, - handle.IsVisible, + // Review fix round finding 3: persist the controller's own show/hide + // INTENT when it reports one, never the outer frame's derived + // IsVisible — a controller (e.g. PluginSidePanel) may fold in an + // availability gate on top of the user's actual request, and an + // availability-driven hide must never be captured as a user hide. + state.RequestedVisible ?? handle.IsVisible, state.Collapsed, state.Maximized, handle.AuthoredGeometryRevision); @@ -292,9 +326,17 @@ public sealed class RetailWindowLayoutPersistence : IDisposable float y = Math.Clamp(FiniteOr(layout.Y, handle.Top), 0f, maxY); handle.MoveTo(x, y); + // Review fix round finding 3: always hand the saved intent to the + // controller (not only when restoreVisibility is true) — a + // state-managed window (restoreVisibility false) restores its own + // show/hide intent THIS way instead of through Show/Hide below, so + // login never fires an extra BringToFront or routes through the + // ordinary Shown/Hidden notification for a window whose visibility + // this persistence layer does not otherwise touch. handle.StateController?.RestoreWindowState(new RetainedWindowState( Collapsed: layout.Collapsed, - Maximized: layout.Maximized)); + Maximized: layout.Maximized, + RequestedVisible: layout.Visible)); if (restoreVisibility) { @@ -335,16 +377,10 @@ public sealed class RetailWindowLayoutPersistence : IDisposable { if (_disposed) return; _disposed = true; - foreach (RetailWindowHandle handle in _attached) - { - handle.Moved -= OnChanged; - handle.Resized -= OnChanged; - if (!_stateManagedVisibilityWindows.Contains(handle.Name)) - { - handle.Shown -= OnChanged; - handle.Hidden -= OnChanged; - } - } + _manager.WindowRegistered -= OnWindowRegistered; + _manager.WindowUnregistered -= OnWindowUnregistered; + foreach (RetailWindowHandle handle in _attached.ToArray()) + Detach(handle); _attached.Clear(); } } diff --git a/src/AcDream.App/UI/WindowNames.cs b/src/AcDream.App/UI/WindowNames.cs index f641c7afc..716e0d60b 100644 --- a/src/AcDream.App/UI/WindowNames.cs +++ b/src/AcDream.App/UI/WindowNames.cs @@ -48,4 +48,9 @@ public static class WindowNames /// Campaign QT slice QT5: the three-tab Contracts/Journal/Page /// List panel (). public const string Journal = "journal"; + + /// Slice A (2026-09-06, + /// docs/plans/2026-09-06-plugin-shelf-and-dat-icons.md): the movable, + /// collapsible plugin shelf (). + public const string PluginShelf = "plugin-shelf"; } diff --git a/tests/AcDream.App.Tests/UI/PluginSidePanelTests.cs b/tests/AcDream.App.Tests/UI/PluginSidePanelTests.cs index 61852ac2c..ba931e580 100644 --- a/tests/AcDream.App.Tests/UI/PluginSidePanelTests.cs +++ b/tests/AcDream.App.Tests/UI/PluginSidePanelTests.cs @@ -1,3 +1,4 @@ +using System.Numerics; using AcDream.App.UI; using AcDream.Plugin.Abstractions; using AcDream.UI.Abstractions.Panels.Settings; @@ -70,8 +71,12 @@ public sealed class PluginSidePanelTests Assert.True(shelf.Visible); Assert.Equal(1, shelf.EntryCount); - UiSimpleButton shelfButton = Assert.IsAssignableFrom( - Assert.Single(shelf.Children)); + // Review fix round finding 1: the grip and collapse toggle are now + // real sibling children too, so shelf.Children is no longer just the + // one entry button — filter by the entry-button TYPE instead of + // asserting a raw child count. + PluginSidePanel.PluginShelfButton shelfButton = Assert.Single( + shelf.Children.OfType()); shelfButton.OnEvent(new UiEvent { Type = UiEventType.Click }); Assert.False(handle.IsVisible); @@ -141,7 +146,7 @@ public sealed class PluginSidePanelTests using var shelf = new PluginSidePanel( root.WindowManager, _ => (0u, 0, 0), font: null); root.AddChild(shelf); - root.WindowManager.Register("plugin-shelf", shelf, shelf, controller: shelf); + root.WindowManager.Register(WindowNames.PluginShelf, shelf, shelf, controller: shelf); var frame = new UiPanel { Width = 200f, Height = 100f }; root.AddChild(frame); @@ -170,7 +175,7 @@ public sealed class PluginSidePanelTests using var shelf = new PluginSidePanel( root.WindowManager, _ => (0u, 0, 0), font: null); root.AddChild(shelf); - root.WindowManager.Register("plugin-shelf", shelf, shelf, controller: shelf); + root.WindowManager.Register(WindowNames.PluginShelf, shelf, shelf, controller: shelf); for (int i = 0; i < 6; i++) { @@ -202,7 +207,7 @@ public sealed class PluginSidePanelTests using var shelf = new PluginSidePanel( root.WindowManager, _ => (0u, 0, 0), font: null); root.AddChild(shelf); - root.WindowManager.Register("plugin-shelf", shelf, shelf, controller: shelf); + root.WindowManager.Register(WindowNames.PluginShelf, shelf, shelf, controller: shelf); var frame = new UiPanel { Width = 200f, Height = 100f }; root.AddChild(frame); @@ -245,7 +250,7 @@ public sealed class PluginSidePanelTests using var shelf = new PluginSidePanel( root.WindowManager, _ => (0u, 0, 0), font: null); root.AddChild(shelf); - root.WindowManager.Register("plugin-shelf", shelf, shelf, controller: shelf); + root.WindowManager.Register(WindowNames.PluginShelf, shelf, shelf, controller: shelf); var frame = new UiPanel { Width = 200f, Height = 100f }; root.AddChild(frame); @@ -278,7 +283,7 @@ public sealed class PluginSidePanelTests using var shelf = new PluginSidePanel( root.WindowManager, _ => (0u, 0, 0), font: null); root.AddChild(shelf); - root.WindowManager.Register("plugin-shelf", shelf, shelf, controller: shelf); + root.WindowManager.Register(WindowNames.PluginShelf, shelf, shelf, controller: shelf); var frame = new UiPanel { Width = 200f, Height = 100f }; root.AddChild(frame); @@ -292,8 +297,8 @@ public sealed class PluginSidePanelTests float expandedWidth = shelf.Width; float leftBeforeCollapse = shelf.Left; - UiSimpleButton button = Assert.IsAssignableFrom( - Assert.Single(shelf.Children)); + PluginSidePanel.PluginShelfButton button = Assert.Single( + shelf.Children.OfType()); Assert.True(button.Visible); // Click the collapse toggle at the grip's right end through the real @@ -329,7 +334,7 @@ public sealed class PluginSidePanelTests using var shelf = new PluginSidePanel( root.WindowManager, _ => (0u, 0, 0), font: null); root.AddChild(shelf); - root.WindowManager.Register("plugin-shelf", shelf, shelf, controller: shelf); + root.WindowManager.Register(WindowNames.PluginShelf, shelf, shelf, controller: shelf); var frame1 = new UiPanel { Width = 200f, Height = 100f }; root.AddChild(frame1); @@ -377,7 +382,7 @@ public sealed class PluginSidePanelTests using var shelf = new PluginSidePanel( root.WindowManager, _ => (0u, 0, 0), font: null); root.AddChild(shelf); - root.WindowManager.Register("plugin-shelf", shelf, shelf, controller: shelf); + root.WindowManager.Register(WindowNames.PluginShelf, shelf, shelf, controller: shelf); var frame = new UiPanel { Width = 200f, Height = 100f }; root.AddChild(frame); @@ -391,12 +396,12 @@ public sealed class PluginSidePanelTests using var persistence = new RetailWindowLayoutPersistence( root.WindowManager, store, () => "Alice", () => (800, 600)); - root.WindowManager.MoveTo("plugin-shelf", 120f, 88f); + root.WindowManager.MoveTo(WindowNames.PluginShelf, 120f, 88f); shelf.RestoreWindowState(new RetainedWindowState(Collapsed: true)); shelf.Hide(); UiWindowLayout saved = Assert.IsType( - store.LoadWindowLayout("Alice", "800x600", "plugin-shelf", default)); + store.LoadWindowLayout("Alice", "800x600", WindowNames.PluginShelf, default)); Assert.Equal((120f, 88f), (saved.X, saved.Y)); Assert.False(saved.Visible); Assert.True(saved.Collapsed); @@ -406,7 +411,7 @@ public sealed class PluginSidePanelTests using var shelf2 = new PluginSidePanel( root2.WindowManager, _ => (0u, 0, 0), font: null); root2.AddChild(shelf2); - root2.WindowManager.Register("plugin-shelf", shelf2, shelf2, controller: shelf2); + root2.WindowManager.Register(WindowNames.PluginShelf, shelf2, shelf2, controller: shelf2); var frame2 = new UiPanel { Width = 200f, Height = 100f }; root2.AddChild(frame2); RetailWindowHandle pluginHandle2 = root2.WindowManager.Register( @@ -430,4 +435,322 @@ public sealed class PluginSidePanelTests Directory.Delete(directory, recursive: true); } } + + // ── Review fix round (2026-09-06): grip/toggle real children, dialog + // z-order, persisted collapse/hide intent ────────────────────────────── + + [Fact] + public void Drag_StartingOnAnEntryButton_DoesNotMoveTheShelf() + { + // Finding 1 / finding 7a: a press that lands on a real entry button + // (UiSimpleButton.HandlesClick => true) must be handled by the button, + // never promoted to a whole-shelf drag. + var root = new UiRoot { Width = 800f, Height = 600f }; + using var shelf = new PluginSidePanel( + root.WindowManager, _ => (0u, 0, 0), font: null); + root.AddChild(shelf); + root.WindowManager.Register(WindowNames.PluginShelf, shelf, shelf, controller: shelf); + + var frame = new UiPanel { Width = 200f, Height = 100f }; + root.AddChild(frame); + RetailWindowHandle pluginHandle = root.WindowManager.Register( + "plugin:acdream.test:main", frame); + shelf.Add( + new PluginUiOwner("acdream.test", "Test Plugin"), + new PluginPanelDescriptor("main", "Test Plugin"), + pluginHandle); + root.Tick(0.016d, 16L); + + PluginSidePanel.PluginShelfButton button = Assert.Single( + shelf.Children.OfType()); + Vector2 buttonScreen = button.ScreenPosition; + int pressX = (int)buttonScreen.X + 5; + int pressY = (int)buttonScreen.Y + 5; + float leftBefore = shelf.Left; + float topBefore = shelf.Top; + + root.OnMouseDown(UiMouseButton.Left, pressX, pressY); + root.OnMouseMove(pressX + 100, pressY + 100); + root.OnMouseUp(UiMouseButton.Left, pressX + 100, pressY + 100); + + Assert.Equal(leftBefore, shelf.Left); + Assert.Equal(topBefore, shelf.Top); + } + + [Fact] + public void PerTickZOrderRaise_Removed_ADialogAddedAfterKeepsItsHigherZOrder() + { + // Finding 2: the OnTick block that used to bump the shelf's ZOrder + // above every sibling, every frame, is gone. Registration's own + // press-to-raise remains (a grip drag calls UiRoot.BringToFront before + // the drag starts) but ticking alone must never re-assert dominance — + // RetailDialogFactory.Tick re-raises open dialogs every frame the same + // way, and the two must not fight. + var root = new UiRoot { Width = 800f, Height = 600f }; + using var shelf = new PluginSidePanel( + root.WindowManager, _ => (0u, 0, 0), font: null); + root.AddChild(shelf); + root.WindowManager.Register(WindowNames.PluginShelf, shelf, shelf, controller: shelf); + + var frame = new UiPanel { Width = 200f, Height = 100f }; + root.AddChild(frame); + RetailWindowHandle pluginHandle = root.WindowManager.Register( + "plugin:acdream.test:main", frame); + shelf.Add( + new PluginUiOwner("acdream.test", "Test Plugin"), + new PluginPanelDescriptor("main", "Test Plugin"), + pluginHandle); + root.Tick(0.016d, 16L); + + int shelfZBefore = shelf.ZOrder; + var dialog = new UiPanel { Width = 100f, Height = 60f, ZOrder = shelfZBefore + 50 }; + root.AddChild(dialog); + + root.Tick(0.016d, 16L); + root.Tick(0.016d, 16L); + + Assert.Equal(shelfZBefore, shelf.ZOrder); + Assert.True(dialog.ZOrder > shelf.ZOrder); + } + + [Fact] + public void ToggleClick_SavesCollapsedOnly_WithNoSubsequentHideOrMove() + { + // Finding 3a. + string directory = Path.Combine( + Path.GetTempPath(), "acdream-plugin-shelf-tests-" + Guid.NewGuid().ToString("N")); + string path = Path.Combine(directory, "settings.json"); + try + { + var store = new SettingsStore(path); + var root = new UiRoot { Width = 800f, Height = 600f }; + using var shelf = new PluginSidePanel( + root.WindowManager, _ => (0u, 0, 0), font: null); + root.AddChild(shelf); + root.WindowManager.Register(WindowNames.PluginShelf, shelf, shelf, controller: shelf); + + var frame = new UiPanel { Width = 200f, Height = 100f }; + root.AddChild(frame); + RetailWindowHandle pluginHandle = root.WindowManager.Register( + "plugin:acdream.test:main", frame); + shelf.Add( + new PluginUiOwner("acdream.test", "Test Plugin"), + new PluginPanelDescriptor("main", "Test Plugin"), + pluginHandle); + root.Tick(0.016d, 16L); + + using var persistence = new RetailWindowLayoutPersistence( + root.WindowManager, store, () => "Alice", () => (800, 600), + stateManagedVisibilityWindows: [WindowNames.PluginShelf]); + + float topBefore = shelf.Top; + float rightEdgeBefore = shelf.Left + shelf.Width; + + int toggleX = (int)shelf.Left + (int)shelf.Width - 8; + int toggleY = (int)shelf.Top + 4; + root.OnMouseDown(UiMouseButton.Left, toggleX, toggleY); + root.OnMouseUp(UiMouseButton.Left, toggleX, toggleY); + + UiWindowLayout saved = Assert.IsType( + store.LoadWindowLayout("Alice", "800x600", WindowNames.PluginShelf, default)); + Assert.True(saved.Collapsed); + Assert.True(saved.Visible); // no Hide happened + Assert.Equal(topBefore, saved.Y); // no vertical Move happened + // Collapsing shrinks Width; the shelf is still DOCKED, so Reflow's + // own anchor math shifts Left to keep the right edge fixed — that + // is not a "Move" (it never goes through handle.MoveTo/NotifyMoved), + // just the collapse's own geometry, captured in the same save. + Assert.Equal(shelf.Left, saved.X); + Assert.Equal(rightEdgeBefore, shelf.Left + shelf.Width, precision: 3); + } + finally + { + if (Directory.Exists(directory)) + Directory.Delete(directory, recursive: true); + } + } + + [Fact] + public void UnregisteringTheLastPlugin_DoesNotPersistTheAvailabilityHideAsAUserHide() + { + // Finding 3b. + string directory = Path.Combine( + Path.GetTempPath(), "acdream-plugin-shelf-tests-" + Guid.NewGuid().ToString("N")); + string path = Path.Combine(directory, "settings.json"); + try + { + var store = new SettingsStore(path); + var root = new UiRoot { Width = 800f, Height = 600f }; + using var shelf = new PluginSidePanel( + root.WindowManager, _ => (0u, 0, 0), font: null); + root.AddChild(shelf); + root.WindowManager.Register(WindowNames.PluginShelf, shelf, shelf, controller: shelf); + + var frame = new UiPanel { Width = 200f, Height = 100f }; + root.AddChild(frame); + RetailWindowHandle pluginHandle = root.WindowManager.Register( + "plugin:acdream.test:main", frame); + shelf.Add( + new PluginUiOwner("acdream.test", "Test Plugin"), + new PluginPanelDescriptor("main", "Test Plugin"), + pluginHandle); + root.Tick(0.016d, 16L); + + using var persistence = new RetailWindowLayoutPersistence( + root.WindowManager, store, () => "Alice", () => (800, 600), + stateManagedVisibilityWindows: [WindowNames.PluginShelf]); + + // Establish a genuine "the user wants this visible" save. + shelf.Show(); + Assert.True(shelf.Visible); + + root.WindowManager.Unregister(pluginHandle.Name); + Assert.Equal(0, shelf.EntryCount); + Assert.False(shelf.Visible); // derived Visible DOES flip (no entries)... + + // ...but the persisted INTENT must still read visible=true: the + // availability-hide is not subscribed for a state-managed window. + UiWindowLayout saved = Assert.IsType( + store.LoadWindowLayout("Alice", "800x600", WindowNames.PluginShelf, default)); + Assert.True(saved.Visible); + } + finally + { + if (Directory.Exists(directory)) + Directory.Delete(directory, recursive: true); + } + } + + [Fact] + public void RestoreWindowState_HiddenAndCollapsedIntent_AppliesDirectly_ThenShowExpandsAndReveals() + { + // Finding 3c: restoring Collapsed=true/RequestedVisible=false onto a + // fresh shelf that already has entries must hide AND collapse it — + // without going through Show/Hide (RestoreWindowState sets the intent + // and re-derives Visible through the same synchronous ApplyVisibility + // path every other state change uses). Show() must then reveal AND + // expand it. + var root = new UiRoot { Width = 800f, Height = 600f }; + using var shelf = new PluginSidePanel( + root.WindowManager, _ => (0u, 0, 0), font: null); + root.AddChild(shelf); + root.WindowManager.Register(WindowNames.PluginShelf, shelf, shelf, controller: shelf); + + var frame = new UiPanel { Width = 200f, Height = 100f }; + root.AddChild(frame); + RetailWindowHandle pluginHandle = root.WindowManager.Register( + "plugin:acdream.test:main", frame); + shelf.Add( + new PluginUiOwner("acdream.test", "Test Plugin"), + new PluginPanelDescriptor("main", "Test Plugin"), + pluginHandle); + root.Tick(0.016d, 16L); + Assert.True(shelf.Visible); + + shelf.RestoreWindowState(new RetainedWindowState(Collapsed: true, RequestedVisible: false)); + + Assert.False(shelf.Visible); + Assert.True(shelf.CaptureWindowState().Collapsed); + + shelf.Show(); + + Assert.True(shelf.Visible); + Assert.False(shelf.CaptureWindowState().Collapsed); + } + + [Fact] + public void CollapseExpandViaTheToggle_DoesNotCollapseTheColumnWrapToOneColumn() + { + // Finding 5: every Reflow() call site OTHER than OnTick's row-wrap + // used to default to an unbounded height, forcing a single-column + // layout for one frame after Add/remove/collapse/expand/restore — + // reusing _lastLayoutHeight keeps the column count OnTick already + // established. 12 entries at 800x260 forces a real multi-column wrap. + var root = new UiRoot { Width = 800f, Height = 260f }; + using var shelf = new PluginSidePanel( + root.WindowManager, _ => (0u, 0, 0), font: null); + root.AddChild(shelf); + root.WindowManager.Register(WindowNames.PluginShelf, shelf, shelf, controller: shelf); + + for (int i = 0; i < 12; i++) + { + var frame = new UiPanel { Width = 200f, Height = 100f }; + root.AddChild(frame); + RetailWindowHandle handle = root.WindowManager.Register($"plugin:test:{i}", frame); + shelf.Add( + new PluginUiOwner($"test.{i}", $"Plugin {i}"), + new PluginPanelDescriptor("main", $"Plugin {i}"), + handle); + } + root.Tick(0.016d, 16L); + + // Collapse via the real toggle click. + int toggleX = (int)shelf.Left + (int)shelf.Width - 8; + int toggleY = (int)shelf.Top + 4; + root.OnMouseDown(UiMouseButton.Left, toggleX, toggleY); + root.OnMouseUp(UiMouseButton.Left, toggleX, toggleY); + + // Expand via the real toggle click (coordinates recomputed: the + // shelf's collapsed Width moved the toggle). + toggleX = (int)shelf.Left + (int)shelf.Width - 8; + toggleY = (int)shelf.Top + 4; + root.OnMouseDown(UiMouseButton.Left, toggleX, toggleY); + root.OnMouseUp(UiMouseButton.Left, toggleX, toggleY); + + root.Tick(0.016d, 16L); + + Assert.True(shelf.Top + shelf.Height <= root.Height); + Assert.All( + shelf.Children.OfType(), + button => Assert.True(button.Top + button.Height <= shelf.Height)); + } + + [Fact] + public void ZeroMovementGripReleaseDoesNotFlipAnchoring_ButARealDragDoes() + { + // Finding 6: RetailWindowHandle.Moved fires unconditionally on every + // completed window-drag gesture — including a press+release on the + // grip with NO intervening mouse move. (RetailWindowManager.MoveTo + // itself short-circuits an already-equal position before ever firing + // Moved, so the meaningful equivalent of "a MoveTo to the identical + // dock position" is this zero-pixel drag-release, not a same-value + // MoveTo call.) That must not be mistaken for a user drag; a REAL + // drag still must. + var root = new UiRoot { Width = 800f, Height = 600f }; + using var shelf = new PluginSidePanel( + root.WindowManager, _ => (0u, 0, 0), font: null); + root.AddChild(shelf); + root.WindowManager.Register(WindowNames.PluginShelf, shelf, shelf, controller: shelf); + + var frame = new UiPanel { Width = 200f, Height = 100f }; + root.AddChild(frame); + RetailWindowHandle pluginHandle = root.WindowManager.Register( + "plugin:acdream.test:main", frame); + shelf.Add( + new PluginUiOwner("acdream.test", "Test Plugin"), + new PluginPanelDescriptor("main", "Test Plugin"), + pluginHandle); + root.Tick(0.016d, 16L); // establishes the initial right-edge dock + + int pressX = (int)shelf.Left + 10; + int pressY = (int)shelf.Top + 5; + root.OnMouseDown(UiMouseButton.Left, pressX, pressY); + root.OnMouseUp(UiMouseButton.Left, pressX, pressY); // zero movement + + float rightEdgeBeforeCollapse = shelf.Left + shelf.Width; + shelf.RestoreWindowState(new RetainedWindowState(Collapsed: true)); + // Still docked: the RIGHT edge survived the width change. + Assert.Equal(rightEdgeBeforeCollapse, shelf.Left + shelf.Width, precision: 3); + shelf.RestoreWindowState(new RetainedWindowState(Collapsed: false)); + + // Now a REAL drag via the grip. + root.OnMouseDown(UiMouseButton.Left, pressX, pressY); + root.OnMouseMove(pressX + 40, pressY + 40); + root.OnMouseUp(UiMouseButton.Left, pressX + 40, pressY + 40); + + float leftAfterDrag = shelf.Left; + shelf.RestoreWindowState(new RetainedWindowState(Collapsed: true)); + // User-positioned now: the LEFT edge survives instead. + Assert.Equal(leftAfterDrag, shelf.Left); + } } diff --git a/tests/AcDream.App.Tests/UI/RetailWindowLayoutPersistenceTests.cs b/tests/AcDream.App.Tests/UI/RetailWindowLayoutPersistenceTests.cs index 4a81e5cec..848f12eba 100644 --- a/tests/AcDream.App.Tests/UI/RetailWindowLayoutPersistenceTests.cs +++ b/tests/AcDream.App.Tests/UI/RetailWindowLayoutPersistenceTests.cs @@ -402,6 +402,28 @@ public sealed class RetailWindowLayoutPersistenceTests : IDisposable Assert.True(state.Restored.Maximized); } + // ── Review fix round finding 4 (plugin-shelf campaign, 2026-09-06): + // attach late registrations, not only windows present at construction ── + + [Fact] + public void WindowRegisteredAfterConstruction_StillRoundTrips() + { + var store = new SettingsStore(PathName); + var root = new UiRoot { Width = 800, Height = 600 }; + using var persistence = new RetailWindowLayoutPersistence( + root.WindowManager, store, () => "Alice", () => (800, 600)); + + // The window is registered AFTER persistence already exists — before + // this fix only windows present in RetailWindowManager.Windows at + // construction time ever got a save subscription. + RetailWindowHandle handle = Mount(root, "late-window"); + handle.MoveTo(77f, 88f); + + UiWindowLayout saved = Assert.IsType( + store.LoadWindowLayout("Alice", "800x600", "late-window", default)); + Assert.Equal((77f, 88f), (saved.X, saved.Y)); + } + private static RetailWindowHandle Mount( UiRoot root, string name, diff --git a/tests/AcDream.App.Tests/UI/UiDatFontBorderPixelTests.cs b/tests/AcDream.App.Tests/UI/UiDatFontBorderPixelTests.cs index 2614f17df..c9cc1a516 100644 --- a/tests/AcDream.App.Tests/UI/UiDatFontBorderPixelTests.cs +++ b/tests/AcDream.App.Tests/UI/UiDatFontBorderPixelTests.cs @@ -1,7 +1,10 @@ using System; using System.Collections.Generic; using System.IO; +using AcDream.App.Rendering; +using AcDream.App.Tests.Rendering.Gpu; using AcDream.App.UI; +using AcDream.Content; using DatReaderWriter; using DatReaderWriter.DBObjs; using DatReaderWriter.Options; @@ -80,6 +83,40 @@ public sealed class UiDatFontBorderPixelTests Assert.True(checkedCount > 10, "expected the documented font sweep to find multiple populated fonts"); } + /// + /// Review fix round finding 9 (plugin-shelf campaign, 2026-09-06): the + /// plugin shelf's collapse toggle now draws ASCII </> + /// instead of the «/» glyphs, which were the only use of + /// those code points anywhere in the App UI and which + /// silently skips when absent ( — + /// callers just don't draw a missing glyph, no error). Pins that the + /// default retail font (0x40000000, the same font + /// PluginSidePanel's buttons already resolve through + /// AcDream.App.RuntimeOptions-style DefaultFont wiring) actually + /// carries both ASCII code points, using the same + /// -backed + /// idiom TextureCacheLinearTwinTests uses to load a real dat font + /// with no live GPU. + /// + [Fact] + [Trait("Lane", "InstalledDat")] + public void RealDatFont_HasLessThanAndGreaterThanGlyphs() + { + string? datDir = ResolveDatDir(); + if (datDir is null) + Assert.Fail("Lane=InstalledDat requires an installed retail DAT directory; see docs/release-gate.md."); + + using var dats = new DatCollection(datDir, DatAccessType.Read); + using var adapter = new DatCollectionAdapter(dats); + var device = new RecordingGpuDevice(); + var cache = new TextureCache(device, adapter); + + UiDatFont? font = UiDatFont.Load(adapter, cache); + Assert.NotNull(font); + Assert.True(font!.TryGetGlyph('<', out _), "default font is missing '<' (toggle glyph)"); + Assert.True(font.TryGetGlyph('>', out _), "default font is missing '>' (toggle glyph)"); + } + /// /// Pure plumbing check — no dat, no GL: the ctor /// stores / verbatim onto