fix: NetworkManager.ShutdownInProgress not being reset (#2661)

* fix

This resolves the issue where the NetworkManager.m_ShuttingDown property could be set to true again if the shutdown method is invoked within OnClientStopped or OnServerStopped.

* test

This validates that if you invoke shutdown within OnClientStopped or OnServerStopped that NetworkManager.ShutdownInProgress remains false once completing the shutdown process.

* update

Adding the changelog entry for this fix.
Making the changelog entry for v1.6.0 branch match #2657

* Update CHANGELOG.md

Removing the adjusted changelog entry

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -14,6 +14,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed  issue where invoking `NetworkManager.Shutdown` within `NetworkManager.OnClientStopped` or `NetworkManager.OnServerStopped` would force `NetworkManager.ShutdownInProgress` to remain true after completing the shutdown process. (#2661)
 - Fixed issue with client synchronization of position when using half precision and the delta position reaches the maximum value and is collapsed on the host prior to being forwarded to the non-owner clients. (#2636)
 - Fixed issue with scale not synchronizing properly depending upon the spawn order of NetworkObjects. (#2636)
 - Fixed issue position was not properly transitioning between ownership changes with an owner authoritative NetworkTransform. (#2636)

com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs

@@ -1013,6 +1013,9 @@ internal void ShutdownInternal()
                 OnServerStopped?.Invoke(ConnectionManager.LocalClient.IsClient);
             }
 
+            // In the event shutdown is invoked within OnClientStopped or OnServerStopped, set it to false again
+            m_ShuttingDown = false;
+
             // Reset the client's roles
             ConnectionManager.LocalClient.SetRole(false, false);
 

testproject/Assets/Tests/Runtime/NetworkManagerTests.cs

@@ -1,4 +1,5 @@
 using System.Collections;
+using System.Linq;
 using NUnit.Framework;
 using Unity.Netcode;
 using Unity.Netcode.TestHelpers.Runtime;
@@ -40,12 +41,15 @@ public NetworkManagerTests(UseSceneManagement useSceneManagement)
 
         private void OnClientConnectedCallback(NetworkObject networkObject, int numberOfTimesInvoked, bool isHost, bool isClient, bool isServer)
         {
-            m_NetworkObject = networkObject;
-            m_NetworkObjectWasSpawned = networkObject.IsSpawned;
-            m_NetworkBehaviourIsHostWasSet = isHost;
-            m_NetworkBehaviourIsClientWasSet = isClient;
-            m_NetworkBehaviourIsServerWasSet = isServer;
-            m_NumberOfTimesInvoked = numberOfTimesInvoked;
+            if (networkObject != null)
+            {
+                m_NetworkObject = networkObject;
+                m_NetworkObjectWasSpawned = networkObject.IsSpawned;
+                m_NetworkBehaviourIsHostWasSet = isHost;
+                m_NetworkBehaviourIsClientWasSet = isClient;
+                m_NetworkBehaviourIsServerWasSet = isServer;
+                m_NumberOfTimesInvoked = numberOfTimesInvoked;
+            }
         }
 
         private bool TestComponentFound()
@@ -96,22 +100,86 @@ public void ValidateHostSettings()
             Assert.IsTrue(m_NetworkBehaviourIsServerWasSet, $"IsServer was not true when OnClientConnectedCallback was invoked!");
         }
 
+        public enum ShutdownChecks
+        {
+            Server,
+            Client
+        }
+
+        protected override void OnNewClientCreated(NetworkManager networkManager)
+        {
+            networkManager.NetworkConfig.EnableSceneManagement = m_EnableSceneManagement;
+            foreach (var prefab in m_ServerNetworkManager.NetworkConfig.Prefabs.Prefabs)
+            {
+                networkManager.NetworkConfig.Prefabs.Add(prefab);
+            }
+            base.OnNewClientCreated(networkManager);
+        }
+
         /// <summary>
         /// Validate shutting down a second time does not cause an exception.
         /// </summary>        
         [UnityTest]
-        public IEnumerator ValidateShutdown()
+        public IEnumerator ValidateShutdown([Values] ShutdownChecks shutdownCheck)
         {
-            // Register for the server stopped notification so we know we have shutdown completely
-            m_ServerNetworkManager.OnServerStopped += M_ServerNetworkManager_OnServerStopped;
-            // Shutdown
-            m_ServerNetworkManager.Shutdown();
+
+
+            if (shutdownCheck == ShutdownChecks.Server)
+            {
+                // Register for the server stopped notification so we know we have shutdown completely
+                m_ServerNetworkManager.OnServerStopped += OnServerStopped;
+                // Shutdown
+                m_ServerNetworkManager.Shutdown();
+            }
+            else
+            {
+                // For this test (simplify the complexity) with a late joining client, just remove the 
+                // in-scene placed NetworkObject prior to the client connecting
+                // (We are testing the shutdown sequence)
+                var spawnedObjects = m_ServerNetworkManager.SpawnManager.SpawnedObjectsList.ToList();
+
+                for (int i = spawnedObjects.Count - 1; i >= 0; i--)
+                {
+                    var spawnedObject = spawnedObjects[i];
+                    if (spawnedObject.IsSceneObject != null && spawnedObject.IsSceneObject.Value)
+                    {
+                        spawnedObject.Despawn();
+                    }
+                }
+
+                yield return s_DefaultWaitForTick;
+
+                yield return CreateAndStartNewClient();
+
+                // Register for the server stopped notification so we know we have shutdown completely
+                m_ClientNetworkManagers[0].OnClientStopped += OnClientStopped;
+                m_ClientNetworkManagers[0].Shutdown();
+            }
+
+            // Let the network manager instance shutdown
             yield return s_DefaultWaitForTick;
+
+            // Validate the shutdown is no longer in progress
+            if (shutdownCheck == ShutdownChecks.Server)
+            {
+                Assert.False(m_ServerNetworkManager.ShutdownInProgress, $"[{shutdownCheck}] Shutdown in progress was still detected!");
+            }
+            else
+            {
+                Assert.False(m_ClientNetworkManagers[0].ShutdownInProgress, $"[{shutdownCheck}] Shutdown in progress was still detected!");
+            }
+        }
+
+        private void OnClientStopped(bool obj)
+        {
+            m_ServerNetworkManager.OnServerStopped -= OnClientStopped;
+            // Verify that we can invoke shutdown again without an exception
+            m_ServerNetworkManager.Shutdown();
         }
 
-        private void M_ServerNetworkManager_OnServerStopped(bool obj)
+        private void OnServerStopped(bool obj)
         {
-            m_ServerNetworkManager.OnServerStopped -= M_ServerNetworkManager_OnServerStopped;
+            m_ServerNetworkManager.OnServerStopped -= OnServerStopped;
             // Verify that we can invoke shutdown again without an exception
             m_ServerNetworkManager.Shutdown();
         }