fix: #2802 NetworkManager uses OnDestroy for shutdown instead of OnApplicationQuit (#3935)

* fix: #2802 NetworkManager uses OnDestroy for shutdown instead of OnApplicationQuit

* imer feedback

---------

Co-authored-by: mischa <info@noobtuts.com>

Assets/Mirror/Core/NetworkManager.cs

@@ -634,31 +634,6 @@ public void StopClient()
             NetworkClient.Disconnect();
         }
 
-        // called when quitting the application by closing the window / pressing
-        // stop in the editor. virtual so that inheriting classes'
-        // OnApplicationQuit() can call base.OnApplicationQuit() too
-        public virtual void OnApplicationQuit()
-        {
-            // stop client first
-            // (we want to send the quit packet to the server instead of waiting
-            //  for a timeout)
-            if (NetworkClient.isConnected)
-            {
-                StopClient();
-                //Debug.Log("OnApplicationQuit: stopped client");
-            }
-
-            // stop server after stopping client (for proper host mode stopping)
-            if (NetworkServer.active)
-            {
-                StopServer();
-                //Debug.Log("OnApplicationQuit: stopped server");
-            }
-
-            // Call ResetStatics to reset statics and singleton
-            ResetStatics();
-        }
-
         /// <summary>Set the frame rate for a headless builds. Override to disable or modify.</summary>
         // useful for dedicated servers.
         // useful for headless benchmark clients.
@@ -772,12 +747,41 @@ public static void ResetStatics()
             singleton = null;
         }
 
-        // virtual so that inheriting classes' OnDestroy() can call base.OnDestroy() too
+        // called when quitting the application by closing the window / pressing
+        // stop in the editor.
+        // use OnDestroy instead of OnApplicationQuit:
+        // fixes: https://github.com/MirrorNetworking/Mirror/issues/2802
         public virtual void OnDestroy()
         {
             //Debug.Log("NetworkManager destroyed");
+
+            // stop client first
+            // (we want to send the quit packet to the server instead of waiting
+            //  for a timeout)
+            if (NetworkClient.isConnected)
+            {
+                StopClient();
+                //Debug.Log("OnApplicationQuit: stopped client");
             }
 
+            // stop server after stopping client (for proper host mode stopping)
+            if (NetworkServer.active)
+            {
+                StopServer();
+                //Debug.Log("OnApplicationQuit: stopped server");
+            }
+
+            // Call ResetStatics to reset statics and singleton
+            ResetStatics();
+        }
+
+        // [Obsolete] in case someone is inheriting it.
+        // don't use this anymore.
+        // fixes: https://github.com/MirrorNetworking/Mirror/issues/2802
+        // DEPRECATED 2024-10-29
+        [Obsolete("Override OnDestroy instead of OnApplicationQuit. Fixes: https://github.com/MirrorNetworking/Mirror/issues/2802")]
+        public virtual void OnApplicationQuit() {}
+
         /// <summary>The name of the current network scene.</summary>
         // set by NetworkManager when changing the scene.
         // new clients will automatically load this scene.

Assets/Mirror/Core/Transport.cs

@@ -197,7 +197,9 @@ public virtual void ServerLateUpdate() {}
         public abstract void Shutdown();
 
         /// <summary>Called by Unity when quitting. Inheriting Transports should call base for proper Shutdown.</summary>
-        public virtual void OnApplicationQuit()
+        // use OnDestroy instead of OnApplicationQuit:
+        // fixes: https://github.com/MirrorNetworking/Mirror/issues/2802
+        public virtual void OnDestroy()
         {
             // stop transport (e.g. to shut down threads)
             // (when pressing Stop in the Editor, Unity keeps threads alive

Assets/Mirror/Examples/AutoLANClientController/Scripts/AutoLANNetworkManager.cs

@@ -62,15 +62,6 @@ public override void LateUpdate()
             base.LateUpdate();
         }
 
-        /// <summary>
-        /// Runs on both Server and Client
-        /// </summary>
-        public override void OnDestroy()
-        {
-            base.OnDestroy();
-            //UnityEngine.Debug.Log("OnDestroy");
-        }
-
 #endregion
 
         #region Start & Stop
@@ -87,10 +78,10 @@ public override void ConfigureHeadlessFrameRate()
         /// <summary>
         /// called when quitting the application by closing the window / pressing stop in the editor
         /// </summary>
-        public override void OnApplicationQuit()
+        public override void OnDestroy()
         {
-            base.OnApplicationQuit();
+            base.OnDestroy();
-            //UnityEngine.Debug.Log("OnApplicationQuit");
+            //UnityEngine.Debug.Log("OnDestroy");
         }
 
         #endregion

Assets/Mirror/Examples/PlayerTest/PlayerTestNetMan.cs

@@ -71,14 +71,6 @@ public override void ConfigureHeadlessFrameRate()
             base.ConfigureHeadlessFrameRate();
         }
 
-        /// <summary>
-        /// called when quitting the application by closing the window / pressing stop in the editor
-        /// </summary>
-        public override void OnApplicationQuit()
-        {
-            base.OnApplicationQuit();
-        }
-
         #endregion
 
         #region Scene Management