fix(CT): CT2 fix round — dedupe client-side title add, drop retail-inexact send guard

Opus dual-lens review of CT2 (bcfddc97) found 4 SHOULD-FIX + notes; this
applies the campaign lead's rulings.

F1 (the important one): retail's client-side table add is DEDUPED —
gmCharacterTitleUI::RecvNotice_AddCharacterTitle @0x0049a990 walks
mTitleList and returns without effect when the id is already present,
only inserting on a miss. The server-side SendNotice_AddCharacterTitle
broadcast is unconditional, but RuntimeCharacterTitleState.ApplyUpdateTitle
models the CLIENT receive side, so TitleAdded now fires only on a genuine
new membership. Inverted the pin:
ApplyUpdateTitle_AlreadyEarnedId_DoesNotFireTitleAddedOrBumpRevision.

F3: removed the send-side titleId==0 rejection from both command
adapters. Retail's own send path (Event_SetDisplayCharacterTitle
@0x006a5720) packs whatever id it is handed, and ACE accepts id 0
(CharacterTitle.Invalid is a defined enum value) — retail's real
protection is the UI ghost-when-current gate (CT3's job), not a
send-side rejection. No register row: this makes acdream MORE
retail-exact.

A2: ResetSession now publishes TableReplaced unconditionally and
DisplayTitleChanged when the display id was non-zero before the clear,
matching the LocalPlayerState.Clear() precedent (publish every category
even when Clear is repeated, so a failed reset can converge on retry).

A3: RuntimeCharacterState.CaptureOwnership reads the new non-allocating
Titles.Count instead of EarnedTitleIds.Count; EarnedTitleIds now carries
an XML warning that every read allocates.

A4/A5: ReplaceTable/ApplyUpdateTitle now mutate under one _gate hold with
change flags computed inside the lock and events raised after release;
every revision bump is gated on an actual state change (a no-op wire
resend produces zero revision edges), matching the change-gated
RuntimeMovementSkillState precedent. TableReplaced itself still fires
unconditionally per retail's own Refresh() dispatch on 0x0029.

A1/A6/A7/A8: CharacterTitleResolverLiveDatTests honors ACDREAM_DAT_DIR
first (CT1 fix-round pattern); documented the EmitResult
primaryObjectId-as-title-id precedent inline; corrected the "third
consumer" comment (CT1 §5 already records gmAttributeUI::PostInit's
icon-DID lookup — CT5 factors the shared GetDIDByEnum helper); added a
titleId -> resolved-string memo to CharacterTitleResolver, the DAT-static
equivalent of retail's lazy-hash cache on the string buffer.

Appended a "CT3 anchors from the CT2 review" list to the plan doc's CT2
ledger entry for CT3 to consume.

Build green. Runtime (102), Core.Net (12), and App (27 + 3 InstalledDat
pins under ACDREAM_RUN_INSTALLED_DAT_TESTS=1) title-scoped tests pass.
Full hermetic solution suite (Lane exclusions per the release gate) is
green: 0 failures across all 15 test projects.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-24 22:27:07 +02:00
parent bcfddc97e7
commit 544f8cb2d7
8 changed files with 350 additions and 69 deletions

View file

@ -155,14 +155,11 @@ setAsDisplay). New sibling owner `RuntimeCharacterTitleState`
(`RuntimeCharacterState.Titles`) holds the earned-title set + display
title id, clears at generation reset (`CaptureOwnership`/`IsConverged`
extended with `TitleCount`/`DisplayTitleIsDefault`), and fires
`TableReplaced`/`TitleAdded`/`DisplayTitleChanged` — matching retail's
own unconditional-add / gated-display-set contract
(`Handle_Social__AddOrSetCharacterTitle @0x00564260`). Outbound
`TableReplaced`/`TitleAdded`/`DisplayTitleChanged`. Outbound
`TitleSet (0x002C)` ships behind `IRuntimeCharacterCommands.SetTitle`
on both hosts (`DirectGameRuntimeCommandAdapter` direct-send,
`CurrentGameRuntimeCommandAdapter` via the `LiveCommandBus`/
`LiveSessionCommandRouter` queue) with **NO optimistic local
mutation** — verified against retail's own
`LiveSessionCommandRouter` queue) — verified against retail's own
`CM_Social::Event_SetDisplayCharacterTitle @0x006a5720`, which sends
the wire message and touches no local field; the display title updates
only from the server's own echo. No register row: this slice
@ -173,10 +170,66 @@ StringTable(`0x2300000E`) chain for CT3/CT4 to consume; Runtime stays
id-only. Conformance tests against ACE's writer shapes
(`tests/AcDream.Core.Net.Tests/Messages/CharacterTitleEventsTests.cs`),
Runtime owner tests (`RuntimeCharacterTitleStateTests.cs` +
`RuntimeCharacterStateTests.cs` integration), a no-local-mutation
command test (`DirectGameRuntimeCommandAdapterTests.cs`), and an
InstalledDat pin (`CharacterTitleResolverLiveDatTests.cs`, ids 0/1/2/3/
5/13/14) all pass.
`RuntimeCharacterStateTests.cs` integration), a wire-send command test
(`DirectGameRuntimeCommandAdapterTests.cs`), and an InstalledDat pin
(`CharacterTitleResolverLiveDatTests.cs`, ids 0/1/2/3/5/13/14) all pass.
**CT2 fix round (Opus dual-lens review, 2026-08-24).** Four SHOULD-FIX
corrections landed. **F1 (the important one):** the NOTICE broadcast is
unconditional (retail's server-side `SendNotice_AddCharacterTitle` fires
regardless of prior membership), but the client-side table ADD is
DEDUPED — `gmCharacterTitleUI::RecvNotice_AddCharacterTitle @0x0049a990`
walks `mTitleList` and returns without effect when the id is already
present, only inserting + adding the row on a miss.
`RuntimeCharacterTitleState.ApplyUpdateTitle` (which models the CLIENT
receive side, not the server send side) now fires `TitleAdded` only on a
genuine new membership; the inverted pin is
`ApplyUpdateTitle_AlreadyEarnedId_DoesNotFireTitleAddedOrBumpRevision`.
**F3:** the send-side `titleId == 0` rejection is REMOVED from both
command adapters — retail's own send path
(`Event_SetDisplayCharacterTitle @0x006a5720`) packs whatever id it is
handed, and ACE accepts id 0 (`CharacterTitle.Invalid` is a defined enum
value); retail's actual protection is the UI ghost-when-current gate
(CT3's job), not a send-side rejection. No register row: removing the
guard makes acdream MORE retail-exact, not less. The fix round also
closed four SHOULD-FIX-adjacent items: A2 (`ResetSession` now publishes
`TableReplaced` unconditionally and `DisplayTitleChanged` when the
display id was non-zero before the clear, matching the
`LocalPlayerState.Clear()` precedent), A3 (`RuntimeCharacterState
.CaptureOwnership` reads the new non-allocating `Titles.Count` instead of
`EarnedTitleIds.Count`), A4 (the whole mutation in `ReplaceTable`/
`ApplyUpdateTitle` now happens under one `_gate` hold, with change flags
computed inside the lock and events raised after release), and A5 (every
revision bump is now gated on an actual state change — a no-op wire
resend produces zero revision edges; `TableReplaced` itself still fires
unconditionally per retail's own `Refresh()` dispatch). A1
(`CharacterTitleResolverLiveDatTests` now honors `ACDREAM_DAT_DIR`
first), A6 (documented the `EmitResult` `primaryObjectId`-as-title-id
precedent inline), A7 (corrected the "third consumer" comment — CT1 §5
already records `gmAttributeUI::PostInit`'s icon-DID lookup as that third
consumer; CT5 is where the shared `GetDIDByEnum` helper gets factored),
and A8 (`CharacterTitleResolver` now memoizes the final resolved string
per title id, the DAT-static equivalent of retail's lazy-hash cache on
the string buffer) round out the fix round.
**CT3 anchors from the CT2 review** (carried forward for CT3 to consume,
not yet acted on):
1. CT3 must refresh the display-title TEXT from `TableReplaced` as well
as `DisplayTitleChanged` — retail's
`RecvNotice_UpdateCharacterTitleTable` unconditionally `Refresh()`es
on every `0x0029` arrival, not only when the display id differs.
2. ACE sends NO echo when re-setting the already-current title — the
Set-as-Display button must not wait for a confirmation that never
arrives; retail prevents the send in the first place via the UI
ghost-when-current gate.
3. Retail's fallback display text when a title id doesn't resolve is the
hardcoded literal `"Unknown"` (`Refresh @0x0049abc0`), not a
StringTable key — `CharacterTitleResolver.Resolve` returning `null`
is the correct signal for CT3 to substitute that literal.
4. The deduped client-side add contract (F1 above) — CT3's title-list
row rendering must not assume every `TitleAdded` firing corresponds
to a wire arrival; the reverse still holds (every genuine new row has
a `TitleAdded` firing).
**CT3 — Titles page UI.** Bind the authored page through the standard
GUI classes (`UiTemplateListBox`/`UiScrollbar`/`UiButton` — zero