diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index b404dd6..ca47532 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -1258,15 +1258,22 @@ public sealed class GameWindow : IDisposable { if (parsed.Body is AcDream.Core.Net.Messages.TurbineChat.Payload.EventSendToRoom ev) { + // Pass the friendly channel name out-of-band via + // ChatLog.OnChannelBroadcast's channelName param so + // ChatVM.FormatEntry can render the retail-style + // "[Trade] +Acdream says, \"hello\"" without us + // mangling the payload text. string label = TurbineRoomDisplayName(ev.RoomId, ev.ChatType); Chat.OnChannelBroadcast( - channelId: ev.RoomId, - sender: ev.SenderName, - text: $"[{label}] {ev.Message}"); + channelId: ev.RoomId, + sender: ev.SenderName, + text: ev.Message, + channelName: label); } // Response (server ack of an outbound RequestSendToRoomById) - // and Unknown payloads are intentionally not surfaced — the - // user already saw their optimistic local echo. + // and Unknown payloads are intentionally not surfaced — + // the inbound EventSendToRoom for our own message acts as + // the canonical echo. }; // Phase I.3: real ICommandBus. Panels publish SendChatCmd here @@ -1302,6 +1309,14 @@ public sealed class GameWindow : IDisposable // Allegiance is double-routed: try TurbineChat first // (when the player has a Turbine allegiance room) and // fall back to the legacy 0x0147 ChatChannel. + // + // We do NOT optimistic-echo channels: ACE's + // TurbineChatHandler broadcasts EventSendToRoom back + // to the sender too, so we always get the canonical + // echo from the server. Optimistic-echoing here + // double-prints the message (one as "[Trade] hello" + // from us, one as "[Trade] +Acdream says, \"hello\"" + // from the server). Trust the server. var turbine = ResolveTurbineForKind(cmd.Channel, turbineChat); if (turbine is not null) { @@ -1323,9 +1338,10 @@ public sealed class GameWindow : IDisposable senderGuid: senderGuid, text: cmd.Text, cookie: cookie); - chat.OnSelfSent( - AcDream.Core.Chat.ChatKind.Channel, cmd.Text, - targetOrChannel: turbine.Value.DisplayName); + // No optimistic echo: server EventSendToRoom + // broadcast comes back with sender="+Acdream" + // and is rendered by ChatVM as + // "[Trade] +Acdream says, \"hello\"". break; } @@ -1347,6 +1363,12 @@ public sealed class GameWindow : IDisposable $"chat: outbound legacy ChatChannel {resolved.Value.DisplayName} " + $"id=0x{resolved.Value.ChannelId:X8} len={cmd.Text.Length}"); liveSession.SendChannel(resolved.Value.ChannelId, cmd.Text); + // Legacy channels (Fellowship / Allegiance / Patron / + // Monarch / Vassals / CoVassals) — keep the optimistic + // echo because legacy ChatChannel does NOT always + // broadcast back to the sender. ChannelName is the + // friendly display name so ChatVM renders it as + // "[Fellowship] +Acdream says, \"hello\"". chat.OnSelfSent( AcDream.Core.Chat.ChatKind.Channel, cmd.Text, targetOrChannel: resolved.Value.DisplayName); diff --git a/src/AcDream.Core.Net/Messages/ChatRequests.cs b/src/AcDream.Core.Net/Messages/ChatRequests.cs index 485961c..af83d26 100644 --- a/src/AcDream.Core.Net/Messages/ChatRequests.cs +++ b/src/AcDream.Core.Net/Messages/ChatRequests.cs @@ -53,18 +53,35 @@ public static class ChatRequests } /// Send a /tell (whisper) by target character name. + /// + /// Wire field order is message FIRST, then target — that's + /// what ACE's GameActionTell.Handle reads (see + /// references/ACE/Source/ACE.Server/Network/GameAction/Actions/GameActionTell.cs + /// lines 17-18: + /// + /// var message = clientMessage.Payload.ReadString16L(); + /// var target = clientMessage.Payload.ReadString16L(); + /// + /// Filed after a 2026-04-25 live trace where every /tell +Je hello + /// failed with WeenieError 0x052B (CharacterNotAvailable). With the + /// previous (target-first) wire shape, ACE was reading our message + /// payload "hello" as the recipient name, looking it up, finding no + /// character "hello" online, and returning the not-available error. + /// Swapping the field order makes /tell <name> <text> + /// actually deliver. + /// public static byte[] BuildTell(uint gameActionSequence, string targetName, string message) { ArgumentNullException.ThrowIfNull(targetName); ArgumentNullException.ThrowIfNull(message); - byte[] name = PackString16L(targetName); byte[] msg = PackString16L(message); - byte[] body = new byte[12 + name.Length + msg.Length]; + byte[] name = PackString16L(targetName); + byte[] body = new byte[12 + msg.Length + name.Length]; BinaryPrimitives.WriteUInt32LittleEndian(body, GameActionEnvelope); BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(4), gameActionSequence); BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(8), TellOpcode); - Array.Copy(name, 0, body, 12, name.Length); - Array.Copy(msg, 0, body, 12 + name.Length, msg.Length); + Array.Copy(msg, 0, body, 12, msg.Length); + Array.Copy(name, 0, body, 12 + msg.Length, name.Length); return body; } diff --git a/src/AcDream.Core/Chat/ChatLog.cs b/src/AcDream.Core/Chat/ChatLog.cs index c2e9aaa..63e4334 100644 --- a/src/AcDream.Core/Chat/ChatLog.cs +++ b/src/AcDream.Core/Chat/ChatLog.cs @@ -128,15 +128,26 @@ public sealed class ChatLog ChannelId: errorId)); } - /// GameEvent ChannelBroadcast (0x0147). - public void OnChannelBroadcast(uint channelId, string sender, string text) + /// + /// Channel broadcast — legacy ChatChannel (0x0147) or the + /// TurbineChat (0xF7DE) global community channels (General, + /// Trade, LFG, Roleplay, Society, Olthoi). Pass + /// when the caller knows the + /// friendly room name (TurbineChat dispatch always does); the + /// ChatVM formatter renders entries as + /// "[ChannelName] Sender says, \"text\"" when set. + /// + public void OnChannelBroadcast(uint channelId, string sender, string text, string channelName = "") { Append(new ChatEntry( Kind: ChatKind.Channel, Sender: sender, Text: text, SenderGuid: 0, - ChannelId: channelId)); + ChannelId: channelId) + { + ChannelName = channelName, + }); } /// GameEvent Tell (0x02BD) — whisper received. @@ -196,15 +207,43 @@ public sealed class ChatLog }); } - /// Echo the player's own outbound message after local send. + /// + /// Echo the player's own outbound message after local send. + /// + /// + /// + /// Say: pass as + /// empty; the formatter renders "You say, \"text\"". + /// Tell: pass the target name; the formatter + /// renders "You tell {target}, \"text\"". + /// Channel: do not call for global community + /// channels — the server (ACE TurbineChatHandler) echoes the + /// broadcast back to the sender already, so optimistic-echoing + /// would double-print. For legacy channels (Fellowship, + /// Allegiance) where the server may not echo, pass the + /// channel name as ; it + /// becomes the entry's ChannelName. + /// + /// SenderGuid == 0 on the resulting entry is the + /// discriminator the formatter uses to render outgoing-vs-incoming + /// (a real incoming Tell carries the sender's player guid). + /// public void OnSelfSent(ChatKind kind, string text, string targetOrChannel = "") { Append(new ChatEntry( Kind: kind, - Sender: targetOrChannel, // used as "to whom" for Tell / channel name for Channel + // For Tell, Sender carries the target name (so the formatter + // can render "You tell {Sender}..."). For LocalSpeech, we + // leave Sender empty and let the formatter substitute "You". + // For Channel callers, Sender stays empty too — ChannelName + // (below) carries the friendly name. + Sender: kind == ChatKind.Tell ? targetOrChannel : "", Text: text, SenderGuid: 0, - ChannelId: 0)); + ChannelId: 0) + { + ChannelName = kind == ChatKind.Channel ? targetOrChannel : "", + }); } private void Append(ChatEntry entry) @@ -256,4 +295,13 @@ public readonly record struct ChatEntry( /// 's TextColored color choice. /// public Combat.CombatLineKind? CombatKind { get; init; } + + /// + /// Friendly name of the channel for + /// entries (e.g. "General", "Trade", "LFG", "Fellowship"). Empty + /// for non-Channel kinds. Used by ChatVM.FormatEntry to + /// render lines as "[ChannelName] Sender says, \"text\"". + /// Falls back to "ch {ChannelId}" if not populated. + /// + public string ChannelName { get; init; } = ""; } diff --git a/src/AcDream.UI.Abstractions/Panels/Chat/ChatVM.cs b/src/AcDream.UI.Abstractions/Panels/Chat/ChatVM.cs index 14758e4..3365868 100644 --- a/src/AcDream.UI.Abstractions/Panels/Chat/ChatVM.cs +++ b/src/AcDream.UI.Abstractions/Panels/Chat/ChatVM.cs @@ -99,10 +99,28 @@ public sealed class ChatVM /// public static string FormatEntry(ChatEntry entry) => entry.Kind switch { - ChatKind.LocalSpeech => $"{entry.Sender}: {entry.Text}", - ChatKind.RangedSpeech => $"{entry.Sender} says distantly: {entry.Text}", - ChatKind.Channel => $"[ch {entry.ChannelId}] {entry.Sender}: {entry.Text}", - ChatKind.Tell => $"[Tell] {entry.Sender}: {entry.Text}", + // Retail style: "Name says, \"text\"" (incoming) / + // "You say, \"text\"" (own echo). Sender is "" for an + // OnSelfSent echo; OnLocalSpeech substitutes "You" when the + // server sends an empty sender (own-shout echoes). Both forms + // collapse to the singular "You say" verb here. + ChatKind.LocalSpeech => IsOwnSpeaker(entry.Sender) + ? $"You say, \"{entry.Text}\"" + : $"{entry.Sender} says, \"{entry.Text}\"", + ChatKind.RangedSpeech => IsOwnSpeaker(entry.Sender) + ? $"You shout, \"{entry.Text}\"" + : $"{entry.Sender} shouts, \"{entry.Text}\"", + // Channel: "[ChannelName] Sender says, \"text\"". ChannelName + // is populated by callers that know the friendly name (the + // TurbineChat inbound dispatch and OnSelfSent for Channel + // kinds); falls back to "ch {ChannelId}" if not set. + ChatKind.Channel => $"[{ChannelLabel(entry)}] {entry.Sender} says, \"{entry.Text}\"", + // Tell: SenderGuid != 0 means an incoming whisper; == 0 is the + // OnSelfSent echo where Sender carries the target name. Retail + // wording: "You tell Caith, \"hi\"" / "Caith tells you, \"hi\"". + ChatKind.Tell => entry.SenderGuid != 0 + ? $"{entry.Sender} tells you, \"{entry.Text}\"" + : $"You tell {entry.Sender}, \"{entry.Text}\"", ChatKind.System => $"[System] {entry.Text}", ChatKind.Popup => $"[Popup] {entry.Text}", // Phase I.5: emote rendering matches retail's leading-asterisk @@ -121,6 +139,27 @@ public sealed class ChatVM _ => entry.Text, }; + /// + /// True when a chat entry's Sender denotes the local player + /// — i.e. the entry came from OnSelfSent (empty sender) or + /// OnLocalSpeech with the empty-sender substitution kicked + /// in (sender == "You"). Used by the formatter to pick the + /// singular "You say" verb over the third-person "Name says". + /// + private static bool IsOwnSpeaker(string sender) => + string.IsNullOrEmpty(sender) || sender == "You"; + + /// + /// Friendly channel label for a Channel entry. Prefers the entry's + /// ChannelName (set by callers that know the room name) + /// and falls back to "ch {ChannelId}" so legacy paths still + /// produce a readable line. + /// + private static string ChannelLabel(ChatEntry entry) => + string.IsNullOrEmpty(entry.ChannelName) + ? $"ch {entry.ChannelId}" + : entry.ChannelName; + /// /// Phase I.7: snapshot of the chat tail with kind metadata so /// can pick the right rendering primitive diff --git a/tests/AcDream.Core.Net.Tests/Messages/ChatTests.cs b/tests/AcDream.Core.Net.Tests/Messages/ChatTests.cs index ad47adc..02bef46 100644 --- a/tests/AcDream.Core.Net.Tests/Messages/ChatTests.cs +++ b/tests/AcDream.Core.Net.Tests/Messages/ChatTests.cs @@ -35,8 +35,13 @@ public sealed class ChatTests } [Fact] - public void BuildTell_IncludesBothStrings() + public void BuildTell_WritesMessageFirstThenTarget() { + // Wire order is message-then-target — ACE GameActionTell.Handle + // reads `var message = ...; var target = ...;` in that sequence. + // The previous (target-first) layout caused a 2026-04-25 live + // bug where every /tell failed with WeenieError 0x052B because + // ACE was looking up the message text as the recipient name. byte[] body = ChatRequests.BuildTell( gameActionSequence: 5, targetName: "Alice", message: "hey"); @@ -45,14 +50,14 @@ public sealed class ChatTests int pos = 12; ushort len1 = BinaryPrimitives.ReadUInt16LittleEndian(body.AsSpan(pos)); - Assert.Equal(5, len1); - Assert.Equal("Alice", Encoding.ASCII.GetString(body.AsSpan(pos + 2, 5))); + Assert.Equal(3, len1); + Assert.Equal("hey", Encoding.ASCII.GetString(body.AsSpan(pos + 2, 3))); - // "Alice" record = 2+5=7, pad 1 → advance by 8. + // "hey" record = 2+3=5, pad 3 → advance by 8. pos += 8; ushort len2 = BinaryPrimitives.ReadUInt16LittleEndian(body.AsSpan(pos)); - Assert.Equal(3, len2); - Assert.Equal("hey", Encoding.ASCII.GetString(body.AsSpan(pos + 2, 3))); + Assert.Equal(5, len2); + Assert.Equal("Alice", Encoding.ASCII.GetString(body.AsSpan(pos + 2, 5))); } [Fact] diff --git a/tests/AcDream.UI.Abstractions.Tests/ChatVMTests.cs b/tests/AcDream.UI.Abstractions.Tests/ChatVMTests.cs index 199ab94..418ca4e 100644 --- a/tests/AcDream.UI.Abstractions.Tests/ChatVMTests.cs +++ b/tests/AcDream.UI.Abstractions.Tests/ChatVMTests.cs @@ -25,8 +25,8 @@ public sealed class ChatVMTests var lines = vm.RecentLines(); Assert.Equal(2, lines.Count); - Assert.Equal("Caith: hello", lines[0]); - Assert.Equal("Regal: world", lines[1]); + Assert.Equal("Caith says, \"hello\"", lines[0]); + Assert.Equal("Regal says, \"world\"", lines[1]); } [Fact] @@ -41,36 +41,62 @@ public sealed class ChatVMTests // Tail = msg25..msg29 (5 entries, oldest first). Assert.Equal(5, lines.Count); - Assert.Equal("A: msg25", lines[0]); - Assert.Equal("A: msg29", lines[4]); + Assert.Equal("A says, \"msg25\"", lines[0]); + Assert.Equal("A says, \"msg29\"", lines[4]); } [Fact] - public void FormatEntry_LocalSpeech_SenderColonText() + public void FormatEntry_LocalSpeech_RetailStyleSays() { - var entry = new ChatEntry(ChatKind.LocalSpeech, "Caith", "hello", 0x5000_0001u, 0); - Assert.Equal("Caith: hello", ChatVM.FormatEntry(entry)); + // Retail format: "Name says, \"text\"" for someone else; + // "You say, \"text\"" for our own /say (sender == "" or "You"). + var incoming = new ChatEntry(ChatKind.LocalSpeech, "Caith", "hello", 0x5000_0001u, 0); + Assert.Equal("Caith says, \"hello\"", ChatVM.FormatEntry(incoming)); + + var ownEcho = new ChatEntry(ChatKind.LocalSpeech, "", "hi there", 0, 0); + Assert.Equal("You say, \"hi there\"", ChatVM.FormatEntry(ownEcho)); + + var ownEchoSubst = new ChatEntry(ChatKind.LocalSpeech, "You", "shouted echo", 0, 0); + Assert.Equal("You say, \"shouted echo\"", ChatVM.FormatEntry(ownEchoSubst)); } [Fact] - public void FormatEntry_RangedSpeech_IncludesDistanceHint() + public void FormatEntry_RangedSpeech_RetailStyleShouts() { - var entry = new ChatEntry(ChatKind.RangedSpeech, "Caith", "hello", 0x5000_0001u, 0); - Assert.Equal("Caith says distantly: hello", ChatVM.FormatEntry(entry)); + var incoming = new ChatEntry(ChatKind.RangedSpeech, "Caith", "hello", 0x5000_0001u, 0); + Assert.Equal("Caith shouts, \"hello\"", ChatVM.FormatEntry(incoming)); + + var ownEcho = new ChatEntry(ChatKind.RangedSpeech, "You", "loud", 0, 0); + Assert.Equal("You shout, \"loud\"", ChatVM.FormatEntry(ownEcho)); } [Fact] - public void FormatEntry_Channel_IncludesChannelId() + public void FormatEntry_Channel_UsesChannelNameWhenPresent() { - var entry = new ChatEntry(ChatKind.Channel, "Caith", "g'day", 0x5000_0001u, 7u); - Assert.Equal("[ch 7] Caith: g'day", ChatVM.FormatEntry(entry)); + // Friendly name takes precedence — "[Trade] Caith says, \"...\"" + var named = new ChatEntry(ChatKind.Channel, "Caith", "g'day", 0x5000_0001u, 7u) + { + ChannelName = "Trade", + }; + Assert.Equal("[Trade] Caith says, \"g'day\"", ChatVM.FormatEntry(named)); + + // Falls back to "ch {id}" when ChannelName isn't set (legacy + // path / older callers). + var unnamed = new ChatEntry(ChatKind.Channel, "Caith", "g'day", 0x5000_0001u, 7u); + Assert.Equal("[ch 7] Caith says, \"g'day\"", ChatVM.FormatEntry(unnamed)); } [Fact] - public void FormatEntry_Tell_PrefixedWithTellTag() + public void FormatEntry_Tell_RetailStyleTells() { - var entry = new ChatEntry(ChatKind.Tell, "Regal", "psst", 0x5000_0002u, 0); - Assert.Equal("[Tell] Regal: psst", ChatVM.FormatEntry(entry)); + // SenderGuid != 0 -> incoming whisper -> "Regal tells you, ..." + var incoming = new ChatEntry(ChatKind.Tell, "Regal", "psst", 0x5000_0002u, 0); + Assert.Equal("Regal tells you, \"psst\"", ChatVM.FormatEntry(incoming)); + + // SenderGuid == 0 -> our own outbound echo -> Sender carries + // the target name -> "You tell Regal, ..." + var ownEcho = new ChatEntry(ChatKind.Tell, "Regal", "psst", 0, 0); + Assert.Equal("You tell Regal, \"psst\"", ChatVM.FormatEntry(ownEcho)); } [Fact] @@ -113,6 +139,6 @@ public sealed class ChatVMTests log.OnLocalSpeech("Caith", "hello", 0x5000_0001u, false); var after = vm.RecentLines(); Assert.Single(after); - Assert.Equal("Caith: hello", after[0]); + Assert.Equal("Caith says, \"hello\"", after[0]); } } diff --git a/tests/AcDream.UI.Abstractions.Tests/Panels/Chat/ChatVMCombatTests.cs b/tests/AcDream.UI.Abstractions.Tests/Panels/Chat/ChatVMCombatTests.cs index fdf9c75..6e8f996 100644 --- a/tests/AcDream.UI.Abstractions.Tests/Panels/Chat/ChatVMCombatTests.cs +++ b/tests/AcDream.UI.Abstractions.Tests/Panels/Chat/ChatVMCombatTests.cs @@ -53,7 +53,7 @@ public sealed class ChatVMCombatTests var line = Assert.Single(vm.RecentLinesDetailed()); Assert.Equal(ChatKind.LocalSpeech, line.Kind); Assert.Null(line.CombatKind); - Assert.Equal("Alice: hi", line.Text); + Assert.Equal("Alice says, \"hi\"", line.Text); } [Fact] @@ -73,7 +73,7 @@ public sealed class ChatVMCombatTests // Plain LocalSpeech entry → Text; combat entry → TextColored. Assert.Contains(renderer.Calls, c => - c.Method == "Text" && (string?)c.Args[0] == "Alice: hi"); + c.Method == "Text" && (string?)c.Args[0] == "Alice says, \"hi\""); var coloredCall = Assert.Single( renderer.Calls, c => c.Method == "TextColored");