From 96c0a4ae1fd4cc9d9beda61d8d488fc75786f3a0 Mon Sep 17 00:00:00 2001 From: Pieter-Jan Briers Date: Mon, 17 May 2021 11:01:22 +0200 Subject: [PATCH] Automatically unsubscribe event bus registrations in entity system shutdown. (#1758) * Automatically unsubscribe event bus registrations in entity system shutdown. * Fix incorrect unsubscription of local events, obsolete unsubscribe methods. That's what we got tests for. * = null instead of .Clear() --- .../GameObjects/EntitySystems/AudioSystem.cs | 11 -- .../EntitySystems/ClientOccluderSystem.cs | 9 -- .../EntitySystems/RenderingTreeSystem.cs | 8 +- .../EntitySystems/UserInterfaceSystem.cs | 8 -- .../Physics/DebugPhysicsIslandSystem.cs | 2 +- .../GameObjects/EntitySystems/InputSystem.cs | 2 + .../TileLookup/GridTileLookupSystem.cs | 4 +- .../EntitySystems/UserInterfaceSystem.cs | 7 - .../GameObjects/EntitySystem.Subscriptions.cs | 128 ++++++++++++++++++ Robust.Shared/GameObjects/EntitySystem.cs | 65 +-------- .../Systems/CollisionWakeSystem.cs | 8 -- .../GameObjects/Systems/OccluderSystem.cs | 1 + .../Systems/SharedGridTraversalSystem.cs | 6 - .../Systems/SharedPhysicsSystem.cs | 8 -- .../GameObjects/Systems/SnapGridSystem.cs | 10 -- .../BroadPhase/SharedBroadPhaseSystem.cs | 8 +- 16 files changed, 140 insertions(+), 145 deletions(-) create mode 100644 Robust.Shared/GameObjects/EntitySystem.Subscriptions.cs diff --git a/Robust.Client/GameObjects/EntitySystems/AudioSystem.cs b/Robust.Client/GameObjects/EntitySystems/AudioSystem.cs index dd874345c0..9e4440ea83 100644 --- a/Robust.Client/GameObjects/EntitySystems/AudioSystem.cs +++ b/Robust.Client/GameObjects/EntitySystems/AudioSystem.cs @@ -41,17 +41,6 @@ namespace Robust.Client.GameObjects _broadPhaseSystem = Get(); } - public override void Shutdown() - { - base.Shutdown(); - UnsubscribeNetworkEvent(); - UnsubscribeNetworkEvent(); - UnsubscribeNetworkEvent(); - UnsubscribeNetworkEvent(); - - UnsubscribeLocalEvent(); - } - private void StopAudioMessageHandler(StopAudioMessageClient ev) { var stream = _playingClydeStreams.Find(p => p.NetIdentifier == ev.Identifier); diff --git a/Robust.Client/GameObjects/EntitySystems/ClientOccluderSystem.cs b/Robust.Client/GameObjects/EntitySystems/ClientOccluderSystem.cs index 38487f48c6..5cdd5b332c 100644 --- a/Robust.Client/GameObjects/EntitySystems/ClientOccluderSystem.cs +++ b/Robust.Client/GameObjects/EntitySystems/ClientOccluderSystem.cs @@ -38,15 +38,6 @@ namespace Robust.Client.GameObjects SubscribeLocalEvent(HandleSnapGridMove); } - /// - public override void Shutdown() - { - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - - base.Shutdown(); - } - public override void FrameUpdate(float frameTime) { base.FrameUpdate(frameTime); diff --git a/Robust.Client/GameObjects/EntitySystems/RenderingTreeSystem.cs b/Robust.Client/GameObjects/EntitySystems/RenderingTreeSystem.cs index 6580a95b9f..e12f69c642 100644 --- a/Robust.Client/GameObjects/EntitySystems/RenderingTreeSystem.cs +++ b/Robust.Client/GameObjects/EntitySystems/RenderingTreeSystem.cs @@ -58,17 +58,11 @@ namespace Robust.Client.GameObjects public override void Shutdown() { base.Shutdown(); + _mapManager.MapCreated -= MapManagerOnMapCreated; _mapManager.MapDestroyed -= MapManagerOnMapDestroyed; _mapManager.OnGridCreated -= MapManagerOnGridCreated; _mapManager.OnGridRemoved -= MapManagerOnGridRemoved; - - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); } // For these next 2 methods (the Remove* ones): diff --git a/Robust.Client/GameObjects/EntitySystems/UserInterfaceSystem.cs b/Robust.Client/GameObjects/EntitySystems/UserInterfaceSystem.cs index 4af7b113ef..ba4f8cbbbc 100644 --- a/Robust.Client/GameObjects/EntitySystems/UserInterfaceSystem.cs +++ b/Robust.Client/GameObjects/EntitySystems/UserInterfaceSystem.cs @@ -22,14 +22,6 @@ namespace Robust.Client.GameObjects } } - public override void Shutdown() - { - base.Shutdown(); - - UnsubscribeNetworkEvent(); - UnsubscribeLocalEvent(); - } - private void MessageReceived(BoundUIWrapMessage ev) { var cmp = ComponentManager.GetComponent(ev.Entity); diff --git a/Robust.Client/Physics/DebugPhysicsIslandSystem.cs b/Robust.Client/Physics/DebugPhysicsIslandSystem.cs index 200bd951bc..5185cd6854 100644 --- a/Robust.Client/Physics/DebugPhysicsIslandSystem.cs +++ b/Robust.Client/Physics/DebugPhysicsIslandSystem.cs @@ -52,7 +52,7 @@ namespace Robust.Client.Physics public override void Shutdown() { base.Shutdown(); - UnsubscribeLocalEvent(); + IoCManager.Resolve().RemoveOverlay(typeof(PhysicsIslandOverlay)); } diff --git a/Robust.Server/GameObjects/EntitySystems/InputSystem.cs b/Robust.Server/GameObjects/EntitySystems/InputSystem.cs index 8356e3ea18..8a38f0535e 100644 --- a/Robust.Server/GameObjects/EntitySystems/InputSystem.cs +++ b/Robust.Server/GameObjects/EntitySystems/InputSystem.cs @@ -30,6 +30,8 @@ namespace Robust.Server.GameObjects /// public override void Shutdown() { + base.Shutdown(); + _playerManager.PlayerStatusChanged -= OnPlayerStatusChanged; } diff --git a/Robust.Server/GameObjects/EntitySystems/TileLookup/GridTileLookupSystem.cs b/Robust.Server/GameObjects/EntitySystems/TileLookup/GridTileLookupSystem.cs index 284bfd69f3..3d0b392063 100644 --- a/Robust.Server/GameObjects/EntitySystems/TileLookup/GridTileLookupSystem.cs +++ b/Robust.Server/GameObjects/EntitySystems/TileLookup/GridTileLookupSystem.cs @@ -219,9 +219,7 @@ namespace Robust.Server.GameObjects public override void Shutdown() { base.Shutdown(); - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); + _mapManager.OnGridCreated -= HandleGridCreated; _mapManager.OnGridRemoved -= HandleGridRemoval; _mapManager.TileChanged -= HandleTileChanged; diff --git a/Robust.Server/GameObjects/EntitySystems/UserInterfaceSystem.cs b/Robust.Server/GameObjects/EntitySystems/UserInterfaceSystem.cs index 7f1b69ed0a..73c5d7b2c9 100644 --- a/Robust.Server/GameObjects/EntitySystems/UserInterfaceSystem.cs +++ b/Robust.Server/GameObjects/EntitySystems/UserInterfaceSystem.cs @@ -36,13 +36,6 @@ namespace Robust.Server.GameObjects } } - public override void Shutdown() - { - base.Shutdown(); - - UnsubscribeNetworkEvent(); - } - internal void SendTo(IPlayerSession session, BoundUIWrapMessage msg) { RaiseNetworkEvent(msg, session.ConnectedClient); diff --git a/Robust.Shared/GameObjects/EntitySystem.Subscriptions.cs b/Robust.Shared/GameObjects/EntitySystem.Subscriptions.cs new file mode 100644 index 0000000000..c947bc5fb8 --- /dev/null +++ b/Robust.Shared/GameObjects/EntitySystem.Subscriptions.cs @@ -0,0 +1,128 @@ +using System; +using System.Collections.Generic; + +namespace Robust.Shared.GameObjects +{ + public abstract partial class EntitySystem + { + private List? _subscriptions; + + protected void SubscribeNetworkEvent(EntityEventHandler handler) + where T : notnull + { + EntityManager.EventBus.SubscribeEvent(EventSource.Network, this, handler); + + _subscriptions ??= new(); + _subscriptions.Add(new SubBroadcast(EventSource.Network)); + } + + protected void SubscribeNetworkEvent(EntitySessionEventHandler handler) + where T : notnull + { + EntityManager.EventBus.SubscribeSessionEvent(EventSource.Network, this, handler); + + _subscriptions ??= new(); + _subscriptions.Add(new SubBroadcast>(EventSource.Network)); + } + + protected void SubscribeLocalEvent(EntityEventHandler handler) + where T : notnull + { + EntityManager.EventBus.SubscribeEvent(EventSource.Local, this, handler); + + _subscriptions ??= new(); + _subscriptions.Add(new SubBroadcast(EventSource.Local)); + } + + protected void SubscribeLocalEvent(EntitySessionEventHandler handler) + where T : notnull + { + EntityManager.EventBus.SubscribeSessionEvent(EventSource.Local, this, handler); + + _subscriptions ??= new(); + _subscriptions.Add(new SubBroadcast>(EventSource.Local)); + } + + [Obsolete("Unsubscribing of entity system events is now automatic")] + protected void UnsubscribeNetworkEvent() + where T : notnull + { + EntityManager.EventBus.UnsubscribeEvent(EventSource.Network, this); + } + + [Obsolete("Unsubscribing of entity system events is now automatic")] + protected void UnsubscribeLocalEvent() + where T : notnull + { + EntityManager.EventBus.UnsubscribeEvent(EventSource.Local, this); + } + + + protected void SubscribeLocalEvent(ComponentEventHandler handler) + where TComp : IComponent + where TEvent : EntityEventArgs + { + EntityManager.EventBus.SubscribeLocalEvent(handler); + + _subscriptions ??= new(); + _subscriptions.Add(new SubLocal()); + } + + [Obsolete("Unsubscribing of entity system events is now automatic")] + protected void UnsubscribeLocalEvent(ComponentEventHandler handler) + where TComp : IComponent + where TEvent : EntityEventArgs + { + EntityManager.EventBus.UnsubscribeLocalEvent(); + } + + [Obsolete("Unsubscribing of entity system events is now automatic")] + protected void UnsubscribeLocalEvent() + where TComp : IComponent + where TEvent : EntityEventArgs + { + EntityManager.EventBus.UnsubscribeLocalEvent(); + } + + private void ShutdownSubscriptions() + { + if (_subscriptions == null) + return; + + foreach (var sub in _subscriptions) + { + sub.Unsubscribe(this, EntityManager.EventBus); + } + + _subscriptions = null; + } + + private abstract class SubBase + { + public abstract void Unsubscribe(EntitySystem sys, IEventBus bus); + } + + private sealed class SubBroadcast : SubBase where T : notnull + { + private readonly EventSource _source; + + public SubBroadcast(EventSource source) + { + _source = source; + } + + public override void Unsubscribe(EntitySystem sys, IEventBus bus) + { + bus.UnsubscribeEvent(_source, sys); + } + } + + private sealed class SubLocal : SubBase where TComp : IComponent where TBase : EntityEventArgs + { + public override void Unsubscribe(EntitySystem sys, IEventBus bus) + { + bus.UnsubscribeLocalEvent(); + } + } + } +} diff --git a/Robust.Shared/GameObjects/EntitySystem.cs b/Robust.Shared/GameObjects/EntitySystem.cs index 4aaaded795..79961aa9d4 100644 --- a/Robust.Shared/GameObjects/EntitySystem.cs +++ b/Robust.Shared/GameObjects/EntitySystem.cs @@ -18,7 +18,7 @@ namespace Robust.Shared.GameObjects /// This class is instantiated by the EntitySystemManager, and any IoC Dependencies will be resolved. /// [Reflect(false), PublicAPI] - public abstract class EntitySystem : IEntitySystem + public abstract partial class EntitySystem : IEntitySystem { [Dependency] protected readonly IEntityManager EntityManager = default!; [Dependency] protected readonly IComponentManager ComponentManager = default!; @@ -40,47 +40,14 @@ namespace Robust.Shared.GameObjects public virtual void FrameUpdate(float frameTime) { } /// - public virtual void Shutdown() { } + public virtual void Shutdown() + { + ShutdownSubscriptions(); + } #region Event Proxy - protected void SubscribeNetworkEvent(EntityEventHandler handler) - where T : notnull - { - EntityManager.EventBus.SubscribeEvent(EventSource.Network, this, handler); - } - - protected void SubscribeNetworkEvent(EntitySessionEventHandler handler) - where T : notnull - { - EntityManager.EventBus.SubscribeSessionEvent(EventSource.Network, this, handler); - } - - protected void SubscribeLocalEvent(EntityEventHandler handler) - where T : notnull - { - EntityManager.EventBus.SubscribeEvent(EventSource.Local, this, handler); - } - - protected void SubscribeLocalEvent(EntitySessionEventHandler handler) - where T : notnull - { - EntityManager.EventBus.SubscribeSessionEvent(EventSource.Local, this, handler); - } - - protected void UnsubscribeNetworkEvent() - where T : notnull - { - EntityManager.EventBus.UnsubscribeEvent(EventSource.Network, this); - } - - protected void UnsubscribeLocalEvent() - where T : notnull - { - EntityManager.EventBus.UnsubscribeEvent(EventSource.Local, this); - } - protected void RaiseLocalEvent(T message) where T : notnull { EntityManager.EventBus.RaiseEvent(EventSource.Local, message); @@ -120,28 +87,6 @@ namespace Robust.Shared.GameObjects return EntityManager.EventBus.AwaitEvent(EventSource.Network, cancellationToken); } - protected void SubscribeLocalEvent(ComponentEventHandler handler) - where TComp : IComponent - where TEvent : EntityEventArgs - { - EntityManager.EventBus.SubscribeLocalEvent(handler); - } - - [Obsolete("Use the overload without the handler argument.")] - protected void UnsubscribeLocalEvent(ComponentEventHandler handler) - where TComp : IComponent - where TEvent : EntityEventArgs - { - EntityManager.EventBus.UnsubscribeLocalEvent(); - } - - protected void UnsubscribeLocalEvent() - where TComp : IComponent - where TEvent : EntityEventArgs - { - EntityManager.EventBus.UnsubscribeLocalEvent(); - } - protected void RaiseLocalEvent(EntityUid uid, TEvent args, bool broadcast = true) where TEvent : EntityEventArgs { diff --git a/Robust.Shared/GameObjects/Systems/CollisionWakeSystem.cs b/Robust.Shared/GameObjects/Systems/CollisionWakeSystem.cs index 3d6da70685..dcfd63b548 100644 --- a/Robust.Shared/GameObjects/Systems/CollisionWakeSystem.cs +++ b/Robust.Shared/GameObjects/Systems/CollisionWakeSystem.cs @@ -12,14 +12,6 @@ namespace Robust.Shared.GameObjects SubscribeLocalEvent(HandleCollisionWakeState); } - public override void Shutdown() - { - base.Shutdown(); - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - } - private void HandleWake(PhysicsWakeMessage message) { if (!message.Body.Owner.TryGetComponent(out var comp) || !comp.Enabled) return; diff --git a/Robust.Shared/GameObjects/Systems/OccluderSystem.cs b/Robust.Shared/GameObjects/Systems/OccluderSystem.cs index dca7e5d115..be59629908 100644 --- a/Robust.Shared/GameObjects/Systems/OccluderSystem.cs +++ b/Robust.Shared/GameObjects/Systems/OccluderSystem.cs @@ -53,6 +53,7 @@ namespace Robust.Shared.GameObjects public override void Shutdown() { base.Shutdown(); + _mapManager.MapCreated -= OnMapCreated; _mapManager.MapDestroyed -= OnMapDestroyed; _mapManager.OnGridCreated -= OnGridCreated; diff --git a/Robust.Shared/GameObjects/Systems/SharedGridTraversalSystem.cs b/Robust.Shared/GameObjects/Systems/SharedGridTraversalSystem.cs index d5e55ca787..a49f4437b9 100644 --- a/Robust.Shared/GameObjects/Systems/SharedGridTraversalSystem.cs +++ b/Robust.Shared/GameObjects/Systems/SharedGridTraversalSystem.cs @@ -20,12 +20,6 @@ namespace Robust.Shared.GameObjects SubscribeLocalEvent(QueueMoveEvent); } - public override void Shutdown() - { - base.Shutdown(); - UnsubscribeLocalEvent(); - } - public override void Update(float frameTime) { base.Update(frameTime); diff --git a/Robust.Shared/GameObjects/Systems/SharedPhysicsSystem.cs b/Robust.Shared/GameObjects/Systems/SharedPhysicsSystem.cs index 7e91e29cd7..bc7d68e759 100644 --- a/Robust.Shared/GameObjects/Systems/SharedPhysicsSystem.cs +++ b/Robust.Shared/GameObjects/Systems/SharedPhysicsSystem.cs @@ -168,14 +168,6 @@ namespace Robust.Shared.GameObjects _mapManager.MapCreated -= HandleMapCreated; _mapManager.MapDestroyed -= HandleMapDestroyed; - - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); } private void HandleMapCreated(object? sender, MapEventArgs eventArgs) diff --git a/Robust.Shared/GameObjects/Systems/SnapGridSystem.cs b/Robust.Shared/GameObjects/Systems/SnapGridSystem.cs index 1d24d938e7..ddc92a13bb 100644 --- a/Robust.Shared/GameObjects/Systems/SnapGridSystem.cs +++ b/Robust.Shared/GameObjects/Systems/SnapGridSystem.cs @@ -21,16 +21,6 @@ namespace Robust.Shared.GameObjects SubscribeLocalEvent(HandleMoveEvent); } - /// - public override void Shutdown() - { - base.Shutdown(); - - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - } - private void HandleComponentStartup(EntityUid uid, SnapGridComponent component, ComponentStartup args) { var transform = ComponentManager.GetComponent(uid); diff --git a/Robust.Shared/Physics/BroadPhase/SharedBroadPhaseSystem.cs b/Robust.Shared/Physics/BroadPhase/SharedBroadPhaseSystem.cs index 8e15289ea5..9a97030344 100644 --- a/Robust.Shared/Physics/BroadPhase/SharedBroadPhaseSystem.cs +++ b/Robust.Shared/Physics/BroadPhase/SharedBroadPhaseSystem.cs @@ -218,13 +218,7 @@ namespace Robust.Shared.Physics.Broadphase public override void Shutdown() { base.Shutdown(); - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); - UnsubscribeLocalEvent(); + _mapManager.OnGridCreated -= HandleGridCreated; _mapManager.OnGridRemoved -= HandleGridRemoval; _mapManager.MapCreated -= HandleMapCreated;