fix(plugin-ui): Slice A review fixes - real grip/toggle children, dialog z-order, persisted collapse/hide intent

Two independent Opus reviews of 01b98ca30 (Slice A - movable, collapsible
plugin shelf) found blocking design and behavior issues plus several
should-fix gaps. All addressed in this commit:

1. Grip/toggle are now REAL children instead of a drawn band + a
   cursor-position-dependent HandlesClick override + an OnEvent toggle
   hack: ShelfGripPanel (WindowMoveHandle=true) spans the top band minus
   the toggle width; a UiSimpleButton toggle sits beside it (HandlesClick
   already wins at UiRoot.OnMouseDown before the Draggable-window
   fallback). The shelf's own Draggable is now FALSE - verified against
   UiRoot.FindDragHandleWindow, which never reads a window's own
   Draggable flag at all (it walks for a WindowMoveHandle ancestor-or-self
   then climbs to the nearest child of UiRoot), so Draggable=true was
   never required for the grip to work and only armed the whole-window-
   drag fallback for clicks on the shelf's own padding - exactly the
   behavior the review asked NOT to have. The two Assert.Single(shelf.
   Children) test sites now filter by the (now internal) PluginShelfButton
   type instead of asserting child count.

2. Deleted the per-tick "always highest ZOrder" raise in OnTick. It fought
   RetailDialogFactory.Tick's own per-frame dialog re-raise, so a dialog
   opened while the shelf was visible could never end up on top of it.
   Registration's ordinary press-to-raise (a grip press calls
   BringToFront before the drag starts) remains.

3. Collapse and hide/show intent are now persisted through their own
   channel. RetainedWindowState gained a nullable RequestedVisible;
   RetailWindowHandle gained an internal StateChanged event that
   RetailWindowLayoutPersistence subscribes to (alongside Moved/Resized/
   Shown/Hidden) and that the shelf raises after a collapse toggle or
   Show/Hide. Capture() now persists state.RequestedVisible (the panel's
   own intent) instead of the derived IsVisible, so an availability hide
   (last plugin window unregistered) is never mistaken for a user hide.
   WindowNames.PluginShelf is now one of RetailUiRuntime's state-managed
   visibility windows, so Apply() restores the intent through
   RestoreWindowState directly rather than via Show()/Hide().

4. RetailWindowLayoutPersistence now subscribes to
   RetailWindowManager.WindowRegistered/WindowUnregistered so a window
   (a plugin window loaded after startup, or the shelf on any path that
   constructs persistence first) attaches even when it registers after
   persistence already exists.

5. Reflow()'s default height argument is now nullable and falls back to
   the last height OnTick actually measured (or unbounded if none yet),
   instead of always forcing a single-column layout - every call site
   OTHER than OnTick's own row-wrap (Add, unregister, Show/Hide, the
   collapse toggle, RestoreWindowState) used to collapse a wrapped
   multi-column layout to one column for a frame.

6. _userPositioned is now flipped only when the handle's position differs
   from the recorded dock placement, not on every RetailWindowHandle.
   Moved (which fires unconditionally on any completed window-drag
   release, including a zero-movement grip click, and on any
   ClampAllToScreen reachable-clamp).

7. New tests cover: a press+drag starting on an entry button does not
   move the shelf; the removed per-tick raise (a sibling with higher
   ZOrder keeps it after a tick); TogglePluginManager's hidden-and-
   collapsed -> shown-and-expanded / visible -> hidden transitions at the
   shelf API (no RetailUiRuntime construction harness exists in this test
   suite to exercise the action-routing switch itself - the "no plugin
   windows registered" message branch is therefore not covered here).

8. The hide branch of TogglePluginManager now displays "Plugin shelf
   hidden. Press Shift+Ctrl+F1 to show it again."; the show branch stays
   silent.

9. The collapse toggle now draws ASCII '<'/'>' instead of the DAT-font-
   dependent '«'/'»' glyphs (the only use of those code points in the App
   UI, silently dropped by UiDatFont when absent), and gets the same
   DatFont + bitmap fallback the shelf's entry buttons already have
   through UiSimpleButton. A new installed-DAT test pins that the default
   font actually carries both ASCII glyphs.

10. ResizeX/ResizeY are false on the shelf so a restored layout's saved
    dimensions can never stomp the derived Width/Height via ResizeTo.

11. WindowNames.PluginShelf replaces the "plugin-shelf" literal at every
    site (RetailUiRuntime, docs comments, tests).

12. The grip dims to half opacity while RetailWindowManager.IsLocked, the
    same visual cue every other retail window gets (the shelf's grip has
    no DatElementId, so RetailWindowLockPresentationController's authored-
    chrome dimming does not reach it on its own).

Every new test was verified to fail against the pre-fix source: reverting
src/AcDream.App/UI/PluginSidePanel.cs, IRetainedWindowStateController.cs,
RetailWindowHandle.cs, WindowNames.cs, and RetailUiRuntime.cs to their
01b98ca30 state makes the whole PluginSidePanelTests.cs file fail to even
compile (missing WindowNames.PluginShelf, the now-internal
PluginShelfButton type, and RetainedWindowState.RequestedVisible);
reverting RetailWindowLayoutPersistence.cs alone (fixed source elsewhere)
makes WindowRegisteredAfterConstruction_StillRoundTrips fail at runtime
with a null saved layout, confirming finding 4 in isolation.

Verified: dotnet build src/AcDream.App (Release) green; dotnet test
tests/AcDream.App.Tests (Release) targeted filter
(PluginSidePanel|RetailWindowLayout|Markup|UiRootInput) 105/105 green;
full suite 7303 passed / 97 skipped / 36 failed (identical failure set to
the pre-fix-round baseline - installed-DAT live-mount probes, Linux-only
pacing/credential tests, and alpha-flush COUNT-only conformance
divergences, none touching plugin UI; +9 tests, all passing, over the
prior 7294/97/36 baseline).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-09-06 13:51:25 +02:00
parent b41ccc87a0
commit 4fada238e6
9 changed files with 725 additions and 161 deletions

View file

@ -1,11 +1,25 @@
namespace AcDream.App.UI;
/// <summary>Panel state that is not completely described by outer-frame bounds.</summary>
/// <param name="RequestedVisible">
/// 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 <see cref="UiElement.Visible"/> (which for a
/// controller like <c>PluginSidePanel</c> also folds in an availability gate —
/// see that class's <c>ApplyVisibility</c>). Null means "no override, fall back
/// to <see cref="RetailWindowHandle.IsVisible"/>" — every existing state
/// controller that never had an intent distinct from derived visibility keeps
/// working unchanged. <see cref="RetailWindowLayoutPersistence"/>'s
/// <c>Capture</c>/<c>Apply</c> 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.
/// </param>
public readonly record struct RetainedWindowState(
bool Collapsed = false,
bool Maximized = false,
float? PersistedTop = null,
float? PersistedHeight = null);
float? PersistedHeight = null,
bool? RequestedVisible = null);
/// <summary>
/// Optional state seam used by retained-window persistence. Bounds are restored

View file

@ -10,11 +10,12 @@ namespace AcDream.App.UI;
/// <para>
/// Slice A (2026-09-06, <c>docs/plans/2026-09-06-plugin-shelf-and-dat-icons.md</c>):
/// the shelf is itself a retained window (<see cref="RetailUiRuntime.MountPlugins"/>
/// registers it as <c>"plugin-shelf"</c>) so it gets drag, the UI lock, and
/// <see cref="RetailWindowLayoutPersistence"/> position/visibility/collapsed
/// persistence for free, exactly like every other retained window. It implements
/// <see cref="IRetainedWindowStateController"/> for the collapse flag and
/// <see cref="IRetainedPanelController"/> to separate "has entries to show"
/// registers it as <see cref="WindowNames.PluginShelf"/>) so it gets drag, the UI
/// lock, and <see cref="RetailWindowLayoutPersistence"/> position/visibility/
/// collapsed persistence for free, exactly like every other retained window. It
/// implements <see cref="IRetainedWindowStateController"/> for the collapse flag
/// (and now the persisted show/hide intent — see <see cref="RestoreWindowState"/>)
/// and <see cref="IRetainedPanelController"/> to separate "has entries to show"
/// (availability) from the user's own show/hide choice — the same pattern
/// <c>PluginWindowVisibilityController</c> uses for individual plugin windows,
/// just applied synchronously (see <see cref="ApplyVisibility"/>) so unit tests
@ -24,19 +25,47 @@ namespace AcDream.App.UI;
/// </para>
///
/// <para>
/// The grip/collapse-toggle band across the top is drawn, not a child element:
/// <see cref="OnDraw"/> paints it and <see cref="HandlesClick"/>/<see cref="OnEvent"/>
/// 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 <see cref="UiElement.WindowMoveHandle"/> grip subtree
/// (nesting it inside would let the drag-handle promotion swallow its press
/// before the button ever sees a click; see <c>UiRoot.OnMouseDown</c>'s
/// <c>handleWindow is not null</c> priority over <c>HandlesClick</c>), and a
/// grip element as a plain sibling would change <see cref="UiElement.Children"/>'s
/// count/shape, which the existing single-button shelf tests assert directly.
/// Keeping the whole shelf <see cref="UiElement.Draggable"/> and excluding just
/// the toggle's pixel rect from <see cref="HandlesClick"/> 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 — <see cref="ShelfGripPanel"/> (<see cref="UiElement.WindowMoveHandle"/>
/// <see langword="true"/>) and a plain <see cref="UiSimpleButton"/> toggle beside
/// it — exactly the mechanism every other retail window with a non-title-bar move
/// strip already uses (<c>ChatLayoutConformanceTests.MountedChatWindow_TopStrip_IsAMoveHandleNotAGrip</c>
/// 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 <see cref="UiElement.HandlesClick"/> override instead — a real design
/// smell (input handling required reading the CURRENT mouse position out of
/// <see cref="UiRoot"/> 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
/// <see cref="UiElement.WindowMoveHandle"/> subtree — which is exactly where it
/// belongs anyway: <see cref="UiRoot.FindDragHandleWindow"/> only walks INTO a
/// <see cref="UiElement.WindowMoveHandle"/> ancestor chain looking for the flag: a
/// SIBLING toggle button never enters that walk, so nesting was never required.
/// </para>
///
/// <para>
/// <see cref="UiElement.Draggable"/> is <see langword="false"/> on the shelf
/// itself (verified against <see cref="UiRoot.FindDragHandleWindow"/>, which does
/// not read a window's own <see cref="UiElement.Draggable"/> at all — it walks
/// from the pressed element for a <see cref="UiElement.WindowMoveHandle"/>
/// ancestor-or-self, then climbs to the nearest child of <see cref="UiRoot"/>. A
/// press on <see cref="_grip"/> therefore resolves <c>handleWindow == this</c> and
/// drags regardless of <see cref="UiElement.Draggable"/>). Leaving the shelf
/// <see cref="UiElement.Draggable"/> would ALSO satisfy the grip drag, but would
/// additionally arm <c>UiRoot.OnMouseDown</c>'s whole-window-drag fallback
/// (<c>window is {{ Draggable: true }}</c>) 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
/// <see cref="UiElement.HandlesClick"/>. That is exactly the behavior Slice A's
/// review round asked NOT to have ("the GRIP drags, the padding between buttons
/// does NOT"), so <see cref="UiElement.Draggable"/> stays false: only
/// <see cref="_grip"/>'s <see cref="UiElement.WindowMoveHandle"/> 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 <c>UiRoot</c>'s
/// <c>FindWindow(target)</c> fallback (which requires <see cref="UiElement.Draggable"/>
/// or <see cref="UiElement.Resizable"/> 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.
/// </para>
/// </summary>
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).</summary>
private const float GripHeight = 12f;
/// <summary>Width of the collapse-toggle glyph's clickable rect, anchored
/// to the grip's right end.</summary>
/// <summary>Width of the collapse-toggle button, anchored to the grip's
/// right end.</summary>
private const float ToggleWidth = 16f;
/// <summary>Collapsed shelf width — just enough for the toggle glyph.</summary>
@ -63,6 +92,8 @@ public sealed class PluginSidePanel : UiPanel, IDisposable, IRetainedWindowState
private readonly Func<uint, (uint tex, int width, int height)> _resolve;
private readonly UiDatFont? _font;
private readonly Dictionary<RetailWindowHandle, ShelfEntry> _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
/// <summary>The user's/persisted show-hide intent, independent of whether
/// there are currently any entries to show (see <see cref="ApplyVisibility"/>).
/// Shift+Ctrl+F1 (<c>InputAction.TogglePluginManager</c>) flips this via
/// <see cref="Show"/>/<see cref="Hide"/>.</summary>
/// <see cref="Show"/>/<see cref="Hide"/>. Persisted directly (review fix
/// round finding 3): see <see cref="CaptureWindowState"/>/
/// <see cref="RestoreWindowState"/> and <see cref="RetainedWindowState.RequestedVisible"/>.</summary>
private bool _requestedVisible = true;
/// <summary>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 <see cref="Reflow"/>
/// 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 <see cref="RetailWindowHandle.Moved"/> on the
/// shelf's own registration, so unregistered (legacy unit-test) use never
/// sets this and behaves like the shelf has always been docked.</summary>
/// <summary>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 <see cref="OnHandleMoved"/>).
/// While false, the shelf is still docked and <see cref="Reflow"/> 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 <see cref="RetailWindowHandle.Moved"/> on the shelf's own
/// registration, so unregistered (legacy unit-test) use never sets this and
/// behaves like the shelf has always been docked.</summary>
private bool _userPositioned;
/// <summary>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).</summary>
private bool _initialDockApplied;
/// <summary>
/// The last position <see cref="Reflow"/> or the one-time dock placement
/// left the shelf at while it was still docked (<see cref="_userPositioned"/>
/// is false). Review fix round finding 6: <see cref="RetailWindowHandle.Moved"/>
/// fires unconditionally on EVERY window-drag release — including a
/// zero-movement grip click, and every <see cref="RetailWindowLayoutPersistence.ClampAllToScreen"/>
/// reachability re-clamp — not only a genuine drag. Comparing the handle's
/// CURRENT position against this recorded dock placement (rather than
/// treating any <see cref="RetailWindowHandle.Moved"/> 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 <see cref="_userPositioned"/>.
/// </summary>
private float _dockLeft;
private float _dockTop;
/// <summary>The shelf's own retained-window handle, captured the moment
/// <c>RetailUiRuntime.MountPlugins</c> registers it as <c>"plugin-shelf"</c>
/// (via <see cref="OnWindowRegistered"/>). Null when the shelf is used
/// unregistered, e.g. by tests that predate Slice A.</summary>
/// <c>RetailUiRuntime.MountPlugins</c> registers it as
/// <see cref="WindowNames.PluginShelf"/> (via <see cref="OnWindowRegistered"/>).
/// Null when the shelf is used unregistered, e.g. by tests that predate
/// Slice A.</summary>
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();
}
/// <summary>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.
}
/// <summary>
/// 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 <c>UiRoot.OnMouseDown</c>'s <c>target.HandlesClick</c> branch,
/// checked BEFORE its <c>window is {{ Draggable: true }}</c> 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 (<c>OnMouseDown</c>), which sets <c>UiRoot.MouseX/MouseY</c>
/// to the press coordinates immediately before reading it.
/// </summary>
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);
/// <summary>
/// Review fix round finding 3: restores BOTH the collapse flag and the
/// persisted show/hide INTENT directly onto <see cref="_requestedVisible"/>,
/// then re-derives <see cref="Visible"/> through the same synchronous
/// <see cref="ApplyVisibility"/> path every other state change uses — never
/// through <see cref="Show"/>/<see cref="Hide"/>, so a login-time restore
/// never fires an extra <c>BringToFront</c> or a redundant state-changed
/// save. <see cref="RetailUiRuntime"/> registers <see cref="WindowNames.PluginShelf"/>
/// as one of <see cref="RetailWindowLayoutPersistence"/>'s state-managed
/// visibility windows, so <c>RetailWindowLayoutPersistence.Apply</c> calls
/// this instead of <see cref="RetailWindowHandle.Show"/>/<see cref="RetailWindowHandle.Hide"/>
/// for visibility.
/// </summary>
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);
}
/// <summary>Repositions the grip/toggle children to span the current
/// Width — called whenever <see cref="Reflow"/> 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.</summary>
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;
}
/// <summary>
/// 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 <c>Visible=false</c>) so
/// expanding is instant, per Slice A plan item 4.
///
/// <para>Review fix round finding 5: the parameterless default no longer
/// forces a single-column layout. Every OTHER call site (<see cref="Add"/>,
/// unregister, <see cref="Show"/>/<see cref="Hide"/>, the collapse toggle,
/// <see cref="RestoreWindowState"/>) calls this with no argument — only
/// <see cref="OnTick"/>'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 <see cref="OnTick"/> row-wrap pass corrected it),
/// visible as a one-frame reflow flash on every collapse/expand. Reusing
/// <see cref="_lastLayoutHeight"/> (the last height <see cref="OnTick"/>
/// actually measured) when no explicit height is given keeps the same
/// column count these calls would already be using.</para>
/// </summary>
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;
}
}
/// <summary>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
/// <summary>The top drag-grip band — a real <see cref="UiElement.WindowMoveHandle"/>
/// child (review fix round finding 1), drawn as three short dashes centered
/// in its own bounds (which already exclude the toggle's rect — see
/// <see cref="LayoutChrome"/>). <see cref="UiElement.Opacity"/> is dimmed by
/// <see cref="OnTick"/> under the global UI lock (nit 12).</summary>
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);

View file

@ -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);

View file

@ -57,6 +57,18 @@ public sealed class RetailWindowHandle
public event Action<RetailWindowHandle>? Moved;
public event Action<RetailWindowHandle>? Resized;
public event Action<RetailWindowHandle>? Closed;
/// <summary>
/// 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. <c>PluginSidePanel</c>'s collapse toggle
/// or its own <see cref="Show"/>/<see cref="Hide"/>, which route through
/// <see cref="IRetainedWindowStateController"/> instead of the outer frame's
/// <c>Visible</c> property. <see cref="RetailWindowLayoutPersistence"/>
/// subscribes here (alongside Moved/Resized/Shown/Hidden) to trigger a save.
/// Internal: no consumer outside this assembly needs it today.
/// </summary>
internal event Action<RetailWindowHandle>? StateChanged;
public event Action<RetailWindowHandle, bool>? LockChanged;
public event Action<RetailWindowHandle, UiElement?>? DescendantFocusChanged;
public event Action<RetailWindowHandle, UiElement?>? DescendantCaptureChanged;
@ -111,6 +123,9 @@ public sealed class RetailWindowHandle
internal void NotifyMoved() => Moved?.Invoke(this);
internal void NotifyResized() => Resized?.Invoke(this);
/// <summary>Raises <see cref="StateChanged"/>. See that event's own doc.</summary>
internal void NotifyStateChanged() => StateChanged?.Invoke(this);
internal void NotifyClosed()
{
if (_closedSinceShown) return;

View file

@ -36,10 +36,22 @@ public sealed class RetailWindowLayoutPersistence : IDisposable
? new HashSet<string>(StringComparer.Ordinal)
: new HashSet<string>(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);
/// <summary>Restore all registered windows after character and screen are
/// known. <paramref name="saveBack"/> (#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();
}
}

View file

@ -48,4 +48,9 @@ public static class WindowNames
/// <summary>Campaign QT slice QT5: the three-tab Contracts/Journal/Page
/// List panel (<see cref="RetailPanelCatalog.Journal"/>).</summary>
public const string Journal = "journal";
/// <summary>Slice A (2026-09-06,
/// docs/plans/2026-09-06-plugin-shelf-and-dat-icons.md): the movable,
/// collapsible plugin shelf (<see cref="PluginSidePanel"/>).</summary>
public const string PluginShelf = "plugin-shelf";
}

View file

@ -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<UiSimpleButton>(
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<PluginSidePanel.PluginShelfButton>());
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<UiSimpleButton>(
Assert.Single(shelf.Children));
PluginSidePanel.PluginShelfButton button = Assert.Single(
shelf.Children.OfType<PluginSidePanel.PluginShelfButton>());
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<UiWindowLayout>(
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<PluginSidePanel.PluginShelfButton>());
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<UiWindowLayout>(
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<UiWindowLayout>(
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<PluginSidePanel.PluginShelfButton>(),
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);
}
}

View file

@ -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<UiWindowLayout>(
store.LoadWindowLayout("Alice", "800x600", "late-window", default));
Assert.Equal((77f, 88f), (saved.X, saved.Y));
}
private static RetailWindowHandle Mount(
UiRoot root,
string name,

View file

@ -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");
}
/// <summary>
/// Review fix round finding 9 (plugin-shelf campaign, 2026-09-06): the
/// plugin shelf's collapse toggle now draws ASCII <c>&lt;</c>/<c>&gt;</c>
/// instead of the <c>«</c>/<c>»</c> glyphs, which were the only use of
/// those code points anywhere in the App UI and which <see cref="UiDatFont"/>
/// silently skips when absent (<see cref="UiDatFont.TryGetGlyph"/> —
/// callers just don't draw a missing glyph, no error). Pins that the
/// default retail font (0x40000000, the same font
/// <c>PluginSidePanel</c>'s buttons already resolve through
/// <c>AcDream.App.RuntimeOptions</c>-style DefaultFont wiring) actually
/// carries both ASCII code points, using the same
/// <see cref="RecordingGpuDevice"/>-backed <see cref="TextureCache"/>
/// idiom <c>TextureCacheLinearTwinTests</c> uses to load a real dat font
/// with no live GPU.
/// </summary>
[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)");
}
/// <summary>
/// Pure plumbing check — no dat, no GL: the <see cref="UiDatFont"/> ctor
/// stores <paramref name="borderX"/>/<paramref name="borderY"/> verbatim onto