erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

fix(ui): OP2 re-review closure (coordinator pass) — AP-195, tooltip port, zero-children pin

Browse source 
Closes the mechanism lens's REOPEN (one MUST-FIX) and both lenses' small
residuals on the OP2 rework (b236a442); the blast lens re-review was
CLOSED outright. Fable-direct per the two-failure escalation rule.

- AP-195 filed: UIOption_CheckboxBitfield64 ports HALF of Refresh
  @0x004859C0 — the ANY-set checkbox predicate is exact, but the ALL-set
  LED media swap (P0x10000082=0x06004D17 / P0x10000083=0x06004D19) and
  the ListBox self-sizing tail (ResizeTo/CalculatePaperSize — the block
  IS a UIElement_ListBox in retail) are unported, and the block's row
  stacking is a second divergent implementation beside UiTemplateListBox.
  All due at OP5 before the Chat tab's connected gate; the IsSet doc
  comment now names both halves instead of quoting only the ported one.
- Row tooltips: UiButton gains settable TooltipText surfaced through the
  shared GetTooltipText hover pipeline (UiCatalogSlot's pattern);
  UiCheckboxBitfield64.AddChild applies the row tooltip retail stamps in
  CreateChildren @0x00485DF0, and documents that the 0x10000084 row-index
  attribute stamp is deliberately replaced by the typed mask closure.
- AD-73 addendum: UiTemplateListBox.ConsumesDatChildren=true is inert
  only while no authored Type-5 element carries children — that premise
  is now conformance-PINNED across all 32 fixtures (a future DAT
  regeneration surfacing an authored child fails the build instead of
  silently dropping it).
- Plan doc: OP2's contract names UiTabPanel.cs (retail UIElement_Panel),
  not the fictional-class-named UiTabControl.cs; ledger records OP1 and
  OP2 both CLOSED.

Full Release suite: 12,871 passed / 4 skipped / 0 failed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-11 01:26:29 +02:00
parent 6f48e34152
commit 5242de9f15
7 changed files with 943 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

5
docs/architecture/retail-divergence-register.md
View file

File diff suppressed because one or more lines are too long

8
docs/plans/2026-08-10-options-panel-campaign.md
View file

@ -178,7 +178,9 @@ byte-exact; both adapters share one code path for the local write.
`0x2E` (struct array `{0x30 button, 0x31 page, 0x32 isDefault}`) and `0x2E` (struct array `{0x30 button, 0x31 page, 0x32 isDefault}`) and
ListBox template-list property `0x64` (entries `{0x63 layout DID, ListBox template-list property `0x64` (entries `{0x63 layout DID,
0x62 element id}`) into `ElementInfo`. 0x62 element id}`) into `ElementInfo`.
- Create `src/AcDream.App/UI/UiTabControl.cs` (element Type 8): tab-button - Create `src/AcDream.App/UI/UiTabPanel.cs` (element Type 8 — retail
`UIElement_Panel`; renamed from this plan's original `UiTabControl` at the
OP2 rework, and a dormant `UiDatElement` subclass per AD-73): tab-button
↔ page-slot switching per lane A §5; default tab honoured. ↔ page-slot switching per lane A §5; default tab honoured.
- Create `src/AcDream.App/UI/UiTemplateListBox.cs` (element Type 5 with - Create `src/AcDream.App/UI/UiTemplateListBox.cs` (element Type 5 with
authored template list): `AddItemFromTemplateList(index)` instantiates a authored template list): `AddItemFromTemplateList(index)` instantiates a
@ -415,8 +417,8 @@ before anything builds on them.
| Slice | Status | Commit(s) | Review | Gate | | Slice | Status | Commit(s) | Review | Gate |
|---|---|---|---|---| |---|---|---|---|---|
| OP1 | CLOSED (residuals R1/R2 owed to a coordinator pass) | `86c0a7e0` + fixes `09029f9f` | dual APPROVE-WITH-FIXES (`2026-08-10-op1-review-{mechanism,blast}.md`) → re-review CLOSED (`2026-08-11-op1-rereview.md`) | automated only — n/a | | OP1 | CLOSED | `86c0a7e0` + fixes `09029f9f` + residuals `6f48e341` | dual APPROVE-WITH-FIXES (`2026-08-10-op1-review-{mechanism,blast}.md`) → re-review CLOSED (`2026-08-11-op1-rereview.md`); residuals R1/R2/R3 landed | automated only — n/a |
| OP2 | REWORK round 1 in flight | `df9c7a35` (REJECTED) | dual REJECT (`2026-08-11-op2-review-{mechanism,blast}.md`) | | | OP2 | CLOSED | `df9c7a35` (REJECTED) → rework `b236a442` → closure (this commit) | dual REJECT (`2026-08-11-op2-review-{mechanism,blast}.md`) → re-review blast CLOSED / mechanism REOPEN-on-one (`2026-08-11-op2-rereview-{mechanism,blast}.md`) → coordinator closure: AP-195 filed, AD-73 addendum, tooltip port, zero-children pin | automated only — n/a |
| OP3 | — | | | | | OP3 | — | | | |
| OP4 | — | | | | | OP4 | — | | | |
| OP5 | — | | | | | OP5 | — | | | |

487
docs/research/2026-08-11-op2-rereview-blast.md Normal file
View file

@ -0,0 +1,487 @@
# OP2 rework re-review — regression / blast-radius lens
**Rework under review:** `b236a442` "fix(ui): OP2 rework — dormant UiDatElement
subclasses, fixed Panel/CheckboxBitfield64 mechanism"
**Original (REJECTed):** `df9c7a35`; round-1 findings
`docs/research/2026-08-11-op2-review-blast.md`
**Reviewer lens:** what OP2 breaks OUTSIDE its own scope (Options panel).
**Date:** 2026-08-11
**Verdict: CLOSED** — all three MUST-FIXes and all three SHOULD-FIXes are
discharged, verified against the committed data rather than against the commit
message. Four residuals, none blocking: 1 SHOULD-FIX (an unpinned premise), 3
NOTEs.
---
## 0. Executive summary
The rework replaces "widen the factory arm and hope" with a **dormancy model**:
`UiDatElement` is unsealed (`src/AcDream.App/UI/Layout/UiDatElement.cs:43`), and
both behavioral widgets now derive from it — `UiTabPanel : UiDatElement`
(`src/AcDream.App/UI/UiTabPanel.cs:79`) and `UiTemplateListBox : UiDatElement`
(`src/AcDream.App/UI/UiTemplateListBox.cs:81`). The Type-8 and Type-5 factory
arms are unconditional again
(`src/AcDream.App/UI/Layout/DatWidgetFactory.cs:106`, `:119`), which is now
**safe rather than lucky**, because an un-activated instance inherits the exact
`UiDatElement` media draw, `ClickThrough = true` default, and `IUiDatStateful`
propagation the pre-OP2 fallback provided, and performs zero import-time work
(`UiTabPanel.cs:126-128`, the empty `OnChildrenAttached`).
That is the right shape. It removes the class of defect instead of narrowing a
guard, and it does so without the "gate on the Options LayoutDesc ids" workaround
round 1 offered as one option.
**All three MUST-FIXes verified closed against primary data**, not against the
commit message:
- The five Type-8 elements are behaviorally identical to pre-OP2, and the six new
pins assert the **failure modes** (media file id non-zero, `ClickThrough` true,
`ActivePageElementId == 0`, child count 16, all children `Visible`), not just
widget class names.
- The ten Type-5 elements gain no viewport child. Verified independently: **all
16 Type-5 elements across all 32 fixtures have zero children**, so the one
genuine remaining delta (`ConsumesDatChildren => true`) drops nothing today.
- All 32 fixtures were regenerated and committed. Verified by structural diff:
**all 1,102 nodes across all 32 files carry the five new fields, and the
regeneration changed no pre-existing field value in any of the 27 previously
stale fixtures** — zero drift, in all 27, not just the 5 spot-checks requested.
**Two of round 1's own findings were factually wrong and the rework absorbed one
of them without correcting the record.** See §5. This matters because SHOULD-FIX
2's stated rationale (base-vs-incorporated id mismatch) is refuted by the raw dat
data, and MUST-FIX 1(d)'s "mitigating: it is luck, three of four panels don't
resolve" was the inverse of the truth — **every** tab-table id resolves as a
direct child, so `df9c7a35` was strictly worse than round 1 assessed. The
rework's dormancy fix is correct either way; only the reasoning under
SHOULD-FIX 2 needs correcting.
---
## 1. Method
- Parsed all **32** committed fixtures under
`tests/AcDream.App.Tests/UI/Layout/fixtures/` at `b236a442`, walked every
`ElementInfo` node (1,102 total), and recorded Type, child ids, raw dat
properties `0x2E`/`0x64`/`0x72`, `StateMedia`, and new-field presence.
- Structural deep-diff of every one of the 27 pre-existing fixtures,
`df9c7a35` → `b236a442`, with the five new field names stripped, to isolate
"did regeneration change anything OLD?"
- Cross-checked the raw `0x2E` struct members (`0x30`/`0x31`/`0x32`) against the
projected `TabTable` field, per element, to settle the base-vs-incorporated-id
question with data.
- Read the widget/factory/importer sources and every new or changed test.
- No build, no test run, no launch (read-only mandate).
---
## 2. MUST-FIX closure
### MUST-FIX 1 — five Type-8 elements re-classed — **CLOSED**
Every one of the five is now behaviorally identical to pre-OP2. The mechanism is
inheritance, not per-element special-casing: `UiTabPanel` adds no `OnDraw`
override, no `ClickThrough` write, no `ConsumesDatChildren` override, and its
`IUiChildrenAttachedListener.OnChildrenAttached` body is empty
(`UiTabPanel.cs:126-128`).
| Element | Round-1 defect | Status | Evidence |
|---|---|---|---|
| vendor `0x1000008D` | lost DirectState backdrop fill | fixed | fixture `StateMedia[""] = (100687632, DrawMode 1)`; `UiDatElement` ctor picks `""` (no `DefaultStateName`, no `Normal`) → `ActiveMedia()` returns it → `UiDatElement.OnDraw` (`UiDatElement.cs:208`) draws it |
| character `0x10000227` | `ClickThrough` true→false; lost state propagation | fixed | `ClickThrough = true` set in `UiDatElement.cs:126`, never overwritten; `TrySetRetailState` is **non-virtual** (`UiDatElement.cs:86`) so a subclass cannot alter propagation |
| spellbook `0x100002A8` | same | fixed | same |
| vendor `0x100000B8` | same | fixed | same |
| combat `0x100000A2` | import-time takeover (8-entry table, no controller re-binder) | fixed | `OnChildrenAttached` is empty; `ActivateTabBehavior` has no caller anywhere in `src/` |
**Do the new pins assert failure modes rather than class names?** Yes, on three
of the four axes, with one weaker:
- **Media drawn** — `OP2ReworkBlastRadiusConformanceTests.cs:48-49`:
`(uint file, int _) = backdrop.ActiveMedia(); Assert.NotEqual(0u, file);`
This is the actual draw input, not a class name. ✅
- **Click-through** — `:47`, `:67`, `:90`, `:106`, `:155`: `Assert.True(...ClickThrough)`
on all five Type-8 elements plus the representative Type-5. This is the exact
field `UiElement.HitTest` gates on. ✅
- **No visibility flip / no takeover** — `:129-131`:
`Assert.Equal(0u, host.ActivePageElementId)`, `Assert.Equal(16, host.Children.Count)`,
`Assert.All(host.Children, child => Assert.True(child.Visible))`. That is the
observable failure mode (a hidden page), not a class name. ✅
- **State propagation** — `:91`, `:107`: `Assert.IsAssignableFrom<IUiDatStateful>(root)`.
This is a *capability* assertion, not a behavior assertion: it does not build a
`PassToChildren` state and check a child received it. Acceptable-as-is because
`TrySetRetailState`/`ActiveRetailStateId` are non-virtual on `UiDatElement`
(`:86`, `:69`), so a subclass structurally cannot diverge — but it is the one
pin of the four that would still pass if the mechanism broke in the base class.
Recorded as NOTE 1, not a residual defect.
Combat's specific claim ("no controller rebind") re-verified: grep of
`src/AcDream.App/UI/Layout/CombatUiController.cs` for `0x100000A2` / `0x100000A3`
returns nothing.
### MUST-FIX 2 — ten Type-5 elements — **CLOSED**
The viewport is now created lazily inside the `Viewport` property
(`UiTemplateListBox.cs:147-162`), reached only from `Scroll` and from a
*successful* `AddItemFromTemplateList` (`:182`). `DatWidgetFactory` leaves
`TemplateResolver` null (`:106` passes no resolver), and `AddItemFromTemplateList`
returns before touching `Viewport` when the resolver is null (`:175-176`) or the
index is out of range (`:174`). So no pre-existing Type-5 element can allocate or
attach the viewport.
**Child structure verified identical to the old `UiDatElement` build** — with one
qualifier the class doc names and I confirmed:
| | old `UiDatElement` | new dormant `UiTemplateListBox` |
|---|---|---|
| viewport child | none | none (lazy) ✅ |
| authored dat children | built and attached (`LayoutImporter.cs:129-136`) | **skipped** — `ConsumesDatChildren => true` (`UiTemplateListBox.cs:145`) |
That second row is the only place the "behaviorally identical" claim is not
literally true. It is **harmless today, and I verified it directly rather than
taking the doc's word**: across all 32 fixtures, all 16 Type-5 elements have
**zero** `Children`. Incorporation (`LayoutImporter.Resolve` → `IncorporateChildren`,
`LayoutImporter.cs:352`) happens in the `ImportInfos` phase, so the committed
fixture child tables *are* the final production child tables — this is not a
fixture-vs-production gap. Nothing is dropped, and no id disappears from `byId`.
The premise being unpinned by any test is SHOULD-FIX A below.
**`EffectsUiControllerTests`' updated assertion is correct, not loosened.** The
diff changes `Assert.IsType<UiDatElement>` → `Assert.IsType<UiTemplateListBox>`
(exact-type, xUnit `IsType` is not assignability), and the line immediately after
is unchanged and is the load-bearing one:
`UiItemList list = Assert.IsType<UiItemList>(Assert.Single(listHost.Children));`
`Assert.Single` is exactly the assertion that fails if a viewport is injected
alongside the controller's own `UiItemList`. The pin got *stronger* in effect,
not weaker.
### MUST-FIX 3 — harness blindness — **CLOSED**
Three independent checks, all measured:
1. **All 32 fixtures regenerated and committed.** `git show --stat b236a442`
lists all 32 fixture files as modified; 27 of them carry substantial line
counts (e.g. `character_2100002E.json` +1001, `combat_21000073.json` +741).
2. **All 27 previously-stale fixtures now carry the five new fields.** Verified
node-by-node, not file-by-file: **1,102 / 1,102 nodes across all 32 files
carry `TabTable`, `TemplateList`, `ScrollbarElementId`, `Outline`, and
`OutlineColor`. Zero nodes missing any of the five.**
3. **The regeneration introduced ONLY the expected field additions.** The task
asked for a spot-diff of at least 5 of the 27; I ran the structural deep-diff
on **all 27**, stripping only the five new field names and comparing every
remaining key/value recursively:
```
character_2100002E.json : 0 non-new-field differences CLEAN
combat_21000073.json : 0 non-new-field differences CLEAN
spellbook_21000034.json : 0 non-new-field differences CLEAN
vendor_21000012_100000B7.json : 0 non-new-field differences CLEAN
effects_positive_2100001B.json : 0 non-new-field differences CLEAN
… and the other 22, all CLEAN (0 differences each)
```
**No pre-existing value drifted.** The hazard the task named — "a changed
pre-existing value would mean the generator's output changed for OLD fields" —
did not occur anywhere. (`chat_2100006f.json`, the one fixture that already
carried `Outline`/`OutlineColor` from CH6a, is included in that sweep with the
same result on its other fields.)
4. **The suite genuinely exercises the new-shape fixtures.** `ElementInfo`'s three
new members are public *fields* (`ElementReader.cs:192`, `:199`, `:207`) and
`FixtureLoader` deserializes with `IncludeFields = true`
(`FixtureLoader.cs:14-17`), so the fixture path now reads real projected
values. Concretely: `OP2ReworkBlastRadiusConformanceTests.cs:64` asserts
`host.Tabs.Count == 3` and `:127` asserts `== 8` — values that were empty
under the stale fixtures. The structural false negative is gone.
5. **The reader itself is now covered independently of the fixtures.** The
remaining structural gap round 1 named — `ApplyCanonicalLegacyProjection` is
never called on the fixture path (`LayoutImporter.cs:465`, `ElementReader.cs:410`
are still its only call sites) — is addressed by five new tests in
`ElementReaderTests.cs` that drive `ApplyCanonicalLegacyProjection` directly
from synthetic raw property bags. A reader regression now fails a test even
without regeneration. That is the right seam.
---
## 3. SHOULD-FIX closure
### SHOULD-FIX 1 — unconditional viewport injection — **CLOSED**
Lazy `Viewport` property (`UiTemplateListBox.cs:147-162`), pinned in both
directions: `OptionsPanelLayoutConformanceTests` asserts `Assert.Empty(listBox.Children)`
after a failed/never-attempted add, and `Assert.Single(listBox.Children)` after the
one successful add. `ConsumesDatChildren => true` also resolves round 1's
"authored children become siblings of the viewport" trap for OP3+ — authored
children are now never built for a Type-5 element, so the sibling-vs-row ambiguity
cannot arise.
### SHOULD-FIX 2 — base-vs-incorporated tab-table ids — **CLOSED, on a corrected premise**
`FindDirectChild` → `FindDescendant`, a pre-order DFS of the whole subtree
(`UiTabPanel.cs:208-219`), correctly cited to retail `GetChildRecursive`.
**Round 1's stated rationale was factually wrong.** I read the raw `0x2E` struct
members straight out of the fixtures and compared them to the projected `TabTable`
and to the actual built child ids:
| Host | raw `0x2E` page ids | projected `TabTable` | actual direct children |
|---|---|---|---|
| vendor `0x100000B8` | `0x100000BC`, `0x100000C4`, `0x100000CD` | identical | contains all three |
| combat `0x100000A2` | `0x100000AA` … `0x100005C3` | identical | contains all eight |
| character `0x10000227` | `0x1000022B`, `0x1000022C`, `0x10000539` | identical | contains all three |
| spellbook `0x100002A8` | `0x100002AC`, `0x100002AD` | identical | contains both |
Round 1 reported vendor's table as naming `0x1000007C/0x10000084/0x1000008D` and
character's third entry as `0x100003F8→0x100003F9`. Neither appears anywhere in
the dat property. Since the deep-diff proves the raw properties are byte-identical
between `df9c7a35` and `b236a442`, this was a round-1 mis-read, not a data change.
Measured consequence: **every tab-table id occurs exactly once, at depth 1, in
every one of the five Type-8 hosts** — so `FindDescendant` returns the same
element `FindDirectChild` did. No behavior change today.
**Does the recursion introduce NEW ambiguity?** Yes, latently — and the codebase
already documents the duplicate-id hazard the task asked about
(`UiDatElement.cs:60-62`: *"footer state groups that appear once per tab page but
share the same dat id"*). Two facts:
- Duplicate `DatElementId`s are **rampant** inside these very subtrees: character
`0x10000227`'s 133-node subtree has 39 duplicated ids (`0x10000244` ×6,
`0x1000023D` ×2, …); combat has four ids at ×8; spellbook has `0x10000328` ×13;
options has `0x100002CE/CF/D0` ×16.
- `FindDescendant` is **pre-order**: it fully recurses into child *n*'s subtree
before testing child *n+1*. So a deep match under an earlier sibling beats a
direct-child match on a later sibling — a strictly larger ambiguity surface
than `FindDirectChild` had.
Safe today only because no tab-table id collides with a duplicated id. Recorded
as NOTE 2, with the OP3+ trigger named.
### SHOULD-FIX 3 — silent no-resolve — **CLOSED**
`UnresolvedEntries` (`UiTabPanel.cs:118`) plus a per-miss `Console.WriteLine`
carrying the host id, both entry ids, and which side missed
(`:154-157`). Detectable enough for this lens: it is a public, testable
collection, not only a log line, and `OptionsPanelLayoutConformanceTests` pins
`Assert.Empty(tabControl.UnresolvedEntries)` after activation. A future
regression that breaks resolution is now an assertable state change rather than a
silent no-op.
---
## 4. New blast radius introduced by the rework
Scanned the rework diff for what it *adds* to the exposure surface.
**(a) Unsealing `UiDatElement` — no invariant becomes mutable by subclasses.**
`Info` is `protected readonly` (`UiDatElement.cs:57`), so the reference cannot be
reseated. The class's identity-carrying members are **non-virtual**:
`ElementId`, `ActiveRetailStateId` (`:69`), `TrySetRetailState` (`:86`),
`ActiveMedia` (`:150`). The only overridable surface a subclass inherits is
`UiElement`'s own (`OnDraw`, `OnHitTest`, `ConsumesDatChildren`, `OnEvent`,
`HandlesClick`) — and of those, exactly one is exercised
(`UiTemplateListBox.ConsumesDatChildren`, §2). `OnDraw` is
`protected override` and **not sealed**, so a future subclass could silently kill
authored-media drawing; neither current subclass does. NOTE 3.
**(b) Unsealing widens every `is`/`as UiDatElement` site in the tree.** Nine
production sites now match Type-5/Type-8 widgets where they previously fell
through: `AppraisalUiController.cs:176`, `:224`;
`CharacterStatController.cs:1582`, `:1585`, `:1799`, `:1839`;
`SelectedObjectController.cs:152`; `CreatureAppraisalRows.cs:312`;
`RetailTabBinding.cs:21`; `SpellcastingUiController.cs:658`;
`WindowChromeController.cs:41`. Every one of them is additionally gated on a
specific `ElementId`/role, and I checked the ids involved
(`InscriptionBackgroundId = 0x10000137`, `SpellIconId = 0x1000015F`,
`FooterStateAId/BId = 0x10000240/41`, `OverlayId = 0x100001A0`) against the
complete Type-5/Type-8 id set — **no overlap**. The widening is also benign in
direction: the behavior these sites reach for (`OnClick`, `MediaVisible`,
`ClickThrough`) is inherited unchanged. Clean.
The inverse — tests using exact-type `Assert.IsType<UiDatElement>` — is the
sharper edge, since xUnit's `IsType` rejects subclasses. Ten such sites exist;
`EffectsUiControllerTests.cs:136` was the one that needed updating and was
updated. The remaining nine target ids outside the Type-5/8 set.
**(c) The factory arms being unconditional again.** Correct now, for the reason
the comments give. Verified there is no arm collision: the switch is a single
`info.Type switch` (`DatWidgetFactory.cs:76`), and the four `UIOption_*` class ids
(`0x10000036/37/38/44`) are distinct Type values from `5` and `8` — the
`0x10000044` template element `0x10000520` is Type `0x10000044`, not Type 5, so it
cannot be captured by the Type-5 arm.
**(d) `BuildCheckboxBitfield64` deleted; `0x10000044u => new UiCheckboxBitfield64(info.TemplateList)`
(`DatWidgetFactory.cs:162`) no longer seeds `SpriteResolve`/`LabelFont`.** Zero
blast radius — class `0x10000044` occurs in exactly one fixture
(`options_2100002B.json`, element `0x10000520`), in a panel that is not yet
shipped. Rows now carry their own art via the resolved template. Noted for
completeness only.
**(e) Fixture regeneration's effect on other conformance suites.** No test pins a
fixture value that regeneration changed, because **regeneration changed no
pre-existing value** (§2, MUST-FIX 3.3, all 27 files). The only way a suite could
have broken is by exact-type assertion on a re-classed widget — one site, updated.
**(f) The `RuntimeCollisionReportingStateTests` allocation flake — not caused by
this rework, and it is exactly ISSUES #250's class.**
`tests/AcDream.Runtime.Tests/Physics/RuntimeCollisionReportingStateTests.cs:2359-2384`
(`WarmedSteadyContactRefreshDoesNotAllocate`) has the precise shape #250 named as
its root cause: **one warmup call, then a 10,000-iteration loop written inline
inside the measured window**, asserted `Assert.Equal(0, allocated)`. #250's own
table lists that shape (`UiDatFontTests`: "1 call" warmup / "a 10,000-iteration
loop written inline") and diagnoses it as on-stack replacement compiling on the
measuring thread inside the window.
The #250 fix moved **every strict-zero site in `AcDream.App.Tests`** onto
`tests/AcDream.App.Tests/ZeroAllocationProbe.cs`. It never swept the other test
assemblies: `AcDream.Runtime.Tests` still has three files with raw
`GC.GetAllocatedBytesForCurrentThread()` sites
(`Physics/RuntimeCollisionReportingStateTests.cs`,
`Physics/RuntimePhysicsStateTests.cs`, `Physics/RuntimeSetPositionStateTests.cs`,
plus `Gameplay/RuntimeLocalPlayerMovementStateTests.cs`), as do
`AcDream.Headless.Tests` and `AcDream.Core.Tests`. #250's own closing lesson
applies verbatim: *"'Not observed failing' only ever meant 'not yet observed'."*
Could the rework have affected it? **No causal path.** `git show --stat b236a442`
touches only `docs/`, `src/AcDream.App/UI/**`, and `tests/AcDream.App.Tests/UI/**`.
Zero files under `src/AcDream.Runtime/` or `tests/AcDream.Runtime.Tests/`. Test
assemblies run in separate processes, so the larger App fixtures cannot raise GC
pressure inside the Runtime test process.
---
## 5. Corrections to the round-1 report (record-keeping)
Two round-1 claims are refuted by the committed data. Both are recorded here so
the next reader does not re-derive them:
1. **"On 3 of the 4 panels the tab table's ids do not match the resolved child
ids, so `FindDirectChild` misses and `SwitchTo` is inert — mitigating, and it
is luck."** False. All 16 tab-table ids across the four pre-existing hosts
resolve as direct children (§3, SHOULD-FIX 2 table). `df9c7a35` was therefore
**worse** than round 1 assessed: all four panels would have had a live
import-time switcher, and combat — with no controller re-binder — would have
hidden seven of its eight stance pages on import. This strengthens, not
weakens, the case for the dormancy fix that landed.
2. **SHOULD-FIX 2's premise, "property `0x2E` carries base-layout ids that
`LayoutImporter.IncorporateChildren` remaps."** False; the raw struct members
equal the incorporated child ids exactly. The `FindDescendant` change is still
defensible on its own merits (retail uses `GetChildRecursive`, and OP3+
cross-layout page mounts genuinely nest), but it was justified by a diagnosis
that does not hold, and it carries the real cost measured in §3.
This is the `docs/research/2026-08-05-c4-closeout-handoff.md` pattern in
miniature — *inferring a fact you can observe*. The observation cost one script.
---
## 6. Residuals
### SHOULD-FIX A — `ConsumesDatChildren => true` rests on a measured-but-unpinned premise
`UiTemplateListBox.cs:145` is the single remaining behavioral delta versus the
pre-OP2 `UiDatElement` fallback: authored dat children of a Type-5 element are no
longer built. The class doc states the premise correctly
(`UiTemplateListBox.cs:28-32`: "None of the ten pre-existing elements author real
dat children of their own"), and I verified it — all 16 Type-5 elements, all 32
fixtures, zero children.
**No test pins it.** Worse, the closest pin cannot distinguish the two outcomes:
`OP2ReworkBlastRadiusConformanceTests.cs:153` asserts
`Assert.Empty(host.Children)` on effects `0x10000123`, which passes both when no
viewport was injected (intended) *and* when authored children were silently
consumed (the failure). A future dat/importer change that gives any of the ten a
child would be dropped with the whole suite green.
Suggested pin (cheap): assert `info.Children.Count == 0` on the `ElementInfo` side
for the ten ids, in the same test file, so the premise is asserted where it is
relied upon. Contrast the Type-8 side, which *does* have the right pin —
`Assert.Equal(16, host.Children.Count)` at `:130`.
Also, only one of the ten Type-5 elements is pinned at all; the other nine
(`0x1000023D`, `0x10000532`, `0x10000123` negative, `0x10000149`, `0x1000032D`,
`0x10000335`, `0x10000174`, `0x10000464`) rely on class-level reasoning. That is
defensible for the dormancy properties, which are class-level; it is *not*
defensible for the child-count premise, which is per-element.
### NOTE 1 — the state-propagation pin is a capability assertion
`OP2ReworkBlastRadiusConformanceTests.cs:91`, `:107` assert
`IsAssignableFrom<IUiDatStateful>`, not that a `PassToChildren` state actually
reached a child. Sound in practice because `TrySetRetailState` is non-virtual
(`UiDatElement.cs:86`), so no subclass can diverge — but it is the one MUST-FIX-1
axis whose pin would survive the mechanism breaking in the base class.
### NOTE 2 — `FindDescendant` pre-order first-match in a duplicate-id tree
`UiTabPanel.cs:208-219` recurses into an earlier sibling's entire subtree before
testing the next sibling, in trees where duplicate `DatElementId`s are common
(up to ×16 in `options_2100002B.json`). Harmless today — every tab id is unique
and at depth 1 — but the OP3+ trigger is concrete: the Options page slots
(`0x10000211`/`0x10000212`/`0x10000213`/`0x1000050C`) mount cross-layout content
that already re-uses ids present elsewhere in the same tree. If it bites, prefer
breadth-first, or search direct children first and fall back to descendants.
Two smaller items in the same class, both unreachable today (nothing calls
`ActivateTabBehavior`):
- `UiTabPanel.cs:161` binds the click even when only the *page* failed to resolve,
so the button would switch to a page that does not exist — and `SwitchTo`
(`:184-199`) would then hide every page and close every tab, i.e. a blank panel.
- Conversely, the new `ReadTabTable` zero-id filter (`ElementReader.cs:520-527`)
usefully protects `SwitchTo`'s `ActivePageElementId == 0` sentinel from ever
being a real page id. Good interaction, worth keeping in mind if that filter is
ever relaxed.
### NOTE 3 — unsealing leaves `OnDraw` overridable
`UiDatElement.OnDraw` (`:208`) is `protected override`, not sealed. A future
`UiDatElement` subclass that overrides it silently loses authored-media drawing —
which is precisely the `df9c7a35` vendor-backdrop defect, reachable again by a
different route. Sealing `OnDraw`, `TrySetRetailState`'s surface is already safe
(non-virtual), would close it. Also: `UiTabPanel` still implements
`IUiChildrenAttachedListener` with an empty body (`:126-128`) purely to document
"deliberately does nothing" — harmless, but it is a live interface registration
that a reader may mistake for an active hook.
### NOTE 4 — blast-radius sweeps remain bounded by the 32 fixtures
Both round 1 and this re-review measure the 27 layouts the fixture generator
covers (`RetailLayoutFixtureGenerator.cs:19-37`, `:95-184`). Production imports at
least a few LayoutDescs with no fixture — `0x21000008`
(`ExternalContainerController`, mounted at `RetailUiRuntime.cs:2177`/`:2223`),
`0x21000037` (shared UIItem catalog), `0x21000043` (vendor popup catalog),
`0x21000011` (SpewBox). With dormancy in place the only unmeasured risk in those
is narrow and specific: **a Type-5 element carrying authored dat children**
(Type-8 is now fully identical to the old fallback, with no `ConsumesDatChildren`
override, so it carries no residual risk at all). `0x21000008`'s two lists build
as `UiItemList` (`ExternalContainerController.cs:80-81` uses a `RequiredList`
helper), i.e. class `0x10000031`, not Type 5 — so the highest-traffic unfixtured
layout is clear. Recorded so the boundary of the claim is explicit, not because a
defect is suspected.
---
## 7. Verdict
**CLOSED.**
The rework fixes the class rather than the instances: dormancy makes an
unactivated Type-8/Type-5 widget indistinguishable from the pre-OP2
`UiDatElement` fallback by construction, so the unconditional factory arms are
now correct instead of accidentally survivable. All three MUST-FIXes are
discharged and — critically for a lens whose round-1 complaint was that the
harness could not see anything — the closure is verifiable from committed data:
1,102/1,102 fixture nodes carry the new fields, 27/27 regenerated fixtures show
zero drift in pre-existing values, all 16 Type-5 elements carry zero children,
and the new pins assert media file ids, `ClickThrough`, child counts, and page
visibility rather than widget class names.
Residuals are one SHOULD-FIX (pin the `ConsumesDatChildren` premise where it is
relied upon) and four NOTEs, none of which changes shipped behavior. The
`RuntimeCollisionReportingStateTests` flake is ISSUES #250's class in an assembly
#250's fix never swept, with no causal path to this commit.
The one thing worth carrying forward beyond OP2: round 1 got two facts wrong that
one fixture-parsing script settles, and one of them became the stated rationale
for a code change that shipped. §5 corrects the record.

376
docs/research/2026-08-11-op2-rereview-mechanism.md Normal file
View file

@ -0,0 +1,376 @@
# Campaign OP slice OP2 rework — MECHANISM-lens FOCUSED RE-REVIEW
**Date:** 2026-08-11
**Rework under review:** `b236a442` — "fix(ui): OP2 rework — dormant UiDatElement
subclasses, fixed Panel/CheckboxBitfield64 mechanism"
**Round-1 findings being closed:** `docs/research/2026-08-11-op2-review-mechanism.md`
(verdict REJECT on `df9c7a35`)
**Mode:** READ-ONLY. No build, no test run, no launch. Every claim is a quote from
committed source, a decode of a committed fixture, or an extract from
`docs/research/named-retail/acclient_2013_pseudo_c.txt`.
## VERDICT: **REOPEN — one MUST-FIX, narrowly scoped**
All four *behavioural* MUST-FIXes and all five SHOULD-FIXes from round 1 are
genuinely closed, with regression tests that would actually catch a relapse. This is
**not** a redo — it is a one-item fix round.
The one thing that fails is the tail of round-1 MUST-FIX 6, and it is the **same
failure mode that caused the round-1 REJECT, on the same element**: the commit
asserts a completeness claim about `0x10000520`'s authored data and about retail's
`Refresh` that the data and the decomp both refute.
- `UiCheckboxBitfield64.cs:194-201`'s doc comment quotes
`UIOption_CheckboxBitfield64::Refresh @0x004859C0` as if the ANY-bit predicate were
the whole function. It is not. `Refresh` computes **both** an all-set and an any-set
result and uses them for **two different things** (§2.2 below). The all-set half
drives a per-row LED media swap between the element's two authored RenderSurface
DIDs — `P0x10000082 = 0x06004D17` and `P0x10000083 = 0x06004D19`, both still present
in the regenerated fixture, both still unread anywhere in `src/AcDream.App`
(verified by grep: zero hits outside one prose mention).
- That unported half has **no register row**. Round-1 MUST-FIX 6 asked for exactly
this ("Also missing rows for … the dropped `0x64`/`0x10000082`/`0x10000083` authored
data"). `0x64` is now consumed; the two DIDs are not, and the register gained only
AD-73 (tab dormancy) while TS-72 was retired.
Fix = one AP row naming the unported `Refresh` media branch + the unported
`ResizeTo(GetWidth(), CalculatePaperSize(0,-1))` tail, and a corrected doc comment
that stops presenting a half-quoted `Refresh` as complete. The port itself belongs to
OP5 with the consumer; nothing is user-reachable today.
---
## 1. Finding-by-finding closure check
### 1.1 `UiTabPanel` — **CLOSED** (all five sub-checks)
| Round-1 item | Rework | Evidence |
|---|---|---|
| MUST-FIX 5 — cite `UIElement_Panel`, not the fictional `UIElement_TabControl` | ✔ | `src/AcDream.App/UI/UiTabPanel.cs:8-21` names `UIElement_Panel::Create @0x0046C6B7`, `SetupTabPageHash @0x0046C2E0`, `Update @0x0046BD00`, `OpenTab @0x0046BE20`, `InqTabFromPage @0x0046BEB0`, `ListenToElementMessage @0x0046BF90`, and explicitly records the naming correction. `DatWidgetFactory.cs:110-119` carries the same correction at the factory arm. Class renamed `UiTabControl` → `UiTabPanel`; the old file is deleted. |
| Switching semantics preserved | ✔ | `UiTabPanel.cs:184-199` is byte-equivalent to the round-1 reading of `UIElement_Panel::Update @0x0046BD00`: exactly one page `Visible`, every tab button driven `Open`/`Closed` via `RetailTabBinding.SetOpen` → `RetailUiStateIds.Open`/`Closed` = retail `0xC`/`0xB`. Nothing in this commit touched `RetailTabBinding.cs` or `RetailUiStateIds`. |
| SHOULD-FIX 8 — recursive descendant resolution | ✔ | `FindDescendant` (`UiTabPanel.cs:208-219`) is a depth-first descendant walk used for **both** the button (`:149`) and the page (`:150`, `:191`, `:194`), matching retail's `GetChildRecursive` at `0x0046C4EE` / `0x0046BD00`. `FindDirectChild` is gone. |
| SHOULD-FIX 9 — no invented `_tabs[0]` fallback | ✔ | `UiTabPanel.cs:172-173` — `if (defaultEntry is { } def) SwitchTo(def.PageElementId);` with no `??=`. **Independently re-verified against the decomp this round:** `SetupTabPageHash` zeroes `m_OpenTabToken`/`m_OpenPageToken` at `0x0046C312`/`0x0046C318`, seeds `ebp_1 = 0` at `0x0046C31E`, only writes `ebp_1`/`var_3c` inside the `0x32`-true branch at `0x0046C4DF`/`0x0046C4E3`, and tail-calls `Update(this, var_3c, ebp_1)` at `0x0046C5EB`. With nothing authoring `0x32`, that is `Update(0,0)` against zeroed tokens, and `Update`'s guard `arg2 != m_OpenPageToken && arg3 != m_OpenTabToken` is false → **no switch**. Confirmed. |
| SHOULD-FIX 7 — reader skips malformed entries | ✔ | `ElementReader.cs:526` — `if (buttonId == 0u \|\| pageId == 0u) continue;` with the retail filter cited inline at `:520-525` (`InqProperty(0x30) @0x0046C3E5`, `InqProperty(0x31) @0x0046C3FC`). I re-read both sites: a missing `0x30` jumps `label_46c510` and a missing `0x31` jumps the same, both landing on `label_46c594` (next entry) without touching either hash. Match. |
Round-1 SHOULD-FIX 3's "a silently-empty table and a wholly-unresolvable table are
indistinguishable" is also addressed: `UnresolvedEntries` (`:118`) plus a `[D.2b]`
diagnostic line (`:154-157`).
Two retail behaviours in `SetupTabPageHash` remain unported and are now *newly*
visible because the rework reads that function properly — both inert on today's data,
both NOTE (§3).
### 1.2 `UiCheckboxBitfield64` — **CLOSED on all three named sub-checks**
**(a) Rows built from the authored template via `AddItemFromTemplateList(0)`.** ✔
`UiCheckboxBitfield64.AddChild` (`:140-180`) resolves `Templates[0]` through
`TemplateResolver`, finds `0x10000219` by recursive descendant search
(`FindCheckboxRecursive`, `:182-192`), and stamps label/checked/click onto that
widget. The synthesized `ElementInfo{Type=1}` and the invented `RowHeight = 14f` are
**deleted**; `BuildCheckboxBitfield64` is deleted from the factory and the arm is now
`0x10000044u => new UiCheckboxBitfield64(info.TemplateList)`
(`DatWidgetFactory.cs:162`). Row geometry now comes from `row.Height`
(`:173-176`) — the authored 272×20.
This matches retail `CreateChildren @0x00485DF0` as I re-read it this round:
`AddItemFromTemplateList(&this->vtable, 0, nullptr)` per row → `GetChildRecursive(row,
0x10000219)` → `DynamicCast(1)` → `SetAttribute_Int(cb, 0x10000084, i)` →
`SetStringInfo` → `SetTooltip`.
**Fixture-verified independently** (I decoded `options_2100002B.json` myself, not the
commit message): `0x10000520` is `T=0x10000044`, 272×100, **zero** children, empty
`StateMedia`, and authors exactly
`TemplateList = [{0x2100002B, 0x10000521}]`; `0x10000521` is `T=3`, 272×20, one child
`0x10000219`. The conformance test pins that exact pair
(`OptionsPanelLayoutConformanceTests` `…Bitfield64Template_BuildsAsEmptyUiCheckboxBitfield64`).
**(b) `IsSet` is retail's ANY-bit predicate.** ✔ `:200-201` —
`(CurrentLow & lowMask) != 0 || (CurrentHigh & highMask) != 0`. Matches
`Refresh @0x00485A11`: `ebx = ((eax_2 | ecx_2) != 0) ? 0 : 1` →
`SetAttribute_Bool(cb, 0xE, ebx == 0)`.
**(c) A multi-bit test that actually discriminates.** ✔
`UiCheckboxBitfield64_MultiBitMask_IsSet_UsesAnyBitNotAllBits` seeds current
`0x00000040` against Combat's real two-bit mask `0x00600040` and asserts
`row.Selected == true`. Under the old all-bits predicate `IsSet` returns false and
`Assert.True(row!.Selected)` fails — the test genuinely discriminates, unlike the
single-bit `Error` case it replaced (which is retained and now also pins that the row
resolves to `TemplateCheckboxElementId`). It further pins that clicking clears
**both** mask bits, i.e. the AND-NOT half.
`ToggleRow` (`:209-224`) is unchanged and remains correct: `turnOn = !IsSet(...)`
derives from the bitfield, not from `Selected`, so a `ToggleBehavior` self-toggle on
the resolved `UiButton` cannot double-flip it.
### 1.3 TS-72 retirement — **PARTIALLY CLOSED (the MUST-FIX)**
The retirement itself is legitimate. TS-72's stated unknown was the click-toggle bit
math; that is now decomp-confirmed
(`ListenToElementMessage @0x00485AE0` → `BitUtils::SetBitsOnOrOff @0x004857D0`:
`arg3 != 0` → OR-in both halves, else AND-NOT both halves). The retirement note at
`docs/architecture/retail-divergence-register.md:350` states plainly that the toggle
half was always right and the checked-state half was wrong and is now fixed. Accurate,
and written per register convention (struck row retained with its original text; the
section header's active count moved 41 → 40).
**What was swept:** the note ends "nothing about its own click/checked mechanism
remains genuinely unverified". Verified is not the same as ported. Reading `Refresh`
in full (§2.2) shows a decomp-confirmed, unported presentation half of the checked
mechanism, on the same element whose "authors nothing" claim caused round 1's REJECT,
with the two authored DIDs it consumes still sitting unread in the fixture. That gap
has no row anywhere in the register.
AD-73 is correct as far as it goes but is scoped to `UiTabPanel` only (§1.4).
### 1.4 The dormancy shape — **CLOSED**, with one undocumented shape change
**Overrides cannot leak pre-activation.** I enumerated every member both subclasses
add or override:
| | `UiTabPanel` | `UiTemplateListBox` |
|---|---|---|
| base | `UiDatElement` (`:79`) | `UiDatElement` (`:81`) |
| `OnDraw` | not overridden → `UiDatElement`'s authored-media draw | not overridden |
| `ClickThrough` | base ctor sets `true` (`UiDatElement.cs:138`) | same |
| `IUiDatStateful` | inherited unchanged | inherited unchanged |
| `ConsumesDatChildren` | **not** overridden → `false`, children recurse | **overridden `true`** (`:145`) ← the one change |
| `AddChild` | not overridden | not overridden |
| import-time hook | `OnChildrenAttached` is an **empty body** (`:126-128`) | none |
| side-effecting members | `ActivateTabBehavior()` / `SwitchTo()` — explicit calls only | `AddItemFromTemplateList()`; `Scroll` getter materialises the viewport (`:107`) |
`UiDatElement` itself changed only `sealed` → `public class` (`:43`) and `_info` →
`protected readonly ElementInfo Info` (`:57`) plus mechanical renames. No behavioural
edit in the base. ✔
**The one shape change:** `UiTemplateListBox.ConsumesDatChildren => true` is *not*
identical to the pre-OP2 fallback, which recursed into dat children. I verified the
class doc's justifying claim myself by walking every committed fixture: **every**
Type-5 element in the corpus has zero authored children —
`character 0x1000023D` / `0x10000532`, `effects_positive`/`effects_negative
0x10000123`, `examine 0x10000149` / `0x10000335` / `0x1000032D`,
`mini_game 0x10000174`, `spellbook 0x10000464`, and the three Options ListBoxes
`0x100001FA` / `0x10000200` / `0x1000050D`. So the change is inert today and the claim
holds. It is a latent divergence (a future Type-5 that authors children loses them
silently), and it is not mentioned in AD-73 or anywhere in the register — NOTE, §3.
**Tab-table entry counts survived the new `0x30`/`0x31` skip** — character 3, combat
8, spellbook 2, vendor 3, options 4, i.e. the same counts round 1 recorded. The filter
is inert on real data, exactly as retail's is.
**AD-73 states the adaptation truthfully.** Register `:108`. I checked each claim:
- "Retail … unconditionally activate the authored default page for ANY instance that
carries a tab table" — correct, and I confirmed the trigger this round:
`SetupTabPageHash` has exactly one caller, `UIElement_Panel::OnSetAttribute
@0x0046C6DC`, fired when property `0x2E` is applied. Per-instance, unconditional.
- Oracle citations `SetupTabPageHash @0x0046C2E0`, `Update @0x0046BD00`,
`OpenTab @0x0046BE20` — all three verified present at those addresses in
`acclient_2013_pseudo_c.txt`.
- The four named hosts and their controller ownership match the fixture decode
(character `0x10000227` tab=3, spellbook `0x100002A8` tab=2, vendor `0x100000B8`
tab=3, combat `0x100000A2` tab=8, plus the tab-table-less vendor backdrop
`0x1000008D`).
- The Risk column names the real failure mode (a forgotten `ActivateTabBehavior()`
leaves every page slot visible at once, silently). Accurate.
AD-73 covers only `UiTabPanel`. `UiTemplateListBox`'s dormancy (lazy viewport +
`ConsumesDatChildren`) rides on the same adaptation and is unregistered — NOTE, §3.
### 1.5 Reader-level tests — **CLOSED**
`tests/AcDream.App.Tests/UI/Layout/ElementReaderTests.cs:354-481` adds five tests that
build a synthetic `ElementInfo`, stuff a raw `UiPropertyValue` into
`States[DirectStateId].Properties.Values[0x2E/0x64/0x72]`, and call
`ElementReader.ApplyCanonicalLegacyProjection(info)` directly.
Would they catch what the fixture tests miss? **Yes, decisively.** The fixture tests
deserialize `ElementInfo.TabTable`/`TemplateList`/`ScrollbarElementId` as already-
projected JSON fields; `LayoutImporter.Build` never re-runs the projection at load, so
deleting the body of `ReadTabTable` would not fail one fixture test. These five drive
the reader functions themselves. Coverage:
- `ReadTabTable_DecodesButtonPageDefaultInAuthoredOrder` — struct-member ids, authored
order, `0x32` present/absent.
- `ReadTabTable_SkipsEntriesMissingButtonOrPage` — the new retail filter, with a
well-formed entry *after* two malformed ones, so an early-`break` regression fails too.
- `ReadTemplateList_DecodesLayoutDidAndElementIdInAuthoredOrder` — `0x63`/`0x62` pair,
order.
- `…ScrollbarLinkage_DecodesEnumProperty` / `…AbsentPropertyStaysZero` — the `0x72`
value and its absent case.
Gap (NOTE, not a miss of the round-1 finding): none of the five exercises
`ReadStructMemberId`'s `Enum`/`DataId`/`Integer` kind coercion — every test uses
`Enum`. A regression narrowing that coercion would still slip through.
The blast-radius pins (`OP2ReworkBlastRadiusConformanceTests.cs`) are the other half
of the answer: six built-widget tests across the five pre-existing Type-8 elements and
one representative Type-5, asserting `BehaviorActive == false`,
`ActivePageElementId == 0`, `ClickThrough == true`, non-zero `ActiveMedia()` on the
vendor backdrop, 16 children still present and all `Visible` on combat, and
`Empty(Children)` + `ContentHeight == 0` on the effects list. Those are exactly the
properties the dormancy model rests on.
---
## 2. New mechanism infidelity introduced or newly exposed by the rework
### 2.1 **MUST-FIX** — `Refresh`'s all-set branch is unported, its two authored DIDs are unread, and neither the doc comment nor the register says so
`UIOption_CheckboxBitfield64::Refresh @0x004859C0`, in full, per row:
```
lowHit = m_current.low & mask.low; // eax_2 @0x004859F9
highHit = m_current.high & mask.high; // ecx_2 @0x004859FB
allSet = (lowHit == mask.low && highHit == mask.high); // var_d_1 @0x004859FF..0x00485A0A
ebx = ((lowHit | highHit) != 0) ? 0 : 1; // anySet @0x00485A11
row = UIElement_ListBox::GetItem(this, i); // @0x00485A21
cb = UIElement::GetChild(row, 0x10000219); // @0x00485A35
UIElement::SetAttribute_Bool(cb, 0xE, ebx == 0); // checked <=> ANY set ← ported ✔
if (ebx == 0) { // @0x00485A51 — only when ANY set
led = UIElement::GetFirstChildElement(cb); // @0x00485A5A (0x10000328)
if (led) {
if (allSet && GetAttribute_DataID(this, 0x10000082, &v)) // @0x00485A69/0x00485A7C
UIElement::SetMediaImageForState(led, v, 1, 6); // @0x00485AA7
else if (GetAttribute_DataID(this, 0x10000083, &v)) // @0x00485A9A
UIElement::SetMediaImageForState(led, v, 1, 6);
}
}
```
Retail therefore renders **three** row states, not two: unchecked, checked-partial
(`0x10000083`), checked-complete (`0x10000082`). Eight of the thirteen Chat filter
masks are multi-bit (Gameplay `0x83912021`, Combat `0x00600040`, Magic `0x00020080`,
AreaSpeech `0x00001004`, Tells `0x00000018`, Allegiance `0x00040C00`, …), so the
partial state is reachable in ordinary play, not a corner case.
Evidence of the gap:
- `src/AcDream.App/UI/UiCheckboxBitfield64.cs:194-201` quotes `Refresh` as
`checked = (lowHit | highHit) != 0` and stops there — the `allSet` computation and
the entire media branch are absent from both the code and the citation.
- `RefreshRowVisuals` (`:226-230`) writes only `row.Toggle.Selected`.
- `grep -i '0x10000082\|0x10000083\|10000328\|0x10000084' src/AcDream.App/**/*.cs`
returns exactly **one** hit, a prose mention at `UiCheckboxBitfield64.cs:28`. The two
DIDs are read by no code path; `ElementReader` does not project them onto
`ElementInfo` either.
- They are unambiguously authored: decoded from `options_2100002B.json`, element
`0x10000520` DirectState carries `P0x10000082 = DataID(0x06004D17)` and
`P0x10000083 = DataID(0x06004D19)` alongside `P0x64`.
- `docs/architecture/retail-divergence-register.md` gained AD-73 only. No AP/TS row
names this.
Adjacent and in the same fix: retail's `CreateChildren` tail is
`ResizeTo(GetWidth(), CalculatePaperSize(0, -1))` at `0x00485E87`/`0x00485E97` — the
block sizes itself to its rows because it is a non-scrolling `UIElement_ListBox`
nested inside the Chat page's scroller. `UiCheckboxBitfield64` tracks `_contentHeight`
(`:53`, `:174-175`) but never applies it to `Height`, so the block stays at its
authored 272×100 while thirteen 20 px rows need 260 px. Also unported, also unrowed.
**Required:** one AP row covering both (unported `Refresh` media branch + unported
`ResizeTo`/`CalculatePaperSize`), naming `0x10000082`/`0x10000083`/`0x10000328` and
pointing at OP5 as the consumer gate; and a corrected `IsSet` doc comment that quotes
`Refresh` completely instead of half. The port itself can land at OP5 with the
consumer — nothing user-reachable observes it today.
### 2.2 SHOULD-FIX — `AddChild` drops retail's tooltip and per-row index stamp
`CreateChildren @0x00485E50`/`@0x00485E65` calls `SetStringInfo(cb, siLabel)` **and**
`SetTooltip(cb, siTooltip)`; `@0x00485E3E` stamps `SetAttribute_Int(cb, 0x10000084,
i)`. acdream sets `checkbox.Label` (`:169`) and stores `tooltip` in the `Row` record
(`:178`) but never applies it; there is no settable tooltip on `UiElement` (only a
virtual `GetTooltipText()` returning null, `UiElement.cs:454`), so this needs a
one-line override or a settable property. The `0x10000084` index stamp is *correctly*
replaced by a mask closure (`:171`) — equivalent and better typed; call that out in
the doc rather than leaving it silently absent.
### 2.3 SHOULD-FIX — the ListBox mechanism is now implemented twice, divergently
`UIOption_CheckboxBitfield64` **is** a `UIElement_ListBox` in retail (it calls
`AddItemFromTemplateList`/`GetItem`/`CalculatePaperSize`/`ListenToElementMessage` on
itself; `PostInit` tail-calls `UIElement_Scrollable::PostInit`). The rework's class
doc argues against reusing `UiTemplateListBox` because "retail's block does not itself
scroll" — an assertion with no citation, and one the `CalculatePaperSize`/`ResizeTo`
tail partly contradicts (the block *does* do ListBox self-sizing). The result is two
independent row-stacking implementations that already differ: `UiTemplateListBox`
stacks into a lazily-created `UiScrollablePanel`
(`UiTemplateListBox.cs:172-187`), `UiCheckboxBitfield64` stacks directly onto itself
(`:173-176`). Not a defect today; it is the seam where §2.1's `ResizeTo` gap and any
future ListBox fix will diverge. Either reuse the class or record why in the register
alongside §2.1's row.
---
## 3. NOTES
1. **`UiTemplateListBox.ConsumesDatChildren => true`** (`:145`) is the one place the
dormant subclass is not byte-identical to the `UiDatElement` fallback. Inert today
(all ten pre-existing Type-5 elements have zero authored children — verified across
every committed fixture), unregistered, and silent if a future Type-5 authors
children. One sentence in AD-73 or its own row.
2. **`UiTemplateListBox.Scroll`** (`:107`) is a side-effecting getter — reading it
materialises and attaches the viewport, breaking dormancy without any explicit
activation call. `ContentHeight` (`:111`) and `LineHeight` (`:116-124`) are both
null-safe; `Scroll` is the odd one out. Nothing reads it today.
3. **AD-73 scopes dormancy to `UiTabPanel` only.** `UiTemplateListBox`'s lazy viewport
is the same adaptation against retail's always-scrollable ListBox and rides
uncovered.
4. **`SetupTabPageHash`'s early return is unported.** At `0x0046C4EE`/`0x0046C4F5`
retail does `GetChildRecursive(this, tabId)` and, on a miss, **returns outright** —
abandoning the remaining entries and never calling `Update`. `UiTabPanel` logs the
miss, keeps iterating, and still performs the default switch (`:149-173`). Inert on
today's data (every options entry resolves) and arguably the better diagnostic, but
it is a divergence the round-1 SHOULD-FIX 3 response introduced.
5. **`tab != page` and `MasterPropertyId == 0x2F` filters still unimplemented**
(`0x0046C3A6`, `0x0046C46B`). Retail adds the hash entries either way and only
gates the default-flag read and `SetMouseVisible(1)` on `tab != page`. Harmless
today. Carried from round-1 NOTE 12.
6. **`SetMouseVisible(1)` per tab button** (`0x0046C4FB`) and
**`BroadcastElementMessage(this, 0x2C, …)`** after each `Update` remain unported.
OP3 needs the second for research §3.6's apply/revert semantics. Carried from
round-1 NOTE 14.
7. **`0x71` (horizontal scrollbar) still unread**; `ScrollbarElementId` is specifically
the *vertical* link (`UIElement_Scrollable::OnSetAttribute @0x00474630`) and is
still not named as such. Carried from round-1 NOTE 13.
8. **`SaveCurrentValue`/`RestoreSavedValue`/`Changed`** (`@0x004856A0`–`@0x00485700`)
are unported on `UiCheckboxBitfield64` — the Apply/Cancel triad OP5 needs. Not
introduced here; `SetDefaultValue`/`RestoreDefaultValue` shipped in OP2 as-is.
9. **`SpriteResolve` and `LabelFont` are now dead** on `UiCheckboxBitfield64` — the
factory no longer sets `SpriteResolve` (the row subtree carries its own resolver)
and `LabelFont`'s own doc says it is unused (`:80-82`). Harmless; delete at OP5.
10. **The campaign plan is stale.**
`docs/plans/2026-08-10-options-panel-campaign.md:181` still says "Create
`src/AcDream.App/UI/UiTabControl.cs`" — the exact fictional-class name this round
corrected — and still cites "lane A §5" for the switching contract when the tab
table is §1.3 and its host §1.4 (round-1 NOTE 18, unaddressed). OP3/OP8 will read
that line.
11. **Reader tests use only `Enum`-kind struct members**, so `ReadStructMemberId`'s
`Enum`/`DataId`/`Integer` coercion tolerance is unpinned.
12. **`IUiChildrenAttachedListener` now has exactly one implementer with an empty
body** (`UiTabPanel.cs:126-128`). Still the right hook for OP3's activation
plumbing; just no longer load-bearing in this commit.
13. Research §1.5's "`0x10000521` … UNKNOWN consumer" / §10.5 U10 are now closed by
the fixture + `CreateChildren`, and the research doc still says UNKNOWN. Carried
from round-1 NOTE 17.
---
## 4. What I re-derived independently this round
Everything below was read from primary source, not taken from the commit message:
| Claim | Source |
|---|---|
| `SetupTabPageHash` has exactly one caller, `OnSetAttribute` on property `0x2E` | `acclient_2013_pseudo_c.txt` `0x0046C6C0`–`0x0046C6DC` (grep: 2 hits total for the symbol) |
| Zeroed tokens + `Update(0,0)` ⇒ no default switch | `0x0046C312`, `0x0046C318`, `0x0046C31E`, `0x0046C4DF`, `0x0046C5EB`; `Update` guard `0x0046BD00` |
| Missing `0x30`/`0x31` skips the entry | `0x0046C3E5`, `0x0046C3FC` → `label_46c510` → `label_46c594` |
| `Refresh` computes all-set AND any-set, and uses all-set for the LED media swap | `0x004859F9`–`0x00485AA7` (full function read) |
| `SetBitsOnOrOff` = OR-on / AND-NOT-off | `0x004857D0`–`0x0048580B` |
| `CreateChildren` = per-row `AddItemFromTemplateList(0)` + `GetChildRecursive(0x10000219)` + index/label/tooltip + `ResizeTo(CalculatePaperSize)` | `0x00485DF0`–`0x00485E97` |
| `0x10000520` authors `P0x64 = [{0x2100002B, 0x10000521}]`, `P0x10000082 = 0x06004D17`, `P0x10000083 = 0x06004D19`, zero children, empty StateMedia | decode of committed `options_2100002B.json` |
| `0x10000521` is `T=3`, 272×20, one child `0x10000219` | same |
| Every Type-5 element in every committed fixture has zero authored children | walk of all 32 committed fixtures |
| Tab-table entry counts unchanged post-regeneration (character 3 / combat 8 / spellbook 2 / vendor 3 / options 4) | same walk |
| Vendor `0x1000008D` is `T=8` with no tab table (the backdrop) | same walk |
Fixture decodes used a throwaway reader in the scratchpad. Nothing in the repository
was modified except this document.

11
src/AcDream.App/UI/UiButton.cs
View file

@ -88,6 +88,17 @@ public sealed class UiButton : UiElement, IUiGlobalTimeListener, IUiDatStateful
/// <summary>Label color (default white).</summary> /// <summary>Label color (default white).</summary>
public Vector4 LabelColor { get; set; } = Vector4.One; public Vector4 LabelColor { get; set; } = Vector4.One;
/// <summary>Settable tooltip, surfaced through the shared
/// <see cref="UiElement.GetTooltipText"/> hover pipeline (same pattern as
/// <see cref="UiCatalogSlot"/>). Retail option rows stamp theirs via
/// <c>UIElement::SetTooltip</c> during row construction
/// (e.g. <c>UIOption_CheckboxBitfield64::CreateChildren @0x00485DF0</c>).</summary>
public string? TooltipText { get; set; }
/// <inheritdoc />
public override string? GetTooltipText() =>
string.IsNullOrWhiteSpace(TooltipText) ? null : TooltipText;
/// <summary>Retail LayoutDesc property <c>0x21</c> (two-pass glyph outline, /// <summary>Retail LayoutDesc property <c>0x21</c> (two-pass glyph outline,
/// <c>UIElement_Text::SetOutline @0x0046a81c</c>). Seeded by DatWidgetFactory /// <c>UIElement_Text::SetOutline @0x0046a81c</c>). Seeded by DatWidgetFactory
/// from the element's effective-default state, same as <see cref="UiText.Outline"/> /// from the element's effective-default state, same as <see cref="UiText.Outline"/>

13
src/AcDream.App/UI/UiCheckboxBitfield64.cs
View file

@ -167,6 +167,13 @@ public sealed class UiCheckboxBitfield64 : UiPanel
} }
checkbox.Label = label; checkbox.Label = label;
// Retail CreateChildren @0x00485DF0 stamps the row tooltip via
// SetTooltip (OP2 re-review §2.2). Its @0x00485E3E companion — the
// SetAttribute_Int(cb, 0x10000084, i) row-INDEX stamp retail later
// reads back to find which row a click meant — is deliberately
// replaced by the typed mask closure below: equivalent routing,
// without a stringly attribute round-trip.
checkbox.TooltipText = tooltip;
checkbox.Selected = IsSet(lowMask, highMask); checkbox.Selected = IsSet(lowMask, highMask);
checkbox.OnClick = () => ToggleRow(lowMask, highMask, checkbox); checkbox.OnClick = () => ToggleRow(lowMask, highMask, checkbox);
@ -195,7 +202,11 @@ public sealed class UiCheckboxBitfield64 : UiPanel
/// Retail <c>UIOption_CheckboxBitfield64::Refresh @0x004859C0</c>: a row is checked /// Retail <c>UIOption_CheckboxBitfield64::Refresh @0x004859C0</c>: a row is checked
/// when ANY bit of its mask is set in the current value — NOT when every bit is /// when ANY bit of its mask is set in the current value — NOT when every bit is
/// set. <c>lowHit = current.low &amp; mask.low; highHit = current.high &amp; /// set. <c>lowHit = current.low &amp; mask.low; highHit = current.high &amp;
/// mask.high; checked = (lowHit | highHit) != 0</c>. /// mask.high; checked = (lowHit | highHit) != 0</c>. This is HALF of Refresh:
/// the same pass also computes the ALL-set predicate to swap each row's LED
/// media between the two authored surfaces (<c>P0x10000082 = 0x06004D17</c> /
/// <c>P0x10000083 = 0x06004D19</c>) — not ported yet, register row AP-195,
/// due with the Chat tab (OP5) alongside the ListBox self-sizing tail.
/// </summary> /// </summary>
private bool IsSet(ulong lowMask, ulong highMask) private bool IsSet(ulong lowMask, ulong highMask)
=> (CurrentLow & lowMask) != 0 || (CurrentHigh & highMask) != 0; => (CurrentLow & lowMask) != 0 || (CurrentHigh & highMask) != 0;

49
tests/AcDream.App.Tests/UI/Layout/OP2ReworkBlastRadiusConformanceTests.cs
View file

@ -1,3 +1,5 @@
using System.IO;
using System.Text.Json;
using AcDream.App.UI; using AcDream.App.UI;
using AcDream.App.UI.Layout; using AcDream.App.UI.Layout;
@ -154,4 +156,51 @@ public class OP2ReworkBlastRadiusConformanceTests
Assert.Equal(0, host.ContentHeight); Assert.Equal(0, host.ContentHeight);
Assert.True(host.ClickThrough); Assert.True(host.ClickThrough);
} }
/// <summary>
/// AD-73 addendum (OP2 re-review closure, 2026-08-11):
/// <see cref="UiTemplateListBox.ConsumesDatChildren"/> is <c>true</c> where the
/// pre-rework generic fallback's was not. That delta drops nothing ONLY while no
/// authored Type-5 element carries children — a premise the re-review verified by
/// hand across all 32 fixtures. This test turns that manual sweep into a build
/// gate: if a future DAT regeneration surfaces a Type-5 element WITH authored
/// children, this fails loudly instead of the children silently vanishing.
/// </summary>
[Fact]
public void EveryAuthoredType5Element_HasZeroChildren_AcrossAllFixtures()
{
string fixturesDir = Path.Combine(
AppContext.BaseDirectory, "UI", "Layout", "fixtures");
string[] fixtures = Directory.GetFiles(fixturesDir, "*.json");
Assert.NotEmpty(fixtures);
var opts = new JsonSerializerOptions { IncludeFields = true };
int type5Seen = 0;
foreach (string file in fixtures)
{
var root = JsonSerializer.Deserialize<ElementInfo>(
File.ReadAllText(file), opts);
Assert.NotNull(root);
Walk(root!, file);
}
// The premise is only meaningful if the sweep actually saw Type-5
// elements (16 across the current fixture set).
Assert.True(type5Seen >= 10, $"sweep saw only {type5Seen} Type-5 elements");
void Walk(ElementInfo node, string file)
{
if (node.Type == 5u)
{
type5Seen++;
Assert.True(
node.Children.Count == 0,
$"{Path.GetFileName(file)}: Type-5 element 0x{node.Id:X8} authors "
+ $"{node.Children.Count} children — UiTemplateListBox.ConsumesDatChildren "
+ "would drop them; see AD-73's addendum.");
}
foreach (ElementInfo child in node.Children)
Walk(child, file);
}
}
} }