docs: FA4 fix-round narrow re-review -- CLOSED with one REOPEN (MUST-FIX 3)
Re-derived each disposition from the actual fix diffs (290f9b58/5499f058/ df000306/300d8189/55b17e15/1d743277/f041b09b), not the commit claims. CLOSED (4/5 MUST-FIX, all 9 SHOULD-FIX, all 4 NIT, blast SF-1): - MUST-FIX 1: (int)((double)pct*100.0) truncation + 6->44%/8->34% pinning cases + gate step corrected. - MUST-FIX 2: intercept deleted, every type routes to the generic controller, type-4 dialog test added; type-1 allegiance path unaffected (was never intercepted). - MUST-FIX 4: world->panel selection sync reproduces retail's found/ fallback arms; AD-82 records the deferred generic UiTemplateListBox selection-model port honestly -- minimal-observable-contract, not a hidden gap. - MUST-FIX 5: AD-82/AD-83 well-formed; AD-78 count corrected to 34/16. - D6/D7/SF-8 dimming (audited from source): 34 dimmed / 16 live is correct, not split-the-difference. FellowshipShareLoot has NO client value-reader (only an editor/display surface; 0x00A2 sends shareXP alone; ACE authors loot server-side) -> dimmed faithful. FellowshipShareXP is genuinely read by the Create click -> Live right. REOPEN (MUST-FIX 3): the 0x00A6 reconnect re-arm is placed at a pre-world reset seam. ResetSessionTransientUi runs via the SessionDialogs reset stage at ResetHostBeforeStart / retired-scope teardown -- both BEFORE _inWorld=true and before command activation for the new generation -- and SetPanelOpen requires world, so the re-declaration returns Inactive and nothing is published, yet _pageVisible is still set true and no post-world-entry hook re-evaluates. The new server never receives 0x00A6 and fellow vitals stay frozen -- the exact bug the fix targets. The unit test passes only because its fake command records unconditionally. Recommend moving the re-declaration to an in-world seam (EnteredWorld). Totals/probe: 109/109 touched App test classes green on post-fix binaries; live-mount probe PASS 1/1; +13/0-deletion delta and 13,285/4/0 corroborated on the touched projects and by arithmetic (not re-run end-to-end). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
f041b09b7c
commit
8bbceff594
1 changed files with 173 additions and 0 deletions
|
|
@ -547,3 +547,176 @@ slice exists to enable), on the missing panel-level D4 conjunction test
|
||||||
(its SHOULD-FIX 4 = SF-5 here), on the leader-gold-tint register omission
|
(its SHOULD-FIX 4 = SF-5 here), on the leader-gold-tint register omission
|
||||||
(its MUST-FIX 1 = part of MUST-FIX 5 here), and on the live-DAT probe
|
(its MUST-FIX 1 = part of MUST-FIX 5 here), and on the live-DAT probe
|
||||||
result.
|
result.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Narrow re-review of the fix round — 2026-08-12
|
||||||
|
|
||||||
|
**Fix commits:** `290f9b58` / `5499f058` / `df000306` / `300d8189` /
|
||||||
|
`55b17e15` / `1d743277` / `f041b09b`.
|
||||||
|
|
||||||
|
**Verdict: CLOSED with ONE REOPEN — MUST-FIX 3.** Dispositions re-derived
|
||||||
|
from the actual diffs (not the commit claims), the D6/D7/SF-8 dimming
|
||||||
|
arithmetic audited from primary source, totals reconciled on the touched
|
||||||
|
projects, and the live-mount probe re-run on the post-fix binaries. Four
|
||||||
|
of five MUST-FIX, all 9 SHOULD-FIX, all 4 NIT, and blast SF-1 are
|
||||||
|
correctly applied. **MUST-FIX 3 is REOPENED:** the latch-clear it ships is
|
||||||
|
necessary but its re-declaration never reaches the wire in the real
|
||||||
|
reconnect path, so the fellow-vitals-freeze defect it targets persists.
|
||||||
|
|
||||||
|
### MUST-FIX 1 — CLOSED
|
||||||
|
`FormatStatsText` now returns `$"…{(int)((double)pct * 100.0)}%"`
|
||||||
|
(`SocialFellowshipPageController.cs:790`). The two failing sizes are
|
||||||
|
pinned: `[InlineData(true, true, 6, "12 44%")]` and
|
||||||
|
`[InlineData(true, true, 8, "12 34%")]` added to
|
||||||
|
`FormatStatsText_MatchesD5Rules`. Gate-script step 23 corrected to "the
|
||||||
|
panel must show 44% and 34% respectively — NOT 45% or 35%". AD-80's
|
||||||
|
"renders retail's byte-decoded table verbatim" claim is now true of the
|
||||||
|
rendered integer, not just the table — no further AD-80 edit owed.
|
||||||
|
|
||||||
|
### MUST-FIX 2 — CLOSED
|
||||||
|
`TryAutoRespondToFellowshipInvite` deleted; `HandleConfirmationRequest`
|
||||||
|
is now `=> _gameplayConfirmationController?.HandleRequest(request) == true`
|
||||||
|
(`RetailUiRuntime.cs:678`), routing every type through the generic
|
||||||
|
controller exactly as retail `Handle_Character__ConfirmationRequest
|
||||||
|
@0x005640A0` does. New test
|
||||||
|
`FellowshipInviteRequest_Type4_OpensDialog_MessageVerbatim_AndSendsAcceptOnClose`
|
||||||
|
proves the type-4 dialog renders the server message verbatim (not
|
||||||
|
`" Continue?"`-suffixed) and sends accept through the generic path.
|
||||||
|
**No allegiance (type-1) path broke:** the deleted intercept keyed only on
|
||||||
|
`ConfirmationType.Fellowship` (type 4) — type 1 always fell through to the
|
||||||
|
generic controller both before and after the fix (verified against the
|
||||||
|
pre-fix `5bdd0528` source). The `IgnoreFellowshipRequests` /
|
||||||
|
`FellowshipAutoAcceptRequests` rows revert with the dimming reversal below.
|
||||||
|
|
||||||
|
### MUST-FIX 3 — REOPEN (latch-clear correct, re-declaration swallowed)
|
||||||
|
The fix adds `SocialPanelController.ResetSessionDeclaration()` (clears the
|
||||||
|
edge-latch via `SocialFellowshipPageController.ResetPageVisibleLatch()`,
|
||||||
|
then re-evaluates the D4 conjunction) and wires it into
|
||||||
|
`RetailUiRuntime.ResetSessionTransientUi()`. The direction is right; the
|
||||||
|
**timing is wrong**, and the unit test cannot see it because its fake
|
||||||
|
command records unconditionally.
|
||||||
|
|
||||||
|
Traced from source:
|
||||||
|
- `ResetSessionTransientUi` runs only via the `SessionDialogs` reset stage
|
||||||
|
(`LiveSessionRuntimeFactory.cs:182` -> `LiveSessionResetManifest`).
|
||||||
|
- Both `ResetSessionState` call sites run that stage **before the new
|
||||||
|
session is in-world**: `ResetHostBeforeStart`
|
||||||
|
(`LiveSessionController.cs:733-751`, run inside `StartCore` *before*
|
||||||
|
`_inWorld = true` at `:642` and before `binding.ActivateCommands()`),
|
||||||
|
and the retired-scope teardown (`:231`, which tears down the OLD scope).
|
||||||
|
- `SetPanelOpen` requires world:
|
||||||
|
`Validate(expectedGeneration, requireWorld: true)`
|
||||||
|
(`CurrentGameRuntimeCommandAdapter.cs:929`), and the deferred App seam is
|
||||||
|
not even bound to the new generation until `ActivateCommands`. So during
|
||||||
|
the reset the send returns `Inactive`/rejected and **nothing is
|
||||||
|
published**.
|
||||||
|
- But `ResetSessionDeclaration` -> `SetPageVisible(true)` still sets
|
||||||
|
`_pageVisible = true` (the transition fires the no-op command). No
|
||||||
|
post-world-entry hook re-evaluates the conjunction (the `EnteredWorld`
|
||||||
|
bindings — RestoreLayout/SyncToolbar/etc. — never touch the social
|
||||||
|
panel's visibility, and the panel is not a state-managed-visibility
|
||||||
|
window so layout restore produces no OnShown edge; `Tick` never calls
|
||||||
|
`SetPageVisible`).
|
||||||
|
|
||||||
|
Net: after an in-process reconnect with the Fellowship page left open,
|
||||||
|
`_pageVisible` ends `true` but `0x00A6` was never sent to the new server —
|
||||||
|
the exact vitals-freeze this MUST-FIX targets (lane B §4.5) persists. The
|
||||||
|
fix relocated the latch reset without achieving the observable outcome.
|
||||||
|
The gate script's new reconnect step (step 9) *will* catch this at the
|
||||||
|
owed connected gate — so this is not silently hidden — but a green
|
||||||
|
automated gate here is not evidence the wire send happens, and the
|
||||||
|
fixer's "resolved" claim is not substantiated in the real path.
|
||||||
|
|
||||||
|
**Recommended direction:** re-declare while in-world on the new session,
|
||||||
|
not during the pre-world reset — e.g. re-run the D4 conjunction from the
|
||||||
|
`EnteredWorld` seam (`LiveSessionRuntimeFactory.cs:158-164`) after clearing
|
||||||
|
the latch, or make `SetPageVisible` refuse to advance `_pageVisible` when
|
||||||
|
the command result is not `Accepted` (paired with an in-world evaluation
|
||||||
|
trigger). Either is a one-seam change; the current placement cannot work
|
||||||
|
because the reset is a pre-connect, local-state-clearing phase.
|
||||||
|
|
||||||
|
### MUST-FIX 4 — CLOSED (deferral is minimal-observable-contract, audited)
|
||||||
|
`SyncSelectionFromWorld`/`SetSelectedFellow` reproduce retail
|
||||||
|
`UpdateFellowSelection @0x0048F0F0`: the "found" arm (a world selection
|
||||||
|
matching a member becomes the panel selection, enabling Dismiss/Leader and
|
||||||
|
tinting the row) and the "fallback" arm (a non-member world selection
|
||||||
|
leaves the existing panel selection untouched; `RefreshRoster` clears it
|
||||||
|
only when that fellow leaves the roster). Called every `Tick` while in a
|
||||||
|
fellowship, mirroring retail's per-`Update` placement. The generic
|
||||||
|
`UiTemplateListBox` selection-model + `0x1000000D` row-instance-id port is
|
||||||
|
**not** done — but AD-82(4) records that honestly as owed, and the
|
||||||
|
OBSERVABLE contract (button-enable + a highlight) is met. This is a
|
||||||
|
scoped, declared page-local reimplementation, not a hidden gap: the only
|
||||||
|
observable divergences (a blue selection tint retail never paints; only
|
||||||
|
the name text is a click target) are both named in AD-82.
|
||||||
|
|
||||||
|
### MUST-FIX 5 — CLOSED
|
||||||
|
AD-82 (invented leader/selection tints, name-text-only click target,
|
||||||
|
page-local world->panel sync with the generic port recorded as owed) and
|
||||||
|
AD-83 (Recruit "is-a-player" enable-gate omission) are well-formed:
|
||||||
|
correct file/symbol cites, honest "Risk" columns, real decomp anchors
|
||||||
|
(`UpdateFellowSelection @0x0048F0F0`, `ListenToElementMessage @0x004901C0`,
|
||||||
|
`UpdateButtons` §2.8). The prior "inline comment, not a register row" call
|
||||||
|
on the Recruit gate is corrected. AD-78's stale "35 of 50" is fixed to
|
||||||
|
"34 of 50 / 16 live" and self-flags the two-campaign register lag as an
|
||||||
|
instance of the exact risk that row warns about.
|
||||||
|
|
||||||
|
### D6/D7/SF-8 dimming arithmetic — CLOSED and CORRECT (audited from source)
|
||||||
|
Landed at **34 of 50 dimmed / 16 live** — one net un-dim from the pre-FA4
|
||||||
|
baseline (`FellowshipShareXP` only), not "splitting the difference".
|
||||||
|
Verified from primary source, not the commit message:
|
||||||
|
- **`FellowshipShareLoot` -> dimmed is faithful.** Grep of `src/` finds
|
||||||
|
exactly two live references (`SocialFellowshipPageController.cs:437`
|
||||||
|
binds the page checkbox; `:578` seeds it from the value) — both an
|
||||||
|
*editor/display* surface, neither a *reader of the value*. The `0x00A2`
|
||||||
|
Create builder carries `shareXP` alone (`SocialActions.cs:139-141`);
|
||||||
|
nothing in acdream reads `FellowshipShareLoot` to drive a wire message
|
||||||
|
or client behavior (ACE authors loot-sharing and its chat lines
|
||||||
|
server-side). A second editor of a stored value is not a consumer — this
|
||||||
|
is precisely AD-78's own store-only definition.
|
||||||
|
- **`FellowshipShareXP` -> Live is right.** The Create-button `OnClick`
|
||||||
|
(`SocialFellowshipPageController.cs:377`) reads
|
||||||
|
`CurrentCharacterOption(FellowshipShareXP)` and passes it as the sent
|
||||||
|
`shareXP` bit — a genuine client-side read of the value that changes
|
||||||
|
what the wire carries (lane B feature 1: "shareXP comes from option
|
||||||
|
0x0F"). That is the exact asymmetry that separates it from ShareLoot.
|
||||||
|
- `IgnoreFellowshipRequests`/`FellowshipAutoAcceptRequests` -> dimmed:
|
||||||
|
their only claimed consumer was the deleted intercept; both are pure
|
||||||
|
server-side filters. Correct.
|
||||||
|
Conformance test updated to `Assert.Equal(34, …)` / `Assert.Equal(16, …)`
|
||||||
|
with the three ids added back to `ExpectedStoreOnlyIds`.
|
||||||
|
|
||||||
|
### SHOULD-FIX / NIT — all applied (spot-verified)
|
||||||
|
SF-1 (`_fellowshipNameLinesProvider` cached, reassigned only on a real
|
||||||
|
name change — `:583`); SF-2/SF-3 (`_memberGuids` is the source of truth
|
||||||
|
for the membership diff and Recruit's already-a-fellow test, `:626`/`:657`
|
||||||
|
— no longer `_rows.Count`); SF-4 (`Dispose` unsubscribes the stored
|
||||||
|
`_onActivePageChanged`); SF-5 (the D4 panel-level conjunction is now
|
||||||
|
tested by `FellowshipPageVisible_Declares0x00A6_OnlyWhenWindowShownANDFellowshipActive`,
|
||||||
|
plus a member-leaves shrink test and a selection-clear test); SF-6 (the
|
||||||
|
probe now `Assert`s the four labels non-empty and the captions exactly
|
||||||
|
`"Open"`/`"Close"`); SF-7 (gate step 3 self-contradiction reconciled);
|
||||||
|
SF-8 (see dimming above); SF-9 / N-0 / N-1 / N-2 / N-3 (AD-78 count,
|
||||||
|
optimistic caption pre-toggle, meter-child + `max>0` + snapshot-atomicity
|
||||||
|
doc notes) all present.
|
||||||
|
|
||||||
|
### Totals and probe
|
||||||
|
`ACDREAM_PROBE_LIVE_MOUNT=1` re-run on the post-fix binaries: PASS 1/1 —
|
||||||
|
the four checkbox labels resolve, captions resolve `Open`/`Close`, tab
|
||||||
|
captions Allegiance/Fellowship/Friends/Squelch. The six touched App test
|
||||||
|
classes pass **109/109** on the post-fix Release binaries (rebuilt
|
||||||
|
07:36). The ledger's **+13 / 0-deletion** delta reconciles by direct count
|
||||||
|
(8 in `SocialFellowshipPageControllerTests` = 2 new `[InlineData]` + 6
|
||||||
|
`[Fact]`; 4 `[Fact]` in `SocialPanelControllerTests`; 1 `[Fact]` in
|
||||||
|
`GameplayConfirmationControllerTests`; the two count-only changes are
|
||||||
|
net-0). The full-suite **13,285/4/0** claim is corroborated on the touched
|
||||||
|
projects and by arithmetic (13,272 + 13); it was not re-run end-to-end
|
||||||
|
here.
|
||||||
|
|
||||||
|
### Disposition
|
||||||
|
One REOPEN (MUST-FIX 3 — re-declaration placed at a pre-world reset seam,
|
||||||
|
so `0x00A6` never reaches the reconnected server). Everything else CLOSED,
|
||||||
|
with the dimming arithmetic audited correct at 34/16. Recommend a focused
|
||||||
|
MUST-FIX-3-only re-fix (move the re-declaration to an in-world seam); no
|
||||||
|
re-review of the other findings is owed.
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue