Converged MUST-FIX: the six mouse-turning macro lines are typed ClientLocal 0x1A where retail's AddTextToScroll sites pass type 7 (Magic, light blue — mechanism lens decoded the stray [4]=7 writes at all six BN call sites); 0x1A additionally routes to the SpewBox where the 4-slot cap discards two of the six lines before they draw (blast lens). Fix: a typed message seam, Magic for the macro, ClientLocal retained for the mid-air refusal and AD-75; byte-verify the type argument during the fix. Mechanism: the Gameplay page is NOT an OptionPage in retail (gmGameplayOptionsUI : UIElement_Field, acclient.h:55857) — the auto-flush-on-visibility acdream gave it flushes the blob at moments retail would not, and two green tests pin the wrong shape; the OnOptionChanged enable-gating seam (@0x004F27D0 — Apply/Reset gated, Defaults never) is missing from the model; the mid-air refusal fires where retail is silent (non-player-mode/null controller). Blast: the plan-assigned docked-host (0x21000017) register ruling was not filed — deemed a divergence, row owed client-wide; Options is the only shared-geometry panel with ResizeX/four-edge/min-size, which the shared _mainPanelGeometry silently reverts when siblings show; three targeted seams lack pins (real-sibling mutual exclusion, DefaultQueueKey, dormant ActivePageChanged silence); UA/RA legibility flagged for the gate. Clean: exactly one toolbar button changed (previously ghosted), F11 same-action since K.1c, no queue key invented, settings additive. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
27 KiB
Campaign OP slice OP3 — MECHANISM-FAITHFULNESS review
Commit under review: 9d26ecc6 ("feat(ui): Campaign OP slice OP3 — Options
panel shell, open paths, Gameplay tab")
Contract: docs/plans/2026-08-10-options-panel-campaign.md §4 OP3 + §2
D1/D5/D6/D9
Oracles: docs/research/2026-08-10-options-panel-structure.md §5/§6/§10.4;
docs/research/2026-08-10-keyboard-config-and-gameplay-tab.md §1–§4/§7.1;
docs/research/named-retail/acclient_2013_pseudo_c.txt +
docs/research/named-retail/acclient.h (re-read directly for every decomp
claim the code's comments make).
Lens: mechanism faithfulness only (blast radius is the other lens).
Method: read-only. No build, no test run, no client launch.
VERDICT: APPROVE-WITH-FIXES
One MUST-FIX (a byte-verifiable wrong LogTextType on six user-visible chat
lines this slice ships), six SHOULD-FIX, eight NOTEs. The core ports —
OptionPage/BoolOptionRow, the mount, the two open paths, the seven-button
dispatch, the mouse-turning macro — are faithful and, unusually for a slice
this size, every element id and every retail anchor I re-checked held up.
0. What I verified as CORRECT (evidence first)
These are not filler: each one was a candidate finding that the source refuted.
0.1 OptionPage/BoolOptionRow vs the decomp — exact
Re-read at acclient_2013_pseudo_c.txt:245757 (OptionPage::SaveCurrentValues @0x004F2C60), :245783 (RestoreDefaultValues @0x004F2CB0), :245809
(RestoreSavedValues @0x004F2D00), :245834 (Changed @0x004F2D60),
:245374 (PlayerOptionPage::OnVisibilityChanged @0x004F26E0), :245390
(PlayerOptionPage::SaveCurrentValues @0x004F2710), :146926-146990
(UIOption_Checkbox::Changed/SaveCurrentValue/RestoreSavedValue/ RestoreDefaultValue/SetDefaultValue/SetCurrentValue).
| Contract item | Retail | OptionPageModel.cs |
Verdict |
|---|---|---|---|
| LED click applies immediately | 0x00486983-88: SetAttribute_Bool(0xE, v); vtable->Apply(1) |
:82-86 _current = value; _apply?.Invoke(value) |
✅ |
| Apply commits unconditionally | 0x004F2C71-93: no Changed test in the loop |
:178-183 plain foreach |
✅ |
| Apply flushes the blob | 0x004F271D SaveToServer(pm, 0) then tailcall |
:182 AfterApply?.Invoke() |
✅ |
| Reset reverts only Changed | 0x004F2D1E: if (ecx != 0 && vtable+0x2BC() != 0) |
:192 .Where(row => row.Changed) |
✅ |
| Reset applies live | 0x00486913 Apply(0) |
:90-94 re-invokes _apply |
✅ |
| Defaults applies live, no commit | 0x004F2CC1-E3 unconditional; 0x00486943 Apply(0); never touches m_saved |
:199-203 / :96-100 |
✅ |
| Defaults never disable-gated | 0x004F27D0 only fetches 0x100001FC/0x100001FD; 0x100001FE absent |
no gate exists in the model | ✅ |
| Hide reverts uncommitted | 0x004F2701 RestoreSavedValues |
:213 OnHidden() => Reset() |
✅ |
| Show applies + commits | 0x004F26F4 SaveCurrentValues |
:208 OnShown() => Apply() |
✅ |
Changed => _saved != _current matches 0x004868D7 exactly, and
OptionPage.Changed matches 0x004F2D7B's any-row short-circuit.
0.2 The mount — both claims independently confirmed from the fixtures
tests/AcDream.App.Tests/UI/Layout/fixtures/options_panel_2100006E_1000018D.json:1-6— rootId: 268435853=0x1000018D,Type: 8, 300 × 362, i.e. the import through host0x2100006Ereally does resolve to the base-merged tab host at the authored slot extent.- Same fixture
:583-587— the slot element itself authors"268435497"(=0x10000029, thegmPanelUIstack key) withUnsignedValue: 10. This corroborates research §10.1's "stack keyP0x10000029 = 10" from the DAT, not just from the decomp. tests/.../fixtures/toolbar_21000016.json:7869— element268435867=0x1000019Bauthors the same"268435497": UnsignedValue: 10and"18"(=P0x12)= 268435482(=0x1000001A).0x10000049(RestorePreviousPropertyId) is absent from the options slot (grep -c 268435529→ 0), soRegisterMainPanel(..., restorePrevious: false)is the correct read.
Is P0x10000029 the property RetailPanelUiController consumes, and did
P0x12 silently win? ToolbarController.cs:49 PanelIdAttribute = 0x10000029u; :170-175 collects (panelId, button) pairs from exactly that
property for the seven ids in PanelButtonIds (:54-55, which already
included 0x1000019B); RetailUiRuntime.BindToolbarPanelButtons :677-687
maps panel id → window name → ToggleWindow. P0x12 is consumed nowhere —
grep -rn "0x12u\b" src/AcDream.App/UI/ returns nothing, and neither
ElementReader, LayoutImporter, DatWidgetFactory nor UiButton reads an
authored input-action id. So there is exactly ONE route, no double-toggle, and
the implementer's "no new plumbing beyond the catalog entry" claim is accurate
(RetailPanelCatalog.cs:30,42 are the only two new lines). Retail's own
toolbar highlight path (gmToolbarUI::RecvNotice_SetPanelVisibility @0x004BD300, cited at ToolbarController.cs:395) is panel-id-based too, so
the surviving route is the retail-consistent one.
Mutual exclusion is real, not asserted: RetailUiRuntime.ToggleWindow
:597-600 routes any catalog-known name into _panelUi.TogglePanel(panelId),
and RetailPanelUiController.SetPanelVisibility :154-172 hides the previous
_activePanel before showing the requested one.
0.3 F11 — the binding really did pre-exist, in the right table
src/AcDream.UI.Abstractions/Input/KeyBindings.cs:216 sits inside
RetailDefaults() (opens :147), binding Key.F11 / None → InputAction.ToggleOptionsPanel, matching
docs/research/named-retail/retail-default.keymap.txt:148
(ToggleOptionsPanel [ "" [ 0 DIK_F11 ] ]). The commit changed only the
landing: GameplayInputCommandController.cs:238-246 now calls
_retained.ToggleOptionsPanel() (→ RetailUiRuntime.ToggleWindow( WindowNames.Options) → the panel controller) instead of the dead
_devTools.ToggleSettingsPanel(). ToggleSettingsPanel itself is untouched
and now unreachable from any action — exactly what D1/OP9 contract for.
0.4 The seven buttons vs research §6 / §1–§4
Element ids in OptionsPanelController.cs:62-68 (0x10000203, 0x10000204,
0x10000205, 0x10000206, 0x10000207, 0x100005CC, 0x10000617) and the
close button 0x10000210 (:58) match research §6's table and §10.1
one-for-one, and OptionsPanelControllerTests.cs:247-268 pins all seven
against the committed fixture (so a future regeneration that drops an id fails
the build rather than degrading to a logged no-op).
- Exit Game →
_bindings.Indicators.EndCharacterSessiondirectly (RetailUiRuntime.cs:1942) — no confirmation, no mid-air check. Matches §3:m_bLogoutConfirmedis set by the button itself andUseTimetakes theQueueUIMode(0x10000009)epilogue branch, which never reaches thetransient_statetest. - Exit to Character Selection → confirm FIRST, mid-air check on accept
(
:1846-1860). MatchesRecvNotice_EndCharacterSession(1)(dialog) →gmGamePlayUI::UseTime @0x004EA3A0(them_bLogoutConfirmed-gated drain that owns thetransient_state & 1test). Ordering is right. - The confirm text is DAT-resolved, not hard-coded:
ResolveEndCharacterSessionConfirmMessage():1834-1845resolvesID_Client_EndCharacterSessionConfirmthroughDatStringResolver.ComputeHashagainst table0x23000001. - Mid-air refusal typing is EXACTLY right. Retail:
ECM_UI::SendNotice_DisplayStringInfo(0x1A, "Cannot log off while in mid-air."). acdream:RetailLogTextType.ClientLocal=RetailLogTextType.cs:600x1A. Same constant. The string itself is byte-anchored at VA0x007C29C0inClientTextRefusals.cs. - Inert buttons are genuinely inert.
Bind:163-174never callsBindButtonfor0x10000204/0x10000205;BindButton:188-197early-returns on a null handler. Tests:220-245assert zero callbacks. No invented text anywhere in the slice.
0.5 The mouse-turning macro vs SetMouseTurningDefaults @0x0049E8F0
Re-read at acclient_2013_pseudo_c.txt:169512-169640. Every structural claim
holds:
| # | Retail condition / target | MouseTurningSettingsMacro.cs |
✓ |
|---|---|---|---|
| 1 | GetCurrentValue() != 0.949999988f → SetCurrentValue(0.95f) @0x0049E93C |
:60-65 vs MouseTurningTarget.Stiffness = 0.95f |
✅ |
| 2 | != 50f → 50f @0x0049E9AC |
:67-72 |
✅ |
| 3 | != 0.699999988f → 0.7f @0x0049EA1C |
:74-79 |
✅ |
| 4 | m_pSlope->GetValue() == 1 → SetCurrentValue(0) @0x0049EA69-77 |
:81-86 (target false) |
✅ |
| 5 | m_pInvert->GetValue() == 0 → SetCurrentValue(1) @0x0049EABB-C9 |
:88-93 |
✅ |
| 6 | m_pMouseTurn->GetValue() == 0 → SetCurrentValue(1) @0x0049EB0D-1B |
:95-99 |
✅ |
| tail | this->vtable->SaveCurrentValues() @0x0049EB57 |
RetailUiRuntime.cs:1908 ConfigPage.Apply() → AfterApply → SaveCharacterOptionsRuntimeCmd |
✅ |
The tail is faithful because gmConfigUI : PlayerOptionPage, gmNoticeHandler
(acclient.h:55846), so its SaveCurrentValues vtable slot IS
PlayerOptionPage::SaveCurrentValues → SaveToServer(pm, 0). acdream's
SaveOptions is dirty-AND-seed-gated
(RuntimeCharacterOptionsState.TryFlush :922-941), which is exactly
SaveToServer(force: 0)'s if (m_bDirty != 0 || arg2 != 0).
The five client-local vs one server-synced split is right, and
UseMouseTurning really is an auto-save id in OP1's table
(CharacterOptionTable.cs:160: Add(UseMouseTurning, false, 0x00400000u, true, false) → autoSave: true), so the SetSingleCharacterOptionRuntimeCmd
publication reaches TrySetOption and emits a 0x0005 immediately, after the
local bit write. Correct per lane C's split.
%f → .ToString("F6") is the right rendering (retail's %f default
precision is 6; the double promotions of 0.45f/0.55f/0.7f all round to
the same six decimals as .NET's float F6).
0.6 TS-74's "camera mode ABSENT" verdict — spot-check holds
src/AcDream.UI.Abstractions/Input/MouseLookState.cs:5-32 documents and
implements only retail's MMB-hold CameraInstantMouseLook (three transitions:
Press/Release/OnWantCaptureMouseChanged; ApplyDelta scales mouse-X into
a turn adjustment). There is no persistent "turn to face camera" mode. The only
other UseMouseTurning references in the tree are
GameplaySettings.cs:42/65, SettingsPanel.cs:366-368 (the unrendered ImGui-era
surface OP9 retires) and SettingsStore.cs:161/617 — i.e. storage, no consumer.
The ABSENT verdict is correct.
0.7 Register rows
AD-74, AD-75, AD-76, TS-74 are accurate against the code they cite,
carry the five-column shape plus retail anchors, and the section counts are
bumped correctly (AD 53 → 56, TS 40 → 41) with the header narrative
updated in the same hunk. No resurrection of retired rows in the diff (the
feedback_register_revert_resurrection check).
1. MUST-FIX
M1 — the six mouse-turning chat lines are typed 0x1A; retail types them 0x07
Where: src/AcDream.App/UI/RetailUiRuntime.cs:1906-1907 (foreach (string line in result.ChatLines) _bindings.Options.DisplaySystemMessage(line);) →
src/AcDream.App/Composition/InteractionRetainedUiComposition.cs:815-816
(DisplaySystemMessage: text => d.Communication.AddText(text, RetailLogTextType.ClientLocal)).
Retail, byte-anchored. All six emission sites in
gmConfigUI::SetMouseTurningDefaults call the same chokepoint with the same
type argument (acclient_2013_pseudo_c.txt:169512-169640, addresses
0x0049E97C, 0x0049E9EC, 0x0049EA5C, 0x0049EAAE, 0x0049EB00,
0x0049EB52):
0049e972 *(uint32_t*)((char*)var_2c)[4] = 7; <- BN's rendering of the pushed type
0049e97c ClientSystem::AddTextToScroll(ClientUISystem::GetUISystem(), var_2c, 1, 0);
The five-parameter signature is
ClientSystem::AddTextToScroll(this, char const* text, uint32 type, uint8 allowPluginFilter, uint32 windowId) (acclient_2013_pseudo_c.txt:148528), and
the fully-decoded sibling site three functions away renders all five:
0x004887B3 → AddTextToScroll(GetUISystem(), <"The key for Camera Zoom In has been…">, 7, 1, 0) (:148984). The stray = 7 line immediately preceding each
truncated call is the missing type argument in the same idiom. Type = 7.
0x07 is Magic / colorLightBlue #3FBFFF
(docs/research/2026-08-09-chat-retail-color-table.md:151). acdream ships
0x1A ClientLocal — bright red, and per that doc's §3.2 0x1A also
bypasses the timestamp prefix and the chat log file, which 0x07 does not.
So this is a colour divergence AND a routing divergence on six lines the OP3
gate script explicitly asks the user to eyeball (step 15).
Why this is MUST and not SHOULD: the answer is byte-verifiable and
unambiguous, the fix is one typed argument, the divergence is user-visible at
the connected gate the user is about to run, OptionsPanelText.cs's own doc
calls these "byte-verified retail string literals" (true of the text, not of
the typing), and no register row covers it — an undocumented deviation is,
per the register rules, "a bug twice over".
Fix: give the Options bindings a typed message seam (e.g.
Action<string, RetailLogTextType>, or a second delegate) and emit the six
macro lines as RetailLogTextType.Magic (0x07). Keep ClientLocal (0x1A) for
the mid-air refusal (§0.4 — that one is exact) and for the UA/RA adaptation
(AD-75 already records that retail used a native MessageBoxA there, so there
is no retail type to match).
2. SHOULD-FIX
S1 — the Gameplay tab is given an OptionPage; retail's Gameplay page is not one
Evidence. acclient.h:55857:
struct __cppobj gmGameplayOptionsUI : UIElement_Field, gmNoticeHandler — it
derives from neither OptionPage (acclient.h:54418) nor PlayerOptionPage
(:55739). Its class id is the Gameplay page slot's authored class
(UIElement::RegisterElementClass(0x10000029, gmGameplayOptionsUI::Create) at
acclient_2013_pseudo_c.txt:169244, and research §10.1 lists page class
0x10000029 for the Gameplay page). The three other pages do derive:
gmCharacterSettingsUI (:55745), gmChatOptionsUI (:55817), gmConfigUI
(:55846), all : PlayerOptionPage.
Consequence in retail: showing/hiding the Gameplay tab runs plain
UIElement::OnVisibilityChanged — no SaveCurrentValues, no
SaveToServer, no baseline traffic of any kind.
What OP3 ships instead. OptionsPanelController.cs:125-129 creates an
OptionPage for all four slots including GameplayPageId, each with the
same AfterApply, and RetailUiRuntime.cs:1944-1946 wires that AfterApply
to Publish(new SaveCharacterOptionsRuntimeCmd()). So:
controller.ActivateTabs()at mount (RetailUiRuntime.cs:1954) firesActivePageChanged(0 → 0x10000212)→GameplayPage.OnShown()→Apply()→ aSaveOptionspublication during UI mount, before login state exists. It is inert today only because OP1's M1 fix makesTryFlushrefuse beforeHasServerSeed(RuntimeCharacterOptionsState.cs:928).- Every later entry into the Gameplay tab publishes another
SaveOptions. After any batched (non-auto-save) Character-tab edit — i.e. from OP4 on — switching to the Gameplay tab flushes the0x01A1blob at a moment retail would not (retail waits for Apply, logout, or the 480 s timer).
The justification in the code is the inverse of the decomp.
OptionPageModel.cs:136-144 argues the empty page still flushes because
"retail's SaveCurrentValues flushes the blob regardless of whether THIS
page's own rows changed anything". That sentence is true — but only for a page
that is a PlayerOptionPage, which the Gameplay page is not. This is
precisely the 2026-08-05-c4-closeout-handoff.md failure class: a contract
asserting a mechanism that does not exist.
And two green tests pin the wrong shape —
OptionPageModelTests.cs:48 EmptyPage_Apply_StillInvokesAfterApply and
:63 EmptyPage_OnShown_InvokesAfterApply — the same "green tests pinning a
wrong shape" that killed the CH3 builder (plan §4 OP1 cites it).
Fix: do not construct an OptionPage for GameplayPageId (or construct it
with AfterApply = null and document why), leave OnActivePageChanged
tolerant of a slot with no page model (it already uses TryGetValue), and
retarget the two tests at a synthetic empty PlayerOptionPage-shaped page so
the model-level property is still pinned without asserting it about the
Gameplay tab. Amend the OptionPageModel XML doc's empty-page paragraph to
say gmGameplayOptionsUI is not an OptionPage at all.
S2 — OptionPage has no OnOptionChanged seam, so "Apply/Reset disable when clean" is only half-ported
Retail runs this->vtable->OnOptionChanged(0) as the last statement of all
three verbs — 0x004F2C95 (Apply), 0x004F2CE5 (Defaults), 0x004F2D4A
(Reset) — and UIOption::HandleDialogAndNotices @0x004EFB90 additionally calls
m_pOCH->OnOptionChanged(this) on a user click (Apply(1) only). The override
that matters, PlayerOptionPage::OnOptionChanged @0x004F27D0
(acclient_2013_pseudo_c.txt:245433-245455), is the ONLY thing that
enable-gates the buttons:
eax = GetChildRecursive(page, 0x100001FC); // Apply
eax_1 = GetChildRecursive(page, 0x100001FD); // Reset
if (OptionPage::Changed(page) == 0) { SetState(0x0D) on both } // disabled
else { SetState(0x01) on both } // enabled
0x100001FE (Defaults) is never fetched — which is exactly research §10.4's
point 3.
The contract (§4 OP3) lists "Apply/Reset disable when clean" as an
OptionPageModel requirement. The model ships only the Changed predicate
(:173) with no notification and no verb-tail hook, so nothing can react
to a value change without polling, and each of OP4/OP5/OP6 will have to invent
its own driver — three chances to get retail's call order wrong.
Fix (small, and it belongs here rather than in OP4): add
Action? OnOptionChanged { get; set; } to OptionPage, invoke it as the last
statement of Apply/Reset/Defaults, and give BoolOptionRow a
page-notification callback fired from SetCurrentValue only (retail's
Apply(1)-only path) — not from RestoreSavedValue/RestoreDefaultValue,
which retail calls with Apply(0) precisely so the page can notify once
itself.
S3 — IsGrounded's IsPlayerMode conjunction emits the mid-air refusal where retail emits nothing, and the comment says the opposite
InteractionRetainedUiComposition.cs:807-810:
// "on the ground" is meaningless outside player mode, so the mid-air
// refusal never fires while flying/spectating.
IsGrounded: () => d.PlayerMode.IsPlayerMode
&& d.PlayerController.Controller is { IsAirborne: false },
RetailUiRuntime.cs:1852-1859 takes the else branch — i.e. emits
"Cannot log off while in mid-air." — whenever IsGrounded() is false. With
IsPlayerMode == false (or a null Controller), IsGrounded() is false, so
the refusal does fire, which is the opposite of what the comment asserts.
Retail's drain (gmGamePlayUI::UseTime @0x004EA3A0, research §2.2) reaches the
transient_state & 1 test only inside else if (smartbox->player); with no
player it emits nothing at all.
The mapping itself is right where it matters —
PlayerMovementController.cs:441 IsAirborne => !_body.OnWalkable is exactly
retail's transient_state & 1 (ON_WALKABLE) inverted. It is only the
no-player fallback that misbehaves, and only by producing a misleading line.
Fix: either drop the IsPlayerMode conjunction from this binding (the
playerOnGround shape at :322-324 is a different question — there it gates
a positive action), or make the binding tri-state (bool?) so "no player"
maps to retail's silent no-op. Either way the comment must be corrected.
S4 — file the docked-gmPanelUI-host register row the plan delegated to this review
Plan §5: "The 0x21000017 docked gmPanelUI host variant — acdream ships the
floating host only (register row in OP3 if the review deems it a
divergence; retail exposes both)."
It is a divergence — retail authors both hosts for the same slot
(research §10.1) and a retail user can dock the Options panel; acdream can
only float it. grep -n "21000017\|docked\|gmFloatyPanelUI" docs/architecture/retail-divergence-register.md returns nothing, so there
is no existing row.
Two nuances for whoever writes it: (a) OP3 did not introduce this — every
gmPanelUI sibling already ships floating-only via
RetailWindowFrame.Mount + RegisterMainPanel, so the row should be scoped
client-wide (all registered main panels), not "the Options panel"; (b) it is
an AD (adaptation), not a TS.
S5 — TS-74's "five unwired preferences" is carried only in the Risk column
The row's What/Where is about the missing camera mode; the fact that all five
CameraTurningSettings values land in a brand-new settings.json
cameraTurning section that no subsystem reads appears only as a
parenthetical in the Risk column ("only the (unwired) preferences
persisting"). That has a concrete user-visible consequence the row does not
state: the chat lines quote a from value (0.550000,0.450000,40.000000
on a fresh profile — CameraTurningSettings.Default) that describes no live
client state, while acdream's actually-live sensitivity lives elsewhere
(MouseLookState.SensitivityPerPixel, F8/F9-adjustable).
Fix: extend TS-74's What/Where to name the five keys as store-only with no consumer (OP6 is their contracted home), so the row can be retired precisely rather than argued about later.
S6 — three gate-visible behaviours missing from the OP3 test script
docs/research/2026-08-11-campaign-op-test-script.md is otherwise strong
(15 steps, retail-expected outcome stated for each, INERT explicitly
contracted, "what to report" and "not in scope" sections). Gaps:
- The toolbar Options button changed state, not just behaviour. Before
this commit, panel id 10 was not in
RetailPanelCatalog, soBindPanelButtons'sisAvailable(10)returned false and the button was ghosted/disabled (ToolbarController.cs:379-381). Step 3 should say "the button is no longer greyed out, and highlights while the panel is open" (SetPanelOpen→Highlight/Normal,:393-405). - Nothing proves the
UseMouseTurning0x0005reached ACE. TS-74 says the bit is sent; the only user-observable proof is a relogin (after which a second click should emit no "Turn to Face Camera" line, becauseIsUseMouseTurningEnabledreads the server-seededOptions2). Add a step. - No cross-relaunch check on the five new preferences. Step 15's "second
click in the same session sees zero lines" only covers in-session state; a
relaunch is what exercises the new
cameraTurningsettings.jsonsection.
(If M1 is fixed, step 15 should also state the expected colour — light blue, not the red the SpewBox uses for client-local refusals.)
3. NOTES (no action required this slice; carry into OP4–OP6)
- N1 — flush/commit ordering. Retail's
PlayerOptionPage::SaveCurrentValuescallsSaveToServerat0x004F271Dand then tailcallsOptionPage::SaveCurrentValues; acdream commits baselines first, thenAfterApply. Content-identical (every value is already live before Apply is pressed), so this is deliberate-looking and harmless — recording it so a future reader does not "discover" it as a bug. - N2 —
SaveCurrentValuere-reads the widget in retail.0x004868E5:m_current = vtable->GetValue(); m_saved = m_current;.BoolOptionRow.SaveCurrentValuecommits its own_current. Equivalent only while every mutation goes throughSetCurrentValue. OP4 must not let an LED widget mutate its own visual state on a path that bypasses the row. - N3 —
Apply(0)vsApply(1)is not modelled. Retail's Reset/Defaults pass0, which skipsUIOption::HandleDialogAndNotices's confirm dialog and the per-optionOnOptionChangednotify (0x004EFB90). The single_applycallback carries no arg. This first bites at OP6 —gmConfigUI::InitOptions @0x0049E5BFis the onlySetConfirmChangeon the four tabs (Display_Resolution). - N4 —
%ffidelity.F6onfloatmatches retail's%fon the double-promoted float for all six values in play. No action. - N5 — hard-coded English confirm fallback.
ResolveEndCharacterSessionConfirmMessage's?? fallbackpredates this slice (it wasRequestEndCharacterSession's own), and the test script correctly instructs the user to report if the fallback is what they see. Plan §6's "never hard-coded English" is about DAT-resident strings; the byte-verified.rdataliterals inOptionsPanelText/ClientTextRefusalsare the Campaign CH-established pattern and are fine. - N6 —
P0x12is dead data client-wide. Not an OP3 defect (see §0.2), but worth an issue eventually: several authored buttons carry an input-action id acdream never reads, and OP8's Configure Keyboard button (P0x12 = 0x1000001F, research §6) is one of them — OP8 will need either that mechanism or an explicit binding. - N7 — UA/RA text is near-verbatim, and AD-75 covers the delta. The
shipped body drops retail's
(Error code %d)clause (documented) and its trailing\n(not documented, immaterial in a chat line). One unverified detail nobody has closed: the research renders the literal wrapped over three lines, so whether "…is listed below." and "Please go there…" are separated by a space (as shipped) or a newline in the binary is not established. Cheap to settle with one push-imm32 read at VA0x007A81E0/0x007A8128if OP9 wants it exact. - N8 — mount ordering.
MountOptionsPanel()runs beforeMountDialogFactory()inRetailUiRuntime.Mount. Safe, becauseShowConfirmationresolvesDialogFactoryat click time, not at bind time — noting it so a future reorder does not silently break Exit-to-Char-Select's dialog.
4. Summary table
| ID | Class | One-line |
|---|---|---|
| M1 | MUST-FIX | Six mouse-turning chat lines typed ClientLocal 0x1A; retail's AddTextToScroll(…, 7, 1, 0) types them 0x07 (light blue, timestamped, logged). No register row. |
| S1 | SHOULD-FIX | Gameplay tab gets an OptionPage + AfterApply; gmGameplayOptionsUI is not an OptionPage (acclient.h:55857) — extra SaveOptions at mount and on every Gameplay-tab entry, with two tests pinning the wrong shape. |
| S2 | SHOULD-FIX | No OnOptionChanged seam; retail runs it as the tail of all three verbs and it is the sole Apply/Reset enable-gate (0x004F27D0). |
| S3 | SHOULD-FIX | IsGrounded's IsPlayerMode conjunction fires the mid-air refusal where retail is silent; the code comment asserts the opposite. |
| S4 | SHOULD-FIX | File the docked-host (0x21000017) register row the plan §5 delegated to this review — scoped client-wide, none exists today. |
| S5 | SHOULD-FIX | TS-74 should name the five store-only camera-turning preferences in What/Where, not only in Risk. |
| S6 | SHOULD-FIX | Test script: toolbar button no-longer-ghosted/highlight, UseMouseTurning bit surviving relogin, prefs surviving relaunch. |
| N1–N8 | NOTE | Ordering, GetValue re-read, Apply(0) arg, %f, fallback string, P0x12, UA/RA whitespace, mount order. |