From 85964c345e4d7a2c37019c670e28f11e5f3cc4dc Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Wed, 18 Mar 2026 19:01:03 -0500 Subject: [PATCH 1/2] fix Fixing the issue where users might try to start NetworkManager within OnClientStopped or OnServerStopped which would result in a failed start. --- .../Runtime/Core/NetworkManager.cs | 30 +++++++++++-------- .../Runtime/Spawning/NetworkSpawnManager.cs | 2 +- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs index 67acb6b7a8..b5f037c4ac 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs @@ -1674,19 +1674,8 @@ internal void ShutdownInternal() ConnectionManager.InvokeOnClientDisconnectCallback(LocalClientId); } - if (ConnectionManager.LocalClient.IsClient) - { - // If we were a client, we want to know if we were a host - // client or not. (why we pass in "IsServer") - OnClientStopped?.Invoke(ConnectionManager.LocalClient.IsServer); - } - - if (ConnectionManager.LocalClient.IsServer) - { - // If we were a server, we want to know if we were a host - // or not. (why we pass in "IsClient") - OnServerStopped?.Invoke(ConnectionManager.LocalClient.IsClient); - } + // Save off the last local client settings + var localClient = ConnectionManager.LocalClient; // In the event shutdown is invoked within OnClientStopped or OnServerStopped, set it to false again m_ShuttingDown = false; @@ -1706,6 +1695,21 @@ internal void ShutdownInternal() // can unsubscribe from tick updates and such. NetworkTimeSystem?.Shutdown(); NetworkTickSystem = null; + + + if (localClient.IsClient) + { + // If we were a client, we want to know if we were a host + // client or not. (why we pass in "IsServer") + OnClientStopped?.Invoke(localClient.IsServer); + } + + if (localClient.IsServer) + { + // If we were a server, we want to know if we were a host + // or not. (why we pass in "IsClient") + OnServerStopped?.Invoke(localClient.IsClient); + } } // Ensures that the NetworkManager is cleaned up before OnDestroy is run on NetworkObjects and NetworkBehaviours when quitting the application. diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index d4768692ec..bf4eacbc1b 100644 --- a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs @@ -1269,7 +1269,7 @@ internal unsafe void UpdateNetworkObjectSceneChanges() foreach (var entry in NetworkObjectsToSynchronizeSceneChanges) { // If it fails the first update then don't add for updates - if (!entry.Value.UpdateForSceneChanges()) + if (entry.Value != null && !entry.Value.UpdateForSceneChanges()) { CleanUpDisposedObjects.Push(entry.Key); } From bd5c75784050892ca89648ced03ade085d632da4 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Wed, 18 Mar 2026 19:09:18 -0500 Subject: [PATCH 2/2] test Adding test that verifies a NetworkManager can be started when OnClientStopped is invoked and that OnServerStopped is not invoked if the NetworkManager is started again during an OnClientStopped invocation (i.e. host). --- .../Runtime/NetworkManagerStartStopTests.cs | 191 ++++++++++++++++++ .../NetworkManagerStartStopTests.cs.meta | 2 + 2 files changed, 193 insertions(+) create mode 100644 com.unity.netcode.gameobjects/Tests/Runtime/NetworkManagerStartStopTests.cs create mode 100644 com.unity.netcode.gameobjects/Tests/Runtime/NetworkManagerStartStopTests.cs.meta diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkManagerStartStopTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkManagerStartStopTests.cs new file mode 100644 index 0000000000..32f9e42856 --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkManagerStartStopTests.cs @@ -0,0 +1,191 @@ +using System.Collections; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using NUnit.Framework; +using Unity.Netcode.TestHelpers.Runtime; +using UnityEngine.TestTools; + +namespace Unity.Netcode.RuntimeTests +{ + [TestFixture(NetworkTopologyTypes.ClientServer)] + internal class NetworkManagerStartStopTests : NetcodeIntegrationTest + { + private const int k_NumberOfSessions = 5; + protected override int NumberOfClients => 2; + private OnClientStoppedHandler m_StoppedHandler; + private int m_ExpectedNumberOfClients = 0; + public NetworkManagerStartStopTests(NetworkTopologyTypes networkTopologyType) : base(networkTopologyType, HostOrServer.Host) { } + + /// + /// This test will not work with the CMB service since it requires the service + /// to remain active after all clients have disconnected. + /// + protected override bool UseCMBService() + { + return false; + } + + private void ShutdownIfListening() + { + var networkManager = m_StoppedHandler.NetworkManager; + if (networkManager.IsListening) + { + m_StoppedHandler.NetworkManager.Shutdown(); + } + } + + private bool NetworkManagerCompletedSessionCount(StringBuilder errorLog) + { + // Once the session count is decremented to zero the condition has been met. + if (m_StoppedHandler.SessionCount != 0) + { + // If we are a host, then only shutdown once all clients have reconnected + if (m_StoppedHandler.IsSessionAuthority && m_StoppedHandler.NetworkManager.ConnectedClientsIds.Count != m_ExpectedNumberOfClients) + { + errorLog.Append($"[{m_StoppedHandler.NetworkManager.name}] Waiting for {m_ExpectedNumberOfClients} clients to connect but there are only {m_StoppedHandler.NetworkManager.ConnectedClientsIds.Count} connected!"); + return false; + } + ShutdownIfListening(); + errorLog.Append($"[{m_StoppedHandler.NetworkManager.name}] Still has a session count of {m_StoppedHandler.SessionCount}!"); + } + return errorLog.Length == 0; + } + + [UnityTest] + public IEnumerator StartFromWithinOnClientStopped() + { + var authority = GetAuthorityNetworkManager(); + m_ExpectedNumberOfClients = authority.ConnectedClientsIds.Count; + + // Validate a client can disconnect and immediately reconnect from within OnClientStopped + m_StoppedHandler = new OnClientStoppedHandler(k_NumberOfSessions, GetNonAuthorityNetworkManager()); + ShutdownIfListening(); + yield return WaitForConditionOrTimeOut(NetworkManagerCompletedSessionCount); + AssertOnTimeout($"Not all {nameof(NetworkManager)} instances finished their sessions!"); + + // Validate a host can disconnect and immediately reconnect from within OnClientStopped + m_StoppedHandler = new OnHostStoppedHandler(k_NumberOfSessions, authority, m_NetworkManagers.ToList()); + ShutdownIfListening(); + yield return WaitForConditionOrTimeOut(NetworkManagerCompletedSessionCount); + AssertOnTimeout($"Not all {nameof(NetworkManager)} instances finished their sessions!"); + + // Verify OnServerStopped is not invoked if NetworkManager is started again within OnClientStopped (it should not invoke if it is listening). + Assert.False((m_StoppedHandler as OnHostStoppedHandler).OnServerStoppedInvoked, $"{nameof(NetworkManager.OnServerStopped)} was invoked when it should not have been invoked!"); + } + } + + internal class OnHostStoppedHandler : OnClientStoppedHandler + { + public bool OnServerStoppedInvoked = false; + + private List m_Clients = new List(); + + private Networking.Transport.NetworkEndpoint m_Endpoint; + + protected override void OnClientStopped(bool wasHost) + { + m_Endpoint.Port++; + var unityTransport = (Transports.UTP.UnityTransport)NetworkManager.NetworkConfig.NetworkTransport; + unityTransport.SetConnectionData(m_Endpoint); + // Make sure all clients are shutdown or shutting down + foreach (var networkManager in m_Clients) + { + if (networkManager.IsListening && !networkManager.ShutdownInProgress) + { + networkManager.Shutdown(); + } + } + + base.OnClientStopped(wasHost); + if (SessionCount != 0) + { + NetworkManager.StartCoroutine(StartClients()); + } + + } + + private IEnumerator StartClients() + { + var nextPhase = false; + var timeout = UnityEngine.Time.realtimeSinceStartup + 5.0f; + while (!nextPhase) + { + if (!nextPhase && timeout < UnityEngine.Time.realtimeSinceStartup) + { + Assert.Fail($"Timed out waiting for all {nameof(NetworkManager)} instances to shutdown!"); + yield break; + } + + nextPhase = true; + foreach (var networkManager in m_Clients) + { + if (networkManager.ShutdownInProgress || networkManager.IsListening) + { + nextPhase = false; + } + } + yield return null; + } + + // Now, start all of the clients and have them connect again + foreach (var networkManager in m_Clients) + { + var unityTransport = (Transports.UTP.UnityTransport)networkManager.NetworkConfig.NetworkTransport; + unityTransport.SetConnectionData(m_Endpoint); + networkManager.StartClient(); + } + } + + public OnHostStoppedHandler(int numberOfSessions, NetworkManager authority, List networkManagers) : base(numberOfSessions, authority) + { + m_Endpoint = ((Transports.UTP.UnityTransport)authority.NetworkConfig.NetworkTransport).GetLocalEndpoint(); + networkManagers.Remove(authority); + m_Clients = networkManagers; + authority.OnServerStopped += OnServerStopped; + } + + private void OnServerStopped(bool wasHost) + { + OnServerStoppedInvoked = SessionCount != 0; + } + } + + internal class OnClientStoppedHandler + { + public NetworkManager NetworkManager { get; private set; } + + public int SessionCount { get; private set; } + public bool IsSessionAuthority { get; private set; } + + protected virtual void OnClientStopped(bool wasHost) + { + SessionCount--; + if (SessionCount <= 0) + { + NetworkManager.OnClientStopped -= OnClientStopped; + return; + } + + if (wasHost) + { + NetworkManager.StartHost(); + } + else + { + NetworkManager.StartClient(); + } + } + + public OnClientStoppedHandler(int sessionCount, NetworkManager networkManager) + { + NetworkManager = networkManager; + NetworkManager.OnClientStopped += OnClientStopped; + SessionCount = sessionCount; + IsSessionAuthority = networkManager.IsServer || networkManager.LocalClient.IsSessionOwner; + } + + public OnClientStoppedHandler() { } + + } +} diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkManagerStartStopTests.cs.meta b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkManagerStartStopTests.cs.meta new file mode 100644 index 0000000000..8192a7454e --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkManagerStartStopTests.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 8ec27192eb899144e82f7a5016ce5cfa \ No newline at end of file