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:
parent
bcfddc97e7
commit
544f8cb2d7
8 changed files with 350 additions and 69 deletions
|
|
@ -155,14 +155,11 @@ setAsDisplay). New sibling owner `RuntimeCharacterTitleState`
|
||||||
(`RuntimeCharacterState.Titles`) holds the earned-title set + display
|
(`RuntimeCharacterState.Titles`) holds the earned-title set + display
|
||||||
title id, clears at generation reset (`CaptureOwnership`/`IsConverged`
|
title id, clears at generation reset (`CaptureOwnership`/`IsConverged`
|
||||||
extended with `TitleCount`/`DisplayTitleIsDefault`), and fires
|
extended with `TitleCount`/`DisplayTitleIsDefault`), and fires
|
||||||
`TableReplaced`/`TitleAdded`/`DisplayTitleChanged` — matching retail's
|
`TableReplaced`/`TitleAdded`/`DisplayTitleChanged`. Outbound
|
||||||
own unconditional-add / gated-display-set contract
|
|
||||||
(`Handle_Social__AddOrSetCharacterTitle @0x00564260`). Outbound
|
|
||||||
`TitleSet (0x002C)` ships behind `IRuntimeCharacterCommands.SetTitle`
|
`TitleSet (0x002C)` ships behind `IRuntimeCharacterCommands.SetTitle`
|
||||||
on both hosts (`DirectGameRuntimeCommandAdapter` direct-send,
|
on both hosts (`DirectGameRuntimeCommandAdapter` direct-send,
|
||||||
`CurrentGameRuntimeCommandAdapter` via the `LiveCommandBus`/
|
`CurrentGameRuntimeCommandAdapter` via the `LiveCommandBus`/
|
||||||
`LiveSessionCommandRouter` queue) with **NO optimistic local
|
`LiveSessionCommandRouter` queue) — verified against retail's own
|
||||||
mutation** — verified against retail's own
|
|
||||||
`CM_Social::Event_SetDisplayCharacterTitle @0x006a5720`, which sends
|
`CM_Social::Event_SetDisplayCharacterTitle @0x006a5720`, which sends
|
||||||
the wire message and touches no local field; the display title updates
|
the wire message and touches no local field; the display title updates
|
||||||
only from the server's own echo. No register row: this slice
|
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
|
id-only. Conformance tests against ACE's writer shapes
|
||||||
(`tests/AcDream.Core.Net.Tests/Messages/CharacterTitleEventsTests.cs`),
|
(`tests/AcDream.Core.Net.Tests/Messages/CharacterTitleEventsTests.cs`),
|
||||||
Runtime owner tests (`RuntimeCharacterTitleStateTests.cs` +
|
Runtime owner tests (`RuntimeCharacterTitleStateTests.cs` +
|
||||||
`RuntimeCharacterStateTests.cs` integration), a no-local-mutation
|
`RuntimeCharacterStateTests.cs` integration), a wire-send command test
|
||||||
command test (`DirectGameRuntimeCommandAdapterTests.cs`), and an
|
(`DirectGameRuntimeCommandAdapterTests.cs`), and an InstalledDat pin
|
||||||
InstalledDat pin (`CharacterTitleResolverLiveDatTests.cs`, ids 0/1/2/3/
|
(`CharacterTitleResolverLiveDatTests.cs`, ids 0/1/2/3/5/13/14) all pass.
|
||||||
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
|
**CT3 — Titles page UI.** Bind the authored page through the standard
|
||||||
GUI classes (`UiTemplateListBox`/`UiScrollbar`/`UiButton` — zero
|
GUI classes (`UiTemplateListBox`/`UiScrollbar`/`UiButton` — zero
|
||||||
|
|
|
||||||
|
|
@ -725,17 +725,22 @@ internal sealed class CurrentGameRuntimeCommandAdapter
|
||||||
RuntimeCommandStatus gate = Validate(expectedGeneration, requireWorld: true);
|
RuntimeCommandStatus gate = Validate(expectedGeneration, requireWorld: true);
|
||||||
if (gate != RuntimeCommandStatus.Accepted)
|
if (gate != RuntimeCommandStatus.Accepted)
|
||||||
return Result(gate);
|
return Result(gate);
|
||||||
if (titleId == 0u)
|
// F3 (CT2 fix round, 2026-08-24): NO id-0 guard here — retail's own
|
||||||
{
|
// send path (Event_SetDisplayCharacterTitle @0x006a5720) packs
|
||||||
return EmitResult(
|
// whatever id it is handed, and ACE accepts id 0
|
||||||
RuntimeCommandDomain.Character,
|
// (CharacterTitle.Invalid is a defined enum value). Retail's
|
||||||
operation: 6,
|
// protection against sending an unearned/invalid id is the UI
|
||||||
RuntimeCommandStatus.Rejected);
|
// 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
|
// CT2: NO optimistic local mutation — matches
|
||||||
// DirectGameRuntimeCommandAdapter.SetTitle; RuntimeCharacterState.
|
// DirectGameRuntimeCommandAdapter.SetTitle; RuntimeCharacterState.
|
||||||
// Titles updates only from the server's own echo.
|
// Titles updates only from the server's own echo.
|
||||||
_commands.Publish(new SetTitleRuntimeCmd(titleId));
|
_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(
|
return EmitResult(
|
||||||
RuntimeCommandDomain.Character,
|
RuntimeCommandDomain.Character,
|
||||||
operation: 6,
|
operation: 6,
|
||||||
|
|
|
||||||
|
|
@ -17,11 +17,16 @@ namespace AcDream.App.UI.Layout;
|
||||||
/// resolved DIDs are hardcoded here rather than porting the indirection
|
/// resolved DIDs are hardcoded here rather than porting the indirection
|
||||||
/// generically — the same precedent <see cref="RetailKeyNames"/> already
|
/// generically — the same precedent <see cref="RetailKeyNames"/> already
|
||||||
/// set for this exact category-4 family (its <c>KeyNameTableId</c>/
|
/// set for this exact category-4 family (its <c>KeyNameTableId</c>/
|
||||||
/// <c>MetaKeyNameTableId</c>/<c>DelimiterTableId</c>); factor out a shared
|
/// <c>MetaKeyNameTableId</c>/<c>DelimiterTableId</c>). A THIRD consumer of
|
||||||
/// helper only if a THIRD consumer of <c>GetDIDByEnum</c> appears. Both DIDs
|
/// <c>GetDIDByEnum</c> has already appeared —
|
||||||
/// were verified end-to-end against ACE's <c>CharacterTitle.WarMage = 13</c>
|
/// <c>gmAttributeUI::PostInit @0x0049DB70</c> resolves per-attribute icon
|
||||||
/// in Campaign CT slice CT1
|
/// DIDs via category <c>0x10000002</c> — so CT5 (not "if a third consumer
|
||||||
/// (docs/research/2026-08-24-campaign-ct-dat-ground-truth.md §5, pinned by
|
/// appears") is where the shared <c>GetDIDByEnum(enumValue, category)</c>
|
||||||
|
/// 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
|
||||||
|
/// <c>CharacterTitle.WarMage = 13</c> in Campaign CT slice CT1 (same §5,
|
||||||
|
/// pinned by
|
||||||
/// <c>CharacterPanelLiveDatTests.TitleStringTable_ResolvesWarMageEndToEnd</c>).
|
/// <c>CharacterPanelLiveDatTests.TitleStringTable_ResolvesWarMageEndToEnd</c>).
|
||||||
/// </para>
|
/// </para>
|
||||||
/// <para>
|
/// <para>
|
||||||
|
|
@ -42,6 +47,7 @@ public sealed class CharacterTitleResolver
|
||||||
|
|
||||||
private readonly IDatReaderWriter _dats;
|
private readonly IDatReaderWriter _dats;
|
||||||
private readonly DatStringResolver _strings;
|
private readonly DatStringResolver _strings;
|
||||||
|
private readonly Dictionary<uint, string?> _resolvedCache = new();
|
||||||
private EnumMapper? _titleEnumMapper;
|
private EnumMapper? _titleEnumMapper;
|
||||||
private bool _loadedMapper;
|
private bool _loadedMapper;
|
||||||
|
|
||||||
|
|
@ -59,27 +65,38 @@ public sealed class CharacterTitleResolver
|
||||||
/// <see cref="DatStringResolver.Resolve(uint, uint, int)"/>'s own
|
/// <see cref="DatStringResolver.Resolve(uint, uint, int)"/>'s own
|
||||||
/// null-on-miss contract.
|
/// null-on-miss contract.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// A8 (CT2 fix round, 2026-08-24): memoizes the final resolved string
|
||||||
|
/// per <paramref name="titleId"/> (including misses). Retail caches the
|
||||||
|
/// computed hash directly on the string buffer
|
||||||
|
/// (<c>GetCharacterTitleFromID</c>'s <c>0xFFFFFFFF</c> lazy-hash
|
||||||
|
/// sentinel); a memo of the final string is the equivalent here given
|
||||||
|
/// our immutable, DAT-static tables — no invalidation is needed.
|
||||||
|
/// </remarks>
|
||||||
public string? Resolve(uint titleId)
|
public string? Resolve(uint titleId)
|
||||||
{
|
{
|
||||||
if (titleId == 0u)
|
if (titleId == 0u)
|
||||||
return null;
|
return null;
|
||||||
|
|
||||||
|
if (_resolvedCache.TryGetValue(titleId, out string? cached))
|
||||||
|
return cached;
|
||||||
|
|
||||||
if (!_loadedMapper)
|
if (!_loadedMapper)
|
||||||
{
|
{
|
||||||
_dats.Portal.TryGet<EnumMapper>(TitleEnumMapperId, out _titleEnumMapper);
|
_dats.Portal.TryGet<EnumMapper>(TitleEnumMapperId, out _titleEnumMapper);
|
||||||
_loadedMapper = true;
|
_loadedMapper = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (_titleEnumMapper is null
|
string? resolved = null;
|
||||||
|| !_titleEnumMapper.IdToStringMap.TryGetValue(titleId, out var rawNameValue))
|
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();
|
_resolvedCache[titleId] = resolved;
|
||||||
if (string.IsNullOrEmpty(rawName))
|
return resolved;
|
||||||
return null;
|
|
||||||
|
|
||||||
return _strings.Resolve(TitleStringTableId, DatStringResolver.ComputeHash(rawName));
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -252,7 +252,7 @@ public sealed class RuntimeCharacterState : IDisposable
|
||||||
&& _movementSkillAugmentations == default,
|
&& _movementSkillAugmentations == default,
|
||||||
AutonomyLevel == FullAutonomyLevel,
|
AutonomyLevel == FullAutonomyLevel,
|
||||||
OptionsAreClean: !Options.IsDirty,
|
OptionsAreClean: !Options.IsDirty,
|
||||||
TitleCount: Titles.EarnedTitleIds.Count,
|
TitleCount: Titles.Count,
|
||||||
DisplayTitleIsDefault: Titles.DisplayTitleId == 0u);
|
DisplayTitleIsDefault: Titles.DisplayTitleId == 0u);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -1238,6 +1238,16 @@ public readonly record struct RuntimeCharacterTitleSnapshot(
|
||||||
/// <c>UpdateTitle</c> (never re-add an optimistic write here — the CA
|
/// <c>UpdateTitle</c> (never re-add an optimistic write here — the CA
|
||||||
/// campaign lesson).
|
/// campaign lesson).
|
||||||
/// </summary>
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// CT2 fix round (2026-08-24), F1: the NOTICE broadcast is unconditional —
|
||||||
|
/// retail's server-side <c>SendNotice_AddCharacterTitle</c> fires
|
||||||
|
/// regardless of prior membership — but the client-side table ADD is
|
||||||
|
/// DEDUPED: <c>gmCharacterTitleUI::RecvNotice_AddCharacterTitle
|
||||||
|
/// @0x0049a990</c> walks <c>mTitleList</c> and returns without effect when
|
||||||
|
/// the id is already present, only inserting + adding the row on a miss.
|
||||||
|
/// <see cref="ApplyUpdateTitle"/> models the client-side receive handler,
|
||||||
|
/// so <see cref="TitleAdded"/> fires only on a genuine new membership.
|
||||||
|
/// </remarks>
|
||||||
public sealed class RuntimeCharacterTitleState
|
public sealed class RuntimeCharacterTitleState
|
||||||
{
|
{
|
||||||
private readonly object _gate = new();
|
private readonly object _gate = new();
|
||||||
|
|
@ -1245,32 +1255,54 @@ public sealed class RuntimeCharacterTitleState
|
||||||
private uint _displayTitleId;
|
private uint _displayTitleId;
|
||||||
private long _revision;
|
private long _revision;
|
||||||
|
|
||||||
/// <summary>Fires after a full <c>0x0029 CharacterTitle</c> table replace.</summary>
|
/// <summary>
|
||||||
|
/// Fires after every <c>0x0029 CharacterTitle</c> table replace,
|
||||||
|
/// unconditionally — matches retail's own
|
||||||
|
/// <c>gmCharacterTitleUI::RecvNotice_UpdateCharacterTitleTable</c>,
|
||||||
|
/// which always calls <c>Refresh()</c> regardless of whether the new
|
||||||
|
/// table differs from the old one.
|
||||||
|
/// </summary>
|
||||||
public event Action? TableReplaced;
|
public event Action? TableReplaced;
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Fires once per <c>0x002B UpdateTitle</c> arrival, UNCONDITIONALLY —
|
/// Fires only when an <c>0x002B UpdateTitle</c> arrival actually adds a
|
||||||
/// matches retail's own <c>SendNotice_AddCharacterTitle</c>, which
|
/// NEW id to the earned set — matches retail's client-side
|
||||||
/// broadcasts regardless of whether the id was already in the earned
|
/// <c>gmCharacterTitleUI::RecvNotice_AddCharacterTitle @0x0049a990</c>,
|
||||||
/// set. Carries the added title id.
|
/// which dedupes against <c>mTitleList</c> before inserting (see the
|
||||||
|
/// class remarks: the F1 fix-round correction). Carries the added
|
||||||
|
/// title id.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public event Action<uint>? TitleAdded;
|
public event Action<uint>? TitleAdded;
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Fires whenever the display title id changes — from either a fresh
|
/// Fires whenever the display title id actually changes — from either a
|
||||||
/// <c>0x0029</c> table (a differing seed) or an <c>0x002B</c> whose
|
/// fresh <c>0x0029</c> table (a differing seed) or an <c>0x002B</c>
|
||||||
/// <c>setAsDisplay</c> flag is set. Carries the NEW display title id.
|
/// whose <c>setAsDisplay</c> flag is set to an id that differs from the
|
||||||
|
/// current display title. Carries the NEW display title id.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public event Action<uint>? DisplayTitleChanged;
|
public event Action<uint>? DisplayTitleChanged;
|
||||||
|
|
||||||
public uint DisplayTitleId => Volatile.Read(ref _displayTitleId);
|
public uint DisplayTitleId => Volatile.Read(ref _displayTitleId);
|
||||||
public long Revision => Interlocked.Read(ref _revision);
|
public long Revision => Interlocked.Read(ref _revision);
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// WARNING: every read allocates a fresh array (<c>ToArray()</c> under
|
||||||
|
/// the gate). Fine for UI refresh call sites (CT3), but NEVER read this
|
||||||
|
/// per-frame — use <see cref="Count"/> or <see cref="HasEarnedTitle"/>
|
||||||
|
/// for hot-path checks.
|
||||||
|
/// </summary>
|
||||||
public IReadOnlyCollection<uint> EarnedTitleIds
|
public IReadOnlyCollection<uint> EarnedTitleIds
|
||||||
{
|
{
|
||||||
get { lock (_gate) return _earnedTitleIds.ToArray(); }
|
get { lock (_gate) return _earnedTitleIds.ToArray(); }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>Non-allocating earned-title count; prefer this over
|
||||||
|
/// <c>EarnedTitleIds.Count</c> in hot paths (A3, CT2 fix round).</summary>
|
||||||
|
public int Count
|
||||||
|
{
|
||||||
|
get { lock (_gate) return _earnedTitleIds.Count; }
|
||||||
|
}
|
||||||
|
|
||||||
public bool HasEarnedTitle(uint titleId)
|
public bool HasEarnedTitle(uint titleId)
|
||||||
{
|
{
|
||||||
lock (_gate) return _earnedTitleIds.Contains(titleId);
|
lock (_gate) return _earnedTitleIds.Contains(titleId);
|
||||||
|
|
@ -1290,20 +1322,32 @@ public sealed class RuntimeCharacterTitleState
|
||||||
/// <c>0x0029 CharacterTitle</c> — a WHOLESALE authoritative replace
|
/// <c>0x0029 CharacterTitle</c> — a WHOLESALE authoritative replace
|
||||||
/// (retail's <c>CharacterTitleTable::UnPack</c> always rebuilds
|
/// (retail's <c>CharacterTitleTable::UnPack</c> always rebuilds
|
||||||
/// <c>mTitleList</c> from scratch; there is no incremental-merge path
|
/// <c>mTitleList</c> 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
|
||||||
|
/// <see cref="_gate"/> 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); <see cref="TableReplaced"/>
|
||||||
|
/// itself still fires unconditionally, matching retail's own
|
||||||
|
/// unconditional <c>Refresh()</c> dispatch on this opcode.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public void ReplaceTable(uint displayTitleId, IReadOnlyList<uint> titleIds)
|
public void ReplaceTable(uint displayTitleId, IReadOnlyList<uint> titleIds)
|
||||||
{
|
{
|
||||||
ArgumentNullException.ThrowIfNull(titleIds);
|
ArgumentNullException.ThrowIfNull(titleIds);
|
||||||
|
bool setChanged;
|
||||||
|
bool displayChanged;
|
||||||
lock (_gate)
|
lock (_gate)
|
||||||
{
|
{
|
||||||
|
setChanged = !_earnedTitleIds.SetEquals(titleIds);
|
||||||
_earnedTitleIds.Clear();
|
_earnedTitleIds.Clear();
|
||||||
foreach (uint id in titleIds)
|
foreach (uint id in titleIds)
|
||||||
_earnedTitleIds.Add(id);
|
_earnedTitleIds.Add(id);
|
||||||
|
displayChanged = _displayTitleId != displayTitleId;
|
||||||
|
if (displayChanged)
|
||||||
|
Volatile.Write(ref _displayTitleId, displayTitleId);
|
||||||
}
|
}
|
||||||
bool displayChanged = DisplayTitleId != displayTitleId;
|
if (setChanged || displayChanged)
|
||||||
Volatile.Write(ref _displayTitleId, displayTitleId);
|
Interlocked.Increment(ref _revision);
|
||||||
Interlocked.Increment(ref _revision);
|
|
||||||
TableReplaced?.Invoke();
|
TableReplaced?.Invoke();
|
||||||
if (displayChanged)
|
if (displayChanged)
|
||||||
DisplayTitleChanged?.Invoke(displayTitleId);
|
DisplayTitleChanged?.Invoke(displayTitleId);
|
||||||
|
|
@ -1311,27 +1355,63 @@ public sealed class RuntimeCharacterTitleState
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// <c>0x002B UpdateTitle</c> — retail's
|
/// <c>0x002B UpdateTitle</c> — retail's
|
||||||
/// <c>ClientUISystem::Handle_Social__AddOrSetCharacterTitle</c>: ALWAYS
|
/// <c>ClientUISystem::Handle_Social__AddOrSetCharacterTitle</c> ALWAYS
|
||||||
/// add, and additionally set-display only when
|
/// broadcasts the add notice server-side, and additionally broadcasts
|
||||||
/// <paramref name="setAsDisplay"/> is true.
|
/// set-display only when <paramref name="setAsDisplay"/> is true. On
|
||||||
|
/// the CLIENT receive side this method models, F1 (CT2 fix round): the
|
||||||
|
/// add is deduped (<see cref="TitleAdded"/> fires only when
|
||||||
|
/// <c>HashSet<uint>.Add</c> reports a genuine new membership,
|
||||||
|
/// matching <c>gmCharacterTitleUI::RecvNotice_AddCharacterTitle
|
||||||
|
/// @0x0049a990</c>'s membership check before insert). A4: both halves
|
||||||
|
/// mutate under one <see cref="_gate"/> 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 <paramref name="setAsDisplay"/> pointing at
|
||||||
|
/// the already-current display id, up to twice when both halves change.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public void ApplyUpdateTitle(uint titleId, bool setAsDisplay)
|
public void ApplyUpdateTitle(uint titleId, bool setAsDisplay)
|
||||||
{
|
{
|
||||||
lock (_gate) _earnedTitleIds.Add(titleId);
|
bool added;
|
||||||
Interlocked.Increment(ref _revision);
|
bool displayChanged;
|
||||||
TitleAdded?.Invoke(titleId);
|
lock (_gate)
|
||||||
if (setAsDisplay)
|
|
||||||
{
|
{
|
||||||
Volatile.Write(ref _displayTitleId, titleId);
|
added = _earnedTitleIds.Add(titleId);
|
||||||
Interlocked.Increment(ref _revision);
|
displayChanged = setAsDisplay && _displayTitleId != titleId;
|
||||||
DisplayTitleChanged?.Invoke(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);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// A2 (CT2 fix round): publishes the clear like
|
||||||
|
/// <c>LocalPlayerState.Clear()</c> — <see cref="TableReplaced"/> fires
|
||||||
|
/// unconditionally and <see cref="DisplayTitleChanged"/> 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.
|
||||||
|
/// </summary>
|
||||||
public void ResetSession()
|
public void ResetSession()
|
||||||
{
|
{
|
||||||
lock (_gate) _earnedTitleIds.Clear();
|
uint previousDisplayTitleId;
|
||||||
Volatile.Write(ref _displayTitleId, 0u);
|
lock (_gate)
|
||||||
|
{
|
||||||
|
previousDisplayTitleId = _displayTitleId;
|
||||||
|
_earnedTitleIds.Clear();
|
||||||
|
Volatile.Write(ref _displayTitleId, 0u);
|
||||||
|
}
|
||||||
Interlocked.Increment(ref _revision);
|
Interlocked.Increment(ref _revision);
|
||||||
|
TableReplaced?.Invoke();
|
||||||
|
if (previousDisplayTitleId != 0u)
|
||||||
|
DisplayTitleChanged?.Invoke(0u);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -731,18 +731,22 @@ public sealed class DirectGameRuntimeCommandAdapter
|
||||||
Validate(expectedGeneration, out WorldSession? session);
|
Validate(expectedGeneration, out WorldSession? session);
|
||||||
if (gate != RuntimeCommandStatus.Accepted)
|
if (gate != RuntimeCommandStatus.Accepted)
|
||||||
return Result(gate);
|
return Result(gate);
|
||||||
if (titleId == 0u)
|
// F3 (CT2 fix round, 2026-08-24): NO id-0 guard here — retail's own
|
||||||
{
|
// send path (Event_SetDisplayCharacterTitle @0x006a5720) packs
|
||||||
return EmitResult(
|
// whatever id it is handed, and ACE accepts id 0
|
||||||
RuntimeCommandDomain.Character,
|
// (CharacterTitle.Invalid is a defined enum value). Retail's
|
||||||
operation: 6,
|
// protection against sending an unearned/invalid id is the UI
|
||||||
RuntimeCommandStatus.Rejected);
|
// 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
|
// CT2: NO optimistic local mutation — retail's own
|
||||||
// Event_SetDisplayCharacterTitle send path touches no local state;
|
// Event_SetDisplayCharacterTitle send path touches no local state;
|
||||||
// RuntimeCharacterState.Titles.DisplayTitleId updates only when the
|
// RuntimeCharacterState.Titles.DisplayTitleId updates only when the
|
||||||
// server echoes UpdateTitle (0x002B) with setAsDisplay=true.
|
// server echoes UpdateTitle (0x002B) with setAsDisplay=true.
|
||||||
session!.SendSetTitle(titleId);
|
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(
|
return EmitResult(
|
||||||
RuntimeCommandDomain.Character,
|
RuntimeCommandDomain.Character,
|
||||||
operation: 6,
|
operation: 6,
|
||||||
|
|
|
||||||
|
|
@ -20,7 +20,8 @@ namespace AcDream.App.Tests.UI.Layout;
|
||||||
public sealed class CharacterTitleResolverLiveDatTests
|
public sealed class CharacterTitleResolverLiveDatTests
|
||||||
{
|
{
|
||||||
private static string DatDirectory =>
|
private static string DatDirectory =>
|
||||||
Path.Combine(
|
System.Environment.GetEnvironmentVariable("ACDREAM_DAT_DIR")
|
||||||
|
?? Path.Combine(
|
||||||
System.Environment.GetFolderPath(System.Environment.SpecialFolder.UserProfile),
|
System.Environment.GetFolderPath(System.Environment.SpecialFolder.UserProfile),
|
||||||
"Documents", "Asheron's Call");
|
"Documents", "Asheron's Call");
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -119,19 +119,59 @@ public sealed class RuntimeCharacterTitleStateTests
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public void ApplyUpdateTitle_AlreadyEarnedId_StillFiresTitleAddedUnconditionally()
|
public void ApplyUpdateTitle_AlreadyEarnedId_DoesNotFireTitleAddedOrBumpRevision()
|
||||||
{
|
{
|
||||||
// Retail's own broadcast is unconditional — it does not check
|
// F1 (CT2 fix round, 2026-08-24): the SERVER-side broadcast
|
||||||
// membership before firing the notice.
|
// (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();
|
var titles = new RuntimeCharacterTitleState();
|
||||||
titles.ApplyUpdateTitle(7u, setAsDisplay: false);
|
titles.ApplyUpdateTitle(7u, setAsDisplay: false);
|
||||||
|
long revisionAfterFirstAdd = titles.Revision;
|
||||||
var added = new List<uint>();
|
var added = new List<uint>();
|
||||||
titles.TitleAdded += id => added.Add(id);
|
titles.TitleAdded += id => added.Add(id);
|
||||||
|
|
||||||
titles.ApplyUpdateTitle(7u, setAsDisplay: false);
|
titles.ApplyUpdateTitle(7u, setAsDisplay: false);
|
||||||
|
|
||||||
Assert.Equal([7u], added);
|
Assert.Empty(added);
|
||||||
Assert.Single(titles.EarnedTitleIds);
|
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<uint>();
|
||||||
|
titles.DisplayTitleChanged += id => displayChanged.Add(id);
|
||||||
|
var added = new List<uint>();
|
||||||
|
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]
|
[Fact]
|
||||||
|
|
@ -147,6 +187,25 @@ public sealed class RuntimeCharacterTitleStateTests
|
||||||
Assert.Equal(99u, titles.DisplayTitleId);
|
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]
|
[Fact]
|
||||||
public void ResetSession_ClearsEarnedIdsAndDisplayTitle()
|
public void ResetSession_ClearsEarnedIdsAndDisplayTitle()
|
||||||
{
|
{
|
||||||
|
|
@ -170,6 +229,57 @@ public sealed class RuntimeCharacterTitleStateTests
|
||||||
Assert.True(titles.Revision > before);
|
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<uint>();
|
||||||
|
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<uint>();
|
||||||
|
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]
|
[Fact]
|
||||||
public void Snapshot_ReflectsDisplayTitleAndCount()
|
public void Snapshot_ReflectsDisplayTitleAndCount()
|
||||||
{
|
{
|
||||||
|
|
|
||||||
|
|
@ -376,8 +376,14 @@ public sealed class DirectGameRuntimeCommandAdapterTests
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[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) =
|
(GameRuntime runtime, DirectGameRuntimeCommandAdapter adapter, FixtureSessionOperations operations) =
|
||||||
CreateStartedHarness();
|
CreateStartedHarness();
|
||||||
var gameActions = new List<byte[]>();
|
var gameActions = new List<byte[]>();
|
||||||
|
|
@ -387,8 +393,13 @@ public sealed class DirectGameRuntimeCommandAdapterTests
|
||||||
runtime.Generation,
|
runtime.Generation,
|
||||||
titleId: 0u);
|
titleId: 0u);
|
||||||
|
|
||||||
Assert.Equal(RuntimeCommandStatus.Rejected, result.Status);
|
Assert.True(result.Accepted);
|
||||||
Assert.Empty(gameActions);
|
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();
|
runtime.Dispose();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue