acdream/docs/research/2026-08-11-op4-review-mechanism.md

Campaign OP slice OP4 — MECHANISM-FAITHFULNESS review

Commit under review: 22b86b9f "Campaign OP slice OP4 — the Character tab" Lens: mechanism faithfulness (retail behaviour vs. the port). The regression/blast-radius lens is a separate document. Contract: docs/plans/2026-08-10-options-panel-campaign.md §4 OP4 + §2 D3/D7. Oracles: docs/research/2026-08-10-character-options-map.md §2/§7.1; docs/research/2026-08-10-options-panel-structure.md §3/§6/§7/§10.4; docs/research/2026-08-10-set-character-options-wire.md; docs/research/named-retail/acclient.h; docs/research/named-retail/acclient_2013_pseudo_c.txt. Mode: read-only. No build, no test run, no client launch. The only file written is this one.

---

Verdict

APPROVE-WITH-FIXES — with one hard sequencing condition:

The connected gate must NOT be scheduled until MUST-FIX 1 lands. The Character tab's 50 LEDs are seeded once, at window startup, before any network session exists, from RuntimeCharacterOptionsState's constructor default word — so the panel cannot show the logged-in character's real options. Test-script items 3, 6, 7 and 8 all assert the opposite and will fail on the first run.

The substance of the slice is right and is the best-evidenced work in the campaign so far. The 50-row binding is exact, the DEFAULTS CORRECTION is independently verified and the plan's own §1 note was genuinely wrong, the cross-page button-id collision is real and the scoped fix is correct, the Group-B/C consumers are correctly anchored, and the register bookkeeping is honest — including two corrections (TS-75, the U1 defaults source) where the implementer refused to wire a plausible-but-wrong seam. Both MUST-FIX items are contained, retail-anchored, and touch three files.

Class	Count	Items
MUST-FIX	2	MF-1 stale row seed (never reads live option word); MF-2 Apply/Reset enable-gating not wired
SHOULD-FIX	3	SF-1 timestamp culture + over-claimed "byte-verified"; SF-2 test gaps (widget click path, header hashes); SF-3 test-script false-failure in item 8 + the enable-gate items
NOTE	5	N-1 FindCheckbox ignores the retail id; N-2 FindDescendant semantics changed on promotion; N-3 sky dome still fogs; N-4 AP-194 anchor mislabel; N-5 no refresh on server-side option echo

---

1. What I verified as CORRECT (evidence first)

These are recorded because several are load-bearing for later slices and because a future reader must not "rediscover" them as bugs.

1.1 The 50-row binding — 18 rows traced end-to-end

src/AcDream.App/UI/Layout/CharacterOptionsPageController.cs:101-186 (Groups) reproduces docs/research/2026-08-10-options-panel-structure.md §7's authored sequence exactly: group sizes 3 / 15 / 6 / 11 / 7 / 7+1, row-for-row in authored order, six header keys in authored order. Every RetailName matches acclient.h:4162-4218's literal enumerator spelling (I diffed all 49 against the header).

I traced 18 rows (the brief asked for 15) row → PlayerOption id → word/mask → auto-save flag → client default → wire route, against CharacterOptionTable (src/AcDream.Runtime/Gameplay/CharacterOptionTable.cs) and, independently, against the two byte-verified jump tables transcribed in character-options-map.md §1.3 (IsAutoSaveOption, VA 0x0059A62C) and §1.4 (GetDefaultOptionValue, VA 0x005D2A5C). Note both tables are inverted (0x00 → jump_table[0] → return 1); the table's columns decode correctly under that inversion in all 18 cases.

Row	id	word · mask	auto-save	ClientDefault	wire route
Keep Combat Targets in View	0x07	O1 0x80	no (idx 7 = 01)	false	dirty → 0x01A1
Salvage Multiple	0x22	O2 0x80	no	false	dirty
Use Main Pack as Default	0x29	O2 0x4000	no	false	dirty
Vivid Targeting Indicator	0x0E	O1 0x8000	no	true (idx 0x0E = 00)	dirty
Display 3D Tooltips	0x08	O1 0x100	no	true	dirty
Show Coordinates By the Radar	0x14	O1 0x400000	no	true	dirty
Disable Distance Fog	0x30	O2 0x200000	no (idx 0x30 = 01)	false (past table end)	dirty
Show Your Helm	0x2F	O2 0x100000	yes (idx 0x2F = 00)	false (past end)	0x0005
Show Your Cloak	0x32	O2 0x800000	yes	false (past end)	0x0005
Ignore Allegiance Requests	0x01	O1 0x04	yes	false	0x0005
Ignore Fellowship Requests	0x02	O1 0x08	yes	true	0x0005
Share Fellowship XP	0x0F	O1 0x40000	yes	true	0x0005
Accept Corpse Looting Permissions	0x10	O1 0x80000	yes	false	0x0005
Automatically Repeat Attacks	0x00	O1 0x02	yes	true	0x0005
Listen to Society Chat	0x2E	O2 0x80000	yes	false (past end)	0x0005
Display Timestamps	0x21	O2 0x40	no	false	dirty
Run as Default Movement	0x0A	O1 0x400	no	true	dirty
Listen to PK death messages	0x34	O2 0x02000000	no (ACE-sourced)	false	dirty — D3 wire+store only, AP-193

The click path is checkbox.OnClick (:381) → BoolOptionRow.SetCurrentValue → the row's apply closure (:371-380) → Bindings.SetOption → SetSingleCharacterOptionRuntimeCmd (RetailUiRuntime.cs:2029-2031) → LiveSessionCommandRouter.cs:173-177 → LiveSessionRuntimeFactory.cs:347-355 SendSingleCharacterOption → RuntimeCharacterOptionsState.TrySetOption (RuntimeCharacterState.cs:806-842), which writes the local bit first and then either sendAutoSave (0x0005) or MarkDirty(). That is retail's CPlayerModule::OnChanged @0x0059A8E0 step order exactly. The batched half flushes through OptionPage.AfterApply → SaveCharacterOptionsRuntimeCmd (RetailUiRuntime.cs:1987-1988) → TryFlush → SendSetCharacterOptions. ✅

D3's 50th row is wire+store only: no consumer, HearPkDeathMessages appears in no Group-B/C binding, and AP-193 carries the ACE-sourced mapping. ✅

The 50th row's label key is right: I computed compute_str_hash("ID_PlayerOption_HearPKDeaths") with the exact algorithm at DatStringResolver.cs:68-82 and got 0x0D16E9A3, matching structure doc §7's byte-verified string id. ✅

1.2 The DEFAULTS CORRECTION is correct — the plan's §1 "U1 closed" note was wrong

This is the highest-stakes claim in the commit and it holds. Read directly from the pseudo-C rather than from the implementer's summary:

acclient_2013_pseudo_c.txt:147331 — UIOption_Checkbox::SetPlayerOption @0x00486e80, guarded at 00486ea3 by if (this->m_propName == 0 && …), contains at 00486f2d:

this->m_default = PlayerModule::GetDefaultOptionValue((__return_1 + 4), this->m_playerOption);

and PlayerOptionPage::AddToggleOption(enum PlayerOption) @0x004F2A00 (:245616) calls SetPlayerOption then OptionPage::RegisterOption for every row on this tab. So the Character tab's m_default is GetDefaultOptionValue — i.e. CharacterOptionTable.ClientDefault, the table OP1 already ported. Structure doc §7's "biggest open unknown" is genuinely closed by this.

The plan's §1 note ("U1 closed: UIOption::InqDefaultGameplayOptionProperty @0x004ef8d0 … the Defaults button restores DAT-authored values") is mis-scoped, and the implementer's rejection of it is right. The pseudo-C has exactly one caller of InqDefaultGameplayOptionProperty — line :145204, inside UIOption_Slider::SetGameplayOptionProperty @0x00485030, the m_propName-bound path. The only three SetGameplayOptionProperty call sites in the whole binary are gmChatOptionsUI::InitOptions' two sliders (0x10000080, 0x10000081 at :170766, :170797) and its filter bitfields (0x1000007F at :170826, :170882). So the claim that DAT collection 0x78000001 holds only those three Chat-tab entries is consistent with the decomp by construction, and — more importantly — it is not load-bearing: every Character-tab row takes the m_propName == 0 PlayerOption branch, so the DBProperty mechanism cannot reach this tab regardless of what the collection contains. Had the implementer followed the plan's note, the Defaults button would have restored nothing at all (no PlayerOption-range keys exist in that collection).

AP-194's amendment (docs/architecture/retail-divergence-register.md:186) states this truthfully: it confirms the directive (use the ClientDefault column) was followed, cites the new independent trace, and names the Chat/Config m_propName mechanism as the thing that does not apply here. ✅

1.3 The cross-page button-id collision is real — verified from the fixture

Counted occurrences of each element id in the committed host fixture tests/AcDream.App.Tests/UI/Layout/fixtures/options_panel_2100006E_1000018D.json:

Element	Occurrences
0x100001FA Character ListBox	1
0x100001FB Character scrollbar	1
0x100001FC Apply	3
0x100001FD Reset	3
0x100001FE Defaults	3
0x10000200 Config ListBox	1

Three physical instances each of Apply/Reset/Defaults (Character, Chat, Config — matching structure doc §3.1's "identical geometry AND identical element ids", all base 0x21000040/0x100002CD). A flat ImportedLayout.FindElement would have wired all three pages' buttons to whichever instance the flat index happened to hold. The per-page scoped UiElement.FindDescendant(pageRoot, id) in OptionsPanelController.cs:214-236 is the correct fix and matches retail's own GetChildRecursive(<the gm*UI element>, 0x100001FC) scoping in PlayerOptionPage::OnOptionChanged @0x004F27D0. The Character ListBox and scrollbar ids are unique, so the flat lookups at CharacterOptionsPageController.cs:246,256 are safe. ✅

1.4 All six group-header keys hash to §7's byte-verified string ids

I ran DatStringResolver.ComputeHash's algorithm by hand over the six HeaderKey literals:

Key	Computed	structure doc §7
ID_CharacterOption_UIBehavior_Section	0x06489B6E	0x06489B6E ✅
ID_CharacterOption_UIDisplay_Section	0x0A9BC99E	0x0A9BC99E ✅
ID_CharacterOption_Grouping_Section	0x0CBAAFAE	0x0CBAAFAE ✅
ID_CharacterOption_OtherPlayers_Section	0x0872DFFE	0x0872DFFE ✅
ID_CharacterOption_CharacterBehavior_Section	0x08674D5E	0x08674D5E ✅
ID_CharacterOption_Chat_Section	0x0987FE8E	0x0987FE8E ✅

And all 49 ID_PlayerOption_<Name> / _Help label keys the controller constructs at :343,352 exist verbatim as globals in the retail binary (grepped out of acclient_2013_pseudo_c.txt; the truncated 32-char variants alongside them are the known BN preview artifact). No invented user-visible English anywhere — every label, tooltip and header resolves by name against table 0x23000003, and an unresolved string degrades to no text (:328-333, :346-351) rather than a fabricated caption. ✅

1.5 Separator cadence, template indices, and the LED click contract

Bind emits header → rows → separator per group (:264-277), i.e. 6 headers and 6 separators = structure doc §7's "5 interior separators + 1 trailing" (§7's PK-death note independently confirms the trailing AddSeperator after HearSocietyChat). Template indices 0/1/2 match §1.5's authored arrays, and index 2 is literally the constant in AddToggleOption's own AddItemFromTemplateList(m_pOptionBox, 2, nullptr) at :245616. ✅

The LED click contract works because the toggle template's checkbox 0x10000219 authors DAT property 0x0B = true (fixture options_2100002B.json:406,428-431 → UiButton.ToggleBehavior), so UiButton self-flips _selected at MouseUp (UiButton.cs:486) before the Click event invokes OnClick, and checkbox.OnClick = () => row_.SetCurrentValue(checkbox.Selected) (:381) therefore reads the new value. ✅ (See SF-2 — nothing pins this.)

1.6 Consumers

• Display Timestamps. Prefixed at RuntimeCommunicationState.AddText (:224-225) — retail's ClientSystem::AddTextToScroll @0x00563C50 chokepoint — after the both-ends trim and after the ClientLocal (0x1A) early return, which is exactly retail's order (the CH2 doc comment on the same method already records that retail "skips the timestamp prefix … for this type"). Format "H:mm:ss " is the correct .NET spelling of %#H:%M:%S (non-padded 24h hour, padded minute/second, trailing space). Prefix applies only when the bit is on and only to new lines. ✅ (See SF-1 for two caveats.)
• Disable Distance Fog. WeatherSystem.Snapshot forces FogMode.Off (WeatherState.cs:326-327), FogMode.Off == 0 (SkyState.cs:15), the UBO carries it as FogParams.w (SceneLightingUbo.cs:130), and both world shaders gate on it — mesh_modern.frag:48-49 and terrain_modern.frag:160-161 both if (mode == 0) return lit;. The shader-gate claim is real. ✅ (See N-3.)
• Run as Default Movement. DispatcherMovementInputSource.cs:84 computes Run: (_movement.RunAsDefaultMovement != walking) || AutoRunActive. This is byte-correct and better-evidenced than the commit claims: retail's CommandInterpreter::SetHoldRun @0x006B3370 (:699110-699118) computes ecx = (hold_run == 0); edx_3 = (UITogglesRun() == 0); eax_3 = (ecx != edx_3) — an XOR of the modifier against the option, which is precisely the port's !=. ACCmdInterp::UITogglesRun @0x0058B180 (:405377) is PlayerModule::ToggleRun, i.e. option 0x0A, which is the id GameWindow binds (GameWindow.cs:676-678). With the option ON the expression collapses to the pre-OP4 !walking, so the default path is bit-identical. ✅
• Group C re-point. CharacterOptionCombatSettingsSource (LiveCombatAttackOperations.cs:31-63) reads GetOptionBit live for AutoTarget/AutoRepeatAttack/ViewCombatTarget and is substituted at both construction sites (SessionPlayerComposition.cs:770, FrameRootComposition.cs:599-603). VividTargetingIndicator, CoordinatesOnRadar, LockUI and AcceptLootPermits likewise (InteractionRetainedUiComposition.cs:286-288,485-487,687-694, LivePresentationComposition.cs:840-847). Toggling writes through TrySetOption (local bit first, then wire) per CH3 — verified for the two non-panel entrances too: ToggleUiLock and SetAcceptLootPermits now both call SendSingleCharacterOption before the legacy settings mirror (LiveSessionRuntimeFactory.cs:394-401,476-481), which closes the two divergences character-options-map.md §0 named. ✅

1.7 Register bookkeeping

TS-73's narrowing is truthful and appropriately scoped — it explicitly says neither closed case lives inside TrySetOption's own switch, so the row's literal title claim stays accurate while the user-observable symptom is recorded as fixed for 0x07/0x30 only.

TS-75's ForcedDayGroupIndex correction is verified: RuntimeWorldDayGroupDefinition(Name, ChanceOfOccur, SkyObjectCount, Sky) (src/AcDream.Runtime/World/RuntimeWorldEnvironmentState.cs:5-14) is a probability-weighted weather-variety group from Region 0x13000000; the field's only consumers are RuntimeOptions.cs:55,122 / GameWindow.cs:639 env-var plumbing. Wiring PersistentAtDay to it would have silently changed the character's weather odds while leaving night falling — the row is right to call that "an incorrect fix masquerading as a correct one". TS-76..TS-80 are each honest no-consumer rows with the correct retail anchors; TS-80 correctly identifies FellowshipShareXP as the one client-sourced option whose packet field, not the stored bit, is what ACE reads. Counts are consistent. ✅

---

2. MUST-FIX

MF-1 — The 50 rows are seeded once at window startup, before login, and never re-read the live option word

The panel cannot display the logged-in character's options.

Chain of evidence:

1. CharacterOptionsPageController.Bind reads each row's initial value once, at build time: bool initial = bindings.CurrentValue(spec.Id); → checkbox.Selected = initial; → new BoolOptionRow(initial, …) (CharacterOptionsPageController.cs:368-380). Nothing re-reads it after that.
2. Bind is called from RetailUiRuntime.MountOptionsPanel (RetailUiRuntime.cs:2010), which runs inside Initialize() (:371) — i.e. once, when the retained UI is composed.
3. The retained UI is composed by the startup graph: GameWindow.OnLoad() (GameWindow.cs:1231) → GameWindowCompositionPipeline.Run(… InteractionRetainedUiCompositionPhase …) (:1326). This runs at window load, before any WorldSession exists.
4. Pre-login, RuntimeCharacterOptionsState holds its constructor defaults — _options1 = DefaultOptions1 (0x50C4A54A), _options2 = DefaultOptions2 (0x00948700) (RuntimeCharacterState.cs:645-647, 671-672). The class even carries a _hasServerSeed flag (:676) that the panel never consults.
5. So every LED shows ACE's character-creation default word — which is neither the character's real state nor even retail's own Defaults column (AP-194: the two disagree on 0x2D/0x2F/0x32).

Consequences on the connected gate as scripted:

• Item 3 ("confirm the checkboxes reflect your character's actual state … should show checked on open") — fails for every option the character has ever changed.
• Item 6 ("relog … rows should still read whatever you left them at (server echo)") — fails; the rows never see the echo.
• Items 7/8 (batched persist-vs-revert, judged by re-reading the row) — the row is not a valid readout of persisted state.

There is no wire corruption (TrySetOption no-ops on an unchanged bit, RuntimeCharacterState.cs:812-814, and Reset only touches Changed rows), and the consumers are all correct because they poll GetOptionBit live. The damage is confined to the panel's own display and baseline — but that is the entire deliverable of this slice.

Retail's own mechanism already fixes this, and the OP3 mechanism review predicted it. docs/research/2026-08-11-op3-review-mechanism.md:424-427 (note N2) reads: "SaveCurrentValue re-reads the widget in retail. 0x004868E5: m_current = vtable->GetValue(); m_saved = m_current;. BoolOptionRow.SaveCurrentValue commits its own _current. Equivalent only while every mutation goes through SetCurrentValue." OP4 is the slice that violates that precondition — the option word has many other writers (the PlayerDescription seed, /consent, @join, ToggleUiLock, headless commands, the fellowship mutual-exclusion recursion inside TrySetOption itself).

And UIOption_Checkbox::GetValue @0x00486f60 is not a widget-local read: character-options-map.md §1.2 establishes it branches m_playerOption != Invalid → PlayerModule::GetOption, i.e. the live server-synced option word. Since OptionPage.OnShown() => Apply() (OptionPageModel.cs:256) runs on the initial default-tab activation, on every tab switch in, and on every window (re)open, porting SaveCurrentValue faithfully makes the panel self-correct on exactly retail's schedule.

Fix. Give BoolOptionRow the GetValue reader it is missing and use it in SaveCurrentValue:

• add a Func<bool>? read ctor parameter (OP4 already has the exact reader: bindings.CurrentValue(spec.Id));
• SaveCurrentValue() becomes _current = _read?.Invoke() ?? _current; _saved = _current; — retail @0x004868E0 verbatim;
• push the re-read value onto the widget (retail's Refresh()), i.e. the row's existing apply-side checkbox.Selected = value needs an equivalent on the save path, or BoolOptionRow gains a refresh callback. Do not route the re-read through apply — that would send the value back on the wire.

Pin it with a test that changes the binding's underlying value behind the row and asserts the row and the checkbox both converge on the next OnShown(). Add a second test for the pre-login case (bind with the constructor default word, seed the state, show the page, assert all 50 rows match).

MF-2 — Apply/Reset enable-gating is never wired in production

Structure doc §3.4 (PlayerOptionPage::OnOptionChanged @0x004F27D0):

eax   = GetChildRecursive(<the gm*UI element>, 0x100001FC);   // Apply
eax_1 = GetChildRecursive(<the gm*UI element>, 0x100001FD);   // Reset
if (OptionPage::Changed(...) == 0) { SetState(0x0D) on both }  // disabled
else                               { SetState(0x01) on both }  // enabled

plus "PostInit calls InitOptions() then OnOptionChanged(0) so the buttons start disabled", and "Defaults (0x100001FE) is never enable-gated". §10.4's "Behaviours that MUST be ported, not improved" lists this as item 3.

OptionPage.OnOptionChanged exists (OptionPageModel.cs:204), fires as the last statement of Apply/Reset/Defaults (:227,240,250) and on every live edit via AttachPageNotify (:211) — but nothing in src/ ever assigns it. Grep across src/ and tests/: the only assignments are in OptionPageModelTests.cs. OP4 is the slice that wired the three buttons (OptionsPanelController.cs:214-236) and owns the first page that has them; OptionPageModel.cs:201-202 says in so many words "OP4-6 bind buttons to this seam".

Everything needed is already present: UiButtonStateMachine.Ghosted == 13 (= retail 0x0D) and Normal == 1 (= 0x01) (UiButtonStateMachine.cs:17,23), and UiButton.TrySetRetailState (UiButton.cs:209-223) already implements Ghosted → Enabled = false and the inverse. UiButton.OnEvent already refuses clicks when !Enabled (:494).

Fix. In OptionsPanelController.Bind's per-page loop, capture the resolved Apply and Reset UiButtons and set page.OnOptionChanged = () => { apply.TrySetRetailState(page.Changed ? UiButtonStateMachine.Normal : UiButtonStateMachine.Ghosted); reset.Try…; }, then invoke it once at bind time so the pair starts disabled (retail's PostInit tail). Leave Defaults untouched. Pin: (a) both start disabled on a freshly-bound page, (b) one LED click enables both, (c) Apply disables both, (d) Defaults leaves them enabled when it actually changed something and (e) the Defaults button is never disabled.

Without this the connected gate silently accepts a missing MUST-PORT behaviour — and the script's own "what to report" bullet ("Any Reset/Defaults button that stays permanently disabled") reads as though the author believed the gating shipped.

---

3. SHOULD-FIX

SF-1 — Timestamp: culture-dependent separator, an over-claimed "byte-verified", and the ignored per-character format string

Three separate problems in one two-line consumer (RuntimeCommunicationState.cs:224-225).

1. Culture. DateTime.Now.ToString("H:mm:ss ") uses the current culture, and : in a .NET custom date/time format string is the time-separator placeholder, replaced by DateTimeFormatInfo.TimeSeparator. On a culture that uses . (fi-FI and friends) both production output and the test's Assert.Matches(@"^\d{1,2}:\d{2}:\d{2} …") (tests/AcDream.Runtime.Tests/Gameplay/RuntimeCommunicationStateTests.cs, the OP4 block) change. Retail's CRT strftime emits a literal :. Use CultureInfo.InvariantCulture (or escape as H\:mm\:ss ) and make the test culture-explicit.
2. The "byte-verified" claim is wrong. The doc comment at RuntimeCommunicationState.cs:100-104 calls "%#H:%M:%S " "the byte-verified CRT strftime string". The wire oracle says the opposite: docs/research/2026-08-10-set-character-options-wire.md:941 open-unknown U6 — "The default timestamp format string "%#H:%M:%S " is BN-sourced, not byte-verified" (and :314-316 repeats it). The campaign's §8 process rule is that decomp claims are byte-verified against the PDB-paired binary; this one asserts verification the source explicitly withholds. Either verify it (the operand decode recipe is in structure doc's appendix) or downgrade the comment to "BN-sourced (wire doc U6)". Note U6's own "only matters when the Chat tab implements timestamp formatting" caveat is now stale — OP4 shipped the consumer.
3. The per-character format is ignored. Retail reads the format from GenericQualitiesData::InqString(m_pPlayerOptionsData, 1, &m_TimeStampFormat) (wire doc :311-313), and acdream's parser reads-and-discards it (:647, "timestamp string (0x80) | read, discarded | ❌ | never sent"). Hardcoding the constructor default is defensible, but it is a divergence with no register row. File one (or fold it into TS-77's neighbourhood).

SF-2 — Two conformance gaps in an otherwise strong test suite

The 52 tests are good — both-directions row conformance, the independently transcribed authored order, the ClientDefault pin, the tab-hide revert against real fixtures. Two things a mechanism reviewer would want pinned:

• The widget click path is never exercised. ClickingARow_PublishesSetOption_WithTheAuthoredId (CharacterOptionsPageControllerTests.cs, "Behavioral" section) calls boolRow.SetCurrentValue(true) directly. Nothing drives checkbox.OnClick, so nothing pins the ToggleBehavior dependency the whole LED interaction rests on (§1.5 above). If DAT property 0x0B on 0x10000219 ever fails to import, all 50 rows go inert and the suite stays green. Add one test that dispatches MouseDown/MouseUp/Click at the built checkbox and asserts both Selected and the published SetOption.
• Only one of the seven string keys is hash-pinned. HearPkDeathMessages_RetailNameHash_MatchesByteVerifiedStringId pins 0x0D16E9A3. The six header keys are not pinned, even though structure doc §7 gives all six byte-verified string ids. I verified all six by hand (§1.4) — they are correct today — but a typo in a HeaderKey literal would produce a silently blank header that only the user's eye catches. A six-row [Theory] is three minutes of work.

SF-3 — Test-script items that can generate a false failure, and items that test nothing

• Item 8 ("Toggle a batched row WITHOUT clicking Apply, then relog. The row should revert to its PRE-toggle value") is only true if the page is hidden first. Structure doc §3.5: CPlayerSystem::LogOffCharacter @0x00563520 calls SaveToServer(pm, 0) — logout flushes the dirty blob. In acdream the same holds (GameRuntime.cs:333, the shared flush body). A batched toggle left un-reverted therefore does persist. It happens to revert in the common path because reaching Exit Game requires switching to the Gameplay tab (→ OnHidden → Reset), but a user who closes the client another way will see the opposite of what the script promises. Add "close the Options panel (F11) first" as an explicit step, or state both outcomes.
• Items 10 and the "what to report" enable/disable bullets ask the user to judge Apply/Reset enable-gating that does not exist (MF-2). Once MF-2 lands, add an explicit step: "on first open of the Character tab, Apply and Reset are greyed out; click any LED and they light up; click Apply and they grey out again; Defaults is never greyed out."
• Item 3 as written is a good check and will correctly fail today — that is the MF-1 detector. Keep it exactly as-is.

Otherwise the section is thorough, states retail-expected outcomes per step, and its "Explicitly NOT in scope" block correctly pre-empts the OP3 R1 lesson for the nine no-consumer rows and the two weather/daylight rows.

---

4. NOTE

• N-1 — FindCheckbox ignores the retail element id. CharacterOptionsPageController.cs:384-391 returns the first UiButton among the row root's direct children. Retail is GetChildRecursive(row, 0x10000219) (:245616). Correct for today's single-child template, but the commit just promoted UiElement.FindDescendant for exactly this purpose — use it with the retail id and delete the bespoke helper.
• N-2 — FindDescendant's semantics changed during promotion. UiTabPanel's private version skipped the root; UiElement.FindDescendant (UiElement.cs:325-337) matches the root itself first. UiTabPanel calls it as FindDescendant(this, entry.ButtonElementId/PageElementId) (UiTabPanel.cs:162-163,205-208), so a tab host whose own id collided with a button/page id would now resolve to itself. No collision exists today (host 0x10000208 vs buttons 0x1000020D… and pages 0x10000211…), but the behaviour change rode along uncommented in a commit whose message describes the promotion as a pure move.
• N-3 — the sky dome still fogs when Disable Distance Fog is on. sky.vert:159-162 computes vFogFactor from uFogParams.xy with no mode == 0 guard, unlike the two world shaders. So distant terrain stops fading but the dome's horizon band still blends to fog colour. Retail's LScape::m_fFogEnabled = !value may or may not reach its sky path — not established either way. TS-73's "0x30 DisableDistanceFog … now behave correctly" is therefore slightly overstated; the gate script's item 14 wisely limits the expectation to "distant terrain/objects", so this is not a false-failure risk, only a register-precision one.
• N-4 — AP-194's anchor column mislabels the symbol. The prose correctly says UIOption_Checkbox::SetPlayerOption @0x00486e80; the "Retail anchor" column at the end of the row says PlayerModule::SetPlayerOption @0x00486e80. Same address, wrong class — worth fixing in a register that exists to be grepped by symbol name.
• N-5 — no refresh on the server's own option echo. Even after MF-1's GetValue re-read lands, a change made while the Character tab is visible (typing /consent on, a mid-session PlayerDescription, a plugin) will not repaint the LED until the next hide/show. Retail has CM_UI::SendNotice_PlayerOptionChanged / SendNotice_RefreshOptionsPanel (structure doc §3.7) and acdream already has the hook — LiveSessionRuntimeFactory's OnCharacterOptionsChanged (character-options-map.md §4.1). Wiring it is small; if it is deferred, file a register row rather than leaving it implicit.

---

5. Fix-order recommendation

1. MF-1 (BoolOptionRow GetValue re-read + widget refresh) — blocks the gate.
2. MF-2 (enable-gating) — blocks the gate's own acceptance criteria.
3. SF-3 (script items 8/10 + the new enable-gate step) — do this with MF-2 so the script and the code land consistent.
4. SF-1, SF-2 — same round, cheap.
5. N-1..N-5 — coordinator's call; N-4 is a one-word register edit and N-5 wants either a wire-up or a row before OP9's reconciliation.