fix: NetworkObject.DeferDespawn not respecting DestroyGameObject parameter

Author: EmandM

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -12,6 +12,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed `NetworkObject.DeferDespawn` to respect the `DestroyGameObject` parameter.
 - Changed the `NetworkTimeSystem.Sync` method to use half RTT to calculate the desired local time offset as opposed to the full RTT. (#3212)
 - Fixed issue where a spawned `NetworkObject` that was registered with a prefab handler and owned by a client would invoke destroy more than once on the host-server side if the client disconnected while the `NetworkObject` was still spawned. (#3200)
 

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/DestroyObjectMessage.cs

@@ -150,7 +150,7 @@ public void Handle(ref NetworkContext context)
                     {
                         networkObject.DeferredDespawnTick = DeferredDespawnTick;
                         var hasCallback = networkObject.OnDeferredDespawnComplete != null;
-                        networkManager.SpawnManager.DeferDespawnNetworkObject(NetworkObjectId, DeferredDespawnTick, hasCallback);
+                        networkManager.SpawnManager.DeferDespawnNetworkObject(NetworkObjectId, DeferredDespawnTick, hasCallback, DestroyGameObject);
                         return;
                     }
                 }

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

@@ -1450,7 +1450,7 @@ internal void ServerSpawnSceneObjectsOnStartSweep()
             }
 
             // Since we are spawing in-scene placed NetworkObjects for already loaded scenes,
-            // we need to add any in-scene placed NetworkObject to our tracking table 
+            // we need to add any in-scene placed NetworkObject to our tracking table
             var clearFirst = true;
             foreach (var sceneLoaded in NetworkManager.SceneManager.ScenesLoaded)
             {
@@ -1582,6 +1582,12 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec
                     }
                 }
 
+                if (networkObject.IsSceneObject == false && destroyGameObject == false)
+                {
+                    // DANGO-TODO: Check that this is still a valid restriction
+                    Debug.LogWarning("Only Scene Objects are valid to not be destroyed when despawned");
+                }
+
                 if (m_TargetClientIds.Count > 0 && !NetworkManager.ShutdownInProgress)
                 {
                     var message = new DestroyObjectMessage
@@ -1920,6 +1926,7 @@ internal struct DeferredDespawnObject
         {
             public int TickToDespawn;
             public bool HasDeferredDespawnCheck;
+            public bool DestroyGameObject;
             public ulong NetworkObjectId;
         }
 
@@ -1931,12 +1938,13 @@ internal struct DeferredDespawnObject
         /// <param name="networkObjectId">associated NetworkObject</param>
         /// <param name="tickToDespawn">when to despawn the NetworkObject</param>
         /// <param name="hasDeferredDespawnCheck">if true, user script is to be invoked to determine when to despawn</param>
-        internal void DeferDespawnNetworkObject(ulong networkObjectId, int tickToDespawn, bool hasDeferredDespawnCheck)
+        internal void DeferDespawnNetworkObject(ulong networkObjectId, int tickToDespawn, bool hasDeferredDespawnCheck, bool destroyGameObject)
         {
             var deferredDespawnObject = new DeferredDespawnObject()
             {
                 TickToDespawn = tickToDespawn,
                 HasDeferredDespawnCheck = hasDeferredDespawnCheck,
+                DestroyGameObject = destroyGameObject,
                 NetworkObjectId = networkObjectId,
             };
             DeferredDespawnObjects.Add(deferredDespawnObject);
@@ -2001,7 +2009,7 @@ internal void DeferredDespawnUpdate(NetworkTime serverTime)
                 }
                 var networkObject = SpawnedObjects[deferredObjectEntry.NetworkObjectId];
                 // Local instance despawns the instance
-                OnDespawnObject(networkObject, true);
+                OnDespawnObject(networkObject, deferredObjectEntry.DestroyGameObject);
                 DeferredDespawnObjects.Remove(deferredObjectEntry);
             }
         }

com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/DeferredDespawningTests.cs

@@ -9,15 +9,25 @@
 
 namespace Unity.Netcode.RuntimeTests
 {
+    [TestFixture(SetDestroyGameObject.DestroyGameObject)]
+    [TestFixture(SetDestroyGameObject.DontDestroyGameObject)]
     internal class DeferredDespawningTests : IntegrationTestWithApproximation
     {
         private const int k_DaisyChainedCount = 5;
         protected override int NumberOfClients => 2;
         private List<GameObject> m_DaisyChainedDespawnObjects = new List<GameObject>();
         private List<ulong> m_HasReachedEnd = new List<ulong>();
 
-        public DeferredDespawningTests() : base(HostOrServer.DAHost)
+        public enum SetDestroyGameObject
         {
+            DestroyGameObject,
+            DontDestroyGameObject,
+        }
+        private bool m_DestroyGameObject;
+
+        public DeferredDespawningTests(SetDestroyGameObject destroyGameObject) : base(HostOrServer.DAHost)
+        {
+            m_DestroyGameObject = destroyGameObject == SetDestroyGameObject.DestroyGameObject;
         }
 
         protected override void OnServerAndClientsCreated()
@@ -45,12 +55,28 @@ protected override void OnServerAndClientsCreated()
         [UnityTest]
         public IEnumerator DeferredDespawning()
         {
+            // Setup for test
+            DeferredDespawnDaisyChained.DestroyGameObject = m_DestroyGameObject;
             DeferredDespawnDaisyChained.EnableVerbose = m_EnableVerboseDebug;
+            DeferredDespawnDaisyChained.ClientRelativeInstances = new Dictionary<ulong, Dictionary<ulong, DeferredDespawnDaisyChained>>();
+
+            // Spawn the initial object
             var rootInstance = SpawnObject(m_DaisyChainedDespawnObjects[0], m_ServerNetworkManager);
             DeferredDespawnDaisyChained.ReachedLastChainInstance = ReachedLastChainObject;
+
+            // Wait for the chain of objects to spawn and despawn
             var timeoutHelper = new TimeoutHelper(300);
             yield return WaitForConditionOrTimeOut(HaveAllClientsReachedEndOfChain, timeoutHelper);
             AssertOnTimeout($"Timed out waiting for all children to reach the end of their chained deferred despawns!", timeoutHelper);
+
+            if (m_DestroyGameObject)
+            {
+                Assert.IsTrue(rootInstance == null); // Assert.IsNull doesn't work here
+            }
+            else
+            {
+                Assert.IsTrue(rootInstance != null); // Assert.IsNotNull doesn't work here
+            }
         }
 
         private bool HaveAllClientsReachedEndOfChain()
@@ -88,9 +114,10 @@ private void ReachedLastChainObject(ulong clientId)
     internal class DeferredDespawnDaisyChained : NetworkBehaviour
     {
         public static bool EnableVerbose;
+        public static bool DestroyGameObject;
         public static Action<ulong> ReachedLastChainInstance;
         private const int k_StartingDeferTick = 4;
-        public static Dictionary<ulong, Dictionary<ulong, DeferredDespawnDaisyChained>> ClientRelativeInstances = new Dictionary<ulong, Dictionary<ulong, DeferredDespawnDaisyChained>>();
+        public static Dictionary<ulong, Dictionary<ulong, DeferredDespawnDaisyChained>> ClientRelativeInstances;
         public bool IsRoot;
         public GameObject PrefabToSpawnWhenDespawned;
         public bool WasContactedByPeviousChainMember { get; private set; }
@@ -182,7 +209,7 @@ private void InvokeDespawn()
             {
                 FailTest($"[{nameof(InvokeDespawn)}] Client is not the authority but this was invoked (integration test logic issue)!");
             }
-            NetworkObject.DeferDespawn(DeferDespawnTick);
+            NetworkObject.DeferDespawn(DeferDespawnTick, DestroyGameObject);
         }
 
         public override void OnDeferringDespawn(int despawnTick)
@@ -241,7 +268,7 @@ private void Update()
                     continue;
                 }
 
-                // This should happen shortly afte the instances spawns (based on the deferred despawn count)
+                // This should happen shortly after the instances spawn (based on the deferred despawn count)
                 if (!IsRoot && !ClientRelativeInstances[clientId][NetworkObjectId].WasContactedByPeviousChainMember)
                 {
                     // exit early if the non-authority instance has not been contacted yet

testproject/Assets/Tests/Runtime/NetworkSceneManager/InScenePlacedNetworkObjectTests.cs

@@ -53,6 +53,12 @@ protected override bool CanStartServerAndClients()
             return m_CanStartServerAndClients;
         }
 
+        public enum DespawnMode
+        {
+            Despawn,
+            DeferredDespawn,
+        }
+
         /// <summary>
         /// This verifies that in-scene placed NetworkObjects will be properly
         /// synchronized if:
@@ -61,8 +67,13 @@ protected override bool CanStartServerAndClients()
         /// NetworkObject as a NetworkPrefab
         /// </summary>
         [UnityTest]
-        public IEnumerator InSceneNetworkObjectSynchAndSpawn()
+        public IEnumerator InSceneNetworkObjectSynchAndSpawn([Values] DespawnMode despawnMode)
         {
+            if (!m_DistributedAuthority && despawnMode == DespawnMode.DeferredDespawn)
+            {
+                Assert.Ignore("Deferred Despawn is only valid with Distributed Authority mode.");
+            }
+
             NetworkObjectTestComponent.VerboseDebug = true;
             // Because despawning a client will cause it to shutdown and clean everything in the
             // scene hierarchy, we have to prevent one of the clients from spawning initially before
@@ -99,7 +110,16 @@ public IEnumerator InSceneNetworkObjectSynchAndSpawn()
 
             // Despawn the in-scene placed NetworkObject
             Debug.Log("Despawning In-Scene placed NetworkObject");
-            serverObject.Despawn(false);
+
+            if (despawnMode == DespawnMode.Despawn)
+            {
+                serverObject.Despawn(false);
+            }
+            else
+            {
+                serverObject.DeferDespawn(1, false);
+            }
+
             yield return WaitForConditionOrTimeOut(() => NetworkObjectTestComponent.SpawnedInstances.Count == 0);
             AssertOnTimeout($"Timed out waiting for all in-scene instances to be despawned!  Current spawned count: {NetworkObjectTestComponent.SpawnedInstances.Count()}");
 
@@ -130,6 +150,12 @@ public IEnumerator InSceneNetworkObjectSynchAndSpawn()
             yield return WaitForConditionOrTimeOut(() => NetworkObjectTestComponent.SpawnedInstances.Count == clientCount);
             AssertOnTimeout($"Timed out waiting for all in-scene instances to be spawned!  Current spawned count: {NetworkObjectTestComponent.SpawnedInstances.Count()} | Expected spawn count: {clientCount}");
 
+            if (despawnMode == DespawnMode.DeferredDespawn)
+            {
+                // TODO: Check if this is the expected behavior
+                serverObject.DeferredDespawnTick = 0;
+            }
+
             // Test NetworkHide on the first client
             var firstClientId = m_ClientNetworkManagers[0].LocalClientId;