diff --git a/docs/plans/2026-08-09-chat-parity-campaign.md b/docs/plans/2026-08-09-chat-parity-campaign.md index f000a3eb..62cc88bb 100644 --- a/docs/plans/2026-08-09-chat-parity-campaign.md +++ b/docs/plans/2026-08-09-chat-parity-campaign.md @@ -102,7 +102,7 @@ implementer per slice against a pinned contract (per |---|---|---|---|---| | R1–R4 research | `see docs/research/2026-08-09-chat-retail-*` | — | — | — | | CH1 colors | `172c6f9a` | 11,835 passed / 4 skipped / 0 failed | APPROVE-WITH-FIXES; fixed `34d8a3c0` | pending | -| CH2 interface text | `77c8296e` | 11,890 passed / 4 skipped / 0 failed | pending | pending | +| CH2 interface text | `77c8296e` | 11,890 passed / 4 skipped / 0 failed | REJECT (`docs/research/2026-08-09-ch2-review-findings.md`); rework in progress | pending | | CH3 side channels | — | — | — | — | | CH4 commands | — | — | — | — | | CH5 closeout | — | — | — | — | diff --git a/docs/research/2026-08-09-ch2-review-findings.md b/docs/research/2026-08-09-ch2-review-findings.md new file mode 100644 index 00000000..d22ce279 --- /dev/null +++ b/docs/research/2026-08-09-ch2-review-findings.md @@ -0,0 +1,163 @@ +# CH2 review findings — `77c8296e` (REJECT; rework list) + +Opus dual-lens review, 2026-08-09. Method: all 339 `case` bodies of +`HandleFailureEvent @0x00571990` extracted, every `push imm32` string +reference in VA `0x571990–0x575480` swept from the PDB-paired binary +(`check_exe_pdb.py` → MATCH), diffed against the landed table, +cross-checked against ACE's `WeenieError.cs`/`WeenieErrorWithString.cs` +doc comments. The two oracles agreed on every discrepancy. + +## BLOCKER 1 — SpewBox never renders; pending queue unbounded + +`SpewBoxController.cs:75` constructs the `UiText` with `Visible = false`; +`:94` (inside `ComputeLines`, the `LinesProvider`) is the only site that +sets it true. `LinesProvider` is only invoked from `UiText.OnDraw` +(`UiText.cs:407/429/451/472`), reached only through +`UiElement.DrawSelfAndChildren`, whose first statement is +`if (!Visible) return;` (`UiElement.cs:460`; `DrawOverlays` gates +identically at `:511`). Start-invisible → provider never called → +never visible → **no SpewBox line ever draws**. Consequence 2: +`SpewBoxVM.Lines` is the sole caller of `SpewBoxState.Tick`, so +`_pending` (`SpewBoxState.cs:66`, uncapped `Queue`) never drains +— an unbounded per-session leak on a hot path. The controller tests +(`SpewBoxControllerTests.cs:28,43`) invoke `text.LinesProvider!()` +directly, bypassing the `Visible` gate, so they cannot see it. + +**Fix:** retail drives `gmSpewBoxUI::Update` from the UI tick (global +message 3, `UIElementManager::UseTime @0x0045CFD0`), not from drawing. +Give `SpewBoxController` an explicit per-frame `Tick(double now)` wired +from the retained-UI host loop (the existing per-UI-tick seam — see +`IUiGlobalTimeListener` / wherever retained controllers receive frame +time); it calls `_vm.Lines(now)`, caches the result, sets +`_text.Visible` there; `LinesProvider` returns the cache (exactly +`PortalWaitNoticeController`'s external-push shape). Tests: drive the +frame hook (not the provider); assert the pending queue drains without +a draw pass. + +## BLOCKER 2 — table: 5 missing ids, ~22 wrong strings, 0x4F8 resolvable + +Transcription by "enumerate `case` labels, read the pseudo-C's ~33-char +inline preview" is structurally unsound: it misses ids dispatched by +`else if (arg2 == N)` and merges siblings whose literals collide in the +preview. Re-derive mechanically: sweep `push imm32` operands in +`0x571990–0x575480`, read each UTF-16LE literal to NUL, attribute per +case block by disassembly (BN's line addresses shift up to ~0x20 bytes); +cross-check every row against ACE's enum comments. + +### 2a. Five missing ids (else-if dispatch) + +| id | retail literal | type | +|---|---|---| +| `0x04F` | `You fail to affect %s because $s cannot be harmed!` | Magic (7) | +| `0x3EE` | `The container is closed!` | ClientLocal (0x1A) | +| `0x408` | `Your spell cannot be cast inside` | ClientLocal (0x1A) | +| `0x48A` | `You must be a monarch to purchase this dwelling.` | Default (0) | +| `0x4E8` | `The %s cannot be used while on a hook, use the '@house hooks on' command to make the hook openable.` | Default (0) | + +`0x43` is correctly absent (no text — only `AbortAutomaticAttack`). +`0x3EE`/`0x408` are live on local ACE (`Player_Inventory.cs:883,955`, +`Player_Magic.cs:513`). The landed test +`Format_0x004F_FallsBackToHex_NoRetailCaseExists` pins the 0x04F +regression and must be replaced. + +### 2b. Wrong strings (landed line → correct retail text) + +| line | id | correct retail string | +|---|---|---| +| `:166` | `0x051` | `You fail to affect %s because you are not a player killer!` | +| `:168` | `0x053` | `You fail to affect %s because you are not the same sort of player killer as %s!` | +| `:169` | `0x054` | `You fail to affect %s because you are acting across a house boundary!` | +| `:218` | `0x466` | `You must purchase Asheron's Call: Dark Majesty to interact with that portal.` | +| `:251` | `0x4A3` | `You must have linked with a portal in order to recall to it!` | +| `:268` | `0x4B5` | `You must specify a character to query.` | +| `:306` | `0x4E0` | `…certain level of skill. Your attributes cannot be transferred…` (transfer wording, not skill-lowering) | +| `:328` | `0x4F7` | `%s fails to affect you because you are not a player killer!` | +| `:401` | `0x544` | `…attempting to remove %s as an allegiance officer.` | +| `:411` | `0x54E` | `The hook does not contain a usable item. You cannot open the hook because you do not own the house to which it belongs.` | +| `:415` | `0x552` | `…Throne of Destiny to use this function.` | +| `:416` | `0x553` | `…Throne of Destiny to use this item.` | +| `:417` | `0x554` | `…Throne of Destiny to use this portal.` | +| `:418` | `0x555` | `…Throne of Destiny to access this quest.` | +| `:460` | `0x57F` | `Your allegiance chat privileges have been temporarily removed by %s. Until they are restored, you may not view or speak in the allegiance chat channel.` | +| `:463` | `0x582` | `Your allegiance chat privileges have been restored by %s.` | + +(`0x581` at `:462` is correct.) For the `0x552`–`0x555` family and +`0x4E0` the reviewer gave shapes — the fixer's binary sweep provides the +exact full texts; every row must come from the swept literal, not this +table's abbreviations. All routing TYPES were verified correct — only +strings change. + +### 2c. 0x4F8 resolves — delete the exclusion + +`data_7d2ee8` = `" fails to affect you because you are not the same sort +of player killer as "`, `data_7d2f80` = `"!\n"`; block stages +`arg3 + literal + arg3 + "!\n"` through three `operator+` calls +(`0x57475e/0x574765/0x574776`); type 7 (Magic) at the block's +`AddTextToScroll (0x00573ba5)`. Row: +`[0x4F8u] = new("%s fails to affect you because you are not the same sort of player killer as %s!", RetailLogTextType.Magic)` +(both `%s` take the same param — replace-all already does this). Delete +`Resolve_0x4F8_IsDeliberatelyExcluded_FallsBackToHex`; the class-doc +claim about 0x4F7's "dangling" literal is a misattribution of 0x4F8's +first operand — fix it. New pinned count: **344** (338 + 5 + 0x4F8). + +## SHOULD-FIX + +1. **`ResetSpewBox()` is dead code** (`RuntimeCommunicationState.cs:135` + — no caller; `RuntimeGenerationReset.cs:318` only resets + ChatIdentity). Fold `SpewBox.Reset()` into the ChatIdentity stage (same + lifetime boundary) or add a stage; add a reset test. +2. **`AddText` trims the wrong end + invents an empty-drop** + (`RuntimeCommunicationState.cs:172-176`). Retail's + `AddTextToScroll` calls `trim(&str, 1, 1, ws)` at `0x00563ce3` — + BOTH ends (the trailing-only trim in research §3.1 is + `gmSpewBoxUI::Update`'s separate call). Retail also broadcasts empty + strings deliberately (`s_NullBuffer` at type 7). Fix: `text.Trim()`; + remove the empty early-return (and its pinning test) — or keep it + with a register row. +3. **`ShowWeenieError` bypasses the chokepoint** — + `LiveSessionRuntimeFactory.cs:341` still routes through + `ChatLog.OnWeenieError` (hardcoded type 0x00, comment cites retired + AP-176). `0x0561` belongs in the SpewBox and lands green in chat. + Fix: route through `Communication.AddText(Resolve(code, param))` and + delete `ChatLog.OnWeenieError` — or restore an AP row. +4. **Unmapped-id fallback is unregistered divergence** + (`WeenieErrorMessages.cs:129-134` emits `WeenieError 0xNNNN` into + chat). Retail's switch has no default — unhandled ids produce NO + text. Fix: match retail (silence toward the player), keep the hex id + as a diagnostics log line only; flip/delete the fallback tests — or + file a register row. + +## NITS + +1. `SpewBoxController.cs:34,42` — "AP-TBD" ×2 → AP-177 (lifetime), + AP-178 (position/colour). +2. Three stale "the split lands with CH2" comments (`ChatVM.cs:122`, + `HeadlessGameplayOperations.cs:231`, `LiveSessionRuntimeFactory.cs:336`) + + `AddText`'s `windowId` accepted but never consumed: retail's §2.3 + dual-destination (0x1A + non-zero windowId → SpewBox AND the + originating chat window, ~40 sites) is unimplemented. File a register + row for it and update the comments (implementing it is CH4/CH5 scope + at the earliest). +3. `SpewBoxLayoutDumpDiagnostic` almost certainly never consulted + `dats.Local` (`client_local_English.dat` — the file research §8.1 + names); also the `0x10000012` probe treats a layout enum as a dat id. + Extend the sweep to `dats.Local` before trusting AP-178's wording; if + still negative, keep the row as-is. +4. `WeenieErrorMessages.cs:36-41` — says "19 ids", lists 18. +5. `SpewBoxState` doc overstates "exactly one frame" decoupling — + same-call `Tick`+`Snapshot` displays same-frame; retail is 0–1. Fix + wording. +6. `GameEventWiring.cs:261,276` apply `IsSilentClientControlStatus` to + 0x028A/0x028B but the UseDone path (`:596-601`) does not — harmless + today; align or comment. + +## CONFIRMED-OK (do not touch) + +Core placement of `SpewBoxState` (canonical instance owned by +`RuntimeCommunicationState`, UI borrows); `0x47` silent (DoJump's jump +table has exactly 4 targets; `CommenceJump`'s fallback unreachable — +brief's §4.2 reading was wrong); all 338 routing TYPES verified against +the decomp with zero mismatches; all 11 `ClientTextRefusals` byte-exact; +jump-family constant sharing; AP-177/178/179 content; placeholder +honesty; architecture/layering; the `ClientLocal`-never-reaches- +transcript test.