From d1695dd16f477fc9edaaedb90032c188bcbba6e2 Mon Sep 17 00:00:00 2001
From: Paul Pacheco <paulpach@gmail.com>
Date: Mon, 20 Apr 2020 03:36:03 -0500
Subject: [PATCH] feat: new virtual OnStopServer called when object is
 unspawned (#1743)

---
 Assets/Mirror/Runtime/NetworkBehaviour.cs     |  6 +++
 Assets/Mirror/Runtime/NetworkIdentity.cs      | 20 ++++++++
 Assets/Mirror/Runtime/NetworkServer.cs        |  2 +
 .../Tests/Editor/NetworkIdentityTests.cs      | 48 +++++++++++++++++++
 4 files changed, 76 insertions(+)

diff --git a/Assets/Mirror/Runtime/NetworkBehaviour.cs b/Assets/Mirror/Runtime/NetworkBehaviour.cs
index 9cbba985c..0f9b66399 100644
--- a/Assets/Mirror/Runtime/NetworkBehaviour.cs
+++ b/Assets/Mirror/Runtime/NetworkBehaviour.cs
@@ -840,6 +840,12 @@ public virtual void OnStopClient()
         /// </summary>
         public virtual void OnStartServer() { }
 
+        /// <summary>
+        /// Invoked on the server when the object is unspawned
+        /// <para>Useful for saving object data in persistant storage</para>
+        /// </summary>
+        public virtual void OnStopServer() { }
+
         /// <summary>
         /// Called on every NetworkBehaviour when it is activated on a client.
         /// <para>Objects on the host have this function called, as there is a local client on the host. The values of SyncVars on object are guaranteed to be initialized correctly with the latest state from the server when this function is called on the client.</para>
diff --git a/Assets/Mirror/Runtime/NetworkIdentity.cs b/Assets/Mirror/Runtime/NetworkIdentity.cs
index 71ee55f68..cdb7c6039 100644
--- a/Assets/Mirror/Runtime/NetworkIdentity.cs
+++ b/Assets/Mirror/Runtime/NetworkIdentity.cs
@@ -598,6 +598,26 @@ internal void OnStartServer()
             }
         }
 
+        internal void OnStopServer()
+        {
+            foreach (NetworkBehaviour comp in NetworkBehaviours)
+            {
+                // an exception in OnStartServer should be caught, so that one
+                // component's exception doesn't stop all other components from
+                // being initialized
+                // => this is what Unity does for Start() etc. too.
+                //    one exception doesn't stop all the other Start() calls!
+                try
+                {
+                    comp.OnStopServer();
+                }
+                catch (Exception e)
+                {
+                    Debug.LogError("Exception in OnStopServer:" + e.Message + " " + e.StackTrace);
+                }
+            }
+        }
+
         bool clientStarted;
         internal void OnStartClient()
         {
diff --git a/Assets/Mirror/Runtime/NetworkServer.cs b/Assets/Mirror/Runtime/NetworkServer.cs
index f57ea9927..798ead6cc 100644
--- a/Assets/Mirror/Runtime/NetworkServer.cs
+++ b/Assets/Mirror/Runtime/NetworkServer.cs
@@ -1094,6 +1094,8 @@ static void DestroyObject(NetworkIdentity identity, bool destroyServerObject)
                 identity.OnStopClient();
             }
 
+            identity.OnStopServer();
+
             // when unspawning, dont destroy the server's object
             if (destroyServerObject)
             {
diff --git a/Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs b/Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs
index 8d79120e9..0e3219285 100644
--- a/Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs
+++ b/Assets/Mirror/Tests/Editor/NetworkIdentityTests.cs
@@ -105,6 +105,22 @@ class NetworkDestroyCalledNetworkBehaviour : NetworkBehaviour
             public override void OnStopClient() { ++called; }
         }
 
+        class StopServerCalledNetworkBehaviour : NetworkBehaviour
+        {
+            public int called;
+            public override void OnStopServer() { ++called; }
+        }
+
+        class StopServerExceptionNetworkBehaviour : NetworkBehaviour
+        {
+            public int called;
+            public override void OnStopServer()
+            {
+                ++called;
+                throw new Exception("some exception");
+            }
+        }
+
         class SetHostVisibilityExceptionNetworkBehaviour : NetworkVisibility
         {
             public int called;
@@ -1038,6 +1054,38 @@ public void OnNetworkDestroy()
             Assert.That(comp.called, Is.EqualTo(1));
         }
 
+        [Test]
+        public void OnStopServer()
+        {
+            // add components
+            StopServerCalledNetworkBehaviour comp = gameObject.AddComponent<StopServerCalledNetworkBehaviour>();
+
+            // make sure our test values are set to 0
+            Assert.That(comp.called, Is.EqualTo(0));
+
+            identity.OnStopServer();
+            Assert.That(comp.called, Is.EqualTo(1));
+        }
+
+        [Test]
+        public void OnStopServerEx()
+        {
+            // add components
+            StopServerExceptionNetworkBehaviour compEx = gameObject.AddComponent<StopServerExceptionNetworkBehaviour>();
+
+            // make sure our test values are set to 0
+            Assert.That(compEx.called, Is.EqualTo(0));
+
+            // call OnNetworkDestroy in identity
+            // one component will throw an exception, but that shouldn't stop
+            // OnNetworkDestroy from being called in the second one
+            // exception will log an error
+            LogAssert.ignoreFailingMessages = true;
+            identity.OnStopServer();
+            LogAssert.ignoreFailingMessages = false;
+            Assert.That(compEx.called, Is.EqualTo(1));
+        }
+
         [Test]
         public void AddObserver()
         {