diff --git a/docs/plans/2026-08-24-character-panel-parity-campaign.md b/docs/plans/2026-08-24-character-panel-parity-campaign.md index ca44648b..22e7524b 100644 --- a/docs/plans/2026-08-24-character-panel-parity-campaign.md +++ b/docs/plans/2026-08-24-character-panel-parity-campaign.md @@ -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 diff --git a/src/AcDream.App/Runtime/CurrentGameRuntimeCommandAdapter.cs b/src/AcDream.App/Runtime/CurrentGameRuntimeCommandAdapter.cs index e7e8ca90..dd8c811b 100644 --- a/src/AcDream.App/Runtime/CurrentGameRuntimeCommandAdapter.cs +++ b/src/AcDream.App/Runtime/CurrentGameRuntimeCommandAdapter.cs @@ -725,17 +725,22 @@ internal sealed class CurrentGameRuntimeCommandAdapter RuntimeCommandStatus gate = Validate(expectedGeneration, requireWorld: true); if (gate != RuntimeCommandStatus.Accepted) return Result(gate); - if (titleId == 0u) - { - return EmitResult( - RuntimeCommandDomain.Character, - operation: 6, - RuntimeCommandStatus.Rejected); - } + // F3 (CT2 fix round, 2026-08-24): NO id-0 guard here — 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 + // protection against sending an unearned/invalid id is the UI + // ghost-when-current gate (CT3's job), not a send-side rejection — + // a client-side guard here blocks a state the server honors. // CT2: NO optimistic local mutation — matches // DirectGameRuntimeCommandAdapter.SetTitle; RuntimeCharacterState. // Titles updates only from the server's own echo. _commands.Publish(new SetTitleRuntimeCmd(titleId)); + // A6 (CT2 fix round): titleId rides the EmitResult objectId slot — + // same in-class precedent as Advance's command.StatId above (see + // the S4 history comment on SaveOptions/EmitResult, which + // established this field as a typed domain-payload-id slot, not + // always an object guid). return EmitResult( RuntimeCommandDomain.Character, operation: 6, diff --git a/src/AcDream.App/UI/Layout/CharacterTitleResolver.cs b/src/AcDream.App/UI/Layout/CharacterTitleResolver.cs index f967b288..6bea0f71 100644 --- a/src/AcDream.App/UI/Layout/CharacterTitleResolver.cs +++ b/src/AcDream.App/UI/Layout/CharacterTitleResolver.cs @@ -17,11 +17,16 @@ namespace AcDream.App.UI.Layout; /// resolved DIDs are hardcoded here rather than porting the indirection /// generically — the same precedent already /// set for this exact category-4 family (its KeyNameTableId/ -/// MetaKeyNameTableId/DelimiterTableId); factor out a shared -/// helper only if a THIRD consumer of GetDIDByEnum appears. Both DIDs -/// were verified end-to-end against ACE's CharacterTitle.WarMage = 13 -/// in Campaign CT slice CT1 -/// (docs/research/2026-08-24-campaign-ct-dat-ground-truth.md §5, pinned by +/// MetaKeyNameTableId/DelimiterTableId). A THIRD consumer of +/// GetDIDByEnum has already appeared — +/// gmAttributeUI::PostInit @0x0049DB70 resolves per-attribute icon +/// DIDs via category 0x10000002 — so CT5 (not "if a third consumer +/// appears") is where the shared GetDIDByEnum(enumValue, category) +/// helper gets factored out, per +/// docs/research/2026-08-24-campaign-ct-dat-ground-truth.md §5 item 1. +/// Both DIDs used here were verified end-to-end against ACE's +/// CharacterTitle.WarMage = 13 in Campaign CT slice CT1 (same §5, +/// pinned by /// CharacterPanelLiveDatTests.TitleStringTable_ResolvesWarMageEndToEnd). /// /// @@ -42,6 +47,7 @@ public sealed class CharacterTitleResolver private readonly IDatReaderWriter _dats; private readonly DatStringResolver _strings; + private readonly Dictionary _resolvedCache = new(); private EnumMapper? _titleEnumMapper; private bool _loadedMapper; @@ -59,27 +65,38 @@ public sealed class CharacterTitleResolver /// 's own /// null-on-miss contract. /// + /// + /// A8 (CT2 fix round, 2026-08-24): memoizes the final resolved string + /// per (including misses). Retail caches the + /// computed hash directly on the string buffer + /// (GetCharacterTitleFromID's 0xFFFFFFFF lazy-hash + /// sentinel); a memo of the final string is the equivalent here given + /// our immutable, DAT-static tables — no invalidation is needed. + /// public string? Resolve(uint titleId) { if (titleId == 0u) return null; + if (_resolvedCache.TryGetValue(titleId, out string? cached)) + return cached; + if (!_loadedMapper) { _dats.Portal.TryGet(TitleEnumMapperId, out _titleEnumMapper); _loadedMapper = true; } - if (_titleEnumMapper is null - || !_titleEnumMapper.IdToStringMap.TryGetValue(titleId, out var rawNameValue)) + string? resolved = null; + if (_titleEnumMapper is not null + && _titleEnumMapper.IdToStringMap.TryGetValue(titleId, out var rawNameValue)) { - return null; + string rawName = rawNameValue.ToString(); + if (!string.IsNullOrEmpty(rawName)) + resolved = _strings.Resolve(TitleStringTableId, DatStringResolver.ComputeHash(rawName)); } - string rawName = rawNameValue.ToString(); - if (string.IsNullOrEmpty(rawName)) - return null; - - return _strings.Resolve(TitleStringTableId, DatStringResolver.ComputeHash(rawName)); + _resolvedCache[titleId] = resolved; + return resolved; } } diff --git a/src/AcDream.Runtime/Gameplay/RuntimeCharacterState.cs b/src/AcDream.Runtime/Gameplay/RuntimeCharacterState.cs index 1fce941c..a32aff5c 100644 --- a/src/AcDream.Runtime/Gameplay/RuntimeCharacterState.cs +++ b/src/AcDream.Runtime/Gameplay/RuntimeCharacterState.cs @@ -252,7 +252,7 @@ public sealed class RuntimeCharacterState : IDisposable && _movementSkillAugmentations == default, AutonomyLevel == FullAutonomyLevel, OptionsAreClean: !Options.IsDirty, - TitleCount: Titles.EarnedTitleIds.Count, + TitleCount: Titles.Count, DisplayTitleIsDefault: Titles.DisplayTitleId == 0u); } @@ -1238,6 +1238,16 @@ public readonly record struct RuntimeCharacterTitleSnapshot( /// UpdateTitle (never re-add an optimistic write here — the CA /// campaign lesson). /// +/// +/// CT2 fix round (2026-08-24), F1: 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. +/// models the client-side receive handler, +/// so fires only on a genuine new membership. +/// public sealed class RuntimeCharacterTitleState { private readonly object _gate = new(); @@ -1245,32 +1255,54 @@ public sealed class RuntimeCharacterTitleState private uint _displayTitleId; private long _revision; - /// Fires after a full 0x0029 CharacterTitle table replace. + /// + /// Fires after every 0x0029 CharacterTitle table replace, + /// unconditionally — matches retail's own + /// gmCharacterTitleUI::RecvNotice_UpdateCharacterTitleTable, + /// which always calls Refresh() regardless of whether the new + /// table differs from the old one. + /// public event Action? TableReplaced; /// - /// Fires once per 0x002B UpdateTitle arrival, UNCONDITIONALLY — - /// matches retail's own SendNotice_AddCharacterTitle, which - /// broadcasts regardless of whether the id was already in the earned - /// set. Carries the added title id. + /// Fires only when an 0x002B UpdateTitle arrival actually adds a + /// NEW id to the earned set — matches retail's client-side + /// gmCharacterTitleUI::RecvNotice_AddCharacterTitle @0x0049a990, + /// which dedupes against mTitleList before inserting (see the + /// class remarks: the F1 fix-round correction). Carries the added + /// title id. /// public event Action? TitleAdded; /// - /// Fires whenever the display title id changes — from either a fresh - /// 0x0029 table (a differing seed) or an 0x002B whose - /// setAsDisplay flag is set. Carries the NEW display title id. + /// Fires whenever the display title id actually changes — from either a + /// fresh 0x0029 table (a differing seed) or an 0x002B + /// whose setAsDisplay flag is set to an id that differs from the + /// current display title. Carries the NEW display title id. /// public event Action? DisplayTitleChanged; public uint DisplayTitleId => Volatile.Read(ref _displayTitleId); public long Revision => Interlocked.Read(ref _revision); + /// + /// WARNING: every read allocates a fresh array (ToArray() under + /// the gate). Fine for UI refresh call sites (CT3), but NEVER read this + /// per-frame — use or + /// for hot-path checks. + /// public IReadOnlyCollection EarnedTitleIds { get { lock (_gate) return _earnedTitleIds.ToArray(); } } + /// Non-allocating earned-title count; prefer this over + /// EarnedTitleIds.Count in hot paths (A3, CT2 fix round). + public int Count + { + get { lock (_gate) return _earnedTitleIds.Count; } + } + public bool HasEarnedTitle(uint titleId) { lock (_gate) return _earnedTitleIds.Contains(titleId); @@ -1290,20 +1322,32 @@ public sealed class RuntimeCharacterTitleState /// 0x0029 CharacterTitle — a WHOLESALE authoritative replace /// (retail's CharacterTitleTable::UnPack always rebuilds /// mTitleList from scratch; there is no incremental-merge path - /// on this opcode). + /// on this opcode). A4 (CT2 fix round): the whole mutation — set clear + /// + rebuild, display-id compare + write — happens under one + /// hold, with change flags computed inside the lock + /// and events raised only after it releases. A5: the revision counter + /// bumps only for an actual content/display change (a byte-identical + /// resend must not produce a revision edge); + /// itself still fires unconditionally, matching retail's own + /// unconditional Refresh() dispatch on this opcode. /// public void ReplaceTable(uint displayTitleId, IReadOnlyList titleIds) { ArgumentNullException.ThrowIfNull(titleIds); + bool setChanged; + bool displayChanged; lock (_gate) { + setChanged = !_earnedTitleIds.SetEquals(titleIds); _earnedTitleIds.Clear(); foreach (uint id in titleIds) _earnedTitleIds.Add(id); + displayChanged = _displayTitleId != displayTitleId; + if (displayChanged) + Volatile.Write(ref _displayTitleId, displayTitleId); } - bool displayChanged = DisplayTitleId != displayTitleId; - Volatile.Write(ref _displayTitleId, displayTitleId); - Interlocked.Increment(ref _revision); + if (setChanged || displayChanged) + Interlocked.Increment(ref _revision); TableReplaced?.Invoke(); if (displayChanged) DisplayTitleChanged?.Invoke(displayTitleId); @@ -1311,27 +1355,63 @@ public sealed class RuntimeCharacterTitleState /// /// 0x002B UpdateTitle — retail's - /// ClientUISystem::Handle_Social__AddOrSetCharacterTitle: ALWAYS - /// add, and additionally set-display only when - /// is true. + /// ClientUISystem::Handle_Social__AddOrSetCharacterTitle ALWAYS + /// broadcasts the add notice server-side, and additionally broadcasts + /// set-display only when is true. On + /// the CLIENT receive side this method models, F1 (CT2 fix round): the + /// add is deduped ( fires only when + /// HashSet<uint>.Add reports a genuine new membership, + /// matching gmCharacterTitleUI::RecvNotice_AddCharacterTitle + /// @0x0049a990's membership check before insert). A4: both halves + /// mutate under one hold with change flags computed + /// inside the lock; events raise after release. A5: the revision + /// counter bumps once per REAL change — zero times for an already- + /// earned id re-sent with pointing at + /// the already-current display id, up to twice when both halves change. /// public void ApplyUpdateTitle(uint titleId, bool setAsDisplay) { - lock (_gate) _earnedTitleIds.Add(titleId); - Interlocked.Increment(ref _revision); - TitleAdded?.Invoke(titleId); - if (setAsDisplay) + bool added; + bool displayChanged; + lock (_gate) { - Volatile.Write(ref _displayTitleId, titleId); - Interlocked.Increment(ref _revision); - DisplayTitleChanged?.Invoke(titleId); + added = _earnedTitleIds.Add(titleId); + displayChanged = setAsDisplay && _displayTitleId != titleId; + if (displayChanged) + Volatile.Write(ref _displayTitleId, titleId); } + if (added) + Interlocked.Increment(ref _revision); + if (displayChanged) + Interlocked.Increment(ref _revision); + if (added) + TitleAdded?.Invoke(titleId); + if (displayChanged) + DisplayTitleChanged?.Invoke(titleId); } + /// + /// A2 (CT2 fix round): publishes the clear like + /// LocalPlayerState.Clear() — fires + /// unconditionally and fires when the + /// display id was non-zero before the clear, so a failed/retried reset + /// attempt can safely converge (process-lived views pull through this + /// object and use these events as their invalidation edge). The + /// revision counter itself stays unconditional, matching this class's + /// pre-existing reset contract. + /// public void ResetSession() { - lock (_gate) _earnedTitleIds.Clear(); - Volatile.Write(ref _displayTitleId, 0u); + uint previousDisplayTitleId; + lock (_gate) + { + previousDisplayTitleId = _displayTitleId; + _earnedTitleIds.Clear(); + Volatile.Write(ref _displayTitleId, 0u); + } Interlocked.Increment(ref _revision); + TableReplaced?.Invoke(); + if (previousDisplayTitleId != 0u) + DisplayTitleChanged?.Invoke(0u); } } diff --git a/src/AcDream.Runtime/Session/DirectGameRuntimeCommandAdapter.cs b/src/AcDream.Runtime/Session/DirectGameRuntimeCommandAdapter.cs index 42e3b18e..e70ece3c 100644 --- a/src/AcDream.Runtime/Session/DirectGameRuntimeCommandAdapter.cs +++ b/src/AcDream.Runtime/Session/DirectGameRuntimeCommandAdapter.cs @@ -731,18 +731,22 @@ public sealed class DirectGameRuntimeCommandAdapter Validate(expectedGeneration, out WorldSession? session); if (gate != RuntimeCommandStatus.Accepted) return Result(gate); - if (titleId == 0u) - { - return EmitResult( - RuntimeCommandDomain.Character, - operation: 6, - RuntimeCommandStatus.Rejected); - } + // F3 (CT2 fix round, 2026-08-24): NO id-0 guard here — 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 + // protection against sending an unearned/invalid id is the UI + // ghost-when-current gate (CT3's job), not a send-side rejection — + // a client-side guard here blocks a state the server honors. // CT2: NO optimistic local mutation — retail's own // Event_SetDisplayCharacterTitle send path touches no local state; // RuntimeCharacterState.Titles.DisplayTitleId updates only when the // server echoes UpdateTitle (0x002B) with setAsDisplay=true. session!.SendSetTitle(titleId); + // A6 (CT2 fix round): titleId rides the EmitResult primaryObjectId + // slot — same in-class precedent established for a typed + // domain-payload id, not always an object guid (see the S4 history + // comment on SaveOptions/EmitResult above). return EmitResult( RuntimeCommandDomain.Character, operation: 6, diff --git a/tests/AcDream.App.Tests/UI/Layout/CharacterTitleResolverLiveDatTests.cs b/tests/AcDream.App.Tests/UI/Layout/CharacterTitleResolverLiveDatTests.cs index bbf3ff9a..bf4a8876 100644 --- a/tests/AcDream.App.Tests/UI/Layout/CharacterTitleResolverLiveDatTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/CharacterTitleResolverLiveDatTests.cs @@ -20,7 +20,8 @@ namespace AcDream.App.Tests.UI.Layout; public sealed class CharacterTitleResolverLiveDatTests { private static string DatDirectory => - Path.Combine( + System.Environment.GetEnvironmentVariable("ACDREAM_DAT_DIR") + ?? Path.Combine( System.Environment.GetFolderPath(System.Environment.SpecialFolder.UserProfile), "Documents", "Asheron's Call"); diff --git a/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCharacterTitleStateTests.cs b/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCharacterTitleStateTests.cs index 0aa1f1a2..fed205fc 100644 --- a/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCharacterTitleStateTests.cs +++ b/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCharacterTitleStateTests.cs @@ -119,19 +119,59 @@ public sealed class RuntimeCharacterTitleStateTests } [Fact] - public void ApplyUpdateTitle_AlreadyEarnedId_StillFiresTitleAddedUnconditionally() + public void ApplyUpdateTitle_AlreadyEarnedId_DoesNotFireTitleAddedOrBumpRevision() { - // Retail's own broadcast is unconditional — it does not check - // membership before firing the notice. + // F1 (CT2 fix round, 2026-08-24): the SERVER-side broadcast + // (SendNotice_AddCharacterTitle) is unconditional, but this method + // models the CLIENT-side receive handler + // (gmCharacterTitleUI::RecvNotice_AddCharacterTitle @0x0049a990), + // which walks mTitleList and returns without effect when the id is + // already present — only a genuine miss inserts + adds the row. var titles = new RuntimeCharacterTitleState(); titles.ApplyUpdateTitle(7u, setAsDisplay: false); + long revisionAfterFirstAdd = titles.Revision; var added = new List(); titles.TitleAdded += id => added.Add(id); titles.ApplyUpdateTitle(7u, setAsDisplay: false); - Assert.Equal([7u], added); + Assert.Empty(added); Assert.Single(titles.EarnedTitleIds); + Assert.Equal(revisionAfterFirstAdd, titles.Revision); + } + + [Fact] + public void ApplyUpdateTitle_SetAsDisplayOnAlreadyCurrentId_DoesNotFireDisplayTitleChangedOrBumpRevision() + { + // A5 (CT2 fix round): a re-notice for the id that is ALREADY the + // display title is a no-op wire message — it must not produce a + // revision edge or a spurious DisplayTitleChanged. + var titles = new RuntimeCharacterTitleState(); + titles.ApplyUpdateTitle(13u, setAsDisplay: true); + long revisionAfterFirst = titles.Revision; + var displayChanged = new List(); + titles.DisplayTitleChanged += id => displayChanged.Add(id); + var added = new List(); + titles.TitleAdded += id => added.Add(id); + + titles.ApplyUpdateTitle(13u, setAsDisplay: true); + + Assert.Empty(displayChanged); + Assert.Empty(added); + Assert.Equal(revisionAfterFirst, titles.Revision); + } + + [Fact] + public void ApplyUpdateTitle_AddsNewIdAndSetsDisplay_BumpsRevisionTwice() + { + // A5: a single 0x002B that both adds a NEW id and changes the + // display title bumps the revision once per real half-change. + var titles = new RuntimeCharacterTitleState(); + long before = titles.Revision; + + titles.ApplyUpdateTitle(13u, setAsDisplay: true); + + Assert.Equal(before + 2, titles.Revision); } [Fact] @@ -147,6 +187,25 @@ public sealed class RuntimeCharacterTitleStateTests Assert.Equal(99u, titles.DisplayTitleId); } + [Fact] + public void ReplaceTable_IdenticalResend_DoesNotBumpRevision_ButStillFiresTableReplaced() + { + // A5: a byte-identical 0x0029 resend (same table, same display id) + // is a no-op wire message for the revision counter. TableReplaced + // itself still fires unconditionally — retail's own + // RecvNotice_UpdateCharacterTitleTable always Refresh()es. + var titles = new RuntimeCharacterTitleState(); + titles.ReplaceTable(13u, [1u, 5u, 13u]); + long revisionAfterFirst = titles.Revision; + int tableReplacedCount = 0; + titles.TableReplaced += () => tableReplacedCount++; + + titles.ReplaceTable(13u, [1u, 5u, 13u]); + + Assert.Equal(revisionAfterFirst, titles.Revision); + Assert.Equal(1, tableReplacedCount); + } + [Fact] public void ResetSession_ClearsEarnedIdsAndDisplayTitle() { @@ -170,6 +229,57 @@ public sealed class RuntimeCharacterTitleStateTests Assert.True(titles.Revision > before); } + [Fact] + public void ResetSession_NonEmptyState_FiresTableReplacedAndDisplayTitleChanged() + { + // A2 (CT2 fix round): matches the LocalPlayerState.Clear() precedent + // — publish every category even when Clear is repeated, so a failed + // reset attempt can safely converge on retry. + var titles = new RuntimeCharacterTitleState(); + titles.ReplaceTable(13u, [1u, 5u, 13u]); + int tableReplacedCount = 0; + titles.TableReplaced += () => tableReplacedCount++; + var displayChanged = new List(); + titles.DisplayTitleChanged += id => displayChanged.Add(id); + + titles.ResetSession(); + + Assert.Equal(1, tableReplacedCount); + Assert.Equal([0u], displayChanged); + } + + [Fact] + public void ResetSession_RepeatedReset_StillFiresTableReplacedButNotDisplayTitleChanged() + { + // A2: TableReplaced publishes unconditionally on every reset; a + // SECOND reset (display id already 0) must not re-fire + // DisplayTitleChanged — there is no real transition to report. + var titles = new RuntimeCharacterTitleState(); + titles.ResetSession(); + int tableReplacedCount = 0; + titles.TableReplaced += () => tableReplacedCount++; + var displayChanged = new List(); + titles.DisplayTitleChanged += id => displayChanged.Add(id); + + titles.ResetSession(); + + Assert.Equal(1, tableReplacedCount); + Assert.Empty(displayChanged); + } + + [Fact] + public void Count_ReflectsEarnedTitleIdsWithoutAllocatingTheArray() + { + // A3 (CT2 fix round): non-allocating count accessor for hot paths + // like RuntimeCharacterState.CaptureOwnership. + var titles = new RuntimeCharacterTitleState(); + Assert.Equal(0, titles.Count); + + titles.ReplaceTable(13u, [1u, 5u, 13u]); + + Assert.Equal(3, titles.Count); + } + [Fact] public void Snapshot_ReflectsDisplayTitleAndCount() { diff --git a/tests/AcDream.Runtime.Tests/Session/DirectGameRuntimeCommandAdapterTests.cs b/tests/AcDream.Runtime.Tests/Session/DirectGameRuntimeCommandAdapterTests.cs index e61390e0..18a3d679 100644 --- a/tests/AcDream.Runtime.Tests/Session/DirectGameRuntimeCommandAdapterTests.cs +++ b/tests/AcDream.Runtime.Tests/Session/DirectGameRuntimeCommandAdapterTests.cs @@ -376,8 +376,14 @@ public sealed class DirectGameRuntimeCommandAdapterTests } [Fact] - public void SetTitle_ZeroTitleId_RejectsWithoutSendingAnything() + public void SetTitle_ZeroTitleId_StillSendsTheWireAction() { + // F3 (CT2 fix round, 2026-08-24): retail's own send path + // (Event_SetDisplayCharacterTitle @0x006a5720) packs whatever id it + // is handed, with no id-0 rejection — and ACE accepts id 0 + // (CharacterTitle.Invalid is a defined enum value). A client-side + // id-0 guard here would block a state the server honors; retail's + // real protection is the UI ghost-when-current gate (CT3's job). (GameRuntime runtime, DirectGameRuntimeCommandAdapter adapter, FixtureSessionOperations operations) = CreateStartedHarness(); var gameActions = new List(); @@ -387,8 +393,13 @@ public sealed class DirectGameRuntimeCommandAdapterTests runtime.Generation, titleId: 0u); - Assert.Equal(RuntimeCommandStatus.Rejected, result.Status); - Assert.Empty(gameActions); + Assert.True(result.Accepted); + Assert.Single(gameActions); + byte[] sent = gameActions[0]; + Assert.Equal( + SocialActions.TitleSetOpcode, + BinaryPrimitives.ReadUInt32LittleEndian(sent.AsSpan(8))); + Assert.Equal(0u, BinaryPrimitives.ReadUInt32LittleEndian(sent.AsSpan(12))); runtime.Dispose(); }