diff --git a/docs/ISSUES.md b/docs/ISSUES.md index dd732a10..1a9e6af9 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -74,32 +74,50 @@ after each deliberate `Top` write for the imported-layout element. Precedent: `MapPageController.cs:235-249` (the same fix already landed for other runtime-repositioned imported/programmatic elements). -## #491 — MossTank extra/blacklisted buff lists are shown but not consumed by BuffPlan.Build +## #493 — In-game rule editors for metas and loot rules (VTank's "Edit Meta Rule" and the loot editor) — deferred by the owner -**Status:** OPEN — found 2026-09-07 at the slice 7 architecture review. -**Severity:** LOW (honest UI after fix round B; behavior owed) -**Component:** `src/AcDream.Plugins.MossTank/BuffPlan.cs` (`Build`), `BuffSettings.ExtraBuffSpellNames`/`BlacklistedBuffFamilyNames` +**Status:** OPEN — filed 2026-09-07 at the slice-7 live look (owner: "we should not implement that right now since most metas are implemented outside of the game. Same with loot rules"). +**Severity:** LOW (feature; post-Campaign VT) +**Component:** `src/AcDream.Plugins.MossTank/mosstank-metaeditor.xml`, `mosstank-loot-editor.xml`, `MossTankPanel.cs` -**Description.** Slice 7 added VTank's "Extra Buff Spells" and "Blacklisted Buff Families" lists to the Buffs tab. They are stored (persisted after fix round B) but `BuffPlan.Build` never reads them: extra exemplars are not cast, blacklisted families are not skipped. Belongs to VT2 slice 4 (buffs); wire `Build` to add the "best similar" spell per extra exemplar and to skip blacklisted families, with plan tests. +**Description.** VTank's Meta tab "Create" opens its "Edit Meta Rule" view (state, condition type + fields, action type + fields, nested All/Any/Not); its loot rules are edited in VTClassic's external editor. MossTank has a minimal popup editor for both. Metas are authored as metaf `.af` outside the game and loot profiles with the external `.utl` editor, so a full in-game editor is not on the campaign path. When picked up: model the editor on VTank's `Edit Meta Rule` view (KB 07 §1.3/§1.4 for every condition/action field) with a tree for nested rules, and a loot-rule editor over the `.utl` requirement vocabulary (KB 05 §1.3); the `.af`/`.utl` writers already exist. -## #490 — Plugin panel host: `StartVisible=false` + `ShowInSidePanel=false` is permanently unshowable; layout persistence has no revision bump +## #492 — Vulkan device loss at frame submit crashes the client and the shutdown path throws again on the lost device -**Status:** OPEN — found live 2026-09-07 at slice 7 fix round A (`78b42a519`), workaround in the plugin. -**Severity:** LOW/MEDIUM (host defect; silent) -**Component:** `src/AcDream.App/UI/RetailUiRuntime.cs` (~5735-5757, `PluginWindowVisibilityController`), `RetailWindowLayoutPersistence` +**Status:** OPEN — observed 2026-09-07 17:56 on the owner's machine during the slice-7 live look (gate build at `2b79ca325`). +**Severity:** MEDIUM (crash; the trigger is external) +**Component:** `src/AcDream.App/Rendering/Gpu/Vk/` (frame submit), `GameWindowLifetime.TryComplete` / `ResourceShutdownTransaction` / `WbMeshAdapter.Dispose` -**Description.** (1) `_requestedVisible = startVisible` and only `OnShown()` ever sets it true; a window registered with `ShowInSidePanel=false` has no shelf entry to raise `OnShown`, so `StartVisible=false` can never become visible, with no error. MossTank works around it by registering its popups `StartVisible=true` and gating on its own visibility bindings. Fix shape: validate the combination at `AddPanel` (throw, or coerce). (2) Plugin windows restore a stale persisted position over a changed authored default with no revision bump — the slice 7 screenshots needed an isolated `ACDREAM_CONFIG_DIR` to open at the authored 28,42. Fix shape: key the persisted layout by the panel's authored geometry hash (or a plugin-declared layout revision) so a redesign resets the stored position once. +**Description.** `vkQueueSubmit2 (frame)` returned `ErrorDeviceLost` (an unhandled `VulkanCallException`); the client then ran its graceful logout (ACE confirmed it) and the ordered shutdown, where the mesh-adapter stage failed on `vkWaitSemaphores … ErrorDeviceLost` and `ResourceShutdownTransaction.CompleteOrThrow` re-threw. Crash report: `C:SERSRIKNAPPDATAocalcdreamchediagnosticsash-20260907t1556317152878z-6224-d80b3162cf1e406fadad21098b322b5a.json`. the owner reported "disconnected and crash" — the disconnect is the client's own logout after the device loss, not a network event. -## #489 — Headless: SpewBox pending queue grows unbounded when no console ticks it; console polish - -**Status:** OPEN — found 2026-09-07 by the Opus re-check of the headless console (`738111239`). -**Severity:** LOW/MEDIUM (leak in long-lived bots) -**Component:** `src/AcDream.Runtime/.../SpewBoxState.cs` (`Enqueue` ~:110, `_pending`), `src/AcDream.Headless/Hosting/HeadlessConsoleSpewBoxPump.cs` - -**Description.** `RuntimeCommunicationState.AddText` routes every `ClientLocal` (0x1A) line into `SpewBoxState.Enqueue`; the only `Tick` caller in the headless host is the console pump, so with the console disabled (every scripted/CI bot) `_pending` grows for the life of the session. Pre-existing before the console; the console merely made it visible. Fix shape: tick the SpewBox from the session tick regardless of the console (or drop `ClientLocal` text when nothing observes it), with a pin that a 10,000-line burst without a console does not grow the queue. - -**Polish carried from the same re-check:** in `--console` mode the JSON diagnostics/resources stream still interleaves with the chat lines on stdout — quiet it or send it to stderr when the console is on; `--console` missing from `--help`; `HeadlessConsoleOptions.cs:51` re-types the env-var literal (the LaunchOptions regex needs it — a const rename would split the two reads); the `/quit`/`/status`/"not handled" writes and `Pump()` sit outside the S4 try/catch (a broken stdout pipe would fault the session); the SpewBox's 4-entry visible cap can drop interface-text lines produced between two pumps. +**fix shape.** (1) treat `errordevicelost` at submit as a terminal render fault: stop rendering, keep the session alive long enough for the graceful logout, and exit with a clear self-report instead of an unhandled exception; (2) the shutdown transaction must tolerate a lost device (skip gpu waits/frees that cannot succeed on a lost device, report once) so teardown converges. not slice-7 scope; belongs with the renderer's fault handling. +## #491 — mosstank extra/blacklisted buff lists are shown but not consumed by BuffPlan.Build + +**Status:** OPEN — found 2026-09-07 at the slice 7 architecture review. +**Severity:** LOW (honest UI after fix round B; behavior owed) +**Component:** `src/AcDream.Plugins.MossTank/BuffPlan.cs` (`Build`), `BuffSettings.ExtraBuffSpellNames`/`BlacklistedBuffFamilyNames` + +**Description.** Slice 7 added VTank's "Extra Buff Spells" and "Blacklisted Buff Families" lists to the Buffs tab. They are stored (persisted after fix round B) but `BuffPlan.Build` never reads them: extra exemplars are not cast, blacklisted families are not skipped. Belongs to VT2 slice 4 (buffs); wire `Build` to add the "best similar" spell per extra exemplar and to skip blacklisted families, with plan tests. + +## #490 — Plugin panel host: `StartVisible=false` + `ShowInSidePanel=false` is permanently unshowable; layout persistence has no revision bump + +**Status:** OPEN — found live 2026-09-07 at slice 7 fix round A (`78b42a519`), workaround in the plugin. +**Severity:** LOW/MEDIUM (host defect; silent) +**Component:** `src/AcDream.App/UI/RetailUiRuntime.cs` (~5735-5757, `PluginWindowVisibilityController`), `RetailWindowLayoutPersistence` + +**Description.** (1) `_requestedVisible = startVisible` and only `OnShown()` ever sets it true; a window registered with `ShowInSidePanel=false` has no shelf entry to raise `OnShown`, so `StartVisible=false` can never become visible, with no error. MossTank works around it by registering its popups `StartVisible=true` and gating on its own visibility bindings. Fix shape: validate the combination at `AddPanel` (throw, or coerce). Still OPEN. (2) **FIXED at `05f22d46f`.** Plugin windows restored a stale persisted SIZE over a changed authored default with no revision bump (MossTank's panel went 856x236 -> 984x271 and every stored layout stayed at 856x236 forever — not a position bug, a size bug; the earlier "28,42" framing above was about part (1)'s workaround, not this). `RetailWindowManager.ComputeAuthoredGeometryRevision` now derives the revision from the panel's own authored `(w, h, minw, minh, resizable)` tuple via a process-stable hash (deliberately not `System.HashCode`, which reseeds per process), and `MountPlugins` passes it instead of the hard-coded `0`. `RetailWindowLayoutPersistence.MigrateAuthoredGeometry` now compares revisions for inequality rather than "newer only", since a hash is not an ordered counter. Position is kept (not reset) and still clamps to the live screen; a genuinely unchanged authored size keeps the user's own resize. See `RetailWindowLayoutPersistence`'s class doc for the full contract. + +## #489 — Headless: SpewBox pending queue grows unbounded when no console ticks it; console polish + +**Status:** OPEN — found 2026-09-07 by the Opus re-check of the headless console (`738111239`). +**Severity:** LOW/MEDIUM (leak in long-lived bots) +**Component:** `src/AcDream.Runtime/.../SpewBoxState.cs` (`Enqueue` ~:110, `_pending`), `src/AcDream.Headless/Hosting/HeadlessConsoleSpewBoxPump.cs` + +**Description.** `RuntimeCommunicationState.AddText` routes every `ClientLocal` (0x1A) line into `SpewBoxState.Enqueue`; the only `Tick` caller in the headless host is the console pump, so with the console disabled (every scripted/CI bot) `_pending` grows for the life of the session. Pre-existing before the console; the console merely made it visible. Fix shape: tick the SpewBox from the session tick regardless of the console (or drop `ClientLocal` text when nothing observes it), with a pin that a 10,000-line burst without a console does not grow the queue. + +**Polish carried from the same re-check:** in `--console` mode the JSON diagnostics/resources stream still interleaves with the chat lines on stdout — quiet it or send it to stderr when the console is on; `--console` missing from `--help`; `HeadlessConsoleOptions.cs:51` re-types the env-var literal (the LaunchOptions regex needs it — a const rename would split the two reads); the `/quit`/`/status`/"not handled" writes and `Pump()` sit outside the S4 try/catch (a broken stdout pipe would fault the session); the SpewBox's 4-entry visible cap can drop interface-text lines produced between two pumps. + ## #488 — MossTank `.utl` expression block: length prefix measured before newline normalization **Status:** OPEN — found 2026-09-07 by the final Opus re-check of Campaign VT diff --git a/docs/plans/2026-09-07-campaign-vt-slice7-tabs.md b/docs/plans/2026-09-07-campaign-vt-slice7-tabs.md index 2b42fbc8..f18053f4 100644 --- a/docs/plans/2026-09-07-campaign-vt-slice7-tabs.md +++ b/docs/plans/2026-09-07-campaign-vt-slice7-tabs.md @@ -141,5 +141,9 @@ re-review, merge to the campaign branch, then the owner's visual gate. - 2026-09-07 16:10 App-side pieces merged into the campaign branch: `b71a8ea37` retail scrollbar chrome on plain `` popups and overflowing ``s (chat/inventory skin ids `0x06004C5F`/`6C`/`69`/`60`/`63`/`66`; markup menus are one-column scrollable), `2e63391cc` `` + `anchor="left top right bottom"` on every element (window-manager resize + persistence already generic). App markup/menu/list/window/anchor filter 294/1 skipped. Panel round D (after fix round C on the panel worktree): merge the campaign branch in, Advanced Options model + category names, VTank's 26-recall table wired, `resizable="true"` + anchors on the main panel with a larger default. - 2026-09-07 16:30 fix round C (architecture re-check of fix round B `8d3c6ad7c`) landed on the panel worktree, seven commits: `466fac426` (D1 — Advanced Options popup's four bindings, previously re-filtering the 163-entry VtankOptionCatalog and re-running GetMetaOption/ToDisplayString per row on every draw, materialized once by a new RefreshAdvancedOptions() called from category toggle/edit-apply/selection-change/profile-load), `23d4376cc` (D4 — AdvancedOptionCategoryEnabled no longer hands out the mutable backing bool[] directly; a ReadOnlyCollection view built once over the same array), `f15667db5` (D2 — `/mt refresh` now calls RefreshMonsterEditor after EnsureDefaultMonsterRule, the one gap found auditing every `_combatSettings.Rules` mutation site in the plugin), `3b8d02194` (D3 — AssertWithinParent's bottom-band check now counts each widget kind's own implicit default height when `h` is absent, label/field 16 toggle 20 button 16, instead of reading an absent `h` as zero), `37055bddc` (F7+F12 — ItemHandsColumn materialized once in RefreshItemEditors from the SAME base-name array that builds the decorated `_itemRows`, replacing a per-frame `.Select().ToArray()` plus a suffix-reparsing BaseItemName helper with one forward-only decoration; CycleItemHandsAtCore now refreshes the cache it mutates), `0724761ad` (F3 — SecondaryPopupPanelsFitTheirOwnBoundsAndEveryBindingResolves discovers popup files with the same glob the sibling-overlap and arrow-glyph tests already use instead of a hand-maintained InlineData list), `d20ad47c5` (F4 — the ExtraBuffSpellNames/BlacklistedBuffFamilyNames field comments and their two Buffs-tab tooltips now cite issue #491). D3's grep audit of every real `mosstank*.xml` file found no label/toggle/field/button missing `h`, so the stricter bottom-band check introduced zero new failures against shipped markup. Every new/changed pin was shown to fail against a targeted mutation (property/getter reverted, or the assertion target broken) before the fix was restored; D1's own fix broke one PRE-EXISTING test (`AdvancedOptionValueColumnMirrorsTheLiveSettingValue`, which expected a setting toggled OUTSIDE the popup's own mutators to appear on the very next read) — updated it to exercise the "selection change" mutator afterward, with a comment explaining the narrowed contract. MossTank suite 677 (D1) → 678 (D4) → 679 (D2) → 680 (D3) → 681 (F7+F12) → 678 (F3, four InlineData cases collapsed into one Fact, no coverage loss) → 678 (F4, comment/tooltip only) → 678 (F13, no new test, an assertion removed). Full solution build green after every commit; App markup/plugin filter held 203/203 throughout. - 2026-09-07 fix round C item F13 folded the `EveryInteractiveControlDeclaresARealHandlerBinding` test's exact-control-count history here, in the same commit that removed it from the test file: Round 3 item 10 added +3 for the Route/Loot/Meta Delete buttons (Settings already had one from round 2 step 5). Campaign VT slice 7 S7.1/S7.2 transcribed Options/Profiles/Vitals control-for-control from VTank's own tabs (docs/research/vtank-kb/08-ui-views.md §1) and repositioned the Advanced Options/Loot Editor popups to VTank's exact 392x300/268x300 geometry — net 194 → 205. The Options tab gained four direct checkboxes for settings that already existed but were only reachable through the generic Advanced Options key-value editor (Enable Autostack, Fastcast Buffs, Don't Shoot at Walls, Fallback Debuffs if Blocked); the Profiles tab gained a real Meta-profile combo/CopyTo row (previously a stub label) and per-type name fields/New buttons, and lost the Loot Priority Boost toggle as a duplicate of the Options-tab control of the same name; the Loot Editor popup lost LootEditorNotice (a status label, not an action). S7.3 replaced the whole Monsters tab body with VTank's own 23-column grid: the select-then-edit surface (1 list + 1 field + 8 buttons + 3 menus + 14 toggles + 3 equip buttons = 30 controls) is gone, replaced by 1 list + 23 columns + 1 field + 2 buttons (27) — net 205 → 202. Fix round A (2026-09-07) moved the Advanced Options and Loot Editor groups OUT of mosstank.xml entirely into their own plugin panels and deleted the Profiles tab's three stacked name-draft field+"New"-button rows and the acdream-only "Loot engine" classifier menu — net 202 → 167. S7.4 replaced the Items tab's single-column list with VTank's own clWeaponName/clHandedness 2-column grid (+2), added the Consumables tab's "Excluded Scarab Types" 2-column grid plus its "Add Selected" button (+4), and added the Buffs tab's Extra Buff Spells / Blacklisted Buff Families lists plus their two "Add..." buttons (+4) — net 167 → 177. S7.5 replaced the Route tab's single-column waypoint list with VTank's own clWP/clWPc 2-column grid (+2) and added the "Select Nearest Point" button (+1) — net 177 → 180. S7.6 replaced the Meta tab's single-column rules list with VTank's own 6-column lstMetaRules grid (+6) — net 180 → 186. Fix round B item 5: VTank's real Meta tab is only 5 controls (the grid, "Create", a caption, and the settable current-state menu). Removed entirely: the profile toolbar (menu/name-draft/New/CopyTo/Clear/Delete, 6) and "Enable Meta" (1) — 7 controls gone for good. Moved into mosstank-metaeditor.xml (a separate file this test doesn't scan): the State/Condition/Action editor (2 fields + 2 menus + 1 field for state + 1 secondary-text field + 4 numeric-stepper buttons + Apply/Remove/MoveUp/MoveDown, 14). Added: the settable cmbMetaCurrentState menu (+1). Net 186 → 166 (-7 -14 +1). Fix round B item 8: the Route tab collapses to VTank's own 2-across button grid + bottom nav-control row, 28 → 18 interactive controls within mosstank.xml. Checkpoint/Jump/Remove/Set Follow Target/Follow Corners/Open Doors/Nav Priority/the Follow-Nav-Min-Distance stepper moved to mosstank-advanced.xml's own new "MossTank Extras" section. The 2-state ToggleRouteAddPosition button became the real 3-option cmbNavInsertMode ``, and the pause "-"/"+" stepper became one editable field. Net 166 → 156 (the count the test asserted immediately before this fix round removed the assertion). The test's own per-control handler/enabled loop remains the live guarantee; a bare `Assert.NotEmpty(controls)` guards the selector itself. Round D dispatched on the same worktree (merges the campaign branch first): category names + VTank's Advanced Options interaction model, the 26-recall table wired to route execution, resizable main panel at a ~15% larger default with anchored lists. -- 2026-09-07 round D landed on the panel worktree (four commits, this session, base `6b42fd68e`): merged `claude/latest-main-sync-497549` first (`4ba0a557f`, bringing in `b71a8ea37` retail scrollbar chrome and `2e63391cc` resizable/anchor markup — resolved the ledger/plugin-ui-markup/UiMarkupList/MarkupDocument/MarkupDocumentTests conflicts keeping both sides' intent, one Advanced Options popup design collision found and resolved in the merge itself: the sync branch's always-scrollable `` model won over this branch's opt-in `scroll=` attribute, since it is the owner-driven shipped design). Item 1 (`ebe670adf`): Advanced Options' category filter shows VTank's real names (Misc/Recharge/MeleeCombat/SpellCombat/Ranges/Navigation/Buffing/Crafting/Looting) instead of raw bitmask hex, derived from real single-category KB rows rather than a typed-in literal. Item 2 (`435ced86f`): VTank's click model — a `tBool` value flips in place, a `tEnum` value cycles to the next label (VtankDefaultSettingsDatabase.SettingEnumValues, parsed from the embedded .usd's own SettingsEnumInfo table), everything else selects into the edit field; Apply/Back buttons removed (retail has neither); panel height 476->450. Item 3 (`5318adbb3`): RouteRecallKind replaced with VTank's real 26-recall cmbRecallType table (metaf's NRecall) plus Marketplace kept as its own slash-command entry (27 total) — deviation: the old Lifestone slash-command member is dropped rather than duplicating the new spell-based LifestoneRecall's "Lifestone Recall" label; Primary/SecondaryPortal upgraded from runtime spell-name lookup to their real hardcoded ids (48/2647); AddRouteRecallCore now writes RecallSpellId+RecallSpellName onto the waypoint so a route-tab-added recall casts exactly like a metaf-imported one. Item 4 (`2d626971d`): main panel `resizable="true"` floored at the pre-round-D authored size (856x236 minw/minh), default enlarged ~15% to 984x271; all nine tab groups anchor="left top right bottom" and grow by the same 128x35 delta; Monsters/Meta's lists get full left/right/top/bottom stretch (nothing beside them); Items/Consumables/Buffs/Route's lists grow height-only, pinned away from a sibling list/button column; a right-pinned sibling list (Consumables' Excluded Scarab, Buffs' Blacklisted Buff Families) repositions +128 so it already sits flush-right at the enlarged default. Every new/changed pin shown to fail against a targeted mutation first (category-name projection reverted, ClickAdvancedOptionValue reduced to a bare select, SubmitRecall reduced to `return false`, enum-order swap, Monsters' anchor attribute removed) and restored green. MossTank suite 678 -> 682 (item 2) -> 713 (item 3) -> 715 (item 4); App markup/plugin filter 242 -> 243 (item 4's real-file re-layout test); full solution builds clean in Release throughout. Owner's connected visual gate against these five fixes remains the next step; #491 (buff-list wiring) and the pre-cutover-JSON RouteRecallKind-ordinal migration risk (documented at its own binding site in MossTankRouteProfileStore.cs) are the carried, accepted deviations from this round. -- 2026-09-07 round E (architecture re-check of round D) landed on the same worktree, six commits, base `2b79ca325`: `3178d9202` (D-4 — an .af "rcl" node whose spell name fails to resolve leaves RecallSpellId at 0 and Recall at its default ordinal, PrimaryPortalRecall; SubmitRecall's own Recall-based fallback used to cast spell 48 for it regardless, since MetafSerializer.ReadNavNode never touches the enum on that path — TickRecall now refuses and skips instead, naming the unresolved spell in the status notice; the dead fallback branch is removed). `a9d1d0a82` (D-1 — the pre-cutover RouteRecallKind was {Lifestone=0, Marketplace=1, PrimaryPortal=2, SecondaryPortal=3}, completely different kinds at the SAME ordinals as the round-D enum, and Enum.IsDefined never caught the silent remap since 0..3 are all still valid new members; LegacyWaypointDocument.Recall is now a bare int translated through an explicit MapLegacyRecall table that also derives RecallSpellId/RecallSpellName). `3fe304359` (D-2 — RouteRecallNames/SelectedRouteRecall showed Enum.GetNames/ToString()'s bare C# identifiers; both now route through RouteWaypoint.RecallDisplayName like RouteInsertModeNames/RouteModeNames already do, SelectRouteRecall parses the same display string back, and the combo widened 120->300 to fit VTank's own full metaf-sourced names, e.g. "Paradox-touched Olthoi Infested Area Recall" — VTank's real combo is also 120 wide only because ITS captions are hand-abbreviated). `64b62fea3` (D-3 — mosstank.xml's own top comment already claimed the Advanced Options popup got resizable+anchored treatment; the popup file itself still carried the dead resize="none", never actually resizable; now resizable="true" minw/minh floored at the pre-fix authored 392x450, lOptionList grows WIDTH ONLY since the value field/description/notice sit directly below with zero vertical slack and none of them anchor to shift down, lFilterList tracks the growing right edge in lockstep so the widening list never walks into it; AdvancedOptionDescription is declared anchor="left top right" for consistency but documented as a real no-op — UiLabel.OnDraw always overwrites Width/Height to the measured text extent every draw, so nothing a label's anchor computes is ever visible). `1a0b0e014` (D-5 — RefreshAdvancedOptions' own doc comment lists every mutation site that must call it, but ShowAdvancedOptions itself was missing one, so a setting changed via /vt opt set or an Options-tab checkbox while the popup was closed stayed stale until some in-popup interaction refreshed it; ShowAdvancedOptions now calls RefreshAdvancedOptions). `7badbee88` (D-6 — a new whole-tree resolved-geometry pin at the main panel's minw/minh floor (856x236) and one enlarged size (1100x320), adapting AssertNoSiblingOverlap/AssertWithinParent's rectangle-overlap algorithm from the authored XElement tree to the resolved UiElement tree for each of the nine tabs in turn; building the test surfaced and fixed two real test-methodology bugs along the way — reusing one built tree across tabs let an earlier tab's resize corrupt a later tab's anchor-capture baseline, and walking into an invisible tab group's never-anchored-for-this-size descendants misreported stale geometry as broken — plus one genuine test-harness false positive (UiLabel's font-dependent measured box, excluded from the size checks for the same reason D-3's own commit documents). Every new/changed test shown to fail first via a targeted revert-then-reapply (git checkout + saved patch) or a deliberate markup mutation (D-6's Monsters-anchor removal), each restored green afterward. MossTank suite 715 (pre-round-E baseline, matching round D's close) -> 716 (D-4) -> 720 (D-1) -> 721 (D-2) -> 721 (D-3, App-side only) -> 722 (D-5); App markup/plugin filter 243 -> 243 (D-2 widened menu, no new pin) -> 244 (D-3) -> 246 (D-6, two theory cases). Full solution builds clean in Release throughout. Carried, accepted deviations unchanged from round D: #491 (buff-list wiring) and (now closed by D-1) the former pre-cutover-JSON RouteRecallKind-ordinal migration risk. +- 2026-09-07 round D landed on the panel worktree (four commits, this session, base `6b42fd68e`): merged `claude/latest-main-sync-497549` first (`4ba0a557f`, bringing in `b71a8ea37` retail scrollbar chrome and `2e63391cc` resizable/anchor markup — resolved the ledger/plugin-ui-markup/UiMarkupList/MarkupDocument/MarkupDocumentTests conflicts keeping both sides' intent, one Advanced Options popup design collision found and resolved in the merge itself: the sync branch's always-scrollable `` model won over this branch's opt-in `scroll=` attribute, since it is the owner-driven shipped design). Item 1 (`ebe670adf`): Advanced Options' category filter shows VTank's real names (Misc/Recharge/MeleeCombat/SpellCombat/Ranges/Navigation/Buffing/Crafting/Looting) instead of raw bitmask hex, derived from real single-category KB rows rather than a typed-in literal. Item 2 (`435ced86f`): VTank's click model — a `tBool` value flips in place, a `tEnum` value cycles to the next label (VtankDefaultSettingsDatabase.SettingEnumValues, parsed from the embedded .usd's own SettingsEnumInfo table), everything else selects into the edit field; Apply/Back buttons removed (retail has neither); panel height 476->450. Item 3 (`5318adbb3`): RouteRecallKind replaced with VTank's real 26-recall cmbRecallType table (metaf's NRecall) plus Marketplace kept as its own slash-command entry (27 total) — deviation: the old Lifestone slash-command member is dropped rather than duplicating the new spell-based LifestoneRecall's "Lifestone Recall" label; Primary/SecondaryPortal upgraded from runtime spell-name lookup to their real hardcoded ids (48/2647); AddRouteRecallCore now writes RecallSpellId+RecallSpellName onto the waypoint so a route-tab-added recall casts exactly like a metaf-imported one. Item 4 (`2d626971d`): main panel `resizable="true"` floored at the pre-round-D authored size (856x236 minw/minh), default enlarged ~15% to 984x271; all nine tab groups anchor="left top right bottom" and grow by the same 128x35 delta; Monsters/Meta's lists get full left/right/top/bottom stretch (nothing beside them); Items/Consumables/Buffs/Route's lists grow height-only, pinned away from a sibling list/button column; a right-pinned sibling list (Consumables' Excluded Scarab, Buffs' Blacklisted Buff Families) repositions +128 so it already sits flush-right at the enlarged default. Every new/changed pin shown to fail against a targeted mutation first (category-name projection reverted, ClickAdvancedOptionValue reduced to a bare select, SubmitRecall reduced to `return false`, enum-order swap, Monsters' anchor attribute removed) and restored green. MossTank suite 678 -> 682 (item 2) -> 713 (item 3) -> 715 (item 4); App markup/plugin filter 242 -> 243 (item 4's real-file re-layout test); full solution builds clean in Release throughout. Owner's connected visual gate against these five fixes remains the next step; #491 (buff-list wiring) and the pre-cutover-JSON RouteRecallKind-ordinal migration risk (documented at its own binding site in MossTankRouteProfileStore.cs) are the carried, accepted deviations from this round. Final narrow re-checks (architecture over C+D; resemblance over the final markup) dispatched; the gate build at `2b79ca325` launched for the owner's look. +- 2026-09-07 18:20 architecture re-check of rounds C+D at `2b79ca325`: round C fully closed; round D sound but NOT merge-ready on two items — D-1 pre-cutover JSON routes silently remap recalls (old ordinals 0..3 now name different kinds; the `Enum.IsDefined` guard never fires), D-2 the recall combo shows C# identifiers instead of the metaf-exact names; follow-ups D-3 (Advanced Options popup claims resizable but is not), D-4 (unknown `.af` recall name falls back to Primary Portal Recall), D-5 (values stale on popup open), D-6 (no resolved-geometry pin at min/enlarged sizes). Category names, the enum table and the click model verified data-driven and single-path; zero App-side production change in round D. Round E dispatched with all six. +- 2026-09-07 18:40 resemblance re-check at `2b79ca325`: all 13 earlier items CLOSED; owner asks 1–3 CLOSED, 4–5 PARTIAL. NOT ready for the gate on: (1) the enlarged default never reaches a machine with a stored layout — plugin windows register with `authoredGeometryRevision = 0`, so the saved 856×236 always wins (App-side fix dispatched on its own worktree: derive the revision from the authored geometry; #490 part 2); (2) the recall combo's full spell names overflow the 120 px box on most of the 27 entries — VTank's `cmbRecallType` uses terse captions ("Primary", "LS Sending", "PtOIA", "Fac. Hub"…): show those, keep the full name in the waypoint and `.af`; (3) Monsters' move-down arrow drifted ~67 px from move-up because the last (auto) column absorbed the new width — add a trailing filler column; (4) Advanced Options popup not resizable (round E D-3). Nits for the same round: single-column `items=` lists still draw the selection band while grids do not; Buffs' 136 px middle gap and the orphaned Coverage label; the Loot Editor's leftover Back button; Route's count column squeezing to 17 px when the list scrolls. Six of nine tabs keep dead margin at 984×271 (Route 366 px right, Profiles 244) — accepted for now under "polish later", noted for the owner. Round F queued after round E. +- 2026-09-07 19:10 App-side merges: `7b7e5e579` plugin `` draws no selection band by default (`selectionband="true"` opts in); `3d7065ebc` #490 part 2 fixed — plugin windows carry a geometry-hash `authoredGeometryRevision`, so a changed authored size replaces a stored size once (position kept, clamped); App window/markup/list filter 254/254. Round F (after round E on the panel worktree): merge the campaign branch in; VTank's terse `cmbRecallType` captions in the recall combo (full spell name stays in the waypoint and `.af`); Monsters trailing filler column so the arrows stay adjacent; Buffs middle gap + Coverage label; Loot Editor's leftover Back; Route count column min width. Then one combined final re-check, merge, gate build. +- 2026-09-07 18:15 OWNER LIVE LOOK at `2b79ca325`: (1) Advanced Options — the whole bottom block ("Editing X." notice + the MossTank Extras section with the Route extras) must go; (2) Advanced Options — the list scrollbar shows but clicks/drags work only sometimes (App-side hit-testing bug; dispatched); (3) `BuffProfile-Prots`/`-Banes` appear "duplicated" beside `BuffProfile_Prots`/`_Banes` and only the enum ones respond — the hyphen ones are `tString`, which VTank's Advanced Options never lists (KB 01, `db.cs:132-166`): hide `tString` settings from the list; (4) Items tab — remove the MossTank block right of the list (Weapon/Offhand readout, Refill Worn Mana slider, status, hint); (5) Meta "Create" should open a VTank-style rule editor — deferred by the owner, filed as #493 (with the loot-rule editor). Items 1, 3, 4 join round F on the panel worktree. +- 2026-09-07 round E (architecture re-check of round D) landed on the same worktree, six commits, base `2b79ca325`: `3178d9202` (D-4 — an .af "rcl" node whose spell name fails to resolve leaves RecallSpellId at 0 and Recall at its default ordinal, PrimaryPortalRecall; SubmitRecall's own Recall-based fallback used to cast spell 48 for it regardless, since MetafSerializer.ReadNavNode never touches the enum on that path — TickRecall now refuses and skips instead, naming the unresolved spell in the status notice; the dead fallback branch is removed). `a9d1d0a82` (D-1 — the pre-cutover RouteRecallKind was {Lifestone=0, Marketplace=1, PrimaryPortal=2, SecondaryPortal=3}, completely different kinds at the SAME ordinals as the round-D enum, and Enum.IsDefined never caught the silent remap since 0..3 are all still valid new members; LegacyWaypointDocument.Recall is now a bare int translated through an explicit MapLegacyRecall table that also derives RecallSpellId/RecallSpellName). `3fe304359` (D-2 — RouteRecallNames/SelectedRouteRecall showed Enum.GetNames/ToString()'s bare C# identifiers; both now route through RouteWaypoint.RecallDisplayName like RouteInsertModeNames/RouteModeNames already do, SelectRouteRecall parses the same display string back, and the combo widened 120->300 to fit VTank's own full metaf-sourced names, e.g. "Paradox-touched Olthoi Infested Area Recall" — VTank's real combo is also 120 wide only because ITS captions are hand-abbreviated). `64b62fea3` (D-3 — mosstank.xml's own top comment already claimed the Advanced Options popup got resizable+anchored treatment; the popup file itself still carried the dead resize="none", never actually resizable; now resizable="true" minw/minh floored at the pre-fix authored 392x450, lOptionList grows WIDTH ONLY since the value field/description/notice sit directly below with zero vertical slack and none of them anchor to shift down, lFilterList tracks the growing right edge in lockstep so the widening list never walks into it; AdvancedOptionDescription is declared anchor="left top right" for consistency but documented as a real no-op — UiLabel.OnDraw always overwrites Width/Height to the measured text extent every draw, so nothing a label's anchor computes is ever visible). `1a0b0e014` (D-5 — RefreshAdvancedOptions' own doc comment lists every mutation site that must call it, but ShowAdvancedOptions itself was missing one, so a setting changed via /vt opt set or an Options-tab checkbox while the popup was closed stayed stale until some in-popup interaction refreshed it; ShowAdvancedOptions now calls RefreshAdvancedOptions). `7badbee88` (D-6 — a new whole-tree resolved-geometry pin at the main panel's minw/minh floor (856x236) and one enlarged size (1100x320), adapting AssertNoSiblingOverlap/AssertWithinParent's rectangle-overlap algorithm from the authored XElement tree to the resolved UiElement tree for each of the nine tabs in turn; building the test surfaced and fixed two real test-methodology bugs along the way — reusing one built tree across tabs let an earlier tab's resize corrupt a later tab's anchor-capture baseline, and walking into an invisible tab group's never-anchored-for-this-size descendants misreported stale geometry as broken — plus one genuine test-harness false positive (UiLabel's font-dependent measured box, excluded from the size checks for the same reason D-3's own commit documents). Every new/changed test shown to fail first via a targeted revert-then-reapply (git checkout + saved patch) or a deliberate markup mutation (D-6's Monsters-anchor removal), each restored green afterward. MossTank suite 715 (pre-round-E baseline, matching round D's close) -> 716 (D-4) -> 720 (D-1) -> 721 (D-2) -> 721 (D-3, App-side only) -> 722 (D-5); App markup/plugin filter 243 -> 243 (D-2 widened menu, no new pin) -> 244 (D-3) -> 246 (D-6, two theory cases). Full solution builds clean in Release throughout. Carried, accepted deviations unchanged from round D: #491 (buff-list wiring) and (now closed by D-1) the former pre-cutover-JSON RouteRecallKind-ordinal migration risk. Scrollbar snap-back fixed App-side and merged (`b4fe18007`: the list re-centred on its selected row every frame). Round F dispatched (merges the campaign branch; the owner's second-look items 1/3/4 + VTank's terse recall captions at 120 wide + Monsters filler column + Buffs/Loot Editor/Route nits). After F: one combined final re-check, merge, gate build. diff --git a/docs/plugin-ui-markup.md b/docs/plugin-ui-markup.md index 93d51d8b..f541b14c 100644 --- a/docs/plugin-ui-markup.md +++ b/docs/plugin-ui-markup.md @@ -109,8 +109,8 @@ vanishing from the built tree. | `toggle` | Lamp-style checkbox | `x y w h text checked onclick color anchor` | | `slider` | Horizontal scalar | `x y w h value onchange min max style anchor` | | `field` | Single-line editable text | `x y w h text maxlength clearonsubmit onchange onsubmit color background anchor` | -| `menu` | Dropdown selector | `x y w h items selected onchange rows rowheight openupward scroll style anchor` | -| `list` | Scrollable row list (+ Slice B icon column, + Campaign VT slice 1 multi-column) | `x y w h selected onchange rowheight anchor` + either the single-column `items colors icons iconkind`, or one-to-many `` children (see "Columns" below) — never both | +| `menu` | Dropdown selector | `x y w h items selected onchange rows rowheight openupward style anchor` | +| `list` | Scrollable row list (+ Slice B icon column, + Campaign VT slice 1 multi-column) | `x y w h selected onchange rowheight selectionband anchor` + either the single-column `items colors icons iconkind`, or one-to-many `` children (see "Columns" below) — never both | `menu style` is `plain` (the default) or `retail`: retail's gold pushbutton art read as an out-of-place "big yellow button" next to a plugin's own dark @@ -218,7 +218,12 @@ No other markup or host wiring is needed to make a panel resizable: once `resizable="true"` sets the window's `Resizable`/`ResizeX`/`ResizeY`/ `MinWidth`/`MinHeight`, the SAME drag-resize, persistence (save/restore across sessions, clamped to `minw`/`minh`), and UI-lock behavior every other -retained window already has just applies. +retained window already has just applies. Changing a panel's authored `w`/ +`h`/`minw`/`minh`/`resizable` in a later plugin update resets every user's +stored SIZE to the new authored default exactly once (their saved position +is kept and re-clamped to the screen) — the host derives a stable revision +from that tuple automatically, so a plugin author never needs to bump one +by hand (#490 part 2). ```xml @@ -418,6 +423,11 @@ attributes: ``` +VVS lists draw no persistent row-selection fill, so `` matches that by +default in both the single-column and `` forms — a plugin that wants +a visible band anyway sets `selectionband="true"` (row selection itself, +including scroll-into-view, is unaffected either way). + This mirrors VTank's own Monsters tab (several boolean flag columns, a name column, and icon-button columns) — see `docs/research/vtank-kb/08-ui-views.md` §3's "Multi-column lists with typed @@ -609,27 +619,28 @@ now optional literal attributes that declare the range the BOUND `value`/ and `0+t*(1-0)` are both no-ops. A declared `min == max` falls back to a range of `1` rather than dividing by zero. -### `` +### `` is always a single scrolling column VVS's `HudCombo` popup is always exactly one scrolling column, at most 10 -rows visible before a scrollbar appears (`HudCombo.cs:35,102-146`). -acdream's `` instead wraps overflow into extra columns unless the -markup opts into `UiMenu.Scrollable` — previously only reachable from C# -(`ConfigOptionsPageController`, `VendorUiController`). `scroll="true"` wires -`Scrollable` plus the same track/thumb/up/down chrome sprites those two -controllers already apply, so a VTank `Choice` with many entries (the -27-option recall menu, a long named-profile list) keeps VVS's one-column -look instead of fanning out sideways: +rows visible before a scrollbar appears (`HudCombo.cs:35,102-146`). An +earlier revision of this slice gated that behind an opt-in `scroll="true"` +attribute; the owner's live-client report on 2026-09-07 ("scrollable +dropdowns... use the SAME scrollbar/window assets as we do in for example +chat or inventory window") made the scrolling, chrome-matching popup the +one and only shape a plugin `` draws — `MarkupDocument` now sets +`UiMenu.Scrollable = true` and applies `RetailScrollbarChrome` for every +`` unconditionally, so a VTank `Choice` with many entries (the +27-option recall menu, a long named-profile list) always keeps VVS's +one-column look instead of ever fanning out sideways: ```xml + rows="7"/> ``` -Omitting `scroll` (every `` written before this slice) keeps -`Scrollable` at its historical `false` default — the column-wrapping -behavior is unchanged. +There is no `scroll` attribute to set or omit — every ``, old and +new, gets the scrolling popup. ## The plugin shelf (Slice A) diff --git a/src/AcDream.App/UI/MarkupDocument.cs b/src/AcDream.App/UI/MarkupDocument.cs index 90d29eac..883d9080 100644 --- a/src/AcDream.App/UI/MarkupDocument.cs +++ b/src/AcDream.App/UI/MarkupDocument.cs @@ -640,6 +640,12 @@ public static class MarkupDocument binding, "list selected"), SelectionChanged = listChanged, + // Campaign VT slice 7 resemblance re-check: VVS lists draw + // no persistent row-selection fill by default (matches + // both single-column and mode now — see + // UiMarkupList.SelectionBandEnabled). A plugin that wants + // one back opts in with . + SelectionBandEnabled = B(el, "selectionband", false), }; if (listUsesColumns) diff --git a/src/AcDream.App/UI/RetailUiRuntime.cs b/src/AcDream.App/UI/RetailUiRuntime.cs index 02600df3..061a839d 100644 --- a/src/AcDream.App/UI/RetailUiRuntime.cs +++ b/src/AcDream.App/UI/RetailUiRuntime.cs @@ -4767,11 +4767,20 @@ public sealed class RetailUiRuntime : IDisposable // later registration/sidepanel failure then rolls the mounted // subtree back through FailMount instead of leaking it. _bindings.Plugins.CompleteMount(panel, Host.Root, element); + // #490 part 2: derive the authored-geometry revision from + // the panel's own authored extent instead of a hard-coded 0 + // — see RetailWindowLayoutPersistence's class doc and + // RetailWindowManager.ComputeAuthoredGeometryRevision's own + // doc for why a plugin window can't use the built-in + // windows' manual-literal scheme. + int authoredGeometryRevision = RetailWindowManager.ComputeAuthoredGeometryRevision( + element.Width, element.Height, element.MinWidth, element.MinHeight, element.Resizable); RetailWindowHandle handle = Host.WindowManager.Register( panel.WindowName, element, element, - visibility); + visibility, + authoredGeometryRevision: authoredGeometryRevision); _bindings.Plugins.CompleteWindowMount( panel, () => Host.WindowManager.Unregister(panel.WindowName)); diff --git a/src/AcDream.App/UI/RetailWindowLayoutPersistence.cs b/src/AcDream.App/UI/RetailWindowLayoutPersistence.cs index ca1bf655..ee2baf95 100644 --- a/src/AcDream.App/UI/RetailWindowLayoutPersistence.cs +++ b/src/AcDream.App/UI/RetailWindowLayoutPersistence.cs @@ -9,6 +9,31 @@ namespace AcDream.App.UI; /// per-resolution settings. It deliberately ignores the temporary pre-login /// default character key so startup layout cannot overwrite a real /// character's state. +/// +/// +/// Authored-geometry revision (#490 part 2). Every registered window +/// carries an authoredGeometryRevision (see +/// ); a restore +/// whose saved revision differs from the handle's current one replaces only +/// the saved WIDTH/HEIGHT with the current authored size +/// () — position, visibility, and +/// collapsed/maximized state are untouched, and the clamp in +/// still re-fits the kept position to the live screen. +/// Built-in retail-imported windows hand-pick that revision as a small +/// incrementing literal at their Register call site (chat windows: +/// authoredGeometryRevision = 1) — a deliberate author decision each +/// time their authored size changes. Plugin windows have no such call site +/// an author remembers to touch, so MountPlugins instead derives the +/// revision automatically from the authored geometry tuple itself via +/// +/// (width, height, min width, min height, resizable): unchanged authored +/// geometry hashes to the same revision (a user's own resize survives +/// restore), and ANY authored geometry change hashes to a different one +/// (the stored size resets to the new default exactly once). Because a hash +/// is not an ordered counter, the comparison is for INEQUALITY — see +/// 's own doc for why the original +/// "newer revision only" read was wrong for this case. +/// /// public sealed class RetailWindowLayoutPersistence : IDisposable { @@ -295,11 +320,26 @@ public sealed class RetailWindowLayoutPersistence : IDisposable handle.AuthoredGeometryRevision); } + /// + /// #490 part 2: compares revisions for INEQUALITY, not ordering. Built-in + /// retail-imported windows hand-pick a small incrementing literal + /// (0, 1, 2…) that only ever grows, so the original "migrate only if + /// saved < authored" read fine for them. Plugin windows instead derive + /// their revision from a hash of the authored geometry itself + /// () so + /// their author never has to remember to bump a literal — but a hash is + /// not a counter, and two different authored sizes can hash in either + /// order. "The authored size changed" therefore means "the value + /// differs", not "the value went up"; treating it as ordered silently + /// dropped every size-decreasing (by hash value, not by pixels) plugin + /// update, which is exactly how MossTank's 856x236 -> 984x271 bump got + /// stuck at the old size for every user with a stored layout. + /// private static UiWindowLayout MigrateAuthoredGeometry( UiWindowLayout saved, UiWindowLayout authored) { - if (saved.AuthoredGeometryRevision >= authored.AuthoredGeometryRevision) + if (saved.AuthoredGeometryRevision == authored.AuthoredGeometryRevision) return saved; return saved with diff --git a/src/AcDream.App/UI/RetailWindowManager.cs b/src/AcDream.App/UI/RetailWindowManager.cs index 3a364047..418e7f9d 100644 --- a/src/AcDream.App/UI/RetailWindowManager.cs +++ b/src/AcDream.App/UI/RetailWindowManager.cs @@ -111,6 +111,45 @@ public sealed class RetailWindowManager : IDisposable return handle; } + /// + /// Derives a stable authored-geometry revision from a window's own + /// authored extent (width, height, min width, min height, resizable), so + /// a plugin window's call can invalidate an + /// obsolete saved size across an authored-size change WITHOUT the plugin + /// author remembering to bump an explicit revision literal the way + /// built-in retail-imported windows do (#490 part 2 — MossTank shipped + /// 856x236 -> 984x271 and every stored layout stayed at 856x236 forever). + /// Deliberately NOT : that type reseeds its + /// internal state once per process specifically to defeat hash-flooding + /// attacks, so the SAME geometry would hash to a DIFFERENT value on + /// every relaunch — every login would look like a fresh authored-geometry + /// revision and reset every plugin window's saved size, every time. This + /// instead combines the exact IEEE-754 bit patterns with a fixed FNV-1a- + /// style multiplier, which is stable across processes, machines, and + /// .NET versions. + /// compares revisions for INEQUALITY, not ordering — a hash is not a + /// counter, so "authored size changed" means "the value differs", + /// whichever direction it moved. The sign bit is masked off the result: + /// clamps a negative authoredGeometryRevision + /// up to 0 (its "no explicit revision" sentinel), and a hash landing + /// there would be indistinguishable from an old, pre-hash saved layout + /// that never had a revision at all. + /// + public static int ComputeAuthoredGeometryRevision( + float width, float height, float minWidth, float minHeight, bool resizable) + { + unchecked + { + int hash = 17; + hash = (hash * 31) + BitConverter.SingleToInt32Bits(width); + hash = (hash * 31) + BitConverter.SingleToInt32Bits(height); + hash = (hash * 31) + BitConverter.SingleToInt32Bits(minWidth); + hash = (hash * 31) + BitConverter.SingleToInt32Bits(minHeight); + hash = (hash * 31) + (resizable ? 1 : 0); + return hash & 0x7FFFFFFF; + } + } + public bool TryGet(string name, out RetailWindowHandle handle) => _byName.TryGetValue(name, out handle!); diff --git a/src/AcDream.App/UI/UiMarkupList.cs b/src/AcDream.App/UI/UiMarkupList.cs index ddc39745..24ff58b2 100644 --- a/src/AcDream.App/UI/UiMarkupList.cs +++ b/src/AcDream.App/UI/UiMarkupList.cs @@ -73,6 +73,25 @@ public sealed class UiMarkupList : UiElement public Vector4 TextColor { get; set; } = new(0.91f, 0.87f, 0.76f, 1f); public Vector4 SelectedColor { get; set; } = new(0.28f, 0.23f, 0.08f, 0.95f); + /// + /// Campaign VT slice 7 resemblance re-check (2026-09-07): real VVS lists + /// (VTank's own HudList) draw no persistent row-selection fill at + /// all — before this fix the column-less items= mode drew + /// under the selected row while the + /// <column> mode did the same, so a plugin's Buffs lists + /// highlighted a row while the Monsters/Meta grids happened not to (or + /// vice versa, depending on which mode a given list used) — same-looking + /// widgets, inconsistent behavior. Default false now suppresses the fill + /// in BOTH 's legacy branch and + /// , matching VVS. <list + /// selectionband="true"> (parsed in 's + /// case "list") opts a single list back into a visible band for + /// plugins that want one. This gates ONLY the fill — , + /// , and the selected-row scroll-into-view + /// logic in / are unchanged. + /// + public bool SelectionBandEnabled { get; set; } + /// /// Owner live-client report 2026-09-07 ("For scrollable dropdown or the /// meta window we use the same assets as we do in for example chat or @@ -97,6 +116,28 @@ public sealed class UiMarkupList : UiElement private int _topRow; private IReadOnlyList? _columns; + /// + /// Owner live-client report 2026-09-07 ("Scrolling in advanced options + /// does not work... or it works sometimes"): the last + /// value the "keep selection visible" + /// clamp below (/) actually + /// reacted to. A real bound list (e.g. MossTank's Advanced Options, + /// selected="{SelectedAdvancedOptionIndex}") keeps a STABLE + /// selected index while the user scrolls elsewhere with the scrollbar — + /// re-running the reveal clamp on EVERY frame regardless of whether + /// selection actually changed snapped straight back + /// to the (unchanged) selected row on the very next draw, undoing the + /// scrollbar arrow/track/thumb interaction the same frame it happened. + /// Gating the clamp on an observed CHANGE in the selected index — the + /// only time retail HudList-style lists scroll to reveal a row — lets a + /// stable selection coexist with the user scrolling away from it via the + /// bar, while still auto-revealing a genuinely NEW selection exactly + /// once. Sentinel so the very first draw with + /// ANY selected index (including the valid -1 "nothing selected") still + /// runs the clamp once. + /// + private int _lastRevealedSelected = int.MinValue; + /// /// Pixel-based scroll projection used ONLY to feed /// 's geometry math (thumb @@ -145,12 +186,16 @@ public sealed class UiMarkupList : UiElement : 0f; int visibleRows = VisibleRows; int selected = SelectedIndexSource(); - if (selected >= 0 && selected < items.Count) + if (selected != _lastRevealedSelected) { - if (selected < _topRow) - _topRow = selected; - else if (selected >= _topRow + visibleRows) - _topRow = selected - visibleRows + 1; + _lastRevealedSelected = selected; + if (selected >= 0 && selected < items.Count) + { + if (selected < _topRow) + _topRow = selected; + else if (selected >= _topRow + visibleRows) + _topRow = selected - visibleRows + 1; + } } ClampTop(items.Count, visibleRows); @@ -163,7 +208,7 @@ public sealed class UiMarkupList : UiElement for (int index = _topRow; index < end; index++) { float y = (index - _topRow) * RowHeight; - if (index == selected) + if (index == selected && SelectionBandEnabled) context.DrawFill(1f, y + 1f, contentWidth - 2f, RowHeight - 1f, SelectedColor); if (iconIds is not null && index < iconIds.Count && IconResolve is { } resolve) @@ -364,12 +409,16 @@ public sealed class UiMarkupList : UiElement ComputeColumnLayout(columns, contentWidth); int selected = SelectedIndexSource(); - if (selected >= 0 && selected < rowCount) + if (selected != _lastRevealedSelected) { - if (selected < _topRow) - _topRow = selected; - else if (selected >= _topRow + visibleRows) - _topRow = selected - visibleRows + 1; + _lastRevealedSelected = selected; + if (selected >= 0 && selected < rowCount) + { + if (selected < _topRow) + _topRow = selected; + else if (selected >= _topRow + visibleRows) + _topRow = selected - visibleRows + 1; + } } ClampTop(rowCount, visibleRows); @@ -379,14 +428,17 @@ public sealed class UiMarkupList : UiElement // Fix round B item 10 (owner/oracle: VVS's own HudList grids have NO // row-selection highlight at all — Monsters/Meta/Route/Items and // every other grid). SelectedIndexSource above still - // drives scroll-into-view; the SelectedColor band draw the legacy - // single-column path (below, unaffected) uses is deliberately - // skipped here. Every per-cell onclick/onchange callback is - // unchanged — only the visual band is gone. + // drives scroll-into-view; the SelectedColor band draw uses the same + // SelectionBandEnabled gate as the legacy single-column path below + // and defaults to false, so plugin lists keep no band unless a + // caller opts in. Every per-cell onclick/onchange callback is + // unchanged — only the visual band is conditional. int end = Math.Min(rowCount, _topRow + visibleRows); for (int index = _topRow; index < end; index++) { float y = (index - _topRow) * RowHeight; + if (index == selected && SelectionBandEnabled) + context.DrawFill(1f, y + 1f, contentWidth - 2f, RowHeight - 1f, SelectedColor); for (int c = 0; c < columns.Count; c++) { diff --git a/tests/AcDream.App.Tests/UI/MarkupListColumnsTests.cs b/tests/AcDream.App.Tests/UI/MarkupListColumnsTests.cs index 69db1f5b..d44c9405 100644 --- a/tests/AcDream.App.Tests/UI/MarkupListColumnsTests.cs +++ b/tests/AcDream.App.Tests/UI/MarkupListColumnsTests.cs @@ -1170,6 +1170,18 @@ public sealed class MarkupListColumnsTests /// entirely — i.e. against the widget's own unmodified /// OnDraw/legacy branch, not a snapshot captured from a different /// commit. Every vertex float must match exactly. + /// + /// + /// Deliberately updated for the slice-7 resemblance re-check's + /// fix: SelectedIndex + /// = 1 (a real selected row) previously meant this byte-for-byte + /// comparison implicitly included the + /// fill quad on BOTH sides. Neither the markup XML nor the hand-built + /// widget below sets selectionband/, + /// so that fill is now absent from both — the explicit assertion at the + /// end locks in that the new no-band default applies here too, rather + /// than leaving it to accidentally fall out of the byte-for-byte diff. + /// /// [Fact] public void ColumnLessList_ProducesTheIdenticalDrawRecordToTheHandBuiltWidget() @@ -1225,6 +1237,119 @@ public sealed class MarkupListColumnsTests } Assert.Null(viaMarkup.Columns); + + // Neither side opted into selectionband="true" — row 1 IS selected + // (LegacyBinding.SelectedIndex = 1) but the new default draws no + // SelectedColor fill for it, on either the markup or the hand-built + // path. + Assert.False(viaMarkup.SelectionBandEnabled); + Assert.False(handBuilt.SelectionBandEnabled); + Assert.DoesNotContain(markupVerts, s => s.Texture == 0u + && Chunk(s.Verts).Any(v => ColorMatches(v, viaMarkup.SelectedColor))); + } + + // ── Slice 7 resemblance re-check: selectionband default + opt-in ──────── + + private sealed class SelectionBandBinding + { + public IReadOnlyList Choices => new[] { "First", "Second" }; + public int Selected { get; set; } = 1; + public Action SelectIndex => _ => { }; + } + + /// Any untextured (fill) quad in whose vertex + /// color matches — used to detect the selection-band + /// fill regardless of its exact geometry. + private static bool HasFillOfColor( + IEnumerable<(uint Texture, IReadOnlyList Verts)> segs, Vector4 color) + => segs.Where(s => s.Texture == 0u) + .SelectMany(s => Chunk(s.Verts)) + .Any(v => ColorMatches(v, color)); + + /// + /// Slice-7 resemblance re-check finding: VVS lists (VTank's real + /// HudList) draw no persistent row-selection fill, so a + /// column-less <list items="..."> must match that by + /// default — defaults + /// false. selectionband="true" opts a single list back into the + /// visible band. + /// + [Theory] + [InlineData(false)] + [InlineData(true)] + public void SingleColumnList_SelectionBand_DefaultsOffAndAttributeOptsIn(bool enabled) + { + var binding = new SelectionBandBinding(); + string attr = enabled ? " selectionband=\"true\"" : ""; + string xml = + "" + + "" + + ""; + + var panel = MarkupDocument.Build(xml, binding, Sprite); + var list = Assert.IsType(panel.Children[0]); + Assert.Equal(enabled, list.SelectionBandEnabled); + + var (renderer, ctx) = MakeContext(200f, 200f); + list.DrawSelfAndChildren(ctx); + + Assert.Equal(enabled, HasFillOfColor(renderer.DebugSpriteSegmentVerts, list.SelectedColor)); + } + + /// Same fix, exercised through <column> mode + /// () — the gate applies to both + /// draw branches of the shared widget, not just the legacy one. + [Theory] + [InlineData(false)] + [InlineData(true)] + public void ColumnModeList_SelectionBand_DefaultsOffAndAttributeOptsIn(bool enabled) + { + var binding = new SelectionBandBinding(); + string attr = enabled ? " selectionband=\"true\"" : ""; + string xml = + "" + + $"" + + " " + + ""; + + var panel = MarkupDocument.Build(xml, binding, Sprite); + var list = Assert.IsType(panel.Children[0]); + Assert.Equal(enabled, list.SelectionBandEnabled); + + var (renderer, ctx) = MakeContext(200f, 200f); + list.DrawSelfAndChildren(ctx); + + Assert.Equal(enabled, HasFillOfColor(renderer.DebugSpriteSegmentVerts, list.SelectedColor)); + } + + /// Parse-only pin: selectionband follows the same silent + /// literal-bool convention as openupward/clearonsubmit + /// ('s private B helper) — omitted + /// defaults false, and "true" sets the property, with no draw + /// involved at all. + [Fact] + public void ListSelectionBandAttribute_ParsesToProperty() + { + var binding = new SelectionBandBinding(); + const string xmlDefault = + "" + + "" + + ""; + const string xmlEnabled = + "" + + "" + + ""; + + var defaultList = Assert.IsType( + MarkupDocument.Build(xmlDefault, binding, Sprite).Children[0]); + var enabledList = Assert.IsType( + MarkupDocument.Build(xmlEnabled, binding, Sprite).Children[0]); + + Assert.False(defaultList.SelectionBandEnabled); + Assert.True(enabledList.SelectionBandEnabled); } // ── Fix round: end-to-end MarkupDocument builds (fix item 10) ─────────── diff --git a/tests/AcDream.App.Tests/UI/RetailWindowLayoutPersistenceTests.cs b/tests/AcDream.App.Tests/UI/RetailWindowLayoutPersistenceTests.cs index 67c0abe1..d82cf0ab 100644 --- a/tests/AcDream.App.Tests/UI/RetailWindowLayoutPersistenceTests.cs +++ b/tests/AcDream.App.Tests/UI/RetailWindowLayoutPersistenceTests.cs @@ -499,6 +499,143 @@ public sealed class RetailWindowLayoutPersistenceTests : IDisposable Assert.Equal((200f, 150f), (panel.Width, panel.Height)); } + // ── #490 part 2: a plugin's authored panel size changes across a plugin + // update (MossTank went 856x236 -> 984x271) and every user's stored + // layout must adopt the new authored size rather than keep the old one + // forever. MountPlugins derives authoredGeometryRevision from the + // authored geometry tuple via RetailWindowManager.ComputeAuthoredGeometryRevision + // (#490 part 2) instead of a manual literal, so these tests register the + // way MountPlugins does: pass the SAME computed hash to RegisterWindow. ── + + [Fact] + public void PluginMarkupPanel_AuthoredSizeChanged_ReplacesStoredSizeButKeepsPosition() + { + const string oldXml = + ""; + const string newXml = + ""; + var store = new SettingsStore(PathName); + + // "Old session": the plugin's previous authored size registers and + // the user drags the window. + var oldPanel = MarkupDocument.Build(oldXml, new object(), _ => (1u, 32, 32)); + var oldRoot = new UiRoot { Width = 1280, Height = 720 }; + oldRoot.AddChild(oldPanel); + RetailWindowHandle oldHandle = oldRoot.RegisterWindow( + "moss-tank", + oldPanel, + authoredGeometryRevision: RetailWindowManager.ComputeAuthoredGeometryRevision( + oldPanel.Width, oldPanel.Height, oldPanel.MinWidth, oldPanel.MinHeight, oldPanel.Resizable)); + using (var oldPersistence = new RetailWindowLayoutPersistence( + oldRoot.WindowManager, store, () => "Alice", () => (1280, 720))) + { + oldHandle.MoveTo(120f, 90f); + } + + // "New session": the plugin ships its new authored 984x271 size. + var newPanel = MarkupDocument.Build(newXml, new object(), _ => (1u, 32, 32)); + var newRoot = new UiRoot { Width = 1280, Height = 720 }; + newRoot.AddChild(newPanel); + newRoot.RegisterWindow( + "moss-tank", + newPanel, + authoredGeometryRevision: RetailWindowManager.ComputeAuthoredGeometryRevision( + newPanel.Width, newPanel.Height, newPanel.MinWidth, newPanel.MinHeight, newPanel.Resizable)); + using var persistence = new RetailWindowLayoutPersistence( + newRoot.WindowManager, store, () => "Alice", () => (1280, 720)); + + persistence.RestoreAll(); + + Assert.Equal((984f, 271f), (newPanel.Width, newPanel.Height)); + Assert.Equal((120f, 90f), (newPanel.Left, newPanel.Top)); + } + + [Fact] + public void PluginMarkupPanel_AuthoredSizeUnchanged_KeepsUserResizedSize() + { + const string xml = + ""; + var store = new SettingsStore(PathName); + + var panel = MarkupDocument.Build(xml, new object(), _ => (1u, 32, 32)); + var root = new UiRoot { Width = 1280, Height = 720 }; + root.AddChild(panel); + RetailWindowHandle handle = root.RegisterWindow( + "moss-tank", + panel, + authoredGeometryRevision: RetailWindowManager.ComputeAuthoredGeometryRevision( + panel.Width, panel.Height, panel.MinWidth, panel.MinHeight, panel.Resizable)); + using (var persistence = new RetailWindowLayoutPersistence( + root.WindowManager, store, () => "Alice", () => (1280, 720))) + { + handle.MoveTo(50f, 50f); + handle.ResizeTo(900f, 300f); + } + + // Fresh session: the SAME authored geometry (same markup) registers + // again — the derived revision is unchanged, so the user's own + // resize must survive. + var freshPanel = MarkupDocument.Build(xml, new object(), _ => (1u, 32, 32)); + var freshRoot = new UiRoot { Width = 1280, Height = 720 }; + freshRoot.AddChild(freshPanel); + freshRoot.RegisterWindow( + "moss-tank", + freshPanel, + authoredGeometryRevision: RetailWindowManager.ComputeAuthoredGeometryRevision( + freshPanel.Width, freshPanel.Height, freshPanel.MinWidth, freshPanel.MinHeight, freshPanel.Resizable)); + using var freshPersistence = new RetailWindowLayoutPersistence( + freshRoot.WindowManager, store, () => "Alice", () => (1280, 720)); + + freshPersistence.RestoreAll(); + + Assert.Equal((900f, 300f), (freshPanel.Width, freshPanel.Height)); + } + + [Fact] + public void PluginMarkupPanel_AuthoredSizeChanged_ClampsPositionToNewScreenBounds() + { + // The authored-size migration keeps the saved POSITION, but that + // position still runs through the ordinary restore clamp (Apply's + // MoveTo clamp) — a larger authored size can push a near-edge saved + // X/Y off the live screen. + const string oldXml = + ""; + const string newXml = + ""; + var store = new SettingsStore(PathName); + + var oldPanel = MarkupDocument.Build(oldXml, new object(), _ => (1u, 32, 32)); + var oldRoot = new UiRoot { Width = 800, Height = 600 }; + oldRoot.AddChild(oldPanel); + RetailWindowHandle oldHandle = oldRoot.RegisterWindow( + "moss-tank", + oldPanel, + authoredGeometryRevision: RetailWindowManager.ComputeAuthoredGeometryRevision( + oldPanel.Width, oldPanel.Height, oldPanel.MinWidth, oldPanel.MinHeight, oldPanel.Resizable)); + using (var oldPersistence = new RetailWindowLayoutPersistence( + oldRoot.WindowManager, store, () => "Alice", () => (800, 600))) + { + oldHandle.MoveTo(590f, 490f); // fits the OLD 200x100 size exactly + } + + var newPanel = MarkupDocument.Build(newXml, new object(), _ => (1u, 32, 32)); + var newRoot = new UiRoot { Width = 800, Height = 600 }; + newRoot.AddChild(newPanel); + newRoot.RegisterWindow( + "moss-tank", + newPanel, + authoredGeometryRevision: RetailWindowManager.ComputeAuthoredGeometryRevision( + newPanel.Width, newPanel.Height, newPanel.MinWidth, newPanel.MinHeight, newPanel.Resizable)); + using var persistence = new RetailWindowLayoutPersistence( + newRoot.WindowManager, store, () => "Alice", () => (800, 600)); + + persistence.RestoreAll(); + + Assert.Equal((220f, 110f), (newPanel.Width, newPanel.Height)); + // maxX = 800 - 220 = 580 (590 clamps down); maxY = 600 - 110 = 490 (already in bounds). + Assert.Equal((580f, 490f), (newPanel.Left, newPanel.Top)); + } + private static RetailWindowHandle Mount( UiRoot root, string name, diff --git a/tests/AcDream.App.Tests/UI/RetailWindowManagerTests.cs b/tests/AcDream.App.Tests/UI/RetailWindowManagerTests.cs index 9a245023..8172897d 100644 --- a/tests/AcDream.App.Tests/UI/RetailWindowManagerTests.cs +++ b/tests/AcDream.App.Tests/UI/RetailWindowManagerTests.cs @@ -291,6 +291,48 @@ public sealed class RetailWindowManagerTests Assert.Equal(0, resized); } + // ── #490 part 2: plugin windows derive their authored-geometry revision + // from the authored geometry itself, so a plugin author who ships a new + // panel size does not also have to remember to bump an explicit revision + // literal the way built-in retail-imported windows do. ──────────────── + + [Fact] + public void ComputeAuthoredGeometryRevision_SameGeometry_IsStable() + { + int a = RetailWindowManager.ComputeAuthoredGeometryRevision(856f, 236f, 400f, 150f, true); + int b = RetailWindowManager.ComputeAuthoredGeometryRevision(856f, 236f, 400f, 150f, true); + + Assert.Equal(a, b); + } + + [Theory] + [InlineData(984f, 236f, 400f, 150f, true)] // width changed (the #490 MossTank case) + [InlineData(856f, 271f, 400f, 150f, true)] // height changed + [InlineData(856f, 236f, 420f, 150f, true)] // minw changed + [InlineData(856f, 236f, 400f, 160f, true)] // minh changed + [InlineData(856f, 236f, 400f, 150f, false)] // resizable changed + public void ComputeAuthoredGeometryRevision_AnyFieldDiffers_ChangesTheValue( + float width, float height, float minWidth, float minHeight, bool resizable) + { + int baseline = RetailWindowManager.ComputeAuthoredGeometryRevision(856f, 236f, 400f, 150f, true); + int changed = RetailWindowManager.ComputeAuthoredGeometryRevision( + width, height, minWidth, minHeight, resizable); + + Assert.NotEqual(baseline, changed); + } + + [Fact] + public void ComputeAuthoredGeometryRevision_IsNeverNegative() + { + // Register clamps a negative authoredGeometryRevision up to 0 (its + // "no explicit revision" sentinel) — a hash landing there would be + // indistinguishable from an old, pre-hash saved layout that never + // had a revision at all, so the function must never produce one. + Assert.True(RetailWindowManager.ComputeAuthoredGeometryRevision(856f, 236f, 400f, 150f, true) >= 0); + Assert.True(RetailWindowManager.ComputeAuthoredGeometryRevision(0f, 0f, 0f, 0f, false) >= 0); + Assert.True(RetailWindowManager.ComputeAuthoredGeometryRevision(-1f, -1f, -1f, -1f, true) >= 0); + } + private sealed class RecordingController : IRetainedPanelController { public int ShownCount { get; private set; } diff --git a/tests/AcDream.App.Tests/UI/UiMarkupListScrollSelectionPersistenceTests.cs b/tests/AcDream.App.Tests/UI/UiMarkupListScrollSelectionPersistenceTests.cs new file mode 100644 index 00000000..e23f2be0 --- /dev/null +++ b/tests/AcDream.App.Tests/UI/UiMarkupListScrollSelectionPersistenceTests.cs @@ -0,0 +1,245 @@ +using System.Linq; +using System.Numerics; +using AcDream.App.Rendering; +using AcDream.App.Rendering.Gpu; +using AcDream.App.Tests.Rendering.Gpu; +using AcDream.App.UI; +using Xunit; + +namespace AcDream.App.Tests.UI; + +/// +/// Owner live-client report 2026-09-07 ("Scrolling in advanced options does +/// not work. Scrollbar shows, but can't click or drag slidebar, or it works +/// sometimes"): root cause is in itself, not the +/// coordinate frame between the popup/group/panel nesting. +/// +/// +/// / +/// run a "keep the selected row visible" clamp UNCONDITIONALLY on every +/// single draw call (every frame), not only when the selection actually +/// changes. A real plugin list (e.g. MossTank's Advanced Options, +/// selected="{SelectedAdvancedOptionIndex}") keeps a STABLE selected +/// index while the user scrolls elsewhere with the scrollbar — but because +/// that index does not change, the very next frame's OnDraw sees the +/// (unchanged) selected row now outside the just-scrolled view and snaps +/// _topRow straight back to reveal it, undoing the arrow click or +/// thumb drag before the user ever sees the new scroll position. This +/// reproduces exactly the "sometimes works" symptom: a scroll that keeps the +/// stable selected row in view survives (looks like it "worked"); one that +/// moves the selected row out of view gets silently reverted on the next +/// frame. +/// +/// +/// +/// Every test here builds a REAL nested Panel > Group > List tree (like +/// MossTank's Advanced Options group living inside its panel) at non-zero +/// offsets at every level, and drives it exclusively through +/// 's public mouse entry points — the same path +/// GameWindow uses — to also rule out a coordinate-frame bug in the +/// hit-test/dispatch chain (candidate 1 in the investigation). A stray +/// coordinate-frame bug would make the FIRST click (the arrow / thumb press) +/// silently miss the scrollbar; these tests assert that press succeeds AND +/// that the resulting scroll position survives a subsequent redraw (the +/// real regression). +/// +/// +public sealed class UiMarkupListScrollSelectionPersistenceTests +{ + private sealed class TestElement : UiElement { } + + private const float ListWidth = 100f; + private const float ListHeight = 100f; // RowHeight 10 => 10 visible rows + private const float RowHeight = 10f; + private const int RowCount = 40; + + // Non-zero nesting offsets at every level (panel > group > list), mirroring + // MossTank's own panel(28,42) > group(8,42) > list(4,24) structure. + private const float PanelLeft = 50f, PanelTop = 60f; + private const float GroupLeft = 10f, GroupTop = 20f; + private const float ListLeft = 5f, ListTop = 15f; + + private static (uint tex, int w, int h) Resolve(uint id) => (id, 16, 16); + + private sealed class Harness + { + public readonly UiRoot Root = new() { Width = 1000f, Height = 800f }; + public readonly TestElement Panel; + public readonly TestElement Group; + public readonly UiMarkupList List; + + public Harness(UiMarkupList list) + { + List = list; + Panel = new TestElement + { + Left = PanelLeft, Top = PanelTop, Width = 900f, Height = 700f, + Draggable = true, + }; + Group = new TestElement + { + Left = GroupLeft, Top = GroupTop, Width = 800f, Height = 600f, + }; + List.Left = ListLeft; List.Top = ListTop; + Group.AddChild(List); + Panel.AddChild(Group); + Root.AddChild(Panel); + } + + /// Absolute root-space screen coords for a point expressed in + /// the list's own local space — the same translation + /// performs by walking every + /// ancestor's Left/Top. + public (int x, int y) ToScreen(float localX, float localY) => + ((int)(PanelLeft + GroupLeft + ListLeft + localX), + (int)(PanelTop + GroupTop + ListTop + localY)); + + public void Click(float localX, float localY) + { + var (x, y) = ToScreen(localX, localY); + Root.OnMouseDown(UiMouseButton.Left, x, y); + Root.OnMouseUp(UiMouseButton.Left, x, y); + } + + public void PressMoveRelease(float downLocalX, float downLocalY, float moveLocalX, float moveLocalY) + { + var (dx, dy) = ToScreen(downLocalX, downLocalY); + Root.OnMouseDown(UiMouseButton.Left, dx, dy); + var (mx, my) = ToScreen(moveLocalX, moveLocalY); + Root.OnMouseMove(mx, my); + Root.OnMouseUp(UiMouseButton.Left, mx, my); + } + } + + // ── Single-column mode ─────────────────────────────────────────────── + + [Fact] + public void SingleColumn_DownArrowClick_TopRowSurvivesTheNextDraw() + { + var list = new UiMarkupList + { + Width = ListWidth, Height = ListHeight, RowHeight = RowHeight, + SpriteResolve = Resolve, + // A REALISTIC stable selection (unlike the -1 the existing + // scrollbar tests use) — the user has row 0 selected and is not + // touching selection while scrolling elsewhere via the bar. + SelectedIndexSource = () => 0, + ItemsSource = () => Enumerable.Range(0, RowCount).Select(i => $"row{i}").ToArray(), + }; + var h = new Harness(list); + h.Root.DrawSelfAndChildren(NullCtx()); + + // Down-arrow occupies the bottom 16px of the reserved scrollbar + // column: x in [84,100), y in [84,100). + h.Click(localX: 90, localY: 90); + + // Simulate the NEXT render frame — this is where the unconditional + // reveal-selected clamp (pre-fix) reverts the scroll. + h.Root.DrawSelfAndChildren(NullCtx()); + + int? selected = null; + list.SelectionChanged = row => selected = row; + // Click the first visible row (local y in row 0's band). + h.Click(localX: 10, localY: 2); + + Assert.Equal(1, selected); + } + + [Fact] + public void SingleColumn_ThumbDrag_TopRowAdvancesThreeRows_AndSurvivesTheNextDraw() + { + var list = new UiMarkupList + { + Width = ListWidth, Height = ListHeight, RowHeight = RowHeight, + SpriteResolve = Resolve, + SelectedIndexSource = () => 0, + ItemsSource = () => Enumerable.Range(0, RowCount).Select(i => $"row{i}").ToArray(), + }; + var h = new Harness(list); + h.Root.DrawSelfAndChildren(NullCtx()); + + // Track: y in [16,84) (Height 100 - 16 up - 16 down). Thumb starts at + // the very top (topRow=0): height = max(8, 68*10/40=17) = 17, so + // y in [16,33). Press at the thumb's own top edge (y=16, zero drag + // offset) and drag down 5px — by the exact geometry in this file's + // header comment, this lands on topRow=3. + h.PressMoveRelease(downLocalX: 90, downLocalY: 16, moveLocalX: 90, moveLocalY: 21); + + h.Root.DrawSelfAndChildren(NullCtx()); + + int? selected = null; + list.SelectionChanged = row => selected = row; + h.Click(localX: 10, localY: 2); + + Assert.Equal(3, selected); + } + + // ── Multi-column mode ──────────────────────────────────────────────── + + [Fact] + public void Columns_DownArrowClick_TopRowSurvivesTheNextDraw() + { + var list = new UiMarkupList + { + Width = ListWidth, Height = ListHeight, RowHeight = RowHeight, + SpriteResolve = Resolve, + SelectedIndexSource = () => 0, + Columns = new[] + { + UiMarkupListColumn.Text( + ListWidth, () => Enumerable.Range(0, RowCount).Select(i => $"row{i}").ToArray(), null), + }, + }; + var h = new Harness(list); + h.Root.DrawSelfAndChildren(NullCtx()); + + h.Click(localX: 90, localY: 90); + h.Root.DrawSelfAndChildren(NullCtx()); + + int? selected = null; + list.SelectionChanged = row => selected = row; + h.Click(localX: 10, localY: 2); + + Assert.Equal(1, selected); + } + + [Fact] + public void Columns_ThumbDrag_TopRowAdvancesThreeRows_AndSurvivesTheNextDraw() + { + var list = new UiMarkupList + { + Width = ListWidth, Height = ListHeight, RowHeight = RowHeight, + SpriteResolve = Resolve, + SelectedIndexSource = () => 0, + Columns = new[] + { + UiMarkupListColumn.Text( + ListWidth, () => Enumerable.Range(0, RowCount).Select(i => $"row{i}").ToArray(), null), + }, + }; + var h = new Harness(list); + h.Root.DrawSelfAndChildren(NullCtx()); + + h.PressMoveRelease(downLocalX: 90, downLocalY: 16, moveLocalX: 90, moveLocalY: 21); + h.Root.DrawSelfAndChildren(NullCtx()); + + int? selected = null; + list.SelectionChanged = row => selected = row; + h.Click(localX: 10, localY: 2); + + Assert.Equal(3, selected); + } + + private static UiRenderContext NullCtx() + { + var device = new RecordingGpuDevice(); + var renderer = new TextRenderer(device, new NullGpuFrameSource(), "unused"); + renderer.Begin(new Vector2(1000f, 800f)); + return new UiRenderContext(renderer, new Vector2(1000f, 800f)); + } + + private sealed class NullGpuFrameSource : ICurrentGpuFrameSource + { + public IGpuFrame? CurrentFrame => null; + } +}