acdream/docs/research/2026-08-11-op3-review-blast.md

# OP3 review — regression / blast-radius lens

**Commit under review:** `9d26ecc6` — *feat(ui): Campaign OP slice OP3 —
Options panel shell, open paths, Gameplay tab*
**Reviewer lens:** regression + blast radius (what ALREADY-SHIPPED behavior
can this change move?). Pure read-only; no build, no test run, no launch.
**Date:** 2026-08-11
**Verdict:** **APPROVE-WITH-FIXES** — 2 MUST-FIX, 4 SHOULD-FIX, 8 NOTE.

The slice's core structural choice is sound and low-blast: it reuses the
generic catalog/`RetailPanelUiController`/toolbar mechanisms rather than
adding a parallel path, and every seam it touches was already generic. The
two MUST-FIX items are both evidence/bookkeeping gaps (an unverified chat
routing decision with a demonstrable line-loss consequence, and a register
row the plan explicitly assigned to this review), not architectural breaks.

---

## Enumeration A — the `gmPanelUI` panel catalog, before and after

`src/AcDream.App/UI/RetailPanelCatalog.cs`. `Mounted` is what
`TryGetWindowName`/`TryGetPanelId` resolve; every entry that also calls
`RetailPanelUiController.RegisterMainPanel` shares ONE parent rectangle
(`RetailPanelUiController.cs:41-65`, `:284-314`) and retail's
"one active child" exclusion (`:139-197`).

| panelId | const | window | mount site | Left/Top | ResizeX / ResizeY | resizable edges | Min W×H | authored `RestorePrevious` (`0x10000049`) |
|---|---|---|---|---|---|---|---|---|
| 3 | `CharacterInformation` | `characterInformation` | `RegisterIndicatorDetailPanel` (`RetailUiRuntime.cs:1713`) | 18 / 18 | false / true | Bottom | — | **yes** (fixture `character_info_2100006E_10000183.json`) |
| 4 | `PositiveEffects` | `positiveEffects` | `MountEffects` (`:1519`) | right-edge / 18 | false / true | Bottom | — | **yes** (`effects_positive_2100001B.json`) |
| 5 | `NegativeEffects` | `negativeEffects` | `MountEffects` (`:1519`) | right-edge / 18 | false / true | Bottom | — | **yes** |
| 7 | `Inventory` | `inventory` | `MountInventory` (`:2307`) | root.Left / root.Top | false / true | Bottom | — | not read (3-arg overload → false) |
| 8 | `LinkStatus` | `linkStatus` | `RegisterIndicatorDetailPanel` | 18 / 18 | false / true | Bottom | — | **yes** (`link_status_2100001D.json`) |
| 9 | `MiniGame` | `miniGame` | `RegisterIndicatorDetailPanel` | 18 / 18 | false / true | Bottom | — | **yes** (`mini_game_2100001E.json`) |
| 11 | `Character` | `character` | `MountCharacter` (`:2226`) | 540 / 18 | false / true | Bottom | — | not read (3-arg overload → false) |
| 13 | `Magic` | `spellbook` | `MountSpellbook` (`:1326`) | 18 / 18 | false / true | Bottom | — | not read (3-arg overload → false) |
| 15 | `Vitae` | `vitae` | `RegisterIndicatorDetailPanel` | 18 / 18 | false / true | Bottom | — | **yes** (`vitae_21000020.json`) |
| **10** | **`Options` (NEW)** | **`options`** | **`MountOptionsPanel` (`:1984`)** | **150 / 80** | **TRUE / true** | **all four** | **300 × 200** | **absent from the fixture → false** |

Authored extents (fixture roots): every one of these panels is **300 wide**;
heights are 600 (spellbook, character) or 362 (inventory, vitae, character
info, effects, **options**). So the shared-rectangle height coupling between
600-tall and 362-tall panels **already existed** pre-OP3; Options joins the
362 cohort and introduces no new height class.

**Does registering Options change any existing panel's open/close/stacking
behavior?** No, with one caveat and one new asymmetry:

- **Exclusion:** unchanged in kind. `SetPanelVisibility` hides the previous
  active panel regardless of which sibling is being shown; Options is just
  one more sibling. No existing pair's behavior changes.
- **Deferred-restore (`RestorePrevious`):** Options does **not** author
  `0x10000049` (verified: `grep -c 268435529
  tests/.../fixtures/options_panel_2100006E_1000018D.json` → 0). Per
  `RetailPanelUiController.cs:164-167` a non-`RestorePrevious` panel **clears
  `_deferredPanel`**. So `Character sheet → Character Info (defers sheet) →
  F11` drops the pending sheet restore. This is byte-identical to what
  opening Inventory/Character/Spellbook already did in that same position
  (all three are `RestorePrevious: false`), so it is not a regression — but
  it is a newly-reachable route to that drop.
- **Geometry asymmetry (new):** Options is the **only** main panel with
  `ResizeX = true`, all-four-edge resize, and explicit `MinWidth`/`MinHeight`
  (`RetailUiRuntime.cs:1970-1980`). See SHOULD-FIX 2.

---

## Enumeration B — toolbar panel-button routing, before and after

`ToolbarController.PanelButtonIds` (`ToolbarController.cs:54-55`) has always
carried all seven ids; each binds by its OWN authored `0x10000029` value
(`:170-175`), and `BindPanelButtons` (`:374-392`) sets
`button.Enabled = isAvailable(panelId)` with `OnClick = null` when not
available. `isAvailable` is
`RetailPanelCatalog.TryGetWindowName(panelId, …) && WindowManager.TryGet(name)`
(`RetailUiRuntime.cs:679-686`).

| element | authored panelId | in catalog pre-OP3? | pre-OP3 state | post-OP3 state |
|---|---|---|---|---|
| `0x10000197` | 12 | no | ghosted, `OnClick = null` | **unchanged** |
| `0x10000198` | 13 (Magic) | yes | enabled → spellbook | **unchanged** |
| `0x10000199` | 11 (Character) | yes | enabled → character | **unchanged** |
| `0x1000055A` | 25 | no | ghosted | **unchanged** |
| `0x1000019A` | 16 | no | ghosted | **unchanged** |
| **`0x1000019B`** | **10** | **no** | **ghosted, `Enabled=false`, `OnClick=null` — clicking did NOTHING** | **enabled → `ToggleWindow("options")`** |
| `0x100001B1` | 7 (Inventory) | yes | enabled → inventory | **unchanged** |

**Answer to "what did `0x1000019B` do pre-OP3":** it was authored with panel
id 10, discovered by the generic binder, found panel 10 absent from the
catalog, and was therefore **ghosted with a null click handler**. Nothing
happened on click. No test pinned that old behavior (the fixture table at
`ToolbarControllerTests.cs:26-35` pins the *element→panelId* mapping only,
and `PanelButtons_bindByDatPanelId_andGhostUnavailablePanels` at `:221`
drives its own `isAvailable` predicate, not the real catalog), so nothing
pinning old behavior had to change. **Exactly one button's routing changed.**
`SyncToolbarWindowButtons` (`RetailUiRuntime.cs:670-675`) now also syncs
panel 10's Highlight/Normal state; `SetPanelOpen` (`ToolbarController.cs:394`)
early-returns per-id, so no other button's state is touched.

Degradation path is clean: if `MountOptionsPanel` bails (`:1954`, `:1962`),
the catalog still maps `options`→10 but `_panelUi` has no id-10 entry, so
`TogglePanel(10)` returns false (`RetailPanelUiController.cs:141`) and the
toolbar button re-ghosts (`WindowManager.TryGet` fails). F11 silently no-ops
rather than falling through to `Host.ToggleWindow`.

---

## Enumeration C — F11 history

| commit / event | `KeyBindings.RetailDefaults()` | `InputAction.ToggleOptionsPanel` routed to | net effect of pressing F11 |
|---|---|---|---|
| `84512d3c` (Phase K.1a) | file created; retail preset not yet built | — | — |
| **`da189103`** (Phase K.1c, retail keymap cutover) | **adds `KeyChord(F11, None) → ToggleOptionsPanel`** (now `KeyBindings.cs:216`), sourced from `retail-default.keymap.txt:148` `ToggleOptionsPanel [ "" [ 0 DIK_F11 ] ]` | `_devTools.ToggleSettingsPanel()` | opened the ImGui-era Settings panel |
| Campaign V slice V11 | unchanged | `DevToolsGameplayCommands.ToggleSettingsPanel()` becomes an **empty method** (`GameplayInputCommandController.cs:70-72`, class doc `:53-59`) | **nothing** |
| **`9d26ecc6` (OP3)** | **unchanged — the row already existed** | `_retained.ToggleOptionsPanel()` → `RetailUiRuntime.ToggleWindow("options")` → `_panelUi.TogglePanel(10)` | opens/closes the retail Options panel |

**Verification of the implementer's claim** (`git log -L 216,216:…/KeyBindings.cs`):
the binding was introduced by `da189103`, not by OP3. OP3 touches
`KeyBindings.cs` **not at all** — only the new pin in
`KeyBindingsRetailTests.cs:239`. The claim is accurate.

**What stopped working:** nothing. Between V11 and OP3, F11 was a hard no-op.

**Keybind conflicts:** `grep -n "F11" KeyBindings.cs` returns exactly one row.
No chord conflict introduced. `InputScope` puts F11 at the bottom of the
stack (`InputScope.cs:19`) — unchanged.

**Dead `ToggleSettingsPanel` wiring:** untouched, as claimed — but now
**orphaned**. `GameplayInputCommandController` was its only production caller;
post-OP3 the only remaining references are the interface declaration
(`:50`), the empty implementation (`:70`), a comment (`:242`), and the test
fake (`GameplayInputCommandControllerTests.cs:141`). The commit comment says
"its no-op wiring elsewhere is untouched" — there is no *elsewhere*. Planned
for retirement in OP9; see NOTE 8.

---

## Findings

### MUST-FIX 1 — the six mouse-turning chat lines are routed to a 4-slot transient overlay on an **unverified** retail log type, and two of them can never be seen

`MouseTurningSettingsMacro.Compute` can emit **six** lines
(`MouseTurningSettingsMacro.cs:58-104`). `RetailUiRuntime.ApplyMouseTurningSettingsMacro`
(`RetailUiRuntime.cs:1907-1908`) pushes every one of them through
`_bindings.Options.DisplaySystemMessage`, which is bound to
`d.Communication.AddText(text, RetailLogTextType.ClientLocal)`
(`InteractionRetainedUiComposition.cs:816-817`).

`RuntimeCommunicationState.AddText` (`RuntimeCommunicationState.cs:197-201`)
sends `ClientLocal (0x1A)` to `SpewBox.Enqueue` and **returns** — it never
reaches the chat transcript. `SpewBoxState.MaxConcurrentItems = 4`
(`SpewBoxState.cs:77`), and `Tick` drains the whole pending queue in one
call, dropping the oldest on overflow (`:132-149`). Six lines enqueued in one
click therefore leave **four visible**: the first two — *Camera Stiffness*
and *Camera Adjustment* — are discarded before they are ever drawn. They also
expire after 5 s (`:88`).

The routing itself is **not byte-verified**. The commit and register call the
*format strings* byte-verified (VAs `0x007A8A70` … `0x007A88D0`), and the
research doc's §4.4 table lists exactly those VAs — but neither the doc nor
the code records the `AddTextToScroll` **type argument** at
`0x0049E8F0..0x0049EB81`. This project has already been burned on precisely
this inference: `ClientTextRefusals.cs:117` documents a site where "the actual
site passes type `0` (Default), not `0x1A`". If retail passes `0` here, all
six lines belong in the scrolling chat window, all six are visible, and OP3's
routing is wrong in a way the user will see.

Fix: either byte-verify the type at the call sites and route accordingly, or —
if the sweep is out of scope this slice — file a register row recording the
assumed `ClientLocal` routing plus the 4-slot truncation, per CLAUDE.md's
"any commit that introduces a deviation adds its register row IN THE SAME
COMMIT". Right now there is neither evidence nor a row. (AD-75 covers the
UA/RA `MessageBoxA` → interface-text substitution; it says nothing about the
mouse-turning macro's six lines.)

### MUST-FIX 2 — the plan assigned this review a register ruling that OP3 did not file

`docs/plans/2026-08-10-options-panel-campaign.md:379-382`:

> 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).

I deem it a divergence: retail exposes both a floating and a docked
`gmPanelUI` host and acdream mounts only `0x2100006E`, so no gmPanelUI child
— Options included — can be docked. The commit files AD-74/75/76 and TS-74
but no row for this. It is inherited rather than introduced by OP3 (every
pre-existing panel already mounts floating-only), which is exactly why it has
gone unrowed until now, and why the plan routed the ruling here. One AD row,
cited at `RetailUiRuntime.MountOptionsPanel` / the shared host id, closes it.

### SHOULD-FIX 1 — `IsGrounded` returns false outside player mode, so "Exit to Character Selection" is unusable in fly mode and emits a mid-air message that is not true; the code comment asserts the opposite

`InteractionRetainedUiComposition.cs:322-324`:

```csharp
IsGrounded: () => d.PlayerMode.IsPlayerMode
    && d.PlayerController.Controller is { IsAirborne: false },
```

with the comment (`:320-322`) claiming *"'on the ground' is meaningless
outside player mode, so the mid-air refusal never fires while flying/
spectating."* The code does the reverse: outside player mode `IsGrounded()`
is **false**, so `RequestExitToCharacterSelection`'s `else` branch
(`RetailUiRuntime.cs:1871-1873`) fires `CantLogOffMidAir`. In fly/chase-camera
mode the button confirms, refuses, and shows a message that is factually
wrong. The underlying expression is the project's established `playerOnGround`
shape (same three lines at `:322-324` feed the item-drop gate), so the shape
is fine — the comment is inverted and the not-in-player-mode case is
unconsidered. Impact is contained (Exit Game right beside it still works, and
retail has no fly mode) but the comment must not survive as-is.

### SHOULD-FIX 2 — Options is the only shared-geometry main panel that resizes horizontally and on all four edges; its width is silently discarded whenever a sibling is shown

`RetailUiRuntime.cs:1970-1980` gives Options `ResizeX = true`,
`ResizableEdges = Left|Right|Top|Bottom`, `MinWidth = 300f`,
`MinHeight = 200f`. Every one of its nine shared-rectangle siblings is
`ResizeX = false`, `ResizableEdges = Bottom`, with no explicit constraints
(Enumeration A). Consequences, all through
`RetailPanelUiController`'s one `_mainPanelGeometry`:

- A user widens Options to 450. `OnWindowResized` → `CaptureAndSynchronize…`
  (`:224-247`) writes 450 into the shared rect; siblings ignore it
  (`RetailWindowManager.ResizeTo:185` forces `width = frame.Width` when
  `!ResizeX`). Open the character sheet, then reopen Options:
  `PrepareMainPanelGeometry` → `ApplyWindowGeometry` (`:265-282`, `:306-314`)
  resizes Options back to the sibling's 300. The width is lost. This
  contradicts the OP3 gate script's own item 5 ("remembers its geometry
  across a close/reopen").
- The 300/200 minimums are invented, not authored — `DatConstraintSource` is
  not passed, so `ResolveConstraint` (`RetailWindowFrame.cs:160-163`) uses
  the literals. Because all ten panels are authored 300 wide, `MinWidth=300`
  currently pins Options to exactly its authored width and no clamp feedback
  loop can start; that is luck, not design.
- The four-edge resize also means Options is the only panel whose *top/left*
  drag moves the shared rect origin for every sibling.

Either match the sibling policy (`ResizeX = false`, bottom-only, drop the
literals) or make the whole shared group consistent — but not one panel with
its own rules inside a single-rectangle group. Needs the connected visual
gate either way; I cannot run the client.

### SHOULD-FIX 3 — no test pins the two seams the review was asked about: mutual exclusion against a *real* existing panel, and the new dialog's queue behavior

The new suites are thorough on the model (`OptionPageModelTests`, 20 facts)
and the controller's own content (`OptionsPanelControllerTests`, 15 facts),
and they do pin the F11 route on both sides
(`GameplayInputCommandControllerTests.cs:21` + `KeyBindingsRetailTests.cs:239`).
What is missing:

- **Panel mutual exclusion with an existing panel.** `RetailPanelCatalogTests`
  asserts catalog membership only. No test registers, say, panel 11
  (character) and panel 10 (options) in a `RetailPanelUiController` and
  asserts that showing Options hides the character sheet — the exact
  interaction the slice's headline claim ("shares retail's mutual exclusion
  for free") rests on. The mechanism is exercised by
  `RetailPanelUiControllerTests` with synthetic ids, so the risk is low, but
  the claim is unpinned.
- **The confirmation dialog's queue key.** `RequestExitToCharacterSelection`
  goes through `ShowConfirmation` → `MakeConfirmation` with the default
  `queueKey = 2` (`RetailDialogFactory.cs:125-136`, `:10`), the same key every
  existing confirmation uses — see NOTE 3. No test pins that the new dialog
  shares the existing key rather than inventing one.
- No test asserts `ActivePageChanged` stays silent on a **dormant** Type-8
  host (AD-73's contract). See NOTE 2.

Fixture/conformance suites are unaffected: `RetailLayoutFixtureGenerator`
gains one row (`+2` lines), `FixtureLoader` two new loaders, and no existing
fixture was regenerated (`git show --stat` shows only the new
`options_panel_2100006E_1000018D.json`). Existing catalog assertions in
`AppraisalUiControllerTests.cs:683-691` and
`EffectsUiControllerTests.cs:256-258` are negative assertions about
Examination/Effects and remain true. No count-based assertion over
`MountedPanels`/`ToolbarPanels` exists to go stale.

### SHOULD-FIX 4 — the Urgent Assistance / Report Abuse bodies are three-line, ~110-character-URL messages delivered into a 4-slot, 5-second, centered transient overlay

`OptionsPanelText.UrgentAssistanceUnavailable` / `ReportAbuseUnavailable`
(`OptionsPanelText.cs:106-118`) embed two `\n`s and the full
`support.turbine.com` URL, and go out through the same `ClientLocal` seam
(`OptionsPanelController.cs:171-174`). `AddText` only trims the ends
(`RuntimeCommunicationState.cs:195`), so the whole three-line body lands as a
**single** `SpewBoxEntry` that expires in 5 s. The *divergence* is registered
(AD-75), but its legibility is not: the user has 5 seconds to transcribe a
110-character URL from a transient overlay. Worth a look at the connected
gate; if it is unreadable, the honest fix is the chat transcript (which
scrolls back), not a longer timeout.

---

### NOTE 1 — the mount-time `AfterApply` publish is harmless, but it does fire before any session exists

`MountOptionsPanel` calls `controller.ActivateTabs()` (`RetailUiRuntime.cs:1958`)
during `Initialize()`, which fires `ActivePageChanged(0, Gameplay)` →
`OptionPage.OnShown()` → `Apply()` → `AfterApply` →
`_bindings.Options.CommandBus().Publish(new SaveCharacterOptionsRuntimeCmd())`
(`:1945-1946`). `DeferredLiveSessionUiAuthority.Commands` returns
`NullCommandBus.Instance` when unbound (`InteractionUiRuntimeSources.cs:222-231`),
so there is **no NRE and no wire traffic** at mount. Post-connect, the same
publish fires on every window show and every tab switch; the router's
`SaveCharacterOptionsIfDirty` is dirty-gated (`LiveSessionRuntimeFactory.cs:359-372`),
so it is a no-op unless the batched module is actually dirty. Retail-cited
(`PlayerOptionPage::SaveCurrentValues @0x004F2710`'s `SaveToServer(0)` tail).
No action needed — recorded because "opening a UI panel sends a character-
options save" is the kind of thing a future reader will flag.

### NOTE 2 — `ActivePageChanged` cannot fire during dormant operation, but only because nobody calls `SwitchTo`

The event is raised from exactly one place: the tail of `SwitchTo`
(`UiTabPanel.cs:212-213`), after `ActivePageElementId` is updated (so handlers
read the new value — which `OptionsPanelController.OnHidden` at `:217` relies
on). `SwitchTo`'s only callers are `ActivateTabBehavior`'s default switch
(`:186`) and the click handlers *that method registers* (`:174`). Dormant
Type-8 hosts (vendor `0x100000B8`, character-sheet root `0x10000227`,
spellbook root `0x100002A8`, combat `0x100000A2` — AD-73) never run either,
so the event never fires for them. **But** `SwitchTo` is public and its own
doc (`:193-195`) invites direct calls — a future controller calling
`SwitchTo` on a dormant host WOULD raise it. Since the only subscriber is
`OptionsPanelController` on its own instance (`:131`), and it unsubscribes in
`Dispose` (`:233`), there is no existing consumer to affect today. Existing
`UiTabPanel` consumers: none subscribe; the added field is 8 bytes/instance.

### NOTE 3 — the new dialog respects the `OpenNextDialog` duplicate-key hazard because it does not introduce a key

`RequestExitToCharacterSelection` reuses `ShowConfirmation`
(`RetailUiRuntime.cs:534-541`) → `MakeConfirmation` with the default
`queueKey = DefaultQueueKey = 2` (`RetailDialogFactory.cs:10`, `:128`). Every
confirmation in the codebase uses that same key (only one `QueueKey` constant
exists outside the factory: `RetailDialogData.cs:19`, the property id). The
chat digest's DO-NOT-RETRY hazard is the `Dictionary.Add` duplicate at
`OpenNextDialog`; the existing guard at `RetailDialogFactory.cs:332-333`
(`if (_activeQueued.ContainsKey(queueKey)) return;`) already covers the
re-entrant case, and OP3 adds no new key for it to miss. Correct by
construction. Behavioral consequence: clicking Exit-to-char-select while a
server-driven confirmation is up queues behind it, same as every other
client-side confirmation.

### NOTE 4 — the Exit-Game / Escape / indicator-bar exit paths are genuinely shared and genuinely unchanged

`ExitGame: _bindings.Indicators.EndCharacterSession` (`RetailUiRuntime.cs:1942`)
resolves to `d.Window.Close` (`InteractionRetainedUiComposition.cs:688`) — the
same delegate behind (a) the indicator bar's end-session button, which wraps
it in `RequestEndCharacterSession`'s confirmation (`:1844-1851`), and (b) the
Escape key's terminal branch, via a *separate* `GameplayWindowCommands(d.Window.Close)`
instance (`SessionPlayerComposition.cs:1200` → `GameplayInputCommandController.cs:269`).
OP3 adds a third caller and modifies neither existing one. The refactor that
extracted `ResolveEndCharacterSessionConfirmMessage` (`:1832-1841`) is
behavior-preserving for `RequestEndCharacterSession` — same DAT id
`0x23000001`, same hash, same fallback string, same `DatLock` scope.
Retail-faithfulness of "Exit Game has no confirmation" is confirmed by the
research doc's `RecvNotice_EndCharacterSession(0)` branch
(`2026-08-10-keyboard-config-and-gameplay-tab.md:151-154`): param 0 sets
`m_shouldQuitOnLogout = 1` and skips both the dialog and the mid-air check —
which is also why OP3 correctly applies the mid-air refusal to
Exit-to-char-select only.

### NOTE 5 — settings-file schema and versioning are safe; the concurrent-write hazard is pre-existing and unchanged in kind

`SaveCameraTurning` goes through the existing `SaveSection`
(`SettingsStore.cs:250-252`, `:658-702`), which preserves every non-target
top-level key as raw text (`:663-683`) and re-stamps `version: 2`
unchanged (`CurrentSchemaVersion` at `:37` is untouched). `cameraTurning` is
a new top-level key, so: older acdream builds ignore it on read and preserve
it on write; newer builds fall back to `CameraTurningSettings.Default`
per-field when the section or a key is missing (`:224-241`). **No schema break
for existing `settings.json` readers.**

Concurrency: `SaveSection` is a non-atomic read-modify-`File.WriteAllText`,
and `SaveWindowLayout` is a *separate* `LoadMutableRoot`/`WriteMutableRoot`
path over the same file (`:435-462`). Interleaving either pair can clobber.
That hazard predates OP3; OP3 adds one more writer to the `SaveSection` side,
driven from a UI click handler on the same thread as the existing settings
saves. No new thread is introduced. Recorded, not charged to this slice.

### NOTE 6 — `LoadCameraTurning`/`SaveCameraTurning` correctly bypass the cached-snapshot pattern, and the interface widening is fully covered

`RuntimeSettingsController` gains two pass-throughs
(`RuntimeSettingsController.cs:444-467`) that read/write storage directly
rather than through the `RuntimeSettingsSnapshot` cache the
Display/Gameplay/Chat sections use. The implementer flagged this as
deliberate; it is correct here — a cached, change-notified copy exists to
drive live UI and there is no Camera/Input UI surface until OP6. The widened
`IRuntimeSettingsStorage` has exactly two implementers
(`JsonRuntimeSettingsStorage` at `:42`, the test fake at
`RuntimeSettingsControllerTests.cs:1280`), both updated. Save failures are
logged not thrown, matching every other `Set*` in the class.

### NOTE 7 — the five persisted camera preferences are a second, disconnected store for a value acdream already owns

`cameraTurning.mouseLookSensitivity` is persisted but never read by the
camera; the live sensitivity acdream actually uses lives in
`CameraPointerInputController` (`_chase.Sensitivity`, `_flySensitivity`,
`_orbitSensitivity` at `:110-111`, `:174-182`) and is adjusted by the
`AcdreamSensitivityUp/Down` actions. TS-74 mentions "(unwired) preferences"
in passing, but only in the context of the `UseMouseTurning` bit; the
five-preference store-without-consumer is not itself rowed. Low risk today
(nothing reads it), but OP6 will surface these in the Config tab, at which
point two stores for one concept becomes user-visible. Worth a sentence in
TS-74 rather than a new row.

Related: `SetSingleCharacterOptionRuntimeCmd` publishes directly on the bus
(`RetailUiRuntime.cs:1900-1903`), matching the established CH3 pattern at
`RuntimeSettingsTargets.cs:321-322` including its documented silent-drop-
while-disconnected behavior. The macro's own wrinkle: the six chat lines are
emitted unconditionally, so a drop while disconnected still prints "Turn to
Face Camera was changed from FALSE … to TRUE" when neither the wire send nor
the local bit write happened (`SendIfActive` skips the whole lambda, so
`TrySetOption` never runs). The Options panel is only reachable in-world, so
the window is narrow.

### NOTE 8 — `IDevToolsGameplayCommands.ToggleSettingsPanel` is now fully orphaned

Zero production callers remain (Enumeration C). The commit comment at
`GameplayInputCommandController.cs:242-244` describes it as having "no-op
wiring elsewhere"; there is none. Slated for OP9 per the same comment —
worth confirming it actually appears on OP9's list rather than relying on the
comment.

Minor inconsistency in the same area: `OptionsPanelController.Bind` logs a
loud warning when any of the seven Gameplay buttons fails to resolve
(`BindButton`, `:188-197`) but the close button (`:160-161`) fails silently
if `0x10000210` ever stops building as a `UiButton` — a panel you cannot
close by its X, with no log. One line to make it symmetric.

---

## Verdict

**APPROVE-WITH-FIXES.**

The structural bet — register through `RetailPanelCatalog` +
`RetailPanelUiController` + the existing generic toolbar binder, rather than
building an Options-specific path — is the right one and is why the blast
radius on already-shipped panels is as small as it is. Both axes walk clean
on the mechanisms: exactly one toolbar button changed state, no other panel's
exclusion/stacking/deferred-restore behavior moved, the dialog queue key is
reused rather than invented, the settings schema is additive and
version-neutral, F11's binding predates the slice and its old target was a
proven no-op, and the new `UiTabPanel` event cannot fire on a dormant host.
The two MUST-FIX items are an unverified chat-routing decision with a
demonstrable line-loss consequence, and a register row the campaign plan
explicitly assigned to this review. Neither blocks the mechanism; both must
land before the connected gate is meaningful, since the first one changes
what the gate is supposed to observe.