erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases 6 Packages Wiki Activity Actions

merge(net): the wire-stack audit, and one reconciled #255

Browse source 
Brings `github/overnight/wire-audit` (`41f74fcd`) forward onto the V11 tree.
Like the enum branch it was cut at `b70b9832`, and like the enum branch its
subject is disjoint from the deletion: the audit lives in `AcDream.Core.Net`
and its tests, V11 emptied `AcDream.App`. One conflict, in `docs/ISSUES.md`,
resolved below.

What it carries: three real parser fixes — ranged speech was carrying a range
float the parser ate, a chat type that is never sent was silently dropping every
transient string on it, and `xpSpent` is a dword on the wire where we were
writing eight bytes. Plus the transport flag word pinned against ACE across all
twenty-three bits, golden fixtures generated from ACE's own writer instead of
hand-typed hex, and the audit document covering all three hundred forty-nine
opcodes.

**The conflict, and how it was resolved.** Both this branch and V11's closeout
reopened #255 — the RetailDatLoader concurrency tests that measure the thread
pool rather than the loader — on the same day, from different trees, without
knowing about each other. Neither reopening is a duplicate of the other: the
V11 gate saw 2 failures in 5 complete-solution Release runs on the
post-deletion tree, the audit session saw 2 in 4 on the pre-deletion tree, and
both saw 124/124 in isolation every time. They independently reached the same
conclusion, that `TaskCreationOptions.LongRunning` is a hint rather than a
guarantee, and independently proposed the same fix, a rendezvous inside the read
stub.

So the two notes are merged into one issue with both evidence sets kept as
labelled subsections rather than one overwriting the other. Four failures across
nine runs on two trees is a materially stronger case than either half, and the
agreement between two blind observations is the part worth preserving. No
assertion was weakened and no retry was added; the fix itself remains open.

Verified on the merge result: Release build 0 errors, and
`AcDream.Core.Net.Tests` at 659 passed / 0 skipped, up exactly the 59 the branch
claimed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-07-29 03:31:25 +02:00
commit 22ae7944b6
14 changed files with 2079 additions and 39 deletions
Download patch file Download diff file Expand all files Collapse all files

10
src/AcDream.Core.Net/GameEventWiring.cs
View file

@ -107,8 +107,14 @@ public static class GameEventWiring
});
registrar.Register(GameEventType.CommunicationTransientString, e =>
{
var p = GameEvents.ParseTransient(e.Payload.Span);
if (p is not null) chat.OnSystemMessage(p.Value.Message, p.Value.ChatType);
// 0x02EB carries no chat type on the wire (see ParseTransient).
// 0 is ACE's ChatMessageType.Broadcast, which its own
// LogTextTypeEnumMapper comment names "Default" — the right
// stand-in for a message the server sends untyped. The exact
// retail rendering style for transient strings belongs to the
// chat colour/text work, not to this parser.
var s = GameEvents.ParseTransient(e.Payload.Span);
if (s is not null) chat.OnSystemMessage(s, chatType: 0u);
});
registrar.Register(GameEventType.PopupString, e =>
{

27
src/AcDream.Core.Net/Messages/CharacterActions.cs
View file

@ -16,9 +16,9 @@ public static class CharacterActions
{
public const uint GameActionEnvelope = 0xF7B1u;
public const uint RaiseAttributeOpcode = 0x0045u; // u32 attr, u64 xpSpent
public const uint RaiseVitalOpcode = 0x0044u; // u32 vital, u64 xpSpent
public const uint RaiseSkillOpcode = 0x0046u; // u32 skillId, u64 xpSpent
public const uint RaiseAttributeOpcode = 0x0045u; // u32 attr, u32 xpSpent
public const uint RaiseVitalOpcode = 0x0044u; // u32 vital, u32 xpSpent
public const uint RaiseSkillOpcode = 0x0046u; // u32 skillId, u32 xpSpent
public const uint TrainSkillOpcode = 0x0047u; // u32 skillId, u32 credits
public const uint ChangeCombatModeOpcode = 0x0053u; // u32 combatMode
@ -74,14 +74,31 @@ public static class CharacterActions
return body;
}
/// <summary>
/// Envelope + sequence + sub-opcode + id + <b>32-bit</b> xpSpent.
///
/// <para>The xpSpent field is a dword on the wire, not a qword. ACE's
/// <c>GameAction/Actions/GameActionRaiseAttribute.cs</c> (and its Vital and
/// Skill siblings) read <c>message.Payload.ReadUInt32()</c>, and
/// holtburger's <c>RaiseAttributeData</c> declares <c>xp_spent: u32</c>
/// and advances the offset by four. We were writing eight, making the
/// message 24 bytes where the server expects 20 and leaving four bytes of
/// tail the server never reads.</para>
///
/// <para>The parameter stays <c>ulong</c> because the cost originates from
/// 64-bit server XP tables several layers up; narrowing that chain end to
/// end is a separate change. No value is lost here: a cost that does not
/// fit in a dword was never expressible on this wire in the first
/// place.</para>
/// </summary>
private static byte[] BuildAttrOrVital(uint seq, uint sub, uint id, ulong xp)
{
byte[] body = new byte[24];
byte[] body = new byte[20];
BinaryPrimitives.WriteUInt32LittleEndian(body, GameActionEnvelope);
BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(4), seq);
BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(8), sub);
BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(12), id);
BinaryPrimitives.WriteUInt64LittleEndian(body.AsSpan(16), xp);
BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(16), (uint)xp);
return body;
}
}

30
src/AcDream.Core.Net/Messages/GameEvents.cs
View file

@ -67,19 +67,27 @@ public static class GameEvents
catch { return null; }
}
/// <summary>0x02EB CommunicationTransientString payload.</summary>
public readonly record struct TransientMessage(string Message, uint ChatType);
public static TransientMessage? ParseTransient(ReadOnlySpan<byte> payload)
/// <summary>
/// 0x02EB CommunicationTransientString payload: a bare string, and
/// nothing else.
///
/// <para>Three oracles agree there is no chat type on this wire. ACE's
/// <c>GameEvent/Events/GameEventCommunicationTransientString.cs</c> writes
/// exactly one <c>WriteString16L(message)</c>. Retail's handler
/// <c>ClientCommunicationSystem::Handle_Communication__TransientString</c>
/// (0x0057d460) takes a single
/// <c>AC1Legacy::PStringBase&lt;char&gt; const*</c> argument. holtburger
/// carries no type field for it either.</para>
///
/// <para>This parser previously demanded a trailing <c>u32 chatType</c>.
/// Because the string is padded to a 4-byte boundary, the remaining length
/// was always 0, so the guard tripped and every single transient string
/// was dropped.</para>
/// </summary>
public static string? ParseTransient(ReadOnlySpan<byte> payload)
{
int pos = 0;
try
{
string message = ReadString16L(payload, ref pos);
if (payload.Length - pos < 4) return null;
uint chatType = BinaryPrimitives.ReadUInt32LittleEndian(payload.Slice(pos));
return new TransientMessage(message, chatType);
}
try { return ReadString16L(payload, ref pos); }
catch { return null; }
}

39
src/AcDream.Core.Net/Messages/HearSpeech.cs
View file

@ -11,12 +11,26 @@ namespace AcDream.Core.Net.Messages;
/// GameMessages dispatched the same way as CreateObject / UpdateMotion.
///
/// <para>
/// The two opcodes do NOT share a payload: ranged speech carries an extra
/// <c>f32 range</c> between the sender guid and the chat type. Both oracles
/// agree — ACE's
/// <c>GameMessages/Messages/GameMessageHearRangedSpeech.cs</c> writes
/// <c>senderID, range, chatMessageType</c> where
/// <c>GameMessageHearSpeech.cs</c> writes only <c>senderID,
/// chatMessageType</c>, and holtburger's
/// <c>crates/holtburger-protocol/src/messages/chat/types.rs</c> declares
/// <c>HearRangedSpeechData</c> with a <c>range: f32</c> that
/// <c>HearSpeechData</c> lacks.
/// </para>
///
/// <para>
/// Wire layout:
/// <code>
/// u32 opcode // 0x02BB or 0x02BC
/// string16L text
/// string16L senderName
/// u32 senderGuid
/// f32 range // 0x02BC ONLY
/// u32 chatType
/// </code>
/// </para>
@ -39,12 +53,17 @@ public static class HearSpeech
public const uint LocalOpcode = 0x02BBu;
public const uint RangedOpcode = 0x02BCu;
/// <param name="Range">
/// Audible radius carried only by <c>0x02BC HearRangedSpeech</c>. Local
/// speech (<c>0x02BB</c>) has no such field on the wire and reports 0.
/// </param>
public readonly record struct Parsed(
string Text,
string SenderName,
uint SenderGuid,
uint ChatType,
bool IsRanged);
bool IsRanged,
float Range);
public static Parsed? TryParse(ReadOnlySpan<byte> body)
{
@ -61,10 +80,22 @@ public static class HearSpeech
{
string text = ReadString16L(body, ref pos);
string sender = ReadString16L(body, ref pos);
if (body.Length - pos < 8) return null;
// 0x02BB: guid + chatType. 0x02BC: guid + range + chatType.
int tailSize = isRanged ? 12 : 8;
if (body.Length - pos < tailSize) return null;
uint senderGuid = BinaryPrimitives.ReadUInt32LittleEndian(body.Slice(pos)); pos += 4;
uint chatType = BinaryPrimitives.ReadUInt32LittleEndian(body.Slice(pos)); pos += 4;
return new Parsed(text, sender, senderGuid, chatType, isRanged);
float range = 0f;
if (isRanged)
{
range = BinaryPrimitives.ReadSingleLittleEndian(body.Slice(pos));
pos += 4;
}
uint chatType = BinaryPrimitives.ReadUInt32LittleEndian(body.Slice(pos)); pos += 4;
return new Parsed(text, sender, senderGuid, chatType, isRanged, range);
}
catch { return null; }
}