From 7d452aa6a27e54ffd686580f5375e0dd297f06f1 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 21 Jul 2026 10:45:15 +0200 Subject: [PATCH] refactor(net): own session wiring subscriptions --- src/AcDream.Core.Net/CombatStateWiring.cs | 17 +- src/AcDream.Core.Net/GameEventWiring.cs | 125 +++++++++----- .../Messages/GameEventDispatcher.cs | 68 +++++++- src/AcDream.Core.Net/ObjectTableWiring.cs | 27 ++- src/AcDream.Core.Net/SubscriptionSet.cs | 61 +++++++ .../Messages/GameEventDispatcherTests.cs | 162 +++++++++++++++++- .../SubscriptionSetTests.cs | 47 +++++ .../WorldSessionWiringOwnershipTests.cs | 131 ++++++++++++++ 8 files changed, 577 insertions(+), 61 deletions(-) create mode 100644 src/AcDream.Core.Net/SubscriptionSet.cs create mode 100644 tests/AcDream.Core.Net.Tests/SubscriptionSetTests.cs create mode 100644 tests/AcDream.Core.Net.Tests/WorldSessionWiringOwnershipTests.cs diff --git a/src/AcDream.Core.Net/CombatStateWiring.cs b/src/AcDream.Core.Net/CombatStateWiring.cs index 44f2514b..9e637e60 100644 --- a/src/AcDream.Core.Net/CombatStateWiring.cs +++ b/src/AcDream.Core.Net/CombatStateWiring.cs @@ -12,13 +12,15 @@ public static class CombatStateWiring { public const uint CombatModePropertyId = 40u; - public static void Wire(WorldSession session, CombatState combat) + public static IDisposable Wire(WorldSession session, CombatState combat) { ArgumentNullException.ThrowIfNull(session); ArgumentNullException.ThrowIfNull(combat); - session.PlayerIntPropertyUpdated += update => + Action handler = update => ApplyPlayerIntProperty(combat, update.Property, update.Value); + session.PlayerIntPropertyUpdated += handler; + return new EventSubscription(session, handler); } /// @@ -44,4 +46,15 @@ public static class CombatStateWiring combat.SetCombatMode(mode); return true; } + + private sealed class EventSubscription( + WorldSession session, + Action handler) : IDisposable + { + private Action? _unsubscribe = + () => session.PlayerIntPropertyUpdated -= handler; + + public void Dispose() => + Interlocked.Exchange(ref _unsubscribe, null)?.Invoke(); + } } diff --git a/src/AcDream.Core.Net/GameEventWiring.cs b/src/AcDream.Core.Net/GameEventWiring.cs index fba3279a..89f9561c 100644 --- a/src/AcDream.Core.Net/GameEventWiring.cs +++ b/src/AcDream.Core.Net/GameEventWiring.cs @@ -18,9 +18,10 @@ namespace AcDream.Core.Net; /// /// /// Call once at startup (or on reconnect) passing the session's -/// dispatcher + the state instances you want to feed. The wiring is -/// additive — if you want to add a custom handler for a specific -/// event, register it AFTER this helper so it overrides the default. +/// dispatcher + the state instances you want to feed. The returned +/// subscription owns exactly these registrations and must be disposed +/// before the session is replaced. A later legacy registration still +/// overrides the corresponding default without becoming owned here. /// /// /// @@ -32,7 +33,7 @@ namespace AcDream.Core.Net; /// public static class GameEventWiring { - public static void WireAll( + public static IDisposable WireAll( GameEventDispatcher dispatcher, ClientObjectTable items, CombatState combat, @@ -88,29 +89,31 @@ public static class GameEventWiring ArgumentNullException.ThrowIfNull(spellbook); ArgumentNullException.ThrowIfNull(chat); clientTime ??= static () => 0d; + var registrar = new OwnedGameEventRegistrar(dispatcher); + using var construction = new RegistrationBuildScope(registrar); // ── Chat ────────────────────────────────────────────────── - dispatcher.Register(GameEventType.ChannelBroadcast, e => + registrar.Register(GameEventType.ChannelBroadcast, e => { var p = GameEvents.ParseChannelBroadcast(e.Payload.Span); if (p is not null) chat.OnChannelBroadcast(p.Value.ChannelId, p.Value.SenderName, p.Value.Message); }); - dispatcher.Register(GameEventType.Tell, e => + registrar.Register(GameEventType.Tell, e => { var p = GameEvents.ParseTell(e.Payload.Span); if (p is not null) chat.OnTellReceived(p.Value.SenderName, p.Value.Message, p.Value.SenderGuid); }); - dispatcher.Register(GameEventType.CommunicationTransientString, e => + 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); }); - dispatcher.Register(GameEventType.PopupString, e => + registrar.Register(GameEventType.PopupString, e => { var s = GameEvents.ParsePopupString(e.Payload.Span); if (s is not null) chat.OnPopup(s); }); - dispatcher.Register(GameEventType.QueryAgeResponse, e => + registrar.Register(GameEventType.QueryAgeResponse, e => { var p = GameEvents.ParseQueryAgeResponse(e.Payload.Span); if (p is null) return; @@ -121,7 +124,7 @@ public static class GameEventWiring }); if (onConfirmationRequest is not null) { - dispatcher.Register(GameEventType.CharacterConfirmationRequest, e => + registrar.Register(GameEventType.CharacterConfirmationRequest, e => { var request = GameEvents.ParseCharacterConfirmationRequest(e.Payload.Span); if (request is not null) @@ -131,7 +134,7 @@ public static class GameEventWiring if (onConfirmationDone is not null) { - dispatcher.Register(GameEventType.CharacterConfirmationDone, e => + registrar.Register(GameEventType.CharacterConfirmationDone, e => { var done = GameEvents.ParseCharacterConfirmationDone(e.Payload.Span); if (done is not null) @@ -141,7 +144,7 @@ public static class GameEventWiring if (friends is not null) { - dispatcher.Register(GameEventType.FriendsListUpdate, e => + registrar.Register(GameEventType.FriendsListUpdate, e => { FriendsUpdate? update = SocialStateMessages.ParseFriendsUpdate(e.Payload.Span); if (update is not null) friends.Apply(update); @@ -150,7 +153,7 @@ public static class GameEventWiring if (squelch is not null) { - dispatcher.Register(GameEventType.SetSquelchDB, e => + registrar.Register(GameEventType.SetSquelchDB, e => { SquelchDatabase? database = SocialStateMessages.ParseSquelchDatabase(e.Payload.Span); if (database is not null) squelch.Replace(database); @@ -167,7 +170,7 @@ public static class GameEventWiring // server-message handling pattern. if (turbineChat is not null) { - dispatcher.Register(GameEventType.SetTurbineChatChannels, e => + registrar.Register(GameEventType.SetTurbineChatChannels, e => { var p = SetTurbineChatChannels.TryParse(e.Payload.Span); if (p is null) return; @@ -202,26 +205,26 @@ public static class GameEventWiring // (GameEvents.ParseWeenieError(WithString)) but were never registered. // The server fires these for game-logic failures: "not enough mana", // "can't pick that up", "your spell fizzled". Routed to chat. - dispatcher.Register(GameEventType.WeenieError, e => + registrar.Register(GameEventType.WeenieError, e => { var code = GameEvents.ParseWeenieError(e.Payload.Span); if (code is not null) chat.OnWeenieError(code.Value, param: null); }); - dispatcher.Register(GameEventType.WeenieErrorWithString, e => + registrar.Register(GameEventType.WeenieErrorWithString, e => { var p = GameEvents.ParseWeenieErrorWithString(e.Payload.Span); if (p is not null) chat.OnWeenieError(p.Value.ErrorCode, p.Value.Interpolation); }); // ── Combat ──────────────────────────────────────────────── - dispatcher.Register(GameEventType.UpdateHealth, e => + registrar.Register(GameEventType.UpdateHealth, e => { var p = GameEvents.ParseUpdateHealth(e.Payload.Span); if (p is not null) combat.OnUpdateHealth(p.Value.TargetGuid, p.Value.HealthPercent); }); if (itemMana is not null) { - dispatcher.Register(GameEventType.QueryItemManaResponse, e => + registrar.Register(GameEventType.QueryItemManaResponse, e => { var p = GameEvents.ParseQueryItemManaResponse(e.Payload.Span); if (p is not null) @@ -229,67 +232,67 @@ public static class GameEventWiring p.Value.ItemGuid, p.Value.ManaPercent, p.Value.Valid); }); } - dispatcher.Register(GameEventType.VictimNotification, e => + registrar.Register(GameEventType.VictimNotification, e => { var p = GameEvents.ParseVictimNotification(e.Payload.Span); if (p is not null) chat.OnCombatLine(p.Value.DeathMessage, CombatLineKind.Error); }); - dispatcher.Register(GameEventType.DefenderNotification, e => + registrar.Register(GameEventType.DefenderNotification, e => { var p = GameEvents.ParseDefenderNotification(e.Payload.Span); if (p is not null) combat.OnDefenderNotification( p.Value.AttackerName, 0u, p.Value.DamageType, p.Value.Damage, p.Value.HitQuadrant, p.Value.Critical); }); - dispatcher.Register(GameEventType.AttackerNotification, e => + registrar.Register(GameEventType.AttackerNotification, e => { var p = GameEvents.ParseAttackerNotification(e.Payload.Span); if (p is not null) combat.OnAttackerNotification( p.Value.DefenderName, p.Value.DamageType, p.Value.Damage, (float)p.Value.HealthPercent); }); - dispatcher.Register(GameEventType.EvasionAttackerNotification, e => + registrar.Register(GameEventType.EvasionAttackerNotification, e => { var name = GameEvents.ParseEvasionAttackerNotification(e.Payload.Span); if (name is not null) combat.OnEvasionAttackerNotification(name); }); - dispatcher.Register(GameEventType.EvasionDefenderNotification, e => + registrar.Register(GameEventType.EvasionDefenderNotification, e => { var name = GameEvents.ParseEvasionDefenderNotification(e.Payload.Span); if (name is not null) combat.OnEvasionDefenderNotification(name); }); - dispatcher.Register(GameEventType.AttackDone, e => + registrar.Register(GameEventType.AttackDone, e => { var p = GameEvents.ParseAttackDone(e.Payload.Span); if (p is not null) combat.OnAttackDone(p.Value.AttackSequence, p.Value.WeenieError); }); - dispatcher.Register(GameEventType.CombatCommenceAttack, e => + registrar.Register(GameEventType.CombatCommenceAttack, e => { if (GameEvents.ParseCombatCommenceAttack(e.Payload.Span)) combat.OnCombatCommenceAttack(); }); - dispatcher.Register(GameEventType.KillerNotification, e => + registrar.Register(GameEventType.KillerNotification, e => { var p = GameEvents.ParseKillerNotification(e.Payload.Span); if (p is not null) chat.OnCombatLine(p.Value.DeathMessage, CombatLineKind.Info); }); // ── Spells ──────────────────────────────────────────────── - dispatcher.Register(GameEventType.MagicUpdateSpell, e => + registrar.Register(GameEventType.MagicUpdateSpell, e => { var spellId = GameEvents.ParseMagicUpdateSpell(e.Payload.Span); if (spellId is not null) spellbook.OnSpellLearned(spellId.Value); }); - dispatcher.Register(GameEventType.MagicRemoveSpell, e => + registrar.Register(GameEventType.MagicRemoveSpell, e => { var spellId = GameEvents.ParseMagicRemoveSpell(e.Payload.Span); if (spellId is not null) spellbook.OnSpellForgotten(spellId.Value); }); - dispatcher.Register(GameEventType.MagicUpdateEnchantment, e => + registrar.Register(GameEventType.MagicUpdateEnchantment, e => { var p = GameEvents.ParseMagicUpdateEnchantment(e.Payload.Span); if (p is not null) spellbook.OnEnchantmentAdded(ToActiveEnchantment(p.Value, clientTime())); }); - dispatcher.Register(GameEventType.MagicUpdateMultipleEnchantments, e => + registrar.Register(GameEventType.MagicUpdateMultipleEnchantments, e => { var entries = GameEvents.ParseMagicUpdateMultipleEnchantments(e.Payload.Span); if (entries is not null) @@ -299,35 +302,35 @@ public static class GameEventWiring ToActiveEnchantment(entry, receivedAt))); } }); - dispatcher.Register(GameEventType.MagicRemoveEnchantment, e => + registrar.Register(GameEventType.MagicRemoveEnchantment, e => { var p = GameEvents.ParseMagicRemoveEnchantment(e.Payload.Span); if (p is not null) spellbook.OnEnchantmentRemoved(p.Value.Layer, p.Value.SpellId); }); - dispatcher.Register(GameEventType.MagicRemoveMultipleEnchantments, e => + registrar.Register(GameEventType.MagicRemoveMultipleEnchantments, e => { var entries = GameEvents.ParseMagicLayeredSpellList(e.Payload.Span); if (entries is not null) spellbook.OnEnchantmentsRemoved(entries.Select(item => ((uint)item.SpellId, (uint)item.Layer))); }); - dispatcher.Register(GameEventType.MagicDispelEnchantment, e => + 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); }); - dispatcher.Register(GameEventType.MagicDispelMultipleEnchantments, e => + 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))); }); - dispatcher.Register(GameEventType.MagicPurgeEnchantments, + registrar.Register(GameEventType.MagicPurgeEnchantments, _ => spellbook.OnPurgeAll()); - dispatcher.Register(GameEventType.MagicPurgeBadEnchantments, + registrar.Register(GameEventType.MagicPurgeBadEnchantments, _ => spellbook.OnPurgeBadEnchantments()); // ── Inventory ───────────────────────────────────────────── - dispatcher.Register(GameEventType.WieldObject, e => + registrar.Register(GameEventType.WieldObject, e => { var p = GameEvents.ParseWieldObject(e.Payload.Span); if (p is null) return; @@ -338,7 +341,7 @@ public static class GameEventWiring wielderGuid, (AcDream.Core.Items.EquipMask)p.Value.EquipLoc); }); - dispatcher.Register(GameEventType.InventoryPutObjInContainer, e => + registrar.Register(GameEventType.InventoryPutObjInContainer, e => { var p = GameEvents.ParsePutObjInContainer(e.Payload.Span); if (p is null) return; @@ -355,7 +358,7 @@ public static class GameEventWiring // opened (Use 0x0036). Treat it as a full projection-only REPLACE: update membership without // inventing ContainerSlot values, then publish one ContainerContentsReplaced notification so // every UI consumer repaints from the same snapshot. Retail: ClientUISystem::OnViewContents. - dispatcher.Register(GameEventType.ViewContents, e => + registrar.Register(GameEventType.ViewContents, e => { var p = GameEvents.ParseViewContents(e.Payload.Span); if (p is null) return; @@ -371,7 +374,7 @@ public static class GameEventWiring // B-Wire: InventoryPutObjectIn3D (0x019A) — server confirms an item dropped // to the world. Unparent it from its container (it's now a ground object) so // the inventory grid drops the cell; the object itself survives. - dispatcher.Register(GameEventType.InventoryPutObjectIn3D, e => + registrar.Register(GameEventType.InventoryPutObjectIn3D, e => { var guid = GameEvents.ParsePutObjectIn3D(e.Payload.Span); if (guid is not null) @@ -386,7 +389,7 @@ public static class GameEventWiring // B-Drag: InventoryServerSaveFailed (0x00A0) — server rejected an optimistic move. // Snap the item back to its pre-move slot. Log only when there was no pending move // (a server-initiated failure on a non-optimistic path). - dispatcher.Register(GameEventType.InventoryServerSaveFailed, e => + registrar.Register(GameEventType.InventoryServerSaveFailed, e => { var p = GameEvents.ParseInventoryServerSaveFailed(e.Payload.Span); if (p is null) return; @@ -404,7 +407,7 @@ public static class GameEventWiring // retail surfaces the string-table text as a chat line. Interim text map: // WeenieErrorText (#202 / register AP-74). Without this line a refused // kit-heal looks like "nothing happened" — the 2026-07-03 session bug. - dispatcher.Register(GameEventType.UseDone, e => + registrar.Register(GameEventType.UseDone, e => { uint? err = GameEvents.ParseUseDone(e.Payload.Span); if (err is null) return; @@ -417,7 +420,7 @@ public static class GameEventWiring // CloseGroundContainer (0x0052): clear ClientUISystem::groundObject and // retire the root plus nested temporary ViewContents projections. Child // objects remain until authoritative move/delete wire says otherwise. - dispatcher.Register(GameEventType.CloseGroundContainer, e => + registrar.Register(GameEventType.CloseGroundContainer, e => { var guid = GameEvents.ParseCloseGroundContainer(e.Payload.Span); if (guid is null) return; @@ -425,7 +428,7 @@ public static class GameEventWiring items.StopViewingContentsTree(guid.Value); }); - dispatcher.Register(GameEventType.IdentifyObjectResponse, e => + registrar.Register(GameEventType.IdentifyObjectResponse, e => { var p = AppraiseInfoParser.TryParse(e.Payload.Span); if (p is null || !p.Value.Success) return; @@ -461,7 +464,7 @@ public static class GameEventWiring // SpellBook flag in IdentifyObjectResponse is for caster // items / scrolls only). bool dumpPd = Environment.GetEnvironmentVariable("ACDREAM_DUMP_VITALS") == "1"; - dispatcher.Register(GameEventType.PlayerDescription, e => + registrar.Register(GameEventType.PlayerDescription, e => { var p = PlayerDescriptionParser.TryParse(e.Payload.Span); if (dumpPd) @@ -652,6 +655,38 @@ public static class GameEventWiring // toolbar can read them without holding a parser reference. onShortcuts?.Invoke(p.Value.Shortcuts); }); + return construction.Complete(); + } + + private sealed class OwnedGameEventRegistrar( + GameEventDispatcher dispatcher) : IDisposable + { + private readonly SubscriptionSet _subscriptions = new(); + + public void Register( + GameEventType type, + GameEventDispatcher.EventHandler handler) => + _subscriptions.Add(dispatcher.RegisterOwned(type, handler)); + + public void Dispose() => _subscriptions.Dispose(); + } + + private sealed class RegistrationBuildScope( + OwnedGameEventRegistrar registration) : IDisposable + { + private bool _complete; + + public IDisposable Complete() + { + _complete = true; + return registration; + } + + public void Dispose() + { + if (!_complete) + registration.Dispose(); + } } private static ActiveEnchantmentRecord ToActiveEnchantment( diff --git a/src/AcDream.Core.Net/Messages/GameEventDispatcher.cs b/src/AcDream.Core.Net/Messages/GameEventDispatcher.cs index c4eb38bb..1a1abfdc 100644 --- a/src/AcDream.Core.Net/Messages/GameEventDispatcher.cs +++ b/src/AcDream.Core.Net/Messages/GameEventDispatcher.cs @@ -24,7 +24,27 @@ public sealed class GameEventDispatcher { public delegate void EventHandler(GameEventEnvelope envelope); - private readonly Dictionary _handlers = new(); + private sealed class RegistrationNode( + GameEventType type, + EventHandler handler, + RegistrationNode? previous) + { + public GameEventType Type { get; } = type; + public EventHandler Handler { get; } = handler; + public RegistrationNode? Previous { get; } = previous; + public bool Retired { get; set; } + } + + private sealed class OwnedRegistration( + GameEventDispatcher owner, + RegistrationNode node) : IDisposable + { + private Action? _retire = () => owner.Retire(node); + + public void Dispose() => Interlocked.Exchange(ref _retire, null)?.Invoke(); + } + + private readonly Dictionary _handlers = new(); private readonly Dictionary _unhandledCounts = new(); /// @@ -34,13 +54,34 @@ public sealed class GameEventDispatcher public void Register(GameEventType type, EventHandler handler) { ArgumentNullException.ThrowIfNull(handler); - _handlers[type] = handler; + // Legacy replacement intentionally severs the predecessor chain. An + // exact owned token installed earlier must never remove or expose a + // later unowned replacement when that token is disposed. + _handlers[type] = new RegistrationNode(type, handler, previous: null); + } + + /// + /// Installs one exact owned handler. Disposal restores the nearest live + /// predecessor only when this registration is still current. Nested A/B + /// ownership is safe in either disposal order. + /// + public IDisposable RegisterOwned(GameEventType type, EventHandler handler) + { + ArgumentNullException.ThrowIfNull(handler); + _handlers.TryGetValue(type, out RegistrationNode? previous); + var node = new RegistrationNode(type, handler, previous); + _handlers[type] = node; + return new OwnedRegistration(this, node); } /// /// Remove the registered handler for a sub-opcode. /// - public void Unregister(GameEventType type) => _handlers.Remove(type); + public void Unregister(GameEventType type) + { + if (_handlers.Remove(type, out RegistrationNode? current)) + current.Retired = true; + } /// /// Route an envelope to its handler, or log as unhandled. Exceptions @@ -49,11 +90,11 @@ public sealed class GameEventDispatcher /// public void Dispatch(GameEventEnvelope envelope) { - if (_handlers.TryGetValue(envelope.EventType, out var handler)) + if (_handlers.TryGetValue(envelope.EventType, out RegistrationNode? registration)) { try { - handler(envelope); + registration.Handler(envelope); } catch (Exception ex) { @@ -87,4 +128,21 @@ public sealed class GameEventDispatcher /// How many distinct sub-opcodes have a handler registered. public int RegisteredHandlerCount => _handlers.Count; + + private void Retire(RegistrationNode node) + { + node.Retired = true; + if (!_handlers.TryGetValue(node.Type, out RegistrationNode? current) + || !ReferenceEquals(current, node)) + return; + + RegistrationNode? predecessor = node.Previous; + while (predecessor?.Retired == true) + predecessor = predecessor.Previous; + + if (predecessor is null) + _handlers.Remove(node.Type); + else + _handlers[node.Type] = predecessor; + } } diff --git a/src/AcDream.Core.Net/ObjectTableWiring.cs b/src/AcDream.Core.Net/ObjectTableWiring.cs index 1f6d3530..b9d320a7 100644 --- a/src/AcDream.Core.Net/ObjectTableWiring.cs +++ b/src/AcDream.Core.Net/ObjectTableWiring.cs @@ -19,7 +19,7 @@ public static class ObjectTableWiring /// Subscribe to quality, inventory, and property /// updates whose freshness is independent of the physics timestamp pack. /// - public static void Wire( + public static IDisposable Wire( WorldSession session, ClientObjectTable table, Func? playerGuid = null, @@ -27,6 +27,7 @@ public static class ObjectTableWiring { ArgumentNullException.ThrowIfNull(session); ArgumentNullException.ThrowIfNull(table); + var subscriptions = new SubscriptionSet(); // Create/Delete/Pickup are deliberately not subscribed here. Their // shared live-object timestamps must be accepted before either the @@ -35,8 +36,10 @@ public static class ObjectTableWiring // B-Wire: apply EVERY PropertyInt update on a visible object (0x02CE), not just // UiEffects — the server is the authority on object properties. UpdateIntProperty // stores it in the bundle and still mirrors UiEffects → the typed Effects field. - session.ObjectIntPropertyUpdated += u => + Action objectIntUpdated = u => table.UpdateIntProperty(u.Guid, u.Property, u.Value); + session.ObjectIntPropertyUpdated += objectIntUpdated; + subscriptions.Add(() => session.ObjectIntPropertyUpdated -= objectIntUpdated); // B-Wire: PrivateUpdatePropertyInt (0x02CD) carries no guid — it targets the // local player. Route it to the player object so live EncumbranceVal updates the @@ -44,26 +47,38 @@ public static class ObjectTableWiring // call (which precedes any live 0x02CD), so UpdateIntProperty finds it. If it somehow // hasn't yet, this no-ops (UpdateIntProperty returns false on an unknown guid) rather // than creating a phantom — the next PD / CreateObject seeds it. - session.PlayerIntPropertyUpdated += u => + Action playerIntUpdated = u => { if (playerGuid is not null) table.UpdateIntProperty(playerGuid(), u.Property, u.Value); }; + session.PlayerIntPropertyUpdated += playerIntUpdated; + subscriptions.Add(() => session.PlayerIntPropertyUpdated -= playerIntUpdated); // Retail's qualities system owns one local-player value and notifies every // registered panel. acdream currently exposes that value through two projections: // ClientObjectTable (retained UI) and LocalPlayerState (Core consumers/fallback). // Apply the authoritative 0x02CF update to both in this one wiring owner so they // cannot drift after the login PlayerDescription snapshot. - session.PlayerInt64PropertyUpdated += u => ApplyPlayerInt64PropertyUpdate( + Action playerInt64Updated = u => + ApplyPlayerInt64PropertyUpdate( table, localPlayer, playerGuid?.Invoke() ?? 0u, u); + session.PlayerInt64PropertyUpdated += playerInt64Updated; + subscriptions.Add(() => session.PlayerInt64PropertyUpdated -= playerInt64Updated); // B-Wire: SetStackSize (0x0197) — update the object's stack count + value. - session.StackSizeUpdated += u => table.UpdateStackSize(u.Guid, u.StackSize, u.Value); + Action stackSizeUpdated = u => + table.UpdateStackSize(u.Guid, u.StackSize, u.Value); + session.StackSizeUpdated += stackSizeUpdated; + subscriptions.Add(() => session.StackSizeUpdated -= stackSizeUpdated); // B-Wire: InventoryRemoveObject (0x0024) — the object left the player's inventory // view; drop it from the table (retail ClientUISystem removes it from maintenance). - session.InventoryObjectRemoved += guid => table.Remove(guid); + Action inventoryObjectRemoved = guid => table.Remove(guid); + session.InventoryObjectRemoved += inventoryObjectRemoved; + subscriptions.Add(() => session.InventoryObjectRemoved -= inventoryObjectRemoved); + + return subscriptions; } /// diff --git a/src/AcDream.Core.Net/SubscriptionSet.cs b/src/AcDream.Core.Net/SubscriptionSet.cs new file mode 100644 index 00000000..5f271c02 --- /dev/null +++ b/src/AcDream.Core.Net/SubscriptionSet.cs @@ -0,0 +1,61 @@ +namespace AcDream.Core.Net; + +/// +/// Owns a transactionally-built set of subscriptions and releases them in +/// reverse registration order. Disposal is idempotent and attempts every +/// release even when one fails. +/// +internal sealed class SubscriptionSet : IDisposable +{ + private List? _subscriptions = []; + + public void Add(IDisposable subscription) + { + ArgumentNullException.ThrowIfNull(subscription); + List? subscriptions = _subscriptions; + if (subscriptions is null) + { + subscription.Dispose(); + throw new ObjectDisposedException(nameof(SubscriptionSet)); + } + + subscriptions.Add(subscription); + } + + public void Add(Action unsubscribe) + { + ArgumentNullException.ThrowIfNull(unsubscribe); + Add(new ActionSubscription(unsubscribe)); + } + + public void Dispose() + { + List? subscriptions = + Interlocked.Exchange(ref _subscriptions, null); + if (subscriptions is null) + return; + + List? errors = null; + for (int i = subscriptions.Count - 1; i >= 0; i--) + { + try + { + subscriptions[i].Dispose(); + } + catch (Exception error) + { + (errors ??= []).Add(error); + } + } + + if (errors is not null) + throw new AggregateException("one or more subscriptions failed to detach", errors); + } + + private sealed class ActionSubscription(Action unsubscribe) : IDisposable + { + private Action? _unsubscribe = unsubscribe; + + public void Dispose() => Interlocked.Exchange(ref _unsubscribe, null)?.Invoke(); + } +} diff --git a/tests/AcDream.Core.Net.Tests/Messages/GameEventDispatcherTests.cs b/tests/AcDream.Core.Net.Tests/Messages/GameEventDispatcherTests.cs index f44bb55d..05cc5289 100644 --- a/tests/AcDream.Core.Net.Tests/Messages/GameEventDispatcherTests.cs +++ b/tests/AcDream.Core.Net.Tests/Messages/GameEventDispatcherTests.cs @@ -15,9 +15,9 @@ public sealed class GameEventDispatcherTests { byte[] body = new byte[GameEventEnvelope.HeaderSize + payload.Length]; var span = body.AsSpan(); - BinaryPrimitives.WriteUInt32LittleEndian(span, GameEventEnvelope.Opcode); - BinaryPrimitives.WriteUInt32LittleEndian(span.Slice(4), playerGuid); - BinaryPrimitives.WriteUInt32LittleEndian(span.Slice(8), sequence); + BinaryPrimitives.WriteUInt32LittleEndian(span, GameEventEnvelope.Opcode); + BinaryPrimitives.WriteUInt32LittleEndian(span.Slice(4), playerGuid); + BinaryPrimitives.WriteUInt32LittleEndian(span.Slice(8), sequence); BinaryPrimitives.WriteUInt32LittleEndian(span.Slice(12), (uint)type); payload.CopyTo(span.Slice(GameEventEnvelope.HeaderSize)); return body; @@ -106,6 +106,162 @@ public sealed class GameEventDispatcherTests Assert.Equal(1, dispatcher.GetUnhandledCount(GameEventType.Tell)); } + [Fact] + public void OwnedRegistration_DisposeRestoresUnownedPredecessor() + { + var dispatcher = new GameEventDispatcher(); + var calls = new List(); + dispatcher.Register(GameEventType.Tell, _ => calls.Add("baseline")); + IDisposable owned = dispatcher.RegisterOwned( + GameEventType.Tell, + _ => calls.Add("owned")); + + dispatcher.Dispatch(new GameEventEnvelope(0, 0, GameEventType.Tell, default)); + owned.Dispose(); + dispatcher.Dispatch(new GameEventEnvelope(0, 0, GameEventType.Tell, default)); + + Assert.Equal(["owned", "baseline"], calls); + Assert.Equal(1, dispatcher.RegisteredHandlerCount); + } + + [Fact] + public void NestedOwnedRegistrations_DisposeOlderFirst_SkipsRetiredPredecessor() + { + var dispatcher = new GameEventDispatcher(); + var calls = new List(); + dispatcher.Register(GameEventType.Tell, _ => calls.Add("baseline")); + IDisposable ownerA = dispatcher.RegisterOwned( + GameEventType.Tell, + _ => calls.Add("A")); + IDisposable ownerB = dispatcher.RegisterOwned( + GameEventType.Tell, + _ => calls.Add("B")); + + ownerA.Dispose(); + dispatcher.Dispatch(new GameEventEnvelope(0, 0, GameEventType.Tell, default)); + ownerB.Dispose(); + dispatcher.Dispatch(new GameEventEnvelope(0, 0, GameEventType.Tell, default)); + + Assert.Equal(["B", "baseline"], calls); + } + + [Fact] + public void NestedOwnedRegistrations_DisposeNewestFirst_RestoresLiveOlderOwner() + { + var dispatcher = new GameEventDispatcher(); + var calls = new List(); + dispatcher.Register(GameEventType.Tell, _ => calls.Add("baseline")); + IDisposable ownerA = dispatcher.RegisterOwned( + GameEventType.Tell, + _ => calls.Add("A")); + IDisposable ownerB = dispatcher.RegisterOwned( + GameEventType.Tell, + _ => calls.Add("B")); + + ownerB.Dispose(); + dispatcher.Dispatch(new GameEventEnvelope(0, 0, GameEventType.Tell, default)); + ownerA.Dispose(); + ownerA.Dispose(); + ownerB.Dispose(); + dispatcher.Dispatch(new GameEventEnvelope(0, 0, GameEventType.Tell, default)); + + Assert.Equal(["A", "baseline"], calls); + } + + [Fact] + public void LaterUnownedReplacement_SurvivesOwnedTokenDisposal() + { + var dispatcher = new GameEventDispatcher(); + var calls = new List(); + IDisposable owned = dispatcher.RegisterOwned( + GameEventType.Tell, + _ => calls.Add("owned")); + dispatcher.Register(GameEventType.Tell, _ => calls.Add("replacement")); + + owned.Dispose(); + dispatcher.Dispatch(new GameEventEnvelope(0, 0, GameEventType.Tell, default)); + + Assert.Equal(["replacement"], calls); + } + + [Fact] + public void OwnedRegistrationWithoutPredecessor_BecomesUnhandledAfterDispose() + { + var dispatcher = new GameEventDispatcher(); + IDisposable owned = dispatcher.RegisterOwned(GameEventType.Tell, _ => { }); + + owned.Dispose(); + owned.Dispose(); + dispatcher.Dispatch(new GameEventEnvelope(0, 0, GameEventType.Tell, default)); + + Assert.Equal(0, dispatcher.RegisteredHandlerCount); + Assert.Equal(1, dispatcher.GetUnhandledCount(GameEventType.Tell)); + } + + [Fact] + public void ExplicitUnregister_RetiresCurrentChainWithoutResurrection() + { + var dispatcher = new GameEventDispatcher(); + dispatcher.Register(GameEventType.Tell, _ => { }); + IDisposable ownerA = dispatcher.RegisterOwned(GameEventType.Tell, _ => { }); + IDisposable ownerB = dispatcher.RegisterOwned(GameEventType.Tell, _ => { }); + + dispatcher.Unregister(GameEventType.Tell); + ownerA.Dispose(); + ownerB.Dispose(); + dispatcher.Dispatch(new GameEventEnvelope(0, 0, GameEventType.Tell, default)); + + Assert.Equal(0, dispatcher.RegisteredHandlerCount); + Assert.Equal(1, dispatcher.GetUnhandledCount(GameEventType.Tell)); + } + + [Fact] + public void HandlerCanDisposeItselfAndInstallRawReplacementDuringDispatch() + { + var dispatcher = new GameEventDispatcher(); + var calls = new List(); + IDisposable? owned = null; + owned = dispatcher.RegisterOwned(GameEventType.Tell, _ => + { + calls.Add("owned"); + owned!.Dispose(); + dispatcher.Register(GameEventType.Tell, _ => calls.Add("replacement")); + }); + + dispatcher.Dispatch(new GameEventEnvelope(0, 0, GameEventType.Tell, default)); + dispatcher.Dispatch(new GameEventEnvelope(0, 0, GameEventType.Tell, default)); + + Assert.Equal(["owned", "replacement"], calls); + Assert.Equal(1, dispatcher.RegisteredHandlerCount); + } + + [Fact] + public void RawReplacementAfterNestedOwners_SurvivesEveryOwnedDisposeOrder() + { + var dispatcher = new GameEventDispatcher(); + var calls = new List(); + IDisposable ownerA = dispatcher.RegisterOwned(GameEventType.Tell, _ => calls.Add("A")); + IDisposable ownerB = dispatcher.RegisterOwned(GameEventType.Tell, _ => calls.Add("B")); + dispatcher.Register(GameEventType.Tell, _ => calls.Add("raw")); + + ownerA.Dispose(); + ownerB.Dispose(); + dispatcher.Dispatch(new GameEventEnvelope(0, 0, GameEventType.Tell, default)); + + Assert.Equal(["raw"], calls); + } + + [Fact] + public void OwnedRegistration_ConcurrentDisposeRetiresExactlyOnce() + { + var dispatcher = new GameEventDispatcher(); + IDisposable owned = dispatcher.RegisterOwned(GameEventType.Tell, _ => { }); + + Parallel.For(0, 64, _ => owned.Dispose()); + + Assert.Equal(0, dispatcher.RegisteredHandlerCount); + } + // ── Per-event parser tests ─────────────────────────────────────────────── private static byte[] MakeString16L(string s) diff --git a/tests/AcDream.Core.Net.Tests/SubscriptionSetTests.cs b/tests/AcDream.Core.Net.Tests/SubscriptionSetTests.cs new file mode 100644 index 00000000..658983b1 --- /dev/null +++ b/tests/AcDream.Core.Net.Tests/SubscriptionSetTests.cs @@ -0,0 +1,47 @@ +namespace AcDream.Core.Net.Tests; + +public sealed class SubscriptionSetTests +{ + [Fact] + public void Dispose_ReleasesInReverseOrderAndIsIdempotent() + { + var order = new List(); + var subscriptions = new SubscriptionSet(); + subscriptions.Add(() => order.Add(1)); + subscriptions.Add(() => order.Add(2)); + subscriptions.Add(() => order.Add(3)); + + subscriptions.Dispose(); + subscriptions.Dispose(); + + Assert.Equal([3, 2, 1], order); + } + + [Fact] + public void Dispose_AttemptsEveryReleaseAndAggregatesFailures() + { + var order = new List(); + var subscriptions = new SubscriptionSet(); + subscriptions.Add(() => order.Add(1)); + subscriptions.Add(() => + { + order.Add(2); + throw new InvalidOperationException("second failed"); + }); + subscriptions.Add(() => + { + order.Add(3); + throw new IOException("third failed"); + }); + + AggregateException error = Assert.Throws( + subscriptions.Dispose); + + Assert.Equal([3, 2, 1], order); + Assert.Collection( + error.InnerExceptions, + exception => Assert.IsType(exception), + exception => Assert.IsType(exception)); + subscriptions.Dispose(); + } +} diff --git a/tests/AcDream.Core.Net.Tests/WorldSessionWiringOwnershipTests.cs b/tests/AcDream.Core.Net.Tests/WorldSessionWiringOwnershipTests.cs new file mode 100644 index 00000000..32cba289 --- /dev/null +++ b/tests/AcDream.Core.Net.Tests/WorldSessionWiringOwnershipTests.cs @@ -0,0 +1,131 @@ +using System.Net; +using System.Reflection; +using AcDream.Core.Chat; +using AcDream.Core.Combat; +using AcDream.Core.Items; +using AcDream.Core.Net.Messages; +using AcDream.Core.Player; +using AcDream.Core.Spells; + +namespace AcDream.Core.Net.Tests; + +public sealed class WorldSessionWiringOwnershipTests +{ + [Fact] + public void ObjectTableWiring_DisposeDetachesEveryExactSessionDelegate() + { + using var session = NewSession(); + var table = new ClientObjectTable(); + var player = new LocalPlayerState(); + + IDisposable registration = ObjectTableWiring.Wire( + session, + table, + playerGuid: () => 0x50000001u, + localPlayer: player); + + Assert.Equal(1, HandlerCount(session, nameof(session.ObjectIntPropertyUpdated))); + Assert.Equal(1, HandlerCount(session, nameof(session.PlayerIntPropertyUpdated))); + Assert.Equal(1, HandlerCount(session, nameof(session.PlayerInt64PropertyUpdated))); + Assert.Equal(1, HandlerCount(session, nameof(session.StackSizeUpdated))); + Assert.Equal(1, HandlerCount(session, nameof(session.InventoryObjectRemoved))); + + registration.Dispose(); + registration.Dispose(); + + Assert.Equal(0, HandlerCount(session, nameof(session.ObjectIntPropertyUpdated))); + Assert.Equal(0, HandlerCount(session, nameof(session.PlayerIntPropertyUpdated))); + Assert.Equal(0, HandlerCount(session, nameof(session.PlayerInt64PropertyUpdated))); + Assert.Equal(0, HandlerCount(session, nameof(session.StackSizeUpdated))); + Assert.Equal(0, HandlerCount(session, nameof(session.InventoryObjectRemoved))); + } + + [Fact] + public void CombatStateWiring_DisposeDetachesOnlyItsExactDelegate() + { + using var session = NewSession(); + var combat = new CombatState(); + Action predecessor = _ => { }; + session.PlayerIntPropertyUpdated += predecessor; + IDisposable registration = CombatStateWiring.Wire(session, combat); + + Assert.Equal(2, HandlerCount(session, nameof(session.PlayerIntPropertyUpdated))); + + registration.Dispose(); + registration.Dispose(); + + Assert.Equal(1, HandlerCount(session, nameof(session.PlayerIntPropertyUpdated))); + session.PlayerIntPropertyUpdated -= predecessor; + } + + [Fact] + public void CombatStateWiring_ConcurrentDisposeDetachesExactlyOnce() + { + using var session = NewSession(); + IDisposable registration = CombatStateWiring.Wire(session, new CombatState()); + + Parallel.For(0, 64, _ => registration.Dispose()); + + Assert.Equal(0, HandlerCount(session, nameof(session.PlayerIntPropertyUpdated))); + } + + [Fact] + public void GameEventWiring_DisposeRestoresPreexistingHandlerAndRemovesOwnedSet() + { + var dispatcher = new GameEventDispatcher(); + int predecessorCalls = 0; + dispatcher.Register(GameEventType.Tell, _ => predecessorCalls++); + + IDisposable registration = GameEventWiring.WireAll( + dispatcher, + new ClientObjectTable(), + new CombatState(), + new Spellbook(), + new ChatLog()); + int wiredCount = dispatcher.RegisteredHandlerCount; + dispatcher.Dispatch(new GameEventEnvelope(0, 0, GameEventType.Tell, default)); + Assert.Equal(0, predecessorCalls); + + registration.Dispose(); + registration.Dispose(); + dispatcher.Dispatch(new GameEventEnvelope(0, 0, GameEventType.Tell, default)); + + Assert.True(wiredCount > 1); + Assert.Equal(1, dispatcher.RegisteredHandlerCount); + Assert.Equal(1, predecessorCalls); + } + + [Fact] + public void GameEventWiring_OnWorldSession_RestoresInternalRegistrations() + { + using var session = NewSession(); + int baselineCount = session.GameEvents.RegisteredHandlerCount; + + IDisposable registration = GameEventWiring.WireAll( + session.GameEvents, + new ClientObjectTable(), + new CombatState(), + new Spellbook(), + new ChatLog(), + turbineChat: new TurbineChatState()); + + Assert.True(session.GameEvents.RegisteredHandlerCount > baselineCount); + registration.Dispose(); + + Assert.Equal(baselineCount, session.GameEvents.RegisteredHandlerCount); + } + + private static WorldSession NewSession() => + new(new IPEndPoint(IPAddress.Loopback, 9)); + + private static int HandlerCount(WorldSession session, string eventName) + { + FieldInfo field = typeof(WorldSession).GetField( + eventName, + BindingFlags.Instance | BindingFlags.NonPublic) + ?? throw new InvalidOperationException($"event field {eventName} not found"); + return (field.GetValue(session) as MulticastDelegate)? + .GetInvocationList() + .Length ?? 0; + } +}