fix: Make NetworkManager.Shutdown idempotent (#1877)

* Add a test for shutting down a NetworkManager twice

* Don't set m_ShuttingDown if not running

* Add CHANGELOG entry

Author: simon-lemay-unity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -20,6 +20,14 @@ Additional documentation and release notes are available at [Multiplayer Documen
 - Fixed issue during client synchronization if 'ValidateSceneBeforeLoading' returned false it would halt the client synchronization process resulting in a client that was approved but not synchronized or fully connected with the server. (#1883)
 - Fixed an issue where UNetTransport.StartServer would return success even if the underlying transport failed to start (#854)
 
+## [Unreleased]
+
+### Removed
+- Removed `SIPTransport` (#1870)
+
+### Fixed
+- Fixed an issue where calling `Shutdown` on a `NetworkManager` that was already shut down would cause an immediate shutdown the next time it was started (basically the fix makes `Shutdown` idempotent). (#1877)
+
 ## [1.0.0-pre.7] - 2022-04-06
 
 ### Added
@@ -38,7 +46,6 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Removed
 
-- Removed `SIPTransport` (#1870)
 - Removed `SnapshotSystem` (#1852)
 - Removed `com.unity.modules.animation`, `com.unity.modules.physics` and `com.unity.modules.physics2d` dependencies from the package (#1812)
 - Removed `com.unity.collections` dependency from the package (#1849)

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

@@ -1167,8 +1167,13 @@ public void Shutdown(bool discardMessageQueue = false)
                 NetworkLog.LogInfo(nameof(Shutdown));
             }
 
-            m_ShuttingDown = true;
-            m_StopProcessingMessages = discardMessageQueue;
+            // If we're not running, don't start shutting down, it would only cause an immediate
+            // shutdown the next time the manager is started.
+            if (IsServer || IsClient)
+            {
+                m_ShuttingDown = true;
+                m_StopProcessingMessages = discardMessageQueue;
+            }
         }
 
         internal void ShutdownInternal()

com.unity.netcode.gameobjects/Tests/Runtime/StopStartRuntimeTests.cs

@@ -44,5 +44,37 @@ public IEnumerator WhenShuttingDownAndRestarting_SDKRestartsSuccessfullyAndStays
             Assert.IsTrue(m_ServerNetworkManager.IsServer);
             Assert.IsTrue(m_ServerNetworkManager.IsListening);
         }
+
+        [UnityTest]
+        public IEnumerator WhenShuttingDownTwiceAndRestarting_SDKRestartsSuccessfullyAndStaysRunning()
+        {
+            // shutdown the server
+            m_ServerNetworkManager.Shutdown();
+
+            // wait 1 frame because shutdowns are delayed
+            var nextFrameNumber = Time.frameCount + 1;
+            yield return new WaitUntil(() => Time.frameCount >= nextFrameNumber);
+
+            // Verify the shutdown occurred
+            Assert.IsFalse(m_ServerNetworkManager.IsServer);
+            Assert.IsFalse(m_ServerNetworkManager.IsListening);
+            Assert.IsFalse(m_ServerNetworkManager.IsHost);
+            Assert.IsFalse(m_ServerNetworkManager.IsClient);
+
+            // Shutdown the server again.
+            m_ServerNetworkManager.Shutdown();
+
+            m_ServerNetworkManager.StartServer();
+            // Verify the server started
+            Assert.IsTrue(m_ServerNetworkManager.IsServer);
+            Assert.IsTrue(m_ServerNetworkManager.IsListening);
+
+            // Wait several frames / one full network tick
+            yield return s_DefaultWaitForTick;
+
+            // Verify the server is still running
+            Assert.IsTrue(m_ServerNetworkManager.IsServer);
+            Assert.IsTrue(m_ServerNetworkManager.IsListening);
+        }
     }
 }