Merge branch 'develop-2.0.0' of https://github.com/Unity-Technologies/com.unity.netcode.gameobjects into fix/mttb-959-exception-with-disabled-gameobjects

EmandM · EmandM · commit 2456a823b8b3 · 2025-02-07T14:17:45.000-05:00
diff --git a/.github/workflows/renovate-validation.yml b/.github/workflows/renovate-validation.yml
@@ -0,0 +1,38 @@
+# This workflow is for validating the Renovate configuration and docker image
+# updates for it.
+name: Renovate Validation
+on:
+  workflow_dispatch:
+    inputs:
+      log-level:
+        type: choice
+        description: Select log level for Renovate
+        options:
+          - trace
+          - debug
+          - info
+          - warn
+          - error
+        default: info
+        required: false
+  pull_request:
+    paths:
+      # we trigger validation on any changes to the renovate workflow files
+      - .github/workflows/renovate*.yml
+      # as well as for any possible location for the renovate config file
+      - .github/renovate.json?
+
+
+jobs:
+  renovate-validation:
+    # The reusable workflow will be updated by renovate if there's a new version
+    uses: Unity-Technologies/renovate-workflows/.github/workflows/run.yml@v5.0.0
+    with:
+      # This is the image that contains our custom renovate and will be auto
+      # updated by Renovate itself.
+      image: europe-docker.pkg.dev/unity-cds-services-prd/ds-docker/renovate:10.1.3@sha256:fdeed7bb524bd67611eb91ee1a5e990c8c73ed62c84a0cd5ef66c87eb5fd0d70
+      dry-run: full
+      log-level: ${{ github.event.inputs.log-level }}
+    secrets:
+      renovate-auth-secret: ${{ secrets.RENOVATE_AUTH_SECRET }}
+      github-com-token: ${{ secrets.GH_COM_TOKEN }}
diff --git a/.github/workflows/renovate.yml b/.github/workflows/renovate.yml
@@ -0,0 +1,33 @@
+# This workflow runs Renovate against the current repo and will create PRs with outdated dependencies.
+name: Renovate
+
+on:
+  workflow_dispatch:
+    inputs:
+      log-level:
+        type: choice
+        description: Select log level for Renovate
+        options:
+          - trace
+          - debug
+          - info
+          - warn
+          - error
+        default: info
+        required: false
+  schedule:
+    # Every 6 hours at the 6th minute.
+    - cron: '06 */6 * * *'
+
+jobs:
+  renovate:
+    # The reusable workflow will be updated by renovate if there's a new version
+    uses: Unity-Technologies/renovate-workflows/.github/workflows/run.yml@v5.0.0
+    with:
+      # This is the image that contains our custom renovate and will be auto
+      # updated by Renovate itself.
+      image: europe-docker.pkg.dev/unity-cds-services-prd/ds-docker/renovate:10.1.3@sha256:fdeed7bb524bd67611eb91ee1a5e990c8c73ed62c84a0cd5ef66c87eb5fd0d70
+      log-level: ${{ github.event.inputs.log-level }}
+    secrets:
+      renovate-auth-secret: ${{ secrets.RENOVATE_AUTH_SECRET }}
+      github-com-token: ${{ secrets.GH_COM_TOKEN }}
diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md
@@ -13,6 +13,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Fixed
 
 - Fixed exception being thrown when a `GameObject` with an associated `NetworkTransform` is disabled. (#3243)
+- Fixed `NetworkObject.DeferDespawn` to respect the `DestroyGameObject` parameter. (#3219)
 - 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)
 - Fixed issue where `NetworkVariableBase` derived classes were not being re-initialized if the associated `NetworkObject` instance was not destroyed and re-spawned. (#3181)
diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/DestroyObjectMessage.cs b/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;
                     }
                 }
diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/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)
             {
@@ -1588,6 +1588,7 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec
                     {
                         NetworkObjectId = networkObject.NetworkObjectId,
                         DeferredDespawnTick = networkObject.DeferredDespawnTick,
+                        // DANGO-TODO: Reconfigure Client-side object destruction on despawn
                         DestroyGameObject = networkObject.IsSceneObject != false ? destroyGameObject : true,
                         IsTargetedDestroy = false,
                         IsDistributedAuthority = distributedAuthority,
@@ -1601,6 +1602,7 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec
             }
 
             networkObject.IsSpawned = false;
+            networkObject.DeferredDespawnTick = 0;
 
             if (SpawnedObjects.Remove(networkObject.NetworkObjectId))
             {
@@ -1920,6 +1922,7 @@ internal struct DeferredDespawnObject
         {
             public int TickToDespawn;
             public bool HasDeferredDespawnCheck;
+            public bool DestroyGameObject;
             public ulong NetworkObjectId;
         }
 
@@ -1931,12 +1934,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 +2005,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);
             }
         }
diff --git a/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs b/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs
@@ -1220,7 +1220,13 @@ public override void DisconnectLocalClient()
         /// <param name="clientId">The client to disconnect</param>
         public override void DisconnectRemoteClient(ulong clientId)
         {
-            Debug.Assert(m_State == State.Listening, "DisconnectRemoteClient should be called on a listening server");
+#if DEBUG
+            if (m_State != State.Listening)
+            {
+                Debug.LogWarning($"{nameof(DisconnectRemoteClient)} should only be called on a listening server!");
+                return;
+            }
+#endif
 
             if (m_State == State.Listening)
             {
@@ -1292,7 +1298,13 @@ public NetworkEndpoint GetEndpoint(ulong clientId)
         /// <param name="networkManager">The NetworkManager that initialized and owns the transport</param>
         public override void Initialize(NetworkManager networkManager = null)
         {
-            Debug.Assert(sizeof(ulong) == UnsafeUtility.SizeOf<NetworkConnection>(), "Netcode connection id size does not match UTP connection id size");
+#if DEBUG
+            if (sizeof(ulong) != UnsafeUtility.SizeOf<NetworkConnection>())
+            {
+                Debug.LogWarning($"Netcode connection id size {sizeof(ulong)} does not match UTP connection id size {UnsafeUtility.SizeOf<NetworkConnection>()}!");
+                return;
+            }
+#endif
 
             m_NetworkManager = networkManager;
 
diff --git a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs
@@ -1156,6 +1156,9 @@ protected void ShutdownAndCleanUp()
             // reset the m_ServerWaitForTick for the next test to initialize
             s_DefaultWaitForTick = new WaitForSecondsRealtime(1.0f / k_DefaultTickRate);
             VerboseDebug($"Exiting {nameof(ShutdownAndCleanUp)}");
+
+            // Assure any remaining NetworkManagers are destroyed
+            DestroyNetworkManagers();
         }
 
         protected IEnumerator CoroutineShutdownAndCleanUp()
@@ -1195,6 +1198,9 @@ protected IEnumerator CoroutineShutdownAndCleanUp()
             // reset the m_ServerWaitForTick for the next test to initialize
             s_DefaultWaitForTick = new WaitForSecondsRealtime(1.0f / k_DefaultTickRate);
             VerboseDebug($"Exiting {nameof(ShutdownAndCleanUp)}");
+
+            // Assure any remaining NetworkManagers are destroyed
+            DestroyNetworkManagers();
         }
 
         /// <summary>
@@ -1244,6 +1250,19 @@ public IEnumerator TearDown()
             VerboseDebug($"Exiting {nameof(TearDown)}");
             LogWaitForMessages();
             NetcodeLogAssert.Dispose();
+
+        }
+
+        /// <summary>
+        /// Destroys any remaining NetworkManager instances
+        /// </summary>
+        private void DestroyNetworkManagers()
+        {
+            var networkManagers = Object.FindObjectsByType<NetworkManager>(FindObjectsSortMode.None);
+            foreach (var networkManager in networkManagers)
+            {
+                Object.DestroyImmediate(networkManager.gameObject);
+            }
         }
 
         /// <summary>
diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/DeferredDespawningTests.cs b/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
diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTests.cs
@@ -531,7 +531,7 @@ public IEnumerator DoesNotActAfterShutdown([Values] AfterShutdownAction afterShu
             {
                 m_Server.DisconnectRemoteClient(m_Client1.ServerClientId);
 
-                LogAssert.Expect(LogType.Assert, "DisconnectRemoteClient should be called on a listening server");
+                LogAssert.Expect(LogType.Warning, $"{nameof(UnityTransport.DisconnectRemoteClient)} should only be called on a listening server!");
             }
             else if (afterShutdownAction == AfterShutdownAction.DisconnectLocalClient)
             {
diff --git a/testproject/Assets/Tests/Runtime/NetworkSceneManager/InScenePlacedNetworkObjectTests.cs b/testproject/Assets/Tests/Runtime/NetworkSceneManager/InScenePlacedNetworkObjectTests.cs
@@ -53,6 +53,12 @@ protected override bool CanStartServerAndClients()
             return m_CanStartServerAndClients;
         }
 
+        public enum DespawnMode
+        {
+            Despawn,
+            DeferDespawn,
+        }
+
         /// <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.DeferDespawn)
+            {
+                Assert.Ignore($"Test ignored as DeferDespawn 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()}");
 

Original file line number	Diff line number	Diff line change
`@@ -150,7 +150,7 @@ public void Handle(ref NetworkContext context)`
`150`	`150`	`{`
`151`	`151`	`networkObject.DeferredDespawnTick = DeferredDespawnTick;`
`152`	`152`	`var hasCallback = networkObject.OnDeferredDespawnComplete != null;`
`153`		`- networkManager.SpawnManager.DeferDespawnNetworkObject(NetworkObjectId, DeferredDespawnTick, hasCallback);`
	`153`	`+ networkManager.SpawnManager.DeferDespawnNetworkObject(NetworkObjectId, DeferredDespawnTick, hasCallback, DestroyGameObject);`
`154`	`154`	`return;`
`155`	`155`	`}`
`156`	`156`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1450,7 +1450,7 @@ internal void ServerSpawnSceneObjectsOnStartSweep()`
`1450`	`1450`	`}`
`1451`	`1451`
`1452`	`1452`	`// Since we are spawing in-scene placed NetworkObjects for already loaded scenes,`
`1453`		`- // we need to add any in-scene placed NetworkObject to our tracking table`
	`1453`	`+ // we need to add any in-scene placed NetworkObject to our tracking table`
`1454`	`1454`	`var clearFirst = true;`
`1455`	`1455`	`foreach (var sceneLoaded in NetworkManager.SceneManager.ScenesLoaded)`
`1456`	`1456`	`{`
`@@ -1588,6 +1588,7 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec`
`1588`	`1588`	`{`
`1589`	`1589`	`NetworkObjectId = networkObject.NetworkObjectId,`
`1590`	`1590`	`DeferredDespawnTick = networkObject.DeferredDespawnTick,`
	`1591`	`+ // DANGO-TODO: Reconfigure Client-side object destruction on despawn`
`1591`	`1592`	`DestroyGameObject = networkObject.IsSceneObject != false ? destroyGameObject : true,`
`1592`	`1593`	`IsTargetedDestroy = false,`
`1593`	`1594`	`IsDistributedAuthority = distributedAuthority,`
`@@ -1601,6 +1602,7 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec`
`1601`	`1602`	`}`
`1602`	`1603`
`1603`	`1604`	`networkObject.IsSpawned = false;`
	`1605`	`+ networkObject.DeferredDespawnTick = 0;`
`1604`	`1606`
`1605`	`1607`	`if (SpawnedObjects.Remove(networkObject.NetworkObjectId))`
`1606`	`1608`	`{`
`@@ -1920,6 +1922,7 @@ internal struct DeferredDespawnObject`
`1920`	`1922`	`{`
`1921`	`1923`	`public int TickToDespawn;`
`1922`	`1924`	`public bool HasDeferredDespawnCheck;`
	`1925`	`+ public bool DestroyGameObject;`
`1923`	`1926`	`public ulong NetworkObjectId;`
`1924`	`1927`	`}`
`1925`	`1928`
`@@ -1931,12 +1934,13 @@ internal struct DeferredDespawnObject`
`1931`	`1934`	`/// <param name="networkObjectId">associated NetworkObject</param>`
`1932`	`1935`	`/// <param name="tickToDespawn">when to despawn the NetworkObject</param>`
`1933`	`1936`	`/// <param name="hasDeferredDespawnCheck">if true, user script is to be invoked to determine when to despawn</param>`
`1934`		`- internal void DeferDespawnNetworkObject(ulong networkObjectId, int tickToDespawn, bool hasDeferredDespawnCheck)`
	`1937`	`+ internal void DeferDespawnNetworkObject(ulong networkObjectId, int tickToDespawn, bool hasDeferredDespawnCheck, bool destroyGameObject)`
`1935`	`1938`	`{`
`1936`	`1939`	`var deferredDespawnObject = new DeferredDespawnObject()`
`1937`	`1940`	`{`
`1938`	`1941`	`TickToDespawn = tickToDespawn,`
`1939`	`1942`	`HasDeferredDespawnCheck = hasDeferredDespawnCheck,`
	`1943`	`+ DestroyGameObject = destroyGameObject,`
`1940`	`1944`	`NetworkObjectId = networkObjectId,`
`1941`	`1945`	`};`
`1942`	`1946`	`DeferredDespawnObjects.Add(deferredDespawnObject);`
`@@ -2001,7 +2005,7 @@ internal void DeferredDespawnUpdate(NetworkTime serverTime)`
`2001`	`2005`	`}`
`2002`	`2006`	`var networkObject = SpawnedObjects[deferredObjectEntry.NetworkObjectId];`
`2003`	`2007`	`// Local instance despawns the instance`
`2004`		`- OnDespawnObject(networkObject, true);`
	`2008`	`+ OnDespawnObject(networkObject, deferredObjectEntry.DestroyGameObject);`
`2005`	`2009`	`DeferredDespawnObjects.Remove(deferredObjectEntry);`
`2006`	`2010`	`}`
`2007`	`2011`	`}`
Original file line number	Diff line number	Diff line change
`@@ -531,7 +531,7 @@ public IEnumerator DoesNotActAfterShutdown([Values] AfterShutdownAction afterShu`
`531`	`531`	`{`
`532`	`532`	`m_Server.DisconnectRemoteClient(m_Client1.ServerClientId);`
`533`	`533`
`534`		`- LogAssert.Expect(LogType.Assert, "DisconnectRemoteClient should be called on a listening server");`
	`534`	`+ LogAssert.Expect(LogType.Warning, $"{nameof(UnityTransport.DisconnectRemoteClient)} should only be called on a listening server!");`
`535`	`535`	`}`
`536`	`536`	`else if (afterShutdownAction == AfterShutdownAction.DisconnectLocalClient)`
`537`	`537`	`{`