refactor!: Unified Shutdown (#1108)

BREAKING CHANGE: NetworkManager.StopX is replaced with NetworkManager.Shutdown

Author: TwoTenPvP

com.unity.netcode.gameobjects/Editor/NetworkManagerEditor.cs

@@ -352,18 +352,7 @@ public override void OnInspectorGUI()
 
                 if (GUILayout.Button(new GUIContent("Stop " + instanceType, "Stops the " + instanceType + " instance.")))
                 {
-                    if (m_NetworkManager.IsHost)
-                    {
-                        m_NetworkManager.StopHost();
-                    }
-                    else if (m_NetworkManager.IsServer)
-                    {
-                        m_NetworkManager.StopServer();
-                    }
-                    else if (m_NetworkManager.IsClient)
-                    {
-                        m_NetworkManager.StopClient();
-                    }
+                    m_NetworkManager.Shutdown();
                 }
             }
         }

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

@@ -724,94 +724,6 @@ public SocketTasks StartClient()
             return socketTasks;
         }
 
-        /// <summary>
-        /// Stops the running server
-        /// </summary>
-        public void StopServer()
-        {
-            if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
-            {
-                NetworkLog.LogInfo(nameof(StopServer));
-            }
-
-            var disconnectedIds = new HashSet<ulong>();
-
-            //Don't know if I have to disconnect the clients. I'm assuming the NetworkTransport does all the cleaning on shtudown. But this way the clients get a disconnect message from server (so long it does't get lost)
-
-            // make sure all messages are flushed before transport disconnect clients
-            if (MessageQueueContainer != null)
-            {
-                MessageQueueContainer.ProcessAndFlushMessageQueue(
-                    queueType: MessageQueueContainer.MessageQueueProcessingTypes.Send,
-                    NetworkUpdateStage.PostLateUpdate); // flushing messages in case transport's disconnect
-            }
-
-            foreach (KeyValuePair<ulong, NetworkClient> pair in ConnectedClients)
-            {
-                if (!disconnectedIds.Contains(pair.Key))
-                {
-                    disconnectedIds.Add(pair.Key);
-
-                    if (pair.Key == NetworkConfig.NetworkTransport.ServerClientId)
-                    {
-                        continue;
-                    }
-
-                    NetworkConfig.NetworkTransport.DisconnectRemoteClient(pair.Key);
-                }
-            }
-
-            foreach (KeyValuePair<ulong, PendingClient> pair in PendingClients)
-            {
-                if (!disconnectedIds.Contains(pair.Key))
-                {
-                    disconnectedIds.Add(pair.Key);
-                    if (pair.Key == NetworkConfig.NetworkTransport.ServerClientId)
-                    {
-                        continue;
-                    }
-
-                    NetworkConfig.NetworkTransport.DisconnectRemoteClient(pair.Key);
-                }
-            }
-
-            IsServer = false;
-            Shutdown();
-        }
-
-        /// <summary>
-        /// Stops the running host
-        /// </summary>
-        public void StopHost()
-        {
-            if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
-            {
-                NetworkLog.LogInfo(nameof(StopHost));
-            }
-
-            IsServer = false;
-            IsClient = false;
-            StopServer();
-
-            //We don't stop client since we dont actually have a transport connection to our own host. We just handle host messages directly in the netcode
-        }
-
-        /// <summary>
-        /// Stops the running client
-        /// </summary>
-        public void StopClient()
-        {
-            if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
-            {
-                NetworkLog.LogInfo(nameof(StopClient));
-            }
-
-            IsClient = false;
-            NetworkConfig.NetworkTransport.DisconnectLocalClient();
-            IsConnectedClient = false;
-            Shutdown();
-        }
-
         /// <summary>
         /// Starts a Host
         /// </summary>
@@ -940,13 +852,71 @@ private void OnDestroy()
             }
         }
 
+        /// <summary>
+        /// Globally shuts down the library.
+        /// Disconnects clients if connected and stops server if running.
+        /// </summary>
         public void Shutdown()
         {
             if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
             {
                 NetworkLog.LogInfo(nameof(Shutdown));
             }
 
+            if (IsServer)
+            {
+                // make sure all messages are flushed before transport disconnect clients
+                if (MessageQueueContainer != null)
+                {
+                    MessageQueueContainer.ProcessAndFlushMessageQueue(
+                        queueType: MessageQueueContainer.MessageQueueProcessingTypes.Send,
+                        NetworkUpdateStage.PostLateUpdate); // flushing messages in case transport's disconnect
+                }
+
+                var disconnectedIds = new HashSet<ulong>();
+
+                //Don't know if I have to disconnect the clients. I'm assuming the NetworkTransport does all the cleaning on shutdown. But this way the clients get a disconnect message from server (so long it does't get lost)
+
+                foreach (KeyValuePair<ulong, NetworkClient> pair in ConnectedClients)
+                {
+                    if (!disconnectedIds.Contains(pair.Key))
+                    {
+                        disconnectedIds.Add(pair.Key);
+
+                        if (pair.Key == NetworkConfig.NetworkTransport.ServerClientId)
+                        {
+                            continue;
+                        }
+
+                        NetworkConfig.NetworkTransport.DisconnectRemoteClient(pair.Key);
+                    }
+                }
+
+                foreach (KeyValuePair<ulong, PendingClient> pair in PendingClients)
+                {
+                    if (!disconnectedIds.Contains(pair.Key))
+                    {
+                        disconnectedIds.Add(pair.Key);
+                        if (pair.Key == NetworkConfig.NetworkTransport.ServerClientId)
+                        {
+                            continue;
+                        }
+
+                        NetworkConfig.NetworkTransport.DisconnectRemoteClient(pair.Key);
+                    }
+                }
+            }
+
+            if (IsClient)
+            {
+                // Client only, send disconnect to server
+                NetworkConfig.NetworkTransport.DisconnectLocalClient();
+            }
+
+            IsConnectedClient = false;
+            IsServer = false;
+            IsClient = false;
+
             // Unregister INetworkUpdateSystem before shutting down the MessageQueueContainer
             this.UnregisterAllNetworkUpdates();
 
@@ -969,8 +939,6 @@ public void Shutdown()
                 NetworkTickSystem = null;
             }
 
-            IsServer = false;
-            IsClient = false;
             NetworkConfig.NetworkTransport.OnTransportEvent -= HandleRawTransportPoll;
 
             if (SpawnManager != null)
@@ -1201,8 +1169,7 @@ private void HandleRawTransportPoll(NetworkEvent networkEvent, ulong clientId, N
                     }
                     else
                     {
-                        IsConnectedClient = false;
-                        StopClient();
+                        Shutdown();
                     }
 
                     OnClientDisconnectCallback?.Invoke(clientId);

com.unity.netcode.gameobjects/Runtime/Transports/Tests/Editor/TransportTest.cs

@@ -47,7 +47,6 @@ public void UNetCustomChannelRegistrationTest()
             Assert.Fail("The UNet transport won't allow registration of a legit user channel");
         }
 
-        nm.StopServer();
         nm.Shutdown();
 
         ut.Channels.Clear();
@@ -64,7 +63,6 @@ public void UNetCustomChannelRegistrationTest()
             Debug.Log(ex.Message);
         }
 
-        nm.StopServer();
         nm.Shutdown();
     }
 }

com.unity.netcode.gameobjects/Tests/Editor/NetworkManagerMessageHandlerTests.cs

@@ -108,7 +108,7 @@ public void MessageHandlerReceivedMessageServerClient()
             networkManager.HandleIncomingData(0, NetworkChannel.Internal, new ArraySegment<byte>(messageStream10.GetBuffer(), 0, (int)messageStream10.Length), 0);
 
             // Stop server to trigger full shutdown
-            networkManager.StopServer();
+            networkManager.Shutdown();
 
             // Replace the real message handler with a dummy one that just prints a result
             networkManager.MessageHandler = new DummyMessageHandler(networkManager);
@@ -196,7 +196,7 @@ public void MessageHandlerReceivedMessageServerClient()
             networkManager.HandleIncomingData(0, NetworkChannel.Internal, new ArraySegment<byte>(messageStream21.GetBuffer(), 0, (int)messageStream21.Length), 0);
 
             // Full cleanup
-            networkManager.StopClient();
+            networkManager.Shutdown();
 
             // Ensure no missmatches with expectations
             LogAssert.NoUnexpectedReceived();

com.unity.netcode.gameobjects/Tests/Editor/StartStopTests.cs

@@ -0,0 +1,62 @@
+using NUnit.Framework;
+using UnityEngine;
+
+namespace Unity.Netcode.EditorTests
+{
+    public class StartStopTests
+    {
+        private NetworkManager m_NetworkManager;
+
+        [SetUp]
+        public void Setup()
+        {
+            // Create the reusable NetworkManager
+            m_NetworkManager = new GameObject(nameof(NetworkManager)).AddComponent<NetworkManager>();
+            var transport = m_NetworkManager.gameObject.AddComponent<DummyTransport>();
+
+            m_NetworkManager.NetworkConfig = new NetworkConfig()
+            {
+                NetworkTransport = transport
+            };
+        }
+
+        [Test]
+        public void TestStopAndRestartForExceptions()
+        {
+            m_NetworkManager.StartServer();
+            m_NetworkManager.Shutdown();
+            m_NetworkManager.StartServer();
+            m_NetworkManager.Shutdown();
+        }
+
+        [Test]
+        public void TestStartupServerState()
+        {
+            m_NetworkManager.StartServer();
+
+            Assert.True(m_NetworkManager.IsServer);
+            Assert.False(m_NetworkManager.IsClient);
+            Assert.False(m_NetworkManager.IsHost);
+
+            m_NetworkManager.Shutdown();
+        }
+
+        [Test]
+        public void TestFlagShutdown()
+        {
+            m_NetworkManager.StartServer();
+            m_NetworkManager.Shutdown();
+
+            Assert.False(m_NetworkManager.IsServer);
+            Assert.False(m_NetworkManager.IsClient);
+            Assert.False(m_NetworkManager.IsHost);
+        }
+
+        [TearDown]
+        public void Teardown()
+        {
+            // Cleanup
+            Object.DestroyImmediate(m_NetworkManager.gameObject);
+        }
+    }
+}

com.unity.netcode.gameobjects/Tests/Editor/StartStopTests.cs.meta

@@ -198,27 +198,7 @@ private static void StartNetworkManagerMode(NetworkManagerOperatingMode managerM
         /// </summary>
         private static void StopNetworkManagerMode()
         {
-            switch (CurrentNetworkManagerMode)
-            {
-                case NetworkManagerOperatingMode.Host:
-                    {
-                        // Stop the host
-                        NetworkManagerObject.StopHost();
-                        break;
-                    }
-                case NetworkManagerOperatingMode.Server:
-                    {
-                        // Stop the server
-                        NetworkManagerObject.StopServer();
-                        break;
-                    }
-                case NetworkManagerOperatingMode.Client:
-                    {
-                        // Stop the client
-                        NetworkManagerObject.StopClient();
-                        break;
-                    }
-            }
+            NetworkManagerObject.Shutdown();
 
             Debug.Log($"{CurrentNetworkManagerMode} stopped.");
             CurrentNetworkManagerMode = NetworkManagerOperatingMode.None;

com.unity.netcode.gameobjects/Tests/Runtime/Helpers/NetworkManagerHelper.cs

@@ -88,7 +88,7 @@ public static bool CreateNewClients(int clientCount, out NetworkManager[] client
         /// <param name="clientToStop"></param>
         public static void StopOneClient(NetworkManager clientToStop)
         {
-            clientToStop.StopClient();
+            clientToStop.Shutdown();
             Object.Destroy(clientToStop.gameObject);
             NetworkManagerInstances.Remove(clientToStop);
         }
@@ -109,18 +109,7 @@ public static void Destroy()
             // Shutdown the server which forces clients to disconnect
             foreach (var networkManager in NetworkManagerInstances)
             {
-                if (networkManager.IsHost)
-                {
-                    networkManager.StopHost();
-                }
-                else if (networkManager.IsServer)
-                {
-                    networkManager.StopServer();
-                }
-                else if (networkManager.IsClient)
-                {
-                    networkManager.StopClient();
-                }
+                networkManager.Shutdown();
             }
 
             // Destroy the network manager instances

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

@@ -14,18 +14,7 @@ public void OnExitScene()
     {
         if (NetworkManager.Singleton)
         {
-            if (NetworkManager.Singleton.IsHost)
-            {
-                NetworkManager.Singleton.StopHost();
-            }
-            else if (NetworkManager.Singleton.IsClient)
-            {
-                NetworkManager.Singleton.StopClient();
-            }
-            else if (NetworkManager.Singleton.IsServer)
-            {
-                NetworkManager.Singleton.StopServer();
-            }
+            NetworkManager.Singleton.Shutdown();
             Destroy(NetworkManager.Singleton.gameObject);
         }
 

testproject/Assets/Scripts/ExitButtonScript.cs

@@ -59,10 +59,10 @@ public IEnumerator Setup()
         [UnityTearDown]
         public IEnumerator Teardown()
         {
-            m_ServerNetworkManager.StopHost();
+            m_ServerNetworkManager.Shutdown();
             foreach (var networkManager in m_ClientNetworkManagers)
             {
-                networkManager.StopClient();
+                networkManager.Shutdown();
             }
             int nextFrameNumber = Time.frameCount + 4;
             yield return new WaitUntil(() => Time.frameCount >= nextFrameNumber);