diff --git a/src/AcDream.Core.Net/GameEventWiring.cs b/src/AcDream.Core.Net/GameEventWiring.cs index a2295608..13afc2be 100644 --- a/src/AcDream.Core.Net/GameEventWiring.cs +++ b/src/AcDream.Core.Net/GameEventWiring.cs @@ -688,14 +688,54 @@ public static class GameEventWiring registrar.Register(GameEventType.MagicDispelEnchantment, e => { var p = GameEvents.ParseMagicDispelEnchantment(e.Payload.Span); - if (p is not null) spellbook.OnEnchantmentRemoved(p.Value.Layer, p.Value.SpellId); + if (p is null) return; + spellbook.OnEnchantmentRemoved(p.Value.Layer, p.Value.SpellId); + NotifyOfEnchantmentRemoval((uint)p.Value.SpellId); }); registrar.Register(GameEventType.MagicDispelMultipleEnchantments, e => { var entries = GameEvents.ParseMagicLayeredSpellList(e.Payload.Span); - if (entries is not null) - spellbook.OnEnchantmentsRemoved(entries.Select(item => ((uint)item.SpellId, (uint)item.Layer))); + if (entries is null) return; + spellbook.OnEnchantmentsRemoved(entries.Select(item => ((uint)item.SpellId, (uint)item.Layer))); + foreach (var entry in entries) + NotifyOfEnchantmentRemoval((uint)entry.SpellId); }); + + // Retail spell id 0x29A, special-cased by name in + // NotifyOfEnchantmentRemoval so the line reads "Vitae penalty has + // expired." rather than "Vitae has expired." + const uint VitaePenaltySpellId = 0x29Au; + + // ClientMagicSystem::NotifyOfEnchantmentRemoval @ 0x005686C0. The + // server sends NO text for an enchantment leaving the PLAYER — it is + // the client that announces it. (Enchantments leaving an ITEM are + // different: those arrive as ordinary server chat, which is why item + // spells were the only ones showing up.) + void NotifyOfEnchantmentRemoval(uint spellId) + { + if (onInterfaceText is null) + return; + + // Retail's own guards, in order: ids at or above 0x8000 are not + // spells and are skipped outright, and a spell missing from the + // table returns without printing. + if (spellId >= 0x8000 + || !spellbook.TryGetMetadata(spellId, out SpellMetadata meta)) + { + return; + } + + // Vitae reads "Vitae penalty has expired." — retail appends the + // word to the spell's own name for this one id. + string name = spellId == VitaePenaltySpellId + ? meta.Name + " penalty" + : meta.Name; + + // AddTextToScroll(text, 7, 1, 0). Retail's literal carries a + // trailing newline because its scroll appends raw text; AddText is + // line-based, so adding one here would print a blank line. + onInterfaceText($"{name} has expired.", RetailLogTextType.Magic); + } registrar.Register(GameEventType.MagicPurgeEnchantments, _ => spellbook.OnPurgeAll()); registrar.Register(GameEventType.MagicPurgeBadEnchantments, diff --git a/src/AcDream.Runtime/Chat/LiveChatCommandRoute.cs b/src/AcDream.Runtime/Chat/LiveChatCommandRoute.cs index 827f3348..b4128b39 100644 --- a/src/AcDream.Runtime/Chat/LiveChatCommandRoute.cs +++ b/src/AcDream.Runtime/Chat/LiveChatCommandRoute.cs @@ -159,16 +159,14 @@ public sealed class LiveChatCommandRoute case ChatChannelKind.Tell: if (string.IsNullOrEmpty(command.TargetName)) return; - if (!SendIfActive(() => - bindings.SendTell(command.TargetName, command.Text))) - { - return; - } - bindings.Chat.OnSelfSent( - ChatKind.Tell, - command.Text, - logTextType: (uint)RetailLogTextType.SpeechDirectSend, - targetOrChannel: command.TargetName); + // No local echo. The server sends the "You tell X, ..." line + // back itself (ACE GameActionTell -> GameMessageSystemChat with + // ChatMessageType.OutgoingTell 0x04), and retail's own send + // path just transmits: Event_TalkDirectByName @ 0x00577CF4 has + // no AddTextToScroll beside it. Echoing optimistically printed + // the line twice. Say above already relies on the server echo + // for exactly this reason. + SendIfActive(() => bindings.SendTell(command.TargetName, command.Text)); return; } diff --git a/tests/AcDream.App.Tests/Net/LiveSessionCommandRouterTests.cs b/tests/AcDream.App.Tests/Net/LiveSessionCommandRouterTests.cs index ad8b7818..80a656e2 100644 --- a/tests/AcDream.App.Tests/Net/LiveSessionCommandRouterTests.cs +++ b/tests/AcDream.App.Tests/Net/LiveSessionCommandRouterTests.cs @@ -161,17 +161,18 @@ public sealed class LiveSessionCommandRouterTests Assert.Equal([("Friend", "hello")], tells); Assert.Equal([(0x00000800u, "group")], channels); - // CH3 (2026-08-09): Fellowship is one of the ACE server-echoing - // legacy channels (resends with an empty sender) — the router must - // NOT also emit a local optimistic echo, or the line double-prints. - // Only the Tell echo (which the server never resends) survives. - Assert.Collection( - chat.Snapshot(), - entry => - { - Assert.Equal(ChatKind.Tell, entry.Kind); - Assert.Equal("Friend", entry.Sender); - }); + // CH3 (2026-08-09) got the Fellowship half right — ACE resends legacy + // channel lines, so a local echo would double-print — but its stated + // reason for keeping the Tell echo, "which the server never resends", + // was wrong: ACE's GameActionTell replies with a GameMessageSystemChat + // carrying the finished "You tell Friend, ..." line + // (ChatMessageType.OutgoingTell). Tells double-printed in the chat + // window for exactly that reason. Retail's own send path + // @0x00577CF4 has no AddTextToScroll beside it either. + // + // So NEITHER emits a local echo now, and Say has always worked this + // way. + Assert.Empty(chat.Snapshot()); } [Fact] diff --git a/tests/AcDream.Core.Net.Tests/GameEventWiringTests.cs b/tests/AcDream.Core.Net.Tests/GameEventWiringTests.cs index f093d5d2..8ca7cb69 100644 --- a/tests/AcDream.Core.Net.Tests/GameEventWiringTests.cs +++ b/tests/AcDream.Core.Net.Tests/GameEventWiringTests.cs @@ -1,5 +1,6 @@ using System; using System.Buffers.Binary; +using System.Collections.Generic; using System.IO; using System.Text; using AcDream.Core.Chat; @@ -1813,6 +1814,99 @@ public sealed class GameEventWiringTests Assert.Empty(rentPayment); } + /// + /// An enchantment leaving the PLAYER is announced by the client, not the + /// server — so nothing printed at all until this was wired. + /// + /// + /// ACE splits the two cases: an enchantment expiring on an ITEM arrives as + /// ordinary server chat ("The spell X on Y has expired."), which is why + /// item spells were the only ones the player ever saw. One expiring on the + /// player arrives as GameEventMagicDispelEnchantment carrying NO text, + /// because retail's client writes the line itself — + /// ClientMagicSystem::NotifyOfEnchantmentRemoval @0x005686C0, printed at + /// LogTextType 7 (Magic). + /// + [Fact] + public void DispelledEnchantment_AnnouncesItsExpiryAtTheRetailTextType() + { + var lines = new List<(string Text, RetailLogTextType Type)>(); + var dispatcher = new GameEventDispatcher(); + GameEventWiring.WireAll( + dispatcher, + new ClientObjectTable(), + new CombatState(), + SpellbookWithNames(), + new ChatLog(), + onInterfaceText: (text, type) => lines.Add((text, type))); + + dispatcher.Dispatch(GameEventEnvelope.TryParse(WrapEnvelope( + GameEventType.MagicDispelEnchantment, + DispelPayload(spellId: 1234, layer: 1)))!.Value); + + Assert.Equal( + ("Fire Protection Self has expired.", RetailLogTextType.Magic), + Assert.Single(lines)); + } + + [Fact] + public void DispelledVitaeReadsAsAPenalty() + { + // Retail appends " penalty" to this one spell's own name, so the line + // reads "Vitae penalty has expired." rather than "Vitae has expired." + var lines = new List<(string Text, RetailLogTextType Type)>(); + var dispatcher = new GameEventDispatcher(); + GameEventWiring.WireAll( + dispatcher, + new ClientObjectTable(), + new CombatState(), + SpellbookWithNames(), + new ChatLog(), + onInterfaceText: (text, type) => lines.Add((text, type))); + + dispatcher.Dispatch(GameEventEnvelope.TryParse(WrapEnvelope( + GameEventType.MagicDispelEnchantment, + DispelPayload(spellId: 0x29A, layer: 1)))!.Value); + + Assert.Equal("Vitae penalty has expired.", Assert.Single(lines).Text); + } + + [Fact] + public void ADispelledSpellMissingFromTheTablePrintsNothing() + { + // Retail returns without printing when InqSpellBase fails, rather than + // announcing a blank name. + var lines = new List<(string Text, RetailLogTextType Type)>(); + var dispatcher = new GameEventDispatcher(); + GameEventWiring.WireAll( + dispatcher, + new ClientObjectTable(), + new CombatState(), + SpellbookWithNames(), + new ChatLog(), + onInterfaceText: (text, type) => lines.Add((text, type))); + + dispatcher.Dispatch(GameEventEnvelope.TryParse(WrapEnvelope( + GameEventType.MagicDispelEnchantment, + DispelPayload(spellId: 4321, layer: 1)))!.Value); + + Assert.Empty(lines); + } + + private static Spellbook SpellbookWithNames() + => new(SpellTable.LoadFromReader(new System.IO.StringReader( + "Spell ID,Name,Flags [Hex]\n" + + "1234,Fire Protection Self,0x4\n" + + "666,Vitae,0x4\n"))); + + private static byte[] DispelPayload(ushort spellId, ushort layer) + { + byte[] payload = new byte[4]; + BinaryPrimitives.WriteUInt16LittleEndian(payload, spellId); + BinaryPrimitives.WriteUInt16LittleEndian(payload.AsSpan(2), layer); + return payload; + } + private static byte[] BuildEnchantment( ushort spellId, ushort layer, diff --git a/tests/AcDream.Runtime.Tests/Chat/LiveChatCommandRouteTests.cs b/tests/AcDream.Runtime.Tests/Chat/LiveChatCommandRouteTests.cs index 04d8fa79..87e0b61c 100644 --- a/tests/AcDream.Runtime.Tests/Chat/LiveChatCommandRouteTests.cs +++ b/tests/AcDream.Runtime.Tests/Chat/LiveChatCommandRouteTests.cs @@ -47,10 +47,13 @@ public sealed class LiveChatCommandRouteTests "client:QueryAge", ], sent); - ChatEntry echo = Assert.Single(communication.Chat.Snapshot()); - Assert.Equal(ChatKind.Tell, echo.Kind); - Assert.Equal("Bob", echo.Sender); - Assert.Equal("secret", echo.Text); + // A tell is SENT and nothing is echoed locally. The server returns the + // "You tell Bob, ..." line itself (ACE GameActionTell -> + // GameMessageSystemChat with ChatMessageType.OutgoingTell), and retail's + // own send path @0x00577CF4 has no AddTextToScroll beside it — so an + // optimistic echo here printed every tell TWICE in the chat window. + // Say has always relied on the server echo for exactly this reason. + Assert.Empty(communication.Chat.Snapshot()); route.Dispose(); route.Publish(new SendServerCommandCmd("@stale"));