Two independent Opus reviews of01b98ca30(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 their01b98ca30state 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>
170 lines
7.4 KiB
C#
170 lines
7.4 KiB
C#
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;
|
|
using DatReaderWriter.Types;
|
|
using SysEnv = System.Environment;
|
|
|
|
namespace AcDream.App.Tests.UI;
|
|
|
|
/// <summary>
|
|
/// Pins <c>Font.NumHorizontalBorderPixels</c>/<c>NumVerticalBorderPixels</c>
|
|
/// (<c>Font::Serialize @0x00443650</c>) against the real installed DAT, and the
|
|
/// <see cref="UiDatFont"/> plumbing that carries them from
|
|
/// <see cref="UiDatFont.Load"/> onto <see cref="UiDatFont.BorderX"/>/
|
|
/// <see cref="UiDatFont.BorderY"/> — the field this project's font reader
|
|
/// dropped entirely before Campaign CH round 4
|
|
/// (<c>docs/research/2026-08-10-retail-ui-text-style.md</c> §1.4). A repo-wide
|
|
/// grep for <c>BorderPixel</c> returned zero hits before this fix; these tests
|
|
/// are the regression guard against that gap reappearing.
|
|
///
|
|
/// <para>
|
|
/// The live-dat tests follow <see cref="AcDream.App.Tests.UI.RetailCursorCatalogTests"/>'s
|
|
/// pattern: skip (not fail) when the real dats aren't present, so the suite stays
|
|
/// green in environments without the installed game.
|
|
/// </para>
|
|
/// </summary>
|
|
public sealed class UiDatFontBorderPixelTests
|
|
{
|
|
// Measured values — docs/research/2026-08-10-retail-ui-text-style.md §1.3.
|
|
[Theory]
|
|
[InlineData(0x40000000u, 4u, 4u)] // 16px bold serif — chat transcript face
|
|
[InlineData(0x40000001u, 4u, 4u)] // 18px bold serif — SpewBox face (round 4)
|
|
[InlineData(0x40000002u, 3u, 3u)] // 14px bold serif
|
|
[InlineData(0x40000025u, 3u, 3u)] // 11px — the pre-round-4 SpewBox placeholder face
|
|
[Trait("Lane", "InstalledDat")]
|
|
public void RealDatFont_HasExpectedBorderPixels(uint fontId, uint expectedHorizontal, uint expectedVertical)
|
|
{
|
|
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);
|
|
Assert.True(dats.TryGet<Font>(fontId, out Font? font), $"Font 0x{fontId:X8} not found");
|
|
Assert.NotNull(font);
|
|
Assert.Equal(expectedHorizontal, font!.NumHorizontalBorderPixels);
|
|
Assert.Equal(expectedVertical, font.NumVerticalBorderPixels);
|
|
}
|
|
|
|
[Fact]
|
|
[Trait("Lane", "InstalledDat")]
|
|
public void RealDatFont_EveryFontWithABackgroundAtlas_HasANonZeroBorder()
|
|
{
|
|
// docs/research/2026-08-10-retail-ui-text-style.md §1.2: "every font that
|
|
// has a background atlas has border >= 3, and every font without one has
|
|
// border == 0" — the data-driven dispatch DrawStringDat's outline pass
|
|
// relies on (background plane vs 8-neighbour fallback) is exactly this
|
|
// correlation. Sweep the documented populated range (0x40000000-0x40000032).
|
|
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);
|
|
int checkedCount = 0;
|
|
for (uint id = 0x40000000u; id <= 0x40000032u; id++)
|
|
{
|
|
if (!dats.TryGet<Font>(id, out Font? font) || font is null)
|
|
continue;
|
|
checkedCount++;
|
|
|
|
bool hasBackground = font.BackgroundSurfaceDataId != 0;
|
|
bool hasBorder = font.NumHorizontalBorderPixels > 0 || font.NumVerticalBorderPixels > 0;
|
|
Assert.True(
|
|
hasBackground == hasBorder,
|
|
$"Font 0x{id:X8}: hasBackground={hasBackground} but hasBorder={hasBorder}");
|
|
}
|
|
|
|
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><</c>/<c>></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
|
|
/// <see cref="UiDatFont.BorderX"/>/<see cref="UiDatFont.BorderY"/>, and existing
|
|
/// callers that omit them (pre-round-4 test fixtures) still default to zero.
|
|
/// </summary>
|
|
[Theory]
|
|
[InlineData(4, 4)]
|
|
[InlineData(3, 3)]
|
|
[InlineData(0, 0)]
|
|
public void Ctor_StoresBorderPixelsVerbatim(int borderX, int borderY)
|
|
{
|
|
var font = new UiDatFont(
|
|
fgTex: 1, fgW: 64, fgH: 64,
|
|
bgTex: 2, bgW: 64, bgH: 64,
|
|
lineHeight: 16f, baselineOffset: 12f,
|
|
glyphs: new Dictionary<char, FontCharDesc>(),
|
|
borderX: borderX, borderY: borderY);
|
|
|
|
Assert.Equal(borderX, font.BorderX);
|
|
Assert.Equal(borderY, font.BorderY);
|
|
}
|
|
|
|
[Fact]
|
|
public void Ctor_OmittedBorderPixels_DefaultToZero()
|
|
{
|
|
// Pins backward compatibility for the existing UiDatFontTests/SpewBoxControllerTests
|
|
// call sites that construct UiDatFont without borderX/borderY.
|
|
var font = new UiDatFont(
|
|
fgTex: 0, fgW: 0, fgH: 0,
|
|
bgTex: 0, bgW: 0, bgH: 0,
|
|
lineHeight: 16f, baselineOffset: 12f,
|
|
glyphs: new Dictionary<char, FontCharDesc>());
|
|
|
|
Assert.Equal(0, font.BorderX);
|
|
Assert.Equal(0, font.BorderY);
|
|
}
|
|
|
|
private static string? ResolveDatDir()
|
|
{
|
|
string? fromEnv = SysEnv.GetEnvironmentVariable("ACDREAM_DAT_DIR");
|
|
if (!string.IsNullOrWhiteSpace(fromEnv) && Directory.Exists(fromEnv))
|
|
return fromEnv;
|
|
|
|
string defaultDir = Path.Combine(
|
|
SysEnv.GetFolderPath(SysEnv.SpecialFolder.UserProfile),
|
|
"Documents",
|
|
"Asheron's Call");
|
|
return Directory.Exists(defaultDir) ? defaultDir : null;
|
|
}
|
|
}
|