docs: night-round review — live verification found the AD-108 icon-resolve mechanism is actually broken

Live verification against the connected client (part of the F1-F15
gate) found the Map tab's player/house icons never mount:
"[D.2b] Map tab: icon 0x100001ED did not resolve" / "...0x100001EE did
not resolve". AD-108 (filed earlier this session for F9) had described
the standalone re-import mechanism as working; it does not.

A throwaway diagnostic (not committed) confirmed the root cause:
LayoutImporter.ImportInfos(dats, hostLayoutId, elementId)'s FindDesc
walks the LayoutDesc's raw top-level Elements table (one entry) and
recurses through ElementDesc.Children with no tab-page/state resolution
— calling it directly with these icon ids returns null. Resolving the
panel's own slot first (what MountMapHousePanel actually does) and
searching THAT tree finds m_pMap with both icon children present, so
the icons are real, just unreachable via a cold standalone import.

This is pre-existing (predates this session, confirmed via git log)
and unrelated to any F1-F15 fix — it means F1's byte-decoded
PlaceMarkerOnMap formula could not be visually confirmed against the
running client this round; it remains verified only at the unit-test/
golden-pixel level. Filed a follow-up task for the fix.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-17 05:19:12 +02:00
parent 0b0c7aa485
commit 38c580ff6d

View file

@ -107,7 +107,7 @@ readiness/requeue adaptation. See
| # | Divergence | Where (file:line) | Why it is safe / justified | Risk if assumption breaks | Retail oracle |
|---|---|---|---|---|---|
| AD-108 | **Filed 2026-08-17 at the night-round review fix round (F9).** `MapPageController.ResolveSwallowedIcon` re-imports the Map tab's player-location and house-location icons (`0x100001ED`/`0x100001EE`) STANDALONE via the panel's template resolver rather than finding them as ordinary descendants of the built page tree. Live-DAT-confirmed structural cause (`MapHousePanelSlotProbeTests`' follow-up dump): `m_pMap` (`0x100001EC`) is itself authored as a Type-1 `UIElement_Button` — the GM click-to-teleport feature `gmMapUI::ListenToElementMessage @0x004a2350` idMessage `0x1c` reads — and the two icons are authored as ITS OWN nested dat children, not siblings. `UiButton.ConsumesDatChildren` swallows a button's dat children as skin/label parts during the normal import walk, so they never appear anywhere `UiElement.FindDescendant` can reach against the built page root. | `src/AcDream.App/UI/Layout/MapPageController.cs:144-145` (the two `ResolveSwallowedIcon` call sites in `Bind`); `:178-191` (`ResolveSwallowedIcon`'s own body) | Reuses the EXACT re-import pattern this same class already uses for the 53 town-hotspot markers (`BuildTownMarkers`'s own `templateResolver` call) — not a new mechanism, the established one applied to two more elements. The icons' authored local position from that standalone import is irrelevant since `PlaceMarker` overwrites `Left`/`Top` on every `Refresh` anyway, so a "wrong" starting position from the standalone re-import has no observable effect. | If a future DAT revision moves `m_pMap` off Type-1 `UIElement_Button` (or `UiButton.ConsumesDatChildren`'s swallow behavior changes), the icons would silently fail to resolve — `ResolveSwallowedIcon` already logs a `[D.2b]` warning and returns null rather than throwing, so the failure mode is "no player/house marker ever shows" (a visible regression), not a crash | `gmMapUI::PostInit @0x004a1c70` (child resolution); `gmMapUI::ListenToElementMessage @0x004a2350` idMessage `0x1c` (confirms `m_pMap` IS a button, not a passive container) |
| AD-108 | **Filed 2026-08-17 at the night-round review fix round (F9); CORRECTED same day during that round's own live-verification step.** `MapPageController.ResolveSwallowedIcon` re-imports the Map tab's player-location and house-location icons (`0x100001ED`/`0x100001EE`) STANDALONE via the panel's template resolver rather than finding them as ordinary descendants of the built page tree. Live-DAT-confirmed structural cause (`MapHousePanelSlotProbeTests`' follow-up dump): `m_pMap` (`0x100001EC`) is itself authored as a Type-1 `UIElement_Button` — the GM click-to-teleport feature `gmMapUI::ListenToElementMessage @0x004a2350` idMessage `0x1c` reads — and the two icons are authored as ITS OWN nested dat children, not siblings. `UiButton.ConsumesDatChildren` swallows a button's dat children as skin/label parts during the normal import walk, so they never appear anywhere `UiElement.FindDescendant` can reach against the built page root. **CORRECTION: this row originally described the standalone re-import as WORKING (reusing the town-hotspot pattern). Live verification the same session found the connected client logging `[D.2b] Map tab: icon 0x100001ED did not resolve` / `...0x100001EE did not resolve` for BOTH icons — the standalone re-import does not actually find them.** A throwaway diagnostic (not committed) confirmed why: `LayoutImporter.ImportInfos(dats, hostLayoutId, elementId)`'s `FindDesc` walks the LayoutDesc's raw top-level `Elements` table (exactly ONE entry for host layout `0x2100006E`) recursing through `ElementDesc.Children` — a purely structural walk with no tab-page/state-descriptor resolution — and calling it directly with `0x100001EC`/`0x100001ED`/`0x100001EE` returns null. Resolving the panel's own SLOT first (`ImportInfos(dats, 0x2100006E, 0x1000018C)` — what `MountMapHousePanel` actually does to build the whole panel) and searching THAT tree DOES find `m_pMap` with both icon children present — so the icons are real and correctly nested, but only reachable through the full panel-slot resolve pathway (likely tab-page wiring), not a cold `ImportInfos(hostLayout, elementId)` call starting from the element id alone. `ResolveSwallowedIcon`'s "re-import as if standalone" approach is architecturally wrong for these two elements, unlike the town-hotspot template (a genuine standalone catalog entry addressable by `(templateLayoutId, templateElementId)`, which DOES work). Filed as a follow-up task (see `spawn_task` "Fix Map tab player/house icon resolution"). | `src/AcDream.App/UI/Layout/MapPageController.cs:144-145` (the two `ResolveSwallowedIcon` call sites in `Bind`); `:178-191` (`ResolveSwallowedIcon`'s own body) | The icons' authored local position from a successful standalone import would be irrelevant since `PlaceMarker` overwrites `Left`/`Top` on every `Refresh` anyway — but this reasoning is currently moot since the import never succeeds at all on the installed DAT. | **This is not a future risk — it is the CURRENT live-DAT state, confirmed 2026-08-17.** `ResolveSwallowedIcon` already logs a `[D.2b]` warning and returns null rather than throwing, so the failure mode is "no player/house marker ever shows" — live-observed, not hypothetical. F1's byte-decoded `PlaceMarkerOnMap` formula (this same session) cannot be visually confirmed against the running client until this is fixed; it remains verified only at the unit-test/golden-pixel level. | `gmMapUI::PostInit @0x004a1c70` (child resolution); `gmMapUI::ListenToElementMessage @0x004a2350` idMessage `0x1c` (confirms `m_pMap` IS a button, not a passive container) |
| AD-106 | **Filed 2026-08-16 at #409 (client-wide retail tooltip system).** Retail's tooltip popup is a separate always-on-top presentation surface — `UIElementManager::StartTooltip @0x00459700` positions and latches it into `m_pTooltipElement`, drawn independently of the ordinary `UIElement` sibling tree (the SAME class of separation the AP-229 register row already establishes for retail's dialogs vs acdream's flat sibling list under one `Host.Root`). `RetailTooltipPresenter` instead mounts the popup as an ordinary `UiRoot` child sibling (`_host.AddChild(root)`) and keeps it topmost by calling `BringToFront` from its OWN `Tick()`, which `RetailUiRuntime.Tick` schedules AFTER both `RetailDialogFactory.Tick()` and `Host.Tick()` in the same frame — guaranteeing the tooltip wins whatever z-order race those two just ran, every frame, regardless of which dialog/screen last called its own `BringToFront`. | `src/AcDream.App/UI/Layout/RetailTooltipPresenter.cs` (`Tick`, `OnTooltipShow`'s `AddChild`/`BringToFront`); `src/AcDream.App/UI/RetailUiRuntime.cs` (`Tick`'s three-call ordering, `MountTooltipPresenter`) | Reproduces the one observable invariant a user can check (tooltips always draw on top of dialogs and screens) without porting retail's literal separate-layer architecture (no second draw pass, no dedicated presentation root) — the SAME tradeoff AP-229 already accepted for dialogs, extended one layer further. The ordering is enforced structurally (three sequential calls in one method), not by convention, so it cannot silently regress from an unrelated edit reordering unrelated `Tick` calls elsewhere. **F10 correction (2026-08-16 review round), two honest additions:** (1) the guarantee is versus dialogs/screens ONLY — `UiRoot.DrawCore`'s own second pass (`ctx.BeginOverlayLayer(); DrawOverlays(ctx); DrawDragGhost(ctx);`) routes open dropdown/menu popups and the drag ghost to a renderer overlay layer that paints over the WHOLE sibling tree unconditionally, so both still paint above a shown tooltip regardless of any `BringToFront` ordering — no z-order fix in the sibling tree can reach that layer. (2) counting the full chain by its own actual participants (not just the three calls local to `RetailUiRuntime.Tick`'s tooltip-adjacent lines), the per-tick `BringToFront` ratchet has FOUR rungs in frame order: `CharacterManagementUiController.Tick`, `CharacterCreationUiController.Tick` (both named in `RetailDialogFactory`'s own GF-15 doc comment as the screens it re-asserts over), `RetailDialogFactory.Tick`, then `RetailTooltipPresenter.Tick`. Four independent per-tick self-reraises stacked by tick ORDER is a design smell — a correct z-order model would need at most one authoritative comparison, not N racing assertions — but is bounded and enumerable in practice (no unbounded surface list, the order is fixed source, not runtime-discovered) so it is left as observed rather than restructured this round. | A FUTURE always-on-top UI surface that calls its own unconditional per-tick `BringToFront` AFTER `TooltipPresenter?.Tick()` in `RetailUiRuntime.Tick`'s ordering could bury a currently-shown tooltip — the exact failure class AP-229 already named for dialogs-vs-screens, now with four layers instead of two. | `UIElementManager::StartTooltip @0x00459700` (`m_pTooltipElement` ownership); AP-229's own dialog/screen precedent |
| AD-73 | Filed 2026-08-11 at the Campaign OP OP2 rework (fix round after a double REJECT). `UiTabPanel` (dat Type 8, formerly `UiTabControl`) does NOT perform retail's automatic tab-table wiring / default-page activation at construction. Retail `UIElement_Panel::SetupTabPageHash @0x0046C2E0` + `::Update @0x0046BD00` unconditionally activate the authored default page for ANY instance that carries a tab table. `UiTabPanel` instead stays DORMANT — no click binding, no page-visibility flip, no tab Open/Closed write — until a controller explicitly calls `ActivateTabBehavior()`. | `src/AcDream.App/UI/UiTabPanel.cs` (`ActivateTabBehavior`); factory site `src/AcDream.App/UI/Layout/DatWidgetFactory.cs` (Type-8 arm) | Four already-shipped Type-8 hosts author a tab table today — character sheet root `0x10000227`, spellbook root `0x100002A8`, and vendor `0x100000B8` already implement this exact switching in their own C# controllers (`CharacterStatController`/`SpellbookWindowController`/`VendorUiController`); activating `UiTabPanel`'s own copy unconditionally would double-drive the same page-visibility/tab-state writes those controllers already own. Combat `0x100000A2` has no controller at all and is INTENTIONALLY left inert (its 8 stance pages have no switching UI yet) rather than have `UiTabPanel` silently take ownership. Only newly-authored hosts opt in (Options panel, Campaign OP slice OP3+; Configure Keyboard, OP8). This is what let the unconditional Type-8 factory mapping become safe after the OP2 REJECT (`docs/research/2026-08-11-op2-review-blast.md`, `docs/research/2026-08-11-op2-review-mechanism.md`). | A future panel that authors a Type-8 tab table but never gets a controller call to `ActivateTabBehavior()` renders with every tab button at its authored default (Closed) and every page slot at its default `Visible=true` — i.e. every page overlapping, no single active page — instead of retail's exactly-one-visible-page behavior. This is silent unless the diagnostic `UnresolvedEntries`/`BehaviorActive` surface is checked; a controller author who forgets the activation call will see a visually broken tab host, not a crash. | `UIElement_Panel::SetupTabPageHash @0x0046C2E0`; `UIElement_Panel::Update @0x0046BD00`; `UIElement_Panel::OpenTab @0x0046BE20`. ADDENDUM (2026-08-11, re-review closure): `UiTemplateListBox` additionally reports `ConsumesDatChildren = true` where the pre-rework fallback did not — inert against every shipped layout because no Type-5 element in any of the 32 fixtures authors children (now conformance-PINNED in `OP2ReworkBlastRadiusConformanceTests`, so an authored child appearing in a future DAT regeneration fails the build instead of silently vanishing) |
| ~~AD-53~~ | **RETIRED 2026-07-31 (Campaign P Slice 1B).** `Transition.CliffSlide` now consumes only `collision_info.last_known_contact_plane.N`, exactly as retail does. The invented `LastWalkablePlane -> LastKnownContactPlane -> UnitZ` fallback chain is gone; invalid/default or parallel data takes retail's degenerate `OK_TS` return. | `src/AcDream.Core/Physics/TransitionTypes.cs` (`CliffSlide`); `tests/AcDream.Core.Tests/Physics/RetailEdgeResponseOrderingTests.cs` | — | — | `CTransition::cliff_slide` pc:272397 (0050a6d0); `last_known_contact_plane` maintenance pc:272659-272668 (~0050ad07) |