fix: OnNetworkDespawn not called during shutdown (#1390)

com.unity.netcode.gameobjects/Components/NetworkRigidbody2D.cs

@@ -32,7 +32,7 @@ private void Awake()
 
         private void FixedUpdate()
         {
-            if (NetworkManager.IsListening)
+            if (IsSpawned)
             {
                 if (HasAuthority != m_IsAuthority)
                 {
@@ -74,7 +74,6 @@ public override void OnNetworkSpawn()
         /// <inheritdoc />
         public override void OnNetworkDespawn()
         {
-            m_IsAuthority = false;
             UpdateRigidbodyKinematicMode();
         }
     }

com.unity.netcode.gameobjects/Components/NetworkTransform.cs

@@ -807,7 +807,7 @@ private void SetStateServerRpc(Vector3 pos, Quaternion rot, Vector3 scale, bool
         // conditional to users only making transform update changes in FixedUpdate.
         protected virtual void Update()
         {
-            if (!NetworkObject.IsSpawned)
+            if (!IsSpawned)
             {
                 return;
             }

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

@@ -282,9 +282,13 @@ public NetworkObject NetworkObject
                     m_NetworkObject = GetComponentInParent<NetworkObject>();
                 }
 
-                if (m_NetworkObject == null && NetworkLog.CurrentLogLevel <= LogLevel.Normal)
+                if (m_NetworkObject == null || NetworkManager.Singleton == null ||
+                    (NetworkManager.Singleton != null && !NetworkManager.Singleton.ShutdownInProgress))
                 {
-                    NetworkLog.LogWarning($"Could not get {nameof(NetworkObject)} for the {nameof(NetworkBehaviour)}. Are you missing a {nameof(NetworkObject)} component?");
+                    if (NetworkLog.CurrentLogLevel < LogLevel.Normal)
+                    {
+                        NetworkLog.LogWarning($"Could not get {nameof(NetworkObject)} for the {nameof(NetworkBehaviour)}. Are you missing a {nameof(NetworkObject)} component?");
+                    }
                 }
 
                 return m_NetworkObject;

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

@@ -336,6 +336,9 @@ public IReadOnlyList<ulong> ConnectedClientsIds
         /// </summary>
         public bool IsConnectedClient { get; internal set; }
 
+
+        public bool ShutdownInProgress { get { return m_ShuttingDown; } }
+
         /// <summary>
         /// The callback to invoke once a client connects. This callback is only ran on the server and on the local client that connects.
         /// </summary>
@@ -1098,7 +1101,7 @@ internal void ShutdownInternal()
             if (SpawnManager != null)
             {
                 SpawnManager.CleanupAllTriggers();
-                SpawnManager.DestroyNonSceneObjects();
+                SpawnManager.DespawnAndDestroyNetworkObjects();
                 SpawnManager.ServerResetShudownStateForSceneObjects();
 
                 SpawnManager = null;

com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs

@@ -225,7 +225,7 @@ private bool TryGetNetworkClient(ulong clientId, out NetworkClient networkClient
                 return NetworkManager.ConnectedClients.TryGetValue(clientId, out networkClient);
             }
 
-            if (clientId == NetworkManager.LocalClient.ClientId)
+            if (NetworkManager.LocalClient != null && clientId == NetworkManager.LocalClient.ClientId)
             {
                 networkClient = NetworkManager.LocalClient;
                 return true;
@@ -598,25 +598,33 @@ internal void ServerDestroySpawnedSceneObjects()
             }
         }
 
-        internal void DestroyNonSceneObjects()
+        internal void DespawnAndDestroyNetworkObjects()
         {
             var networkObjects = UnityEngine.Object.FindObjectsOfType<NetworkObject>();
 
             for (int i = 0; i < networkObjects.Length; i++)
             {
                 if (networkObjects[i].NetworkManager == NetworkManager)
                 {
-                    if (networkObjects[i].IsSceneObject != null && networkObjects[i].IsSceneObject.Value == false)
+                    if (NetworkManager.PrefabHandler.ContainsHandler(networkObjects[i]))
                     {
-                        if (NetworkManager.PrefabHandler.ContainsHandler(networkObjects[i]))
-                        {
-                            NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(networkObjects[i]);
-                            OnDespawnObject(networkObjects[i], false);
-                        }
-                        else
-                        {
-                            UnityEngine.Object.Destroy(networkObjects[i].gameObject);
-                        }
+                        OnDespawnObject(networkObjects[i], false);
+                        // Leave destruction up to the handler
+                        NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(networkObjects[i]);
+                    }
+                    else if (networkObjects[i].IsSpawned)
+                    {
+                        // If it is an in-scene placed NetworkObject then just despawn
+                        // and let it be destroyed when the scene is unloaded.  Otherwise,
+                        // despawn and destroy it.
+                        var shouldDestroy = !(networkObjects[i].IsSceneObject != null
+                                    && networkObjects[i].IsSceneObject.Value);
+
+                        OnDespawnObject(networkObjects[i], shouldDestroy);
+                    }
+                    else
+                    {
+                        UnityEngine.Object.Destroy(networkObjects[i].gameObject);
                     }
                 }
             }
@@ -686,17 +694,21 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec
                 return;
             }
 
-            // Move child NetworkObjects to the root when parent NetworkObject is destroyed
-            foreach (var spawnedNetObj in SpawnedObjectsList)
+            // If we are shutting down the NetworkManager, then ignore resetting the parent
+            if (!NetworkManager.ShutdownInProgress)
             {
-                var (isReparented, latestParent) = spawnedNetObj.GetNetworkParenting();
-                if (isReparented && latestParent == networkObject.NetworkObjectId)
+                // Move child NetworkObjects to the root when parent NetworkObject is destroyed
+                foreach (var spawnedNetObj in SpawnedObjectsList)
                 {
-                    spawnedNetObj.gameObject.transform.parent = null;
-
-                    if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
+                    var (isReparented, latestParent) = spawnedNetObj.GetNetworkParenting();
+                    if (isReparented && latestParent == networkObject.NetworkObjectId)
                     {
-                        NetworkLog.LogWarning($"{nameof(NetworkObject)} #{spawnedNetObj.NetworkObjectId} moved to the root because its parent {nameof(NetworkObject)} #{networkObject.NetworkObjectId} is destroyed");
+                        spawnedNetObj.gameObject.transform.parent = null;
+
+                        if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
+                        {
+                            NetworkLog.LogWarning($"{nameof(NetworkObject)} #{spawnedNetObj.NetworkObjectId} moved to the root because its parent {nameof(NetworkObject)} #{networkObject.NetworkObjectId} is destroyed");
+                        }
                     }
                 }
             }

com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOnNetworkDespawnTests.cs

@@ -0,0 +1,132 @@
+using System.Collections;
+using NUnit.Framework;
+using UnityEngine;
+using UnityEngine.TestTools;
+
+
+namespace Unity.Netcode.RuntimeTests
+{
+    /// <summary>
+    /// Tests that check OnNetworkDespawn being invoked
+    /// </summary>
+    public class NetworkObjectOnNetworkDespawnTests
+    {
+        private NetworkManager m_ServerHost;
+        private NetworkManager[] m_Clients;
+
+        private GameObject m_ObjectToSpawn;
+        private NetworkObject m_NetworkObject;
+
+        internal class OnNetworkDespawnTestComponent : NetworkBehaviour
+        {
+            public bool OnNetworkDespawnCalled { get; internal set; }
+
+            public override void OnNetworkSpawn()
+            {
+                OnNetworkDespawnCalled = false;
+                base.OnNetworkSpawn();
+            }
+
+            public override void OnNetworkDespawn()
+            {
+                OnNetworkDespawnCalled = true;
+                base.OnNetworkDespawn();
+            }
+        }
+
+        [UnitySetUp]
+        public IEnumerator Setup()
+        {
+            Assert.IsTrue(MultiInstanceHelpers.Create(1, out m_ServerHost, out m_Clients));
+
+            m_ObjectToSpawn = new GameObject();
+            m_NetworkObject = m_ObjectToSpawn.AddComponent<NetworkObject>();
+            m_ObjectToSpawn.AddComponent<OnNetworkDespawnTestComponent>();
+
+            // Make it a prefab
+            MultiInstanceHelpers.MakeNetworkObjectTestPrefab(m_NetworkObject);
+
+            var networkPrefab = new NetworkPrefab();
+            networkPrefab.Prefab = m_ObjectToSpawn;
+            m_ServerHost.NetworkConfig.NetworkPrefabs.Add(networkPrefab);
+
+            foreach (var client in m_Clients)
+            {
+                client.NetworkConfig.NetworkPrefabs.Add(networkPrefab);
+            }
+
+            yield return null;
+        }
+
+        [UnityTearDown]
+        public IEnumerator Teardown()
+        {
+            // Shutdown and clean up both of our NetworkManager instances
+            if (m_ObjectToSpawn)
+            {
+                Object.Destroy(m_ObjectToSpawn);
+                m_ObjectToSpawn = null;
+            }
+            MultiInstanceHelpers.Destroy();
+            yield return null;
+        }
+
+        public enum InstanceType
+        {
+            Server,
+            Host,
+            Client
+        }
+
+        /// <summary>
+        /// Tests that a spawned NetworkObject's associated NetworkBehaviours will have
+        /// their OnNetworkDespawn invoked during NetworkManager shutdown.
+        /// </summary>
+        [UnityTest]
+        public IEnumerator TestNetworkObjectDespawnOnShutdown([Values(InstanceType.Server, InstanceType.Host, InstanceType.Client)] InstanceType despawnCheck)
+        {
+            var useHost = despawnCheck == InstanceType.Server ? false : true;
+            var networkManager = despawnCheck == InstanceType.Host || despawnCheck == InstanceType.Server ? m_ServerHost : m_Clients[0];
+
+            // Start the instances
+            if (!MultiInstanceHelpers.Start(useHost, m_ServerHost, m_Clients))
+            {
+                Debug.LogError("Failed to start instances");
+                Assert.Fail("Failed to start instances");
+            }
+
+            // [Client-Side] Wait for a connection to the server
+            yield return MultiInstanceHelpers.WaitForClientsConnected(m_Clients, null, 512);
+
+            // [Host-Server-Side] Check to make sure all clients are connected
+            var clientCount = useHost ? m_Clients.Length + 1 : m_Clients.Length;
+            yield return MultiInstanceHelpers.WaitForClientsConnectedToServer(m_ServerHost, clientCount, null, 512);
+
+            // Spawn the test object
+            var spawnedObject = Object.Instantiate(m_NetworkObject);
+            var spawnedNetworkObject = spawnedObject.GetComponent<NetworkObject>();
+            spawnedNetworkObject.NetworkManagerOwner = m_ServerHost;
+            spawnedNetworkObject.Spawn(true);
+
+            // Get the spawned object relative to which NetworkManager instance we are testing.
+            var relativeSpawnedObject = new MultiInstanceHelpers.CoroutineResultWrapper<NetworkObject>();
+            yield return MultiInstanceHelpers.Run(MultiInstanceHelpers.GetNetworkObjectByRepresentation((x => x.GetComponent<OnNetworkDespawnTestComponent>() != null), networkManager, relativeSpawnedObject));
+            var onNetworkDespawnTestComponent = relativeSpawnedObject.Result.GetComponent<OnNetworkDespawnTestComponent>();
+
+            // Confirm it is not set before shutting down the NetworkManager
+            Assert.IsFalse(onNetworkDespawnTestComponent.OnNetworkDespawnCalled);
+
+            // Shutdown the NetworkManager instance we are testing.
+            networkManager.Shutdown();
+
+            // Since shutdown is now delayed until the post frame update
+            // just wait 2 frames before checking to see if OnNetworkDespawnCalled is true
+            var currentFrame = Time.frameCount + 2;
+            yield return new WaitUntil(() => Time.frameCount <= currentFrame);
+
+            // Confirm that OnNetworkDespawn is invoked after shutdown
+            Assert.IsTrue(onNetworkDespawnTestComponent.OnNetworkDespawnCalled);
+        }
+    }
+}
+

com.unity.netcode.gameobjects/Tests/Runtime/Physics/NetworkRigidbody2DTest.cs

@@ -70,6 +70,7 @@ public IEnumerator TestRigidbodyKinematicEnableDisable()
 
             yield return NetworkRigidbodyTestBase.WaitForFrames(5);
 
+            // This should equal Kinematic
             Assert.IsTrue(serverPlayer.GetComponent<Rigidbody2D>().isKinematic == Kinematic);
 
             yield return NetworkRigidbodyTestBase.WaitForFrames(5);