diff --git a/docs/research/2026-08-12-fa4-review-mechanism.md b/docs/research/2026-08-12-fa4-review-mechanism.md new file mode 100644 index 00000000..1bc316e3 --- /dev/null +++ b/docs/research/2026-08-12-fa4-review-mechanism.md @@ -0,0 +1,263 @@ +# FA4 — mechanism-faithfulness review + +**Slice:** Campaign FA slice FA4 — the Fellowship page fully live. +**Targets:** `357d2032` (Runtime), `5bdd0528` (App+tests), `38f08314` (docs). +**Reviewer lens:** mechanism-faithfulness (does the code do what retail does, +byte-for-byte where a byte exists, and is every divergence honestly filed). +**Method:** re-derived from the code, the lane docs +(`2026-08-11-fa-fellowship-wire.md`, `-fa-panel-structure.md`), the +PDB symbol table (`docs/research/named-retail/symbols.json`), and a LIVE run +of the env-gated mount probe against the real installed DATs. + +--- + +## VERDICT: APPROVE-WITH-FIXES + +One MUST-FIX (a doc-only register omission), three SHOULD-FIX, two NITs. No +code-behavior REJECT: every wire-touching mechanism this slice ships is +retail-faithful and the divergences it makes are (with the one exception +below) honestly registered. + +**Strongest single piece of evidence:** the live-mount probe +(`ACDREAM_PROBE_LIVE_MOUNT=1`, real installed DATs, not a fixture) **PASSES** +(1/1). It confirms, end-to-end against the production mount, that the name +field builds as `UiField`, all 11 buttons/checkboxes resolve as `UiButton`, +the sole row template `0x21000030/0x10000281` resolves through the production +`RowTemplateResolver` with all five checked fields at the right widget types +(`UiText`/`UiText`/`UiMeter`/`UiMeter`/`UiMeter`), the four checkbox labels +resolve to real retail English, the Open/Close captions resolve to +`"Open"`/`"Close"`, and a full production-path `Bind()` emits zero +"not found" warnings. FA4 test suites also pass under `--no-build`: +`SocialFellowshipPageController`/`RowTemplateResolver`/`UiTemplateListBoxFlushPreservingScroll`/`CharacterOptions` += 75/75; `RuntimeFellowshipStateTests` = 28/28; probe = 1/1. + +--- + +## What verified CLEAN (re-derived, not trusted) + +### D4 — the `0x00A6` gate (the highest-stakes mechanism) +The five contracted transitions all resolve correctly. The conjunction is +`_fellowship?.SetPageVisible(_visible && IsShowingFellowship)` +(`SocialPanelController.cs:267-268`), driven from three sources: +`ActivePageChanged` (ctor `:140`), `OnShown` (`:250-254`), `OnHidden` +(`:258-262`). `SetPageVisible` sends `0x00A6` only on an actual flip +(`SocialFellowshipPageController.cs:453-458`, `if (_pageVisible == visible) return;`). + +- **open-on-fellowship:** `OnShown` → `_visible=true` ∧ `IsShowingFellowship=true` + → `SetPageVisible(true)` → one `true` send. ✓ +- **tab-away:** `ActivePageChanged` → `_visible=true` ∧ `false` + → `SetPageVisible(false)` (flip from true) → one `false` send. ✓ +- **tab-back:** → `SetPageVisible(true)` → one `true` send. ✓ +- **close-while-on-fellowship:** `OnHidden` → `_visible=false` + → `SetPageVisible(false)` → one `false` send. ✓ +- **close-while-on-other-tab:** `_pageVisible` already `false` (we left the + Fellowship tab earlier), `OnHidden` → `SetPageVisible(false)` → no-op, **no + redundant send**. ✓ +- **switch between two non-Fellowship tabs while shown:** + `SetPageVisible(false)` while already `false` → no spurious send. ✓ + +The default authored tab is Allegiance (probe: `button=0x1000028C +page=0x10000291 default=True`), so `ActivateTabs()` at mount time +(`RetailUiRuntime.cs:2775`, before the window is shown with `Visible=false`) +fires `ActivePageChanged` for Allegiance while `_visible=false` → no spurious +`0x00A6` at mount. ✓ + +**No-send-while-disconnected** verified at the command layer: `Invoke` +returns `RuntimeCommandStatus.Inactive` without dispatching when +`_deactivated || _view is null || _commands is null` +(`InteractionUiRuntimeSources.cs:181-186`); `FellowshipSetPanelOpen` just +forwards through it (`:157-159`). **Idempotence** verified twice (the flip +guard + the pinning test `SetPageVisible_SendsPanelOpen_OnlyOnATransition`, +`SocialFellowshipPageControllerTests.cs:537-554`). + +### Leader-quit rule (FA2 hand-off not bypassed) +FA4's Quit routes `_bindings.Quit(disband)` → +`late.GameRuntime.FellowshipQuit` → `commands.Fellowship.Quit` +(`InteractionUiRuntimeSources.cs:138-140`), whose adapter body sends +`0x0290 SendFellowshipAssignNewLeader` BEFORE `0x00A3 SendFellowshipQuit` +when `RequiresLeaderHandoffBeforeQuit` returns true +(`DirectGameRuntimeCommandAdapter.cs:888-895`, mirrored +`CurrentGameRuntimeCommandAdapter.cs:883`). No raw `WorldSession.Send*` +bypass exists in the UI layer. The FA2 rule itself is correct: fires only +when `!disband && _isInFellowship && _leaderGuid == selfGuid`, picks the +first non-self member (`RuntimeFellowshipState.cs:292-313`). ✓ + +### D5 — the XP-share table +`EvenSplitPercentTable = [1.0, 0.75, 0.6, 0.55, 0.5, 0.45, 0.4, 0.35, +0.3111111, 0.28]` (`SocialFellowshipPageController.cs:103-104`) reproduces +lane B §7.2's byte-decoded `.rdata` floats **exactly**, including the +out-of-range default `0.0` (`EvenSplitPercent`, `:655-656`, returns `0f` for +`memberCount` outside `[1,10]`). Branch structure matches +`gmFellowshipUI::UpdateFellowStats` §2.9 verbatim (`FormatStatsText`, +`:643-653`): `!ShareXp` → `0%` (retail's literal `pct=0.0f` branch), +even-split → table, proportional → **omitted** (level only) rather than a +guessed formula — the honest, CLAUDE.md-compliant disposition. AD-80 +accurately states the ACE display divergence (`.3` vs `.3111111` at 9, no +10-fellow row, wrong `1.0` out-of-range default) as ACE-vs-retail, not +acdream-vs-retail. ✓ + +### D6 — invite auto-response +`HandleConfirmationRequest` intercepts type-4 (`ConfirmationType.Fellowship=4`, +`GameEvents.cs:580`) BEFORE the generic `GameplayConfirmationController` +(`RetailUiRuntime.cs:677-683`). `TryAutoRespondToFellowshipInvite` (`:685-693`) +reads both bits and sends `SendConfirmationResponse(type, ctx, autoAccept)` +(the shared confirmation-response primitive, +`InteractionRetainedUiComposition.cs:807-812`) — decline when only Ignore is +set, accept when AutoAccept is set. The mutual exclusion IS Runtime's: +`RuntimeCharacterState.TrySetOption` clears the opposite bit when either is +turned on (`:821-834`), so both-set is unreachable through the write path. +Neither bit set → `return false` → falls through unchanged. ✓ + +### D7 — un-dims +Exactly four rows flip `StoreOnly`→`Live` +(`CharacterOptionsPageController.cs:206-208`): `IgnoreFellowshipRequests`, +`FellowshipAutoAcceptRequests`, `FellowshipShareXP`, `FellowshipShareLoot`. +Each has a live read site: the first two in +`TryAutoRespondToFellowshipInvite`; `ShareXP` at the Create-button click +(`SocialFellowshipPageController.cs:325`) and its own checkbox; +`ShareLoot` via its own live checkbox surface (`RefreshCheckboxSelections` +`:484-486`). `IgnoreAllegianceRequests`/`DisplayAllegianceLogonNotifications` +stay `StoreOnly` (`:206`/`:208`). Conformance test updated 35→31 store-only / +15→19 live with the exact four ids removed from `ExpectedStoreOnlyIds` +(`CharacterOptionsPageControllerTests.cs:729-733`, `:762-765`). ✓ + +### Roster scroll preservation +`FlushPreservingScroll` (`UiTemplateListBox.cs:290-296`) saves `ScrollY`, +`ClearContent()`s, restores `ScrollY`. It genuinely preserves across a +membership-change rebuild because `ClearContent` zeroes only the *panel's* +`ContentHeight` and never syncs `Scroll.ContentHeight` +(`UiScrollablePanel.cs:54-61`), so the restore's `SetScrollY` clamps against +the STALE (pre-rebuild) `MaxScroll` (`UiScrollable.cs:56`) — exactly as the +method's own doc claims. The next `LayoutScrollableChildren` re-clamps +against the fresh height. Tests exercise the real cases: preserve across +same-count rebuild, clamp-down on shorter content, dormant no-op +(`UiTemplateListBoxFlushPreservingScrollTests.cs:50-98`). The no-change path +(`UpdateRow`) only sets `LinesProvider`/meter delegates — no ListBox +mutation (`SocialFellowshipPageController.cs:607-627`). ✓ + +### Create-flow refusal-by-enabled-state +`RefreshCreateButtonState` sets `Enabled = !IsNullOrWhiteSpace(name)` +(`:460-465`); the click handler re-guards (`:322-327`). Matches lane B §2.2 +(`gmFellowshipUI::CreateFellowship` @0x0048F730): the button IS the guard, +no separate error text. `ShareXP` read from the character option at click +time (lane B row 67: "shareXP comes from option 0x0F"), not a dialog field. ✓ + +### Button-enable rules (§2.8 verbatim) +`RefreshButtonStates` (`:510-537`) ports the `gmFellowshipUI::UpdateButtons` +@0x0048E6C0 table exactly: Quit always; Disband/Open `= isLeader`; +Leader/Dismiss `= isLeader && hasSelection && !selectedIsSelf`; Recruit +`= targetValid && notFull && (isLeader || IsOpen)`. The one gap (Recruit does +not gate on "target is a player") is honestly disclosed as a +superset-of-retail rule in an inline comment (`:529-534`) with the server +refusing a non-player target the same way retail's click-handler no-ops. + +### §6 compliance / anchors +String resolver threaded to every `LayoutImporter.Build` +(`RetailUiRuntime.cs:2714-2720`, the `RowTemplateResolver` build closure); +scoped `FindDescendant(pageRoot,…)`/`FindDescendant(row,…)` throughout; +scrollbar id-0 guard (`:247-256`); `RowTemplateResolver` caches null misses +so a permanently-unresolvable template imports ONCE (`RowTemplateResolver.cs:63-72`), +and the revision latch advances before `RefreshRoster` +(`SocialFellowshipPageController.cs:433-437`) so a partial resolver failure +never becomes a per-frame retry. Five retail anchors byte-verified in the +PDB: `0x0048E6C0`→`gmFellowshipUI::UpdateButtons`, +`0x0048F730`→`::CreateFellowship`, `0x005B9070`→`Fellowship::IsFull`, +`0x005B9BA0`→`FellowshipSystem::GetEvenSplitXPPctg`, +`0x004901C0`→`gmFellowshipUI::ListenToElementMessage`, +`0x0048EB20`→`::UpdateFellowStats`, `0x006A6380`→`CM_Fellowship::Event_UpdateRequest` +— all match their citations. `SelectionChangeSource.Social` +(`SelectionState.cs:21-26`) is anchored to `::ListenToElementMessage`'s +`SetSelectedObject` call and matches the shared selection primitive. Gate +script §FA4 claims all trace to the code (create gating, caption swap, button +states, D5 display, invite bits, leader hand-off). ✓ + +--- + +## Findings + +### MUST-FIX 1 — the leader-gold-tint adaptation has no divergence-register row +`SocialFellowshipPageController` tints the fellowship LEADER's roster name a +distinct gold (`LeaderNameColor = (1, 0.84, 0, 1)`, applied in `UpdateRow`, +`SocialFellowshipPageController.cs:110`/`:614`). Its own class doc +(`:57-65`) is honest that lane A's row template has **no** dedicated leader +marker and retail "may simply not mark leadership in the row at all" — i.e. +this is an *invented, user-visible visual cue retail (probably) does not +have*. That is precisely the class of deviation the divergence register +exists to hold, and CLAUDE.md's binding rule is unambiguous: "Any commit that +introduces a deviation … adds its register row IN THE SAME COMMIT" / "A +deviation found without a row is a bug twice over." The FA4 docs commit filed +AD-80 (XP-share display) and AD-81 (the numeric-composite text gap) but +**neither covers the gold tint** — a register grep for `gold`/`tint` returns +only the unrelated AP-178 (SpewBox). This is exactly the same reasoning that +earned AD-78 its row ("caption colors retail never has → register row"). Fix: +add a one-line AD row (or fold a clause into AD-81) recording the leader +gold-tint adaptation and citing `SocialFellowshipPageController.cs:110`/`:614`. +*(Doc-only; the code behavior is a reasonable, honestly-code-documented +adaptation — the defect is purely the missing auditable row.)* + +### SHOULD-FIX 2 — AD-78's "35 of 50 rows dimmed" parenthetical is now stale +`38f08314`'s message claims "AD-78's derivation table gains its D7 addendum," +but the addendum landed in the CODE class-doc (in `5bdd0528`), while the +**register row itself** was not touched: AD-78's "Where" column still reads +`CharacterOptionsPageController.cs (35 of 50 rows dimmed …)` +(`retail-divergence-register.md:175`) when the post-FA4 count is **31 of 50**. +The register is "the single auditable list"; a stale count in an active row +is a bookkeeping drift. Fix: update `35`→`31` in AD-78's Where column. + +### SHOULD-FIX 3 — D4 is not re-armed across a generation reset (reconnect) +`0x00A6` is a per-session, server-latched declaration; `_pageVisible` +(App-side UI state) is NOT reset by a generation reset, and +`ResetSessionTransientUi` (`RetailUiRuntime.cs:731-736`) does not hide/close +the social panel. So if the Fellowship tab is open across a reconnect, +`_pageVisible` stays `true`, no `OnShown`/`OnHidden`/`ActivePageChanged` +transition fires, `SetPageVisible` is never re-called, and the fresh session +never receives `0x00A6`. Since `RuntimeFellowshipState.ResetSession` clears +the roster and ACE only sends `0x02BE`/`0x02C0` while `FellowshipPanelOpen` +(gated by `0x00A6`, lane B §4.5/row 78), the roster can present empty/frozen +after reconnect until the user manually toggles the tab. This is the exact +"missed true-send freezes the roster" failure the D4 contract warns about, +just on the reconnect edge (outside the five in-session transitions, which +are all correct). Recommend re-declaring on session reset (drive +`SetPageVisible` from the reset hook, or clear `_pageVisible` so the next +`Tick`/transition re-arms) — and confirm ACE's world-enter re-send behavior +before the connected gate closes. *(I could not establish ACE's exact +world-enter `0x02BE` behavior from the client decomp; flag it for the gate.)* + +### SHOULD-FIX 4 — no panel-level test pins the D4 conjunction +The controller-level idempotence IS tested +(`SetPageVisible_SendsPanelOpen_OnlyOnATransition`), but the panel-level +wiring in `SocialPanelController` — `OnShown`/`OnHidden`/`SwitchTo` × +active-tab → the correct `SetPanelOpen` calls, including the "close on other +tab sends nothing" and "tab-away sends false" cases — has **no** direct +assertion. The callbacks record `fellowship-set-panel-open:{bool}` +(`SocialPanelControllerTests.cs:51`) but nothing asserts on it. For the +highest-stakes mechanism, add a transition test that opens on Fellowship, +tabs away/back, and closes on each tab, asserting the exact +`set-panel-open` call sequence. + +### NIT 5 — Open/Close caption does not optimistically pre-toggle +Lane B row 77 notes retail "pre-toggles its own `_open_fellow`" on the Open +button click, i.e. the caption flips immediately client-side, then the server +confirms. acdream's `RefreshOpenCaption` keys the caption off the +server-authoritative `snapshot.IsOpen` (`:495-508`), so the caption flips +only when the `0x02BE` echo lands. Functionally convergent (and instant on +localhost), but not byte-faithful to retail's optimistic local pre-toggle. + +### NIT 6 — `MathF.Round` vs retail `_ftol2` truncation (non-issue, documented) +`FormatStatsText` uses `(int)MathF.Round(pct*100f)` (`:650`) where retail's +`UpdateFellowStats` truncates via `_ftol2`. For every value in the even-split +table (×100 = 100/75/60/55/50/45/40/35/31.11/28) round and truncation agree, +so there is no observable difference on any reachable input. Worth a one-word +note that the choice is deliberate/equivalent; no change required. + +--- + +## Coverage confirmation +- Live-mount probe (real DATs): **PASS 1/1** — the FA4 fellowship assertions + (name field `UiField`, 11 controls `UiButton`, row template + 5 fields, + labels/captions, zero-warning `Bind()`) all hold against the production mount. +- `--no-build` targeted suites: App FA4 = 75/75, Runtime fellowship = 28/28. +- The structural finding in the ledger (Create-flow controls are children of + the not-in-fellowship frame `0x1000026B`) is confirmed by the probe dump: + `0x1000026B` children include `0x10000270/71/72/73` (all four checkboxes), + `0x1000026F` (name field), `0x10000274` (Create).