Skip to content

feat: PubSub improvement: ISubscriber unsub [MTT-2765] #612

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Sep 29, 2022
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ public class ServerBossRoomState : GameStateBehaviour
/// </summary>
[Inject] ISubscriber<LifeStateChangedEventMessage> m_LifeStateChangedEventMessageSubscriber;

IDisposable m_Subscription;

[Inject] ConnectionManager m_ConnectionManager;
[Inject] PersistentGameState m_PersistentGameState;

Expand All @@ -79,7 +77,7 @@ void OnNetworkSpawn()
return;
}
m_PersistentGameState.Reset();
m_Subscription = m_LifeStateChangedEventMessageSubscriber.Subscribe(OnLifeStateChangedEventMessage);
m_LifeStateChangedEventMessageSubscriber.Subscribe(OnLifeStateChangedEventMessage);

NetworkManager.Singleton.OnClientDisconnectCallback += OnClientDisconnect;
NetworkManager.Singleton.SceneManager.OnLoadEventCompleted += OnLoadEventCompleted;
Expand All @@ -90,16 +88,15 @@ void OnNetworkSpawn()

void OnNetworkDespawn()
{
m_Subscription?.Dispose();

m_LifeStateChangedEventMessageSubscriber?.Unsubscribe(OnLifeStateChangedEventMessage);
NetworkManager.Singleton.OnClientDisconnectCallback -= OnClientDisconnect;
NetworkManager.Singleton.SceneManager.OnLoadEventCompleted -= OnLoadEventCompleted;
NetworkManager.Singleton.SceneManager.OnSynchronizeComplete -= OnSynchronizeComplete;
}

protected override void OnDestroy()
{
m_Subscription?.Dispose();
m_LifeStateChangedEventMessageSubscriber?.Unsubscribe(OnLifeStateChangedEventMessage);

if (m_NetcodeHooks)
{
Expand Down
7 changes: 4 additions & 3 deletions Assets/Scripts/Gameplay/UI/IPConnectionWindow.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@ public class IPConnectionWindow : MonoBehaviour

[Inject] IPUIMediator m_IPUIMediator;

IDisposable m_Subscription;
ISubscriber<ConnectStatus> m_ConnectStatusSubscriber;

[Inject]
void InjectDependencies(ISubscriber<ConnectStatus> connectStatusSubscriber)
{
m_Subscription = connectStatusSubscriber.Subscribe(OnConnectStatusMessage);
m_ConnectStatusSubscriber = connectStatusSubscriber;
m_ConnectStatusSubscriber.Subscribe(OnConnectStatusMessage);
}

void Awake()
Expand All @@ -35,7 +36,7 @@ void Awake()

void OnDestroy()
{
m_Subscription.Dispose();
m_ConnectStatusSubscriber?.Unsubscribe(OnConnectStatusMessage);
}

void OnConnectStatusMessage(ConnectStatus connectStatus)
Expand Down
7 changes: 4 additions & 3 deletions Assets/Scripts/Gameplay/UI/IPUIMediator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,13 @@ public class IPUIMediator : MonoBehaviour

public IPHostingUI IPHostingUI => m_IPHostingUI;

IDisposable m_Subscription;
ISubscriber<ConnectStatus> m_ConnectStatusSubscriber;

[Inject]
void InjectDependencies(ISubscriber<ConnectStatus> connectStatusSubscriber)
{
m_Subscription = connectStatusSubscriber.Subscribe(OnConnectStatusMessage);
m_ConnectStatusSubscriber = connectStatusSubscriber;
m_ConnectStatusSubscriber.Subscribe(OnConnectStatusMessage);
}

void Awake()
Expand All @@ -63,7 +64,7 @@ void Start()

void OnDestroy()
{
m_Subscription.Dispose();
m_ConnectStatusSubscriber?.Unsubscribe(OnConnectStatusMessage);
}

void OnConnectStatusMessage(ConnectStatus connectStatus)
Expand Down
10 changes: 6 additions & 4 deletions Assets/Scripts/Gameplay/UI/Lobby/LobbyJoiningUI.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ public class LobbyJoiningUI : MonoBehaviour
IObjectResolver m_Container;
LobbyUIMediator m_LobbyUIMediator;
UpdateRunner m_UpdateRunner;
IDisposable m_Subscriptions;
ISubscriber<LobbyListFetchedMessage> m_LocalLobbiesRefreshedSub;

List<LobbyListItemUI> m_LobbyListItems = new List<LobbyListItemUI>();

void Awake()
Expand All @@ -40,13 +41,13 @@ void OnDisable()
{
if (m_UpdateRunner != null)
{
m_UpdateRunner.Unsubscribe(PeriodicRefresh);
m_UpdateRunner?.Unsubscribe(PeriodicRefresh);
}
}

void OnDestroy()
{
m_Subscriptions?.Dispose();
m_LocalLobbiesRefreshedSub?.Unsubscribe(UpdateUI);
}

[Inject]
Expand All @@ -59,7 +60,8 @@ void InjectDependenciesAndInitialize(
m_Container = container;
m_LobbyUIMediator = lobbyUIMediator;
m_UpdateRunner = updateRunner;
m_Subscriptions = localLobbiesRefreshedSub.Subscribe(UpdateUI);
m_LocalLobbiesRefreshedSub = localLobbiesRefreshedSub;
m_LocalLobbiesRefreshedSub.Subscribe(UpdateUI);
}

/// <summary>
Expand Down
8 changes: 4 additions & 4 deletions Assets/Scripts/Gameplay/UI/Lobby/LobbyUIMediator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public class LobbyUIMediator : MonoBehaviour
LocalLobby m_LocalLobby;
NameGenerationData m_NameGenerationData;
ConnectionManager m_ConnectionManager;
IDisposable m_Subscriptions;
ISubscriber<ConnectStatus> m_ConnectStatusSubscriber;

const string k_DefaultLobbyName = "no-name";

Expand All @@ -50,10 +50,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)
Expand All @@ -66,7 +66,7 @@ void OnConnectStatus(ConnectStatus status)

void OnDestroy()
{
m_Subscriptions?.Dispose();
m_ConnectStatusSubscriber?.Unsubscribe(OnConnectStatus);
}

//Lobby and Relay calls done from UI
Expand Down
7 changes: 4 additions & 3 deletions Assets/Scripts/Gameplay/UI/UnityServicesUIHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Unity.BossRoom.Gameplay.UI
{
public class UnityServicesUIHandler : MonoBehaviour
{
IDisposable m_Subscriptions;
ISubscriber<UnityServiceErrorMessage> m_ServiceErrorSubscription;

void Awake()
{
Expand All @@ -20,7 +20,8 @@ void Awake()
[Inject]
void Initialize(ISubscriber<UnityServiceErrorMessage> serviceError)
{
m_Subscriptions = serviceError.Subscribe(ServiceErrorHandler);
m_ServiceErrorSubscription = serviceError;
m_ServiceErrorSubscription.Subscribe(ServiceErrorHandler);
}

void ServiceErrorHandler(UnityServiceErrorMessage error)
Expand Down Expand Up @@ -86,7 +87,7 @@ void HandleLobbyError(UnityServiceErrorMessage error)

void OnDestroy()
{
m_Subscriptions?.Dispose();
m_ServiceErrorSubscription?.Unsubscribe(ServiceErrorHandler);
}
}
}
2 changes: 1 addition & 1 deletion Assets/Scripts/Infrastructure/PubSub/IMessageChannel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ public interface IPublisher<T>
public interface ISubscriber<T>
{
IDisposable Subscribe(Action<T> handler);
void Unsubscribe(Action<T> handler);
}

public interface IMessageChannel<T> : IPublisher<T>, ISubscriber<T>, IDisposable
{
bool IsDisposed { get; }
void Unsubscribe(Action<T> handler);
}

public interface IBufferedMessageChannel<T> : IMessageChannel<T>
Expand Down
19 changes: 10 additions & 9 deletions Assets/Scripts/Infrastructure/PubSub/MessageChannel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,18 +68,19 @@ public virtual IDisposable Subscribe(Action<T> handler)

public void Unsubscribe(Action<T> 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;
}
}

Expand Down
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> 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)
Expand Down