From 8baa6072fb968f20ae090dedcc0cd0d8e0b34f50 Mon Sep 17 00:00:00 2001 From: Philipp Deschain Date: Wed, 13 Apr 2022 13:21:57 -0400 Subject: [PATCH 1/7] ISubscriber now contains Unsubscribe method --- .../Scripts/Shared/Infrastructure/PubSub/IMessageChannel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Assets/BossRoom/Scripts/Shared/Infrastructure/PubSub/IMessageChannel.cs b/Assets/BossRoom/Scripts/Shared/Infrastructure/PubSub/IMessageChannel.cs index a94a90e6e..3e748af1d 100644 --- a/Assets/BossRoom/Scripts/Shared/Infrastructure/PubSub/IMessageChannel.cs +++ b/Assets/BossRoom/Scripts/Shared/Infrastructure/PubSub/IMessageChannel.cs @@ -10,12 +10,12 @@ public interface IPublisher public interface ISubscriber { IDisposable Subscribe(Action handler); + void Unsubscribe(Action handler); } public interface IMessageChannel : IPublisher, ISubscriber, IDisposable { bool IsDisposed { get; } - void Unsubscribe(Action handler); } public interface IBufferedMessageChannel : IMessageChannel From 43eda3fbb2fef143e15da7f46dd40a5842a92015 Mon Sep 17 00:00:00 2001 From: Philipp Deschain Date: Thu, 11 Aug 2022 18:07:03 -0400 Subject: [PATCH 2/7] Used the new API to unsubscribe directly from an ISubscriber - better readability in trivial cases --- Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs | 8 +++----- Assets/Scripts/Gameplay/UI/IPConnectionWindow.cs | 7 ++++--- Assets/Scripts/Gameplay/UI/IPUIMediator.cs | 7 ++++--- Assets/Scripts/Gameplay/UI/Lobby/LobbyJoiningUI.cs | 8 +++++--- Assets/Scripts/Gameplay/UI/Lobby/LobbyUIMediator.cs | 8 ++++---- Assets/Scripts/Gameplay/UI/UnityServicesUIHandler.cs | 7 ++++--- 6 files changed, 24 insertions(+), 21 deletions(-) diff --git a/Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs b/Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs index 3452b4647..98246f919 100644 --- a/Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs +++ b/Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs @@ -51,8 +51,6 @@ public class ServerBossRoomState : GameStateBehaviour /// [Inject] ISubscriber m_LifeStateChangedEventMessageSubscriber; - IDisposable m_Subscription; - [Inject] ConnectionManager m_ConnectionManager; protected override void Awake() @@ -71,7 +69,7 @@ void OnNetworkSpawn() return; } - m_Subscription = m_LifeStateChangedEventMessageSubscriber.Subscribe(OnLifeStateChangedEventMessage); + m_LifeStateChangedEventMessageSubscriber.Subscribe(OnLifeStateChangedEventMessage); NetworkManager.Singleton.SceneManager.OnLoadComplete += OnServerLoadComplete; NetworkManager.Singleton.SceneManager.OnUnloadComplete += OnServerUnloadComplete; @@ -79,7 +77,7 @@ void OnNetworkSpawn() void OnNetworkDespawn() { - m_Subscription?.Dispose(); + m_LifeStateChangedEventMessageSubscriber.Unsubscribe(OnLifeStateChangedEventMessage); NetworkManager.Singleton.SceneManager.OnLoadComplete -= OnServerLoadComplete; NetworkManager.Singleton.SceneManager.OnUnloadComplete -= OnServerUnloadComplete; @@ -116,7 +114,7 @@ void OnServerUnloadComplete(ulong clientId, string sceneName) protected override void OnDestroy() { - m_Subscription?.Dispose(); + m_LifeStateChangedEventMessageSubscriber?.Unsubscribe(OnLifeStateChangedEventMessage); if (m_NetcodeHooks) { diff --git a/Assets/Scripts/Gameplay/UI/IPConnectionWindow.cs b/Assets/Scripts/Gameplay/UI/IPConnectionWindow.cs index 90c75151a..550c82079 100644 --- a/Assets/Scripts/Gameplay/UI/IPConnectionWindow.cs +++ b/Assets/Scripts/Gameplay/UI/IPConnectionWindow.cs @@ -19,12 +19,13 @@ public class IPConnectionWindow : MonoBehaviour [Inject] IPUIMediator m_IPUIMediator; - IDisposable m_Subscription; + ISubscriber m_ConnectStatusSubscriber; [Inject] void InjectDependencies(ISubscriber connectStatusSubscriber) { - m_Subscription = connectStatusSubscriber.Subscribe(OnConnectStatusMessage); + m_ConnectStatusSubscriber = connectStatusSubscriber; + m_ConnectStatusSubscriber.Subscribe(OnConnectStatusMessage); } void Awake() @@ -34,7 +35,7 @@ void Awake() void OnDestroy() { - m_Subscription.Dispose(); + m_ConnectStatusSubscriber.Unsubscribe(OnConnectStatusMessage); } void OnConnectStatusMessage(ConnectStatus connectStatus) diff --git a/Assets/Scripts/Gameplay/UI/IPUIMediator.cs b/Assets/Scripts/Gameplay/UI/IPUIMediator.cs index 2d8957482..04ef436a9 100644 --- a/Assets/Scripts/Gameplay/UI/IPUIMediator.cs +++ b/Assets/Scripts/Gameplay/UI/IPUIMediator.cs @@ -39,12 +39,13 @@ public class IPUIMediator : MonoBehaviour public IPHostingUI IPHostingUI => m_IPHostingUI; - IDisposable m_Subscription; + ISubscriber m_ConnectStatusSubscriber; [Inject] void InjectDependencies(ISubscriber connectStatusSubscriber) { - m_Subscription = connectStatusSubscriber.Subscribe(OnConnectStatusMessage); + m_ConnectStatusSubscriber = connectStatusSubscriber; + m_ConnectStatusSubscriber.Subscribe(OnConnectStatusMessage); } void Awake() @@ -61,7 +62,7 @@ void Start() void OnDestroy() { - m_Subscription.Dispose(); + m_ConnectStatusSubscriber.Unsubscribe(OnConnectStatusMessage); } void OnConnectStatusMessage(ConnectStatus connectStatus) diff --git a/Assets/Scripts/Gameplay/UI/Lobby/LobbyJoiningUI.cs b/Assets/Scripts/Gameplay/UI/Lobby/LobbyJoiningUI.cs index f75bbcb3c..9d28f3b03 100644 --- a/Assets/Scripts/Gameplay/UI/Lobby/LobbyJoiningUI.cs +++ b/Assets/Scripts/Gameplay/UI/Lobby/LobbyJoiningUI.cs @@ -22,7 +22,8 @@ public class LobbyJoiningUI : MonoBehaviour IObjectResolver m_Container; LobbyUIMediator m_LobbyUIMediator; UpdateRunner m_UpdateRunner; - IDisposable m_Subscriptions; + ISubscriber m_LocalLobbiesRefreshedSub; + List m_LobbyListItems = new List(); void Awake() @@ -40,7 +41,7 @@ void OnDisable() void OnDestroy() { - m_Subscriptions?.Dispose(); + m_LocalLobbiesRefreshedSub.Unsubscribe(UpdateUI); } [Inject] @@ -53,7 +54,8 @@ void InjectDependenciesAndInitialize( m_Container = container; m_LobbyUIMediator = lobbyUIMediator; m_UpdateRunner = updateRunner; - m_Subscriptions = localLobbiesRefreshedSub.Subscribe(UpdateUI); + m_LocalLobbiesRefreshedSub = localLobbiesRefreshedSub; + m_LocalLobbiesRefreshedSub.Subscribe(UpdateUI); } /// diff --git a/Assets/Scripts/Gameplay/UI/Lobby/LobbyUIMediator.cs b/Assets/Scripts/Gameplay/UI/Lobby/LobbyUIMediator.cs index 4f5fef1e5..057229b3f 100644 --- a/Assets/Scripts/Gameplay/UI/Lobby/LobbyUIMediator.cs +++ b/Assets/Scripts/Gameplay/UI/Lobby/LobbyUIMediator.cs @@ -28,7 +28,7 @@ public class LobbyUIMediator : MonoBehaviour LocalLobby m_LocalLobby; NameGenerationData m_NameGenerationData; ConnectionManager m_ConnectionManager; - IDisposable m_Subscriptions; + ISubscriber m_ConnectStatusSubscriber; const string k_DefaultLobbyName = "no-name"; @@ -49,10 +49,10 @@ ConnectionManager connectionManager m_LobbyServiceFacade = lobbyServiceFacade; m_LocalLobby = localLobby; m_ConnectionManager = connectionManager; - + m_ConnectStatusSubscriber = connectStatusSub; RegenerateName(); - m_Subscriptions = connectStatusSub.Subscribe(OnConnectStatus); + m_ConnectStatusSubscriber.Subscribe(OnConnectStatus); } void OnConnectStatus(ConnectStatus status) @@ -65,7 +65,7 @@ void OnConnectStatus(ConnectStatus status) void OnDestroy() { - m_Subscriptions?.Dispose(); + m_ConnectStatusSubscriber.Unsubscribe(OnConnectStatus); } //Lobby and Relay calls done from UI diff --git a/Assets/Scripts/Gameplay/UI/UnityServicesUIHandler.cs b/Assets/Scripts/Gameplay/UI/UnityServicesUIHandler.cs index b6cfdb73c..143d6dea8 100644 --- a/Assets/Scripts/Gameplay/UI/UnityServicesUIHandler.cs +++ b/Assets/Scripts/Gameplay/UI/UnityServicesUIHandler.cs @@ -10,7 +10,7 @@ namespace Unity.Multiplayer.Samples.BossRoom.Visual { public class UnityServicesUIHandler : MonoBehaviour { - IDisposable m_Subscriptions; + ISubscriber m_ServiceErrorSubscription; void Awake() { @@ -20,7 +20,8 @@ void Awake() [Inject] void Initialize(ISubscriber serviceError) { - m_Subscriptions = serviceError.Subscribe(ServiceErrorHandler); + m_ServiceErrorSubscription = serviceError; + m_ServiceErrorSubscription.Subscribe(ServiceErrorHandler); } void ServiceErrorHandler(UnityServiceErrorMessage error) @@ -86,7 +87,7 @@ void HandleLobbyError(UnityServiceErrorMessage error) void OnDestroy() { - m_Subscriptions?.Dispose(); + m_ServiceErrorSubscription.Unsubscribe(ServiceErrorHandler); } } } From 13f708d94169da6bce3978a4ad98926974d037d3 Mon Sep 17 00:00:00 2001 From: Philipp Deschain Date: Thu, 11 Aug 2022 18:09:22 -0400 Subject: [PATCH 3/7] added changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3f0fb0d1..1eacd541e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ## [Unreleased] - yyyy-mm-dd ### Added -* +* Unsubscribe API for the ISubscriber along with refactoring of the codebase to use this API instead of IDisposable handle when there is just one subscription. ### Changed * Updated tools, authentication and relay packages (#690) * Replaced our dependency injection solution with VContainer. (#679) From 047dac0aa01231b16f755e3608f70f59ca6ec01f Mon Sep 17 00:00:00 2001 From: Philipp Deschain Date: Thu, 29 Sep 2022 13:01:17 -0400 Subject: [PATCH 4/7] Update Assets/Scripts/Gameplay/UI/IPConnectionWindow.cs Co-authored-by: Fernando Cortez --- Assets/Scripts/Gameplay/UI/IPConnectionWindow.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Assets/Scripts/Gameplay/UI/IPConnectionWindow.cs b/Assets/Scripts/Gameplay/UI/IPConnectionWindow.cs index 550c82079..e73da01a1 100644 --- a/Assets/Scripts/Gameplay/UI/IPConnectionWindow.cs +++ b/Assets/Scripts/Gameplay/UI/IPConnectionWindow.cs @@ -35,7 +35,7 @@ void Awake() void OnDestroy() { - m_ConnectStatusSubscriber.Unsubscribe(OnConnectStatusMessage); + m_ConnectStatusSubscriber?.Unsubscribe(OnConnectStatusMessage); } void OnConnectStatusMessage(ConnectStatus connectStatus) From 5844c0cb0f0b29ca3c365c4c08a1619afdeadc1a Mon Sep 17 00:00:00 2001 From: Philipp Deschain Date: Thu, 29 Sep 2022 13:03:58 -0400 Subject: [PATCH 5/7] Addressing comments --- Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs | 2 +- Assets/Scripts/Gameplay/UI/IPUIMediator.cs | 2 +- Assets/Scripts/Gameplay/UI/Lobby/LobbyJoiningUI.cs | 4 ++-- Assets/Scripts/Gameplay/UI/Lobby/LobbyUIMediator.cs | 2 +- Assets/Scripts/Gameplay/UI/UnityServicesUIHandler.cs | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs b/Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs index 98246f919..4e988651e 100644 --- a/Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs +++ b/Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs @@ -77,7 +77,7 @@ void OnNetworkSpawn() void OnNetworkDespawn() { - m_LifeStateChangedEventMessageSubscriber.Unsubscribe(OnLifeStateChangedEventMessage); + m_LifeStateChangedEventMessageSubscriber?.Unsubscribe(OnLifeStateChangedEventMessage); NetworkManager.Singleton.SceneManager.OnLoadComplete -= OnServerLoadComplete; NetworkManager.Singleton.SceneManager.OnUnloadComplete -= OnServerUnloadComplete; diff --git a/Assets/Scripts/Gameplay/UI/IPUIMediator.cs b/Assets/Scripts/Gameplay/UI/IPUIMediator.cs index 04ef436a9..f1d1844c7 100644 --- a/Assets/Scripts/Gameplay/UI/IPUIMediator.cs +++ b/Assets/Scripts/Gameplay/UI/IPUIMediator.cs @@ -62,7 +62,7 @@ void Start() void OnDestroy() { - m_ConnectStatusSubscriber.Unsubscribe(OnConnectStatusMessage); + m_ConnectStatusSubscriber?.Unsubscribe(OnConnectStatusMessage); } void OnConnectStatusMessage(ConnectStatus connectStatus) diff --git a/Assets/Scripts/Gameplay/UI/Lobby/LobbyJoiningUI.cs b/Assets/Scripts/Gameplay/UI/Lobby/LobbyJoiningUI.cs index 9d28f3b03..f9c7f26e8 100644 --- a/Assets/Scripts/Gameplay/UI/Lobby/LobbyJoiningUI.cs +++ b/Assets/Scripts/Gameplay/UI/Lobby/LobbyJoiningUI.cs @@ -35,13 +35,13 @@ void OnDisable() { if (m_UpdateRunner != null) { - m_UpdateRunner.Unsubscribe(PeriodicRefresh); + m_UpdateRunner?.Unsubscribe(PeriodicRefresh); } } void OnDestroy() { - m_LocalLobbiesRefreshedSub.Unsubscribe(UpdateUI); + m_LocalLobbiesRefreshedSub?.Unsubscribe(UpdateUI); } [Inject] diff --git a/Assets/Scripts/Gameplay/UI/Lobby/LobbyUIMediator.cs b/Assets/Scripts/Gameplay/UI/Lobby/LobbyUIMediator.cs index 057229b3f..264efed53 100644 --- a/Assets/Scripts/Gameplay/UI/Lobby/LobbyUIMediator.cs +++ b/Assets/Scripts/Gameplay/UI/Lobby/LobbyUIMediator.cs @@ -65,7 +65,7 @@ void OnConnectStatus(ConnectStatus status) void OnDestroy() { - m_ConnectStatusSubscriber.Unsubscribe(OnConnectStatus); + m_ConnectStatusSubscriber?.Unsubscribe(OnConnectStatus); } //Lobby and Relay calls done from UI diff --git a/Assets/Scripts/Gameplay/UI/UnityServicesUIHandler.cs b/Assets/Scripts/Gameplay/UI/UnityServicesUIHandler.cs index 143d6dea8..30ff086fe 100644 --- a/Assets/Scripts/Gameplay/UI/UnityServicesUIHandler.cs +++ b/Assets/Scripts/Gameplay/UI/UnityServicesUIHandler.cs @@ -87,7 +87,7 @@ void HandleLobbyError(UnityServiceErrorMessage error) void OnDestroy() { - m_ServiceErrorSubscription.Unsubscribe(ServiceErrorHandler); + m_ServiceErrorSubscription?.Unsubscribe(ServiceErrorHandler); } } } From 9ffda69fbd75b6a55ea5a366bf924a4958def9b7 Mon Sep 17 00:00:00 2001 From: Philipp Deschain Date: Thu, 29 Sep 2022 13:18:06 -0400 Subject: [PATCH 6/7] removed unnecessary assert in MessageChannel Unsubscribe and replaced it with conditional --- .../Infrastructure/PubSub/MessageChannel.cs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/Assets/Scripts/Infrastructure/PubSub/MessageChannel.cs b/Assets/Scripts/Infrastructure/PubSub/MessageChannel.cs index 8d8a31fb2..faa1090d3 100644 --- a/Assets/Scripts/Infrastructure/PubSub/MessageChannel.cs +++ b/Assets/Scripts/Infrastructure/PubSub/MessageChannel.cs @@ -68,18 +68,19 @@ public virtual IDisposable Subscribe(Action handler) public void Unsubscribe(Action handler) { - Assert.IsTrue(IsSubscribed(handler), "Attempting to unsubscribe with a handler that is not subscribed"); - - if (m_PendingHandlers.ContainsKey(handler)) + if (IsSubscribed(handler)) { - if (m_PendingHandlers[handler]) + if (m_PendingHandlers.ContainsKey(handler)) { - m_PendingHandlers.Remove(handler); + if (m_PendingHandlers[handler]) + { + m_PendingHandlers.Remove(handler); + } + } + else + { + m_PendingHandlers[handler] = false; } - } - else - { - m_PendingHandlers[handler] = false; } } From 83d53122b970ce06391c1a0ef11952f9e6dc6922 Mon Sep 17 00:00:00 2001 From: Philipp Deschain Date: Thu, 29 Sep 2022 15:12:58 -0400 Subject: [PATCH 7/7] added PR number in changelog item --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4e30414a..70b5817ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ Additional documentation and release notes are available at [Multiplayer Documen * NetworkObjectSpawner handles dynamically spawning in-scene placed NetworkObjects (#717) - You can't place a NetworkObject in scene directly and destroy it at runtime. This PR showcases proper handling of NetworkObjects that you'd wish to place inside of scenes, but would still want to destroy at game-time. Examples of these are: Imps, VandalImps, ImpBoss. NetworkObjects such as doors, crystals, door switch, etc. remain the same, statically-placed in scene. * Quality levels settings set up for Desktop [MTT-4450] (#713) * Added custom RNSM config with graph for RTT instead of single value (#747) -* Added Unsubscribe API for the ISubscriber along with refactoring of the codebase to use this API instead of IDisposable handle when there is just one subscription. +* Added Unsubscribe API for the ISubscriber along with refactoring of the codebase to use this API instead of IDisposable handle when there is just one subscription (#612) ### Changed * Updated tools, authentication and relay packages (#690) * Replaced our dependency injection solution with VContainer. (#679)