From f416c577d69d13b64e4869a279aea89ea2e18c6d Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 29 Jul 2026 01:58:02 +0200 Subject: [PATCH] fix(net): stop dropping every transient string on a chat type that isn't sent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CommunicationTransientString (0x02EB) required a trailing u32 chat type after the message. The server does not send one. Because the string is padded to a four-byte boundary, the remaining length after reading it was always zero, the guard tripped, and the parser returned null for every transient string the server has ever sent. Not most. Every one. Three oracles agree there is no such field. ACE's GameEventCommunicationTransientString writes exactly one WriteString16L and stops. Retail's ClientCommunicationSystem::Handle_Communication__TransientString at 0x0057d460 takes a single PStringBase argument. holtburger carries no type field for the event either. ParseTransient now returns the string. The wiring supplies chat type 0, which is ACE's ChatMessageType.Broadcast and which ACE's own LogTextTypeEnumMapper comment names "Default" — the honest stand-in for a message the server sends untyped. What retail's transient strings should actually look like is a rendering question and belongs with the chat colour work, not here. The existing round-trip test was itself appending the phantom trailing dword, which is exactly why the wrong guard looked correct for as long as it did. It is corrected to the real payload and joined by a case sweeping string lengths zero through four, so no future padding-residue assumption can hide here again. Core.Net tests go 654 to 655. Co-Authored-By: Claude Fable 5 --- src/AcDream.Core.Net/GameEventWiring.cs | 10 ++++-- src/AcDream.Core.Net/Messages/GameEvents.cs | 30 ++++++++++------ .../Messages/GameEventDispatcherTests.cs | 34 +++++++++++++------ 3 files changed, 51 insertions(+), 23 deletions(-) diff --git a/src/AcDream.Core.Net/GameEventWiring.cs b/src/AcDream.Core.Net/GameEventWiring.cs index d2401cde..84c6e8a4 100644 --- a/src/AcDream.Core.Net/GameEventWiring.cs +++ b/src/AcDream.Core.Net/GameEventWiring.cs @@ -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 => { diff --git a/src/AcDream.Core.Net/Messages/GameEvents.cs b/src/AcDream.Core.Net/Messages/GameEvents.cs index ac05cf2f..5401b02d 100644 --- a/src/AcDream.Core.Net/Messages/GameEvents.cs +++ b/src/AcDream.Core.Net/Messages/GameEvents.cs @@ -67,19 +67,27 @@ public static class GameEvents catch { return null; } } - /// 0x02EB CommunicationTransientString payload. - public readonly record struct TransientMessage(string Message, uint ChatType); - - public static TransientMessage? ParseTransient(ReadOnlySpan payload) + /// + /// 0x02EB CommunicationTransientString payload: a bare string, and + /// nothing else. + /// + /// Three oracles agree there is no chat type on this wire. ACE's + /// GameEvent/Events/GameEventCommunicationTransientString.cs writes + /// exactly one WriteString16L(message). Retail's handler + /// ClientCommunicationSystem::Handle_Communication__TransientString + /// (0x0057d460) takes a single + /// AC1Legacy::PStringBase<char> const* argument. holtburger + /// carries no type field for it either. + /// + /// This parser previously demanded a trailing u32 chatType. + /// 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. + /// + public static string? ParseTransient(ReadOnlySpan 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; } } diff --git a/tests/AcDream.Core.Net.Tests/Messages/GameEventDispatcherTests.cs b/tests/AcDream.Core.Net.Tests/Messages/GameEventDispatcherTests.cs index 05cc5289..a254f733 100644 --- a/tests/AcDream.Core.Net.Tests/Messages/GameEventDispatcherTests.cs +++ b/tests/AcDream.Core.Net.Tests/Messages/GameEventDispatcherTests.cs @@ -376,17 +376,31 @@ public sealed class GameEventDispatcherTests [Fact] public void ParseTransient_RoundTrip() { - byte[] msg = MakeString16L("Your spell fizzled!"); - byte[] chatType = new byte[4]; - BinaryPrimitives.WriteUInt32LittleEndian(chatType, 5u); - byte[] payload = new byte[msg.Length + 4]; - Buffer.BlockCopy(msg, 0, payload, 0, msg.Length); - Buffer.BlockCopy(chatType, 0, payload, msg.Length, 4); + // 0x02EB is a bare string. ACE's + // GameEventCommunicationTransientString writes one WriteString16L and + // stops; retail's Handle_Communication__TransientString takes a single + // string argument. This test used to append a phantom trailing u32 + // chat type, which is what let the parser's wrong guard look correct. + byte[] payload = MakeString16L("Your spell fizzled!"); - var parsed = GameEvents.ParseTransient(payload); - Assert.NotNull(parsed); - Assert.Equal("Your spell fizzled!", parsed!.Value.Message); - Assert.Equal(5u, parsed.Value.ChatType); + string? parsed = GameEvents.ParseTransient(payload); + + Assert.Equal("Your spell fizzled!", parsed); + } + + /// + /// Regression pin: the padded string consumes the whole payload, so a + /// parser demanding four more bytes returned null for every real + /// transient string the server has ever sent. + /// + [Fact] + public void ParseTransient_ExactAcePayload_IsNotDropped() + { + foreach (string text in new[] { "", "a", "bb", "ccc", "dddd", "Your spell fizzled!" }) + { + byte[] payload = MakeString16L(text); + Assert.Equal(text, GameEvents.ParseTransient(payload)); + } } [Fact]