Merge branch 'develop-2.0.0' into PVP-151-fix

Author: michalChrobot

.github/CODEOWNERS

@@ -2,15 +2,10 @@
 # Order is important; the last matching pattern takes the most precedence.
 
 * @Unity-Technologies/multiplayer-sdk
-Profiling/ @Unity-Technologies/multiplayer-tools
-Metrics/ @Unity-Technologies/multiplayer-tools
-/com.unity.netcode.gameobjects/Runtime/Transports/ @Unity-Technologies/multiplayer-workflows
-/com.unity.netcode.gameobjects/Tests/Editor/Transports/ @Unity-Technologies/multiplayer-workflows
-/com.unity.netcode.gameobjects/Tests/Runtime/Transports/ @Unity-Technologies/multiplayer-workflows
-*.asmdef @chrispope @miniwolf
-package.json @chrispope @miniwolf 
-AssemblyInfo.cs @chrispope @miniwolf
-.editorconfig @chrispope @miniwolf
-.gitignore @chrispope @miniwolf
-.github/ @chrispope @miniwolf
-.yamato/ @chrispope @miniwolf
+*.asmdef @miniwolf @NoelStephensUnity @fluong6 @michalChrobot @EmandM
+package.json @miniwolf @NoelStephensUnity @fluong6 @michalChrobot @EmandM
+AssemblyInfo.cs @miniwolf @NoelStephensUnity @fluong6 @michalChrobot @EmandM
+.editorconfig @miniwolf @NoelStephensUnity @fluong6 @michalChrobot @EmandM
+.gitignore @miniwolf @NoelStephensUnity @fluong6 @michalChrobot @EmandM
+.github/ @miniwolf @NoelStephensUnity @fluong6 @michalChrobot @EmandM
+.yamato/ @miniwolf @NoelStephensUnity @fluong6 @michalChrobot @EmandM

.github/renovate.json5

@@ -7,15 +7,19 @@
     "local>Unity-Technologies/unity-renovate-config"
   ],
   "prConcurrentLimit": 100,
-  // Ignore commits produced by github actions workflows
+  // Ignore commits produced by GitHub actions workflows
   "gitIgnoredAuthors": ["githubaction@githubaction.com"],
   "ignorePaths": [
     "**/node_modules/**",
     // Don't renovate files in special folders using ~ as suffix
-    "**/*~/**"
+    "**/*~/**",
+    "com.unity.netcode.gameobjects/**/*",
+    "Examples/**/*",
+    "testproject/**/*",
+    "minimalproject/**/*",
+    "testproject-tools-integration/**/*"
   ],
   "packageRules": [
-
     // Run unity-upm-project and unity-upm-package only on weekends to reduce PR noise
     // Also ensure dependencies won't be downgraded when they don't exist in the public repositories
     {
@@ -27,13 +31,7 @@
       "schedule": [
         "every weekend"
       ],
-      "rollbackPrs": false
-    },
-
-    // Enable automerge for Bokken image updates
-    {
-      "matchDatasources": ["unity-bokken"],
-      "automerge": false,
-    },
+      "rollbackPrs": false,
+    }
   ],
 }

.github/workflows/conventional-pr.yml

@@ -18,7 +18,7 @@ jobs:
     steps:
       - name: semantic-pull-request
         # Internal Unity mirror available at jesseo/action-semantic-pull-request, but actions from private repos aren't supported, so continue to use the public one below
-        uses: amannn/action-semantic-pull-request@b7a9a97cb10fa6e1ae02647e718798175f6b1f1d
+        uses: amannn/action-semantic-pull-request@40166f00814508ec3201fc8595b393d451c8cd80
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         with:

Tools/CI/global.json

@@ -12,6 +12,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed DestroyObject flow on non-authority game clients. (#3291)
 - Fixed exception being thrown when a `GameObject` with an associated `NetworkTransform` is disabled. (#3243)
 - Fixed issue where the scene migration synchronization table was not cleaned up if the `GameObject` of a `NetworkObject` is destroyed before it should have been. (#3230)
 - Fixed issue where the scene migration synchronization table was not cleaned up upon `NetworkManager` shutting down. (#3230)

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -1,4 +1,5 @@
 using System.Linq;
+using System.Runtime.CompilerServices;
 
 namespace Unity.Netcode
 {
@@ -81,7 +82,7 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
 
             reader.ReadValueSafe(out DestroyGameObject);
 
-            if (!networkManager.SpawnManager.SpawnedObjects.TryGetValue(NetworkObjectId, out var networkObject))
+            if (!networkManager.SpawnManager.SpawnedObjects.ContainsKey(NetworkObjectId))
             {
                 // Client-Server mode we always defer where in distributed authority mode we only defer if it is not a targeted destroy
                 if (!networkManager.DistributedAuthorityMode || (networkManager.DistributedAuthorityMode && !IsTargetedDestroy))
@@ -95,80 +96,90 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
         public void Handle(ref NetworkContext context)
         {
             var networkManager = (NetworkManager)context.SystemOwner;
+            networkManager.SpawnManager.SpawnedObjects.TryGetValue(NetworkObjectId, out var networkObject);
 
-            var networkObject = (NetworkObject)null;
-            if (!networkManager.DistributedAuthorityMode)
+            // The DAHost needs to forward despawn messages to the other clients
+            if (networkManager.DAHost)
             {
-                // If this NetworkObject does not exist on this instance then exit early
-                if (!networkManager.SpawnManager.SpawnedObjects.TryGetValue(NetworkObjectId, out networkObject))
-                {
-                    return;
-                }
-            }
-            else
-            {
-                networkManager.SpawnManager.SpawnedObjects.TryGetValue(NetworkObjectId, out networkObject);
-                if (!networkManager.DAHost && networkObject == null)
+                HandleDAHostForwardMessage(context.SenderId, ref networkManager, networkObject);
+
+                // DAHost adds the object to the queue only if it is not a targeted destroy, or it is and the target is the DAHost client.
+                if (networkObject && DeferredDespawnTick > 0 && (!IsTargetedDestroy || (IsTargetedDestroy && TargetClientId == 0)))
                 {
-                    // If this NetworkObject does not exist on this instance then exit early
+                    HandleDeferredDespawn(ref networkManager, ref networkObject);
                     return;
                 }
             }
-            // DANGO-TODO: This is just a quick way to foward despawn messages to the remaining clients
-            if (networkManager.DistributedAuthorityMode && networkManager.DAHost)
+
+            // If this NetworkObject does not exist on this instance then exit early
+            if (!networkObject)
             {
-                var message = new DestroyObjectMessage
-                {
-                    NetworkObjectId = NetworkObjectId,
-                    DestroyGameObject = DestroyGameObject,
-                    IsDistributedAuthority = true,
-                    IsTargetedDestroy = IsTargetedDestroy,
-                    TargetClientId = TargetClientId, // Just always populate this value whether we write it or not
-                    DeferredDespawnTick = DeferredDespawnTick,
-                };
-                var ownerClientId = networkObject == null ? context.SenderId : networkObject.OwnerClientId;
-                var clientIds = networkObject == null ? networkManager.ConnectedClientsIds.ToList() : networkObject.Observers.ToList();
-
-                foreach (var clientId in clientIds)
+                if (networkManager.LogLevel <= LogLevel.Developer)
                 {
-                    if (clientId == networkManager.LocalClientId || clientId == ownerClientId)
-                    {
-                        continue;
-                    }
-                    networkManager.ConnectionManager.SendMessage(ref message, NetworkDelivery.ReliableSequenced, clientId);
+                    NetworkLog.LogWarning($"[{nameof(DestroyObjectMessage)}] Received destroy object message for NetworkObjectId ({NetworkObjectId}) on Client-{networkManager.LocalClientId}, but that {nameof(NetworkObject)} does not exist!");
                 }
+                return;
             }
 
-            // If we are deferring the despawn, then add it to the deferred despawn queue
             if (networkManager.DistributedAuthorityMode)
             {
-                if (DeferredDespawnTick > 0)
+                // If we are deferring the despawn, then add it to the deferred despawn queue
+                // If DAHost has reached this point, it is not valid to add to the queue
+                if (DeferredDespawnTick > 0 && !networkManager.DAHost)
                 {
-                    // Clients always add it to the queue while DAHost will only add it to the queue if it is not a targeted destroy or it is and the target is the
-                    // DAHost client.
-                    if (!networkManager.DAHost || (networkManager.DAHost && (!IsTargetedDestroy || (IsTargetedDestroy && TargetClientId == 0))))
-                    {
-                        networkObject.DeferredDespawnTick = DeferredDespawnTick;
-                        var hasCallback = networkObject.OnDeferredDespawnComplete != null;
-                        networkManager.SpawnManager.DeferDespawnNetworkObject(NetworkObjectId, DeferredDespawnTick, hasCallback, DestroyGameObject);
-                        return;
-                    }
+                    HandleDeferredDespawn(ref networkManager, ref networkObject);
+                    return;
                 }
 
                 // If this is targeted and we are not the target, then just update our local observers for this object
-                if (IsTargetedDestroy && TargetClientId != networkManager.LocalClientId && networkObject != null)
+                if (IsTargetedDestroy && TargetClientId != networkManager.LocalClientId)
                 {
                     networkObject.Observers.Remove(TargetClientId);
                     return;
                 }
             }
 
-            if (networkObject != null)
+            // Otherwise just despawn the NetworkObject right now
+            networkManager.SpawnManager.OnDespawnNonAuthorityObject(networkObject);
+            networkManager.NetworkMetrics.TrackObjectDestroyReceived(context.SenderId, networkObject, context.MessageSize);
+        }
+
+        /// <summary>
+        /// Handles forwarding the <see cref="DestroyObjectMessage"/> when acting as the DA Host
+        /// </summary>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private void HandleDAHostForwardMessage(ulong senderId, ref NetworkManager networkManager, NetworkObject networkObject)
+        {
+            var message = new DestroyObjectMessage
             {
-                // Otherwise just despawn the NetworkObject right now
-                networkManager.SpawnManager.OnDespawnObject(networkObject, DestroyGameObject);
-                networkManager.NetworkMetrics.TrackObjectDestroyReceived(context.SenderId, networkObject, context.MessageSize);
+                NetworkObjectId = NetworkObjectId,
+                DestroyGameObject = DestroyGameObject,
+                IsDistributedAuthority = true,
+                IsTargetedDestroy = IsTargetedDestroy,
+                TargetClientId = TargetClientId, // Just always populate this value whether we write it or not
+                DeferredDespawnTick = DeferredDespawnTick,
+            };
+            var ownerClientId = networkObject == null ? senderId : networkObject.OwnerClientId;
+            var clientIds = networkObject == null ? networkManager.ConnectedClientsIds.ToList() : networkObject.Observers.ToList();
+
+            foreach (var clientId in clientIds)
+            {
+                if (clientId != networkManager.LocalClientId && clientId != ownerClientId)
+                {
+                    networkManager.ConnectionManager.SendMessage(ref message, NetworkDelivery.ReliableSequenced, clientId);
+                }
             }
         }
+
+        /// <summary>
+        /// Handles adding to the deferred despawn queue when the <see cref="DestroyObjectMessage"/> indicates a deferred despawn
+        /// </summary>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private void HandleDeferredDespawn(ref NetworkManager networkManager, ref NetworkObject networkObject)
+        {
+            networkObject.DeferredDespawnTick = DeferredDespawnTick;
+            var hasCallback = networkObject.OnDeferredDespawnComplete != null;
+            networkManager.SpawnManager.DeferDespawnNetworkObject(NetworkObjectId, DeferredDespawnTick, hasCallback);
+        }
     }
 }

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

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Text;
 using UnityEngine;
@@ -1470,7 +1471,6 @@ internal void ServerSpawnSceneObjectsOnStartSweep()
 #else
             var networkObjects = UnityEngine.Object.FindObjectsOfType<NetworkObject>();
 #endif
-            var isConnectedCMBService = NetworkManager.CMBServiceConnection;
             var networkObjectsToSpawn = new List<NetworkObject>();
             for (int i = 0; i < networkObjects.Length; i++)
             {
@@ -1510,15 +1510,30 @@ internal void ServerSpawnSceneObjectsOnStartSweep()
             networkObjectsToSpawn.Clear();
         }
 
+        /// <summary>
+        /// Called when destroying an object after receiving a <see cref="DestroyObjectMessage"/>.
+        /// Processes logic for how to destroy objects on the non-authority client.
+        /// </summary>
+        internal void OnDespawnNonAuthorityObject([NotNull] NetworkObject networkObject)
+        {
+            if (networkObject.HasAuthority)
+            {
+                NetworkLog.LogError($"OnDespawnNonAuthorityObject called on object {networkObject.NetworkObjectId} when is current client {NetworkManager.LocalClientId} has authority on this object.");
+            }
+
+            // On the non-authority, never destroy the game object when InScenePlaced, otherwise always destroy on non-authority side
+            OnDespawnObject(networkObject, networkObject.IsSceneObject == false);
+        }
+
         internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObject, bool modeDestroy = false)
         {
-            if (NetworkManager == null)
+            if (!NetworkManager)
             {
                 return;
             }
 
             // We have to do this check first as subsequent checks assume we can access NetworkObjectId.
-            if (networkObject == null)
+            if (!networkObject)
             {
                 Debug.LogWarning($"Trying to destroy network object but it is null");
                 return;
@@ -1632,8 +1647,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,
+                        DestroyGameObject = destroyGameObject,
                         IsTargetedDestroy = false,
                         IsDistributedAuthority = distributedAuthority,
                     };
@@ -2006,7 +2020,6 @@ internal struct DeferredDespawnObject
         {
             public int TickToDespawn;
             public bool HasDeferredDespawnCheck;
-            public bool DestroyGameObject;
             public ulong NetworkObjectId;
         }
 
@@ -2018,13 +2031,12 @@ 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, bool destroyGameObject)
+        internal void DeferDespawnNetworkObject(ulong networkObjectId, int tickToDespawn, bool hasDeferredDespawnCheck)
         {
             var deferredDespawnObject = new DeferredDespawnObject()
             {
                 TickToDespawn = tickToDespawn,
                 HasDeferredDespawnCheck = hasDeferredDespawnCheck,
-                DestroyGameObject = destroyGameObject,
                 NetworkObjectId = networkObjectId,
             };
             DeferredDespawnObjects.Add(deferredDespawnObject);
@@ -2041,15 +2053,21 @@ internal void DeferredDespawnUpdate(NetworkTime serverTime)
                 return;
             }
             var currentTick = serverTime.Tick;
-            var deferredCallbackCount = DeferredDespawnObjects.Count();
-            for (int i = 0; i < deferredCallbackCount; i++)
+            var deferredDespawnCount = DeferredDespawnObjects.Count;
+            // Loop forward and to process user callbacks and update despawn ticks
+            for (int i = 0; i < deferredDespawnCount; i++)
             {
                 var deferredObjectEntry = DeferredDespawnObjects[i];
                 if (!deferredObjectEntry.HasDeferredDespawnCheck)
                 {
                     continue;
                 }
-                var networkObject = SpawnedObjects[deferredObjectEntry.NetworkObjectId];
+
+                if (!SpawnedObjects.TryGetValue(deferredObjectEntry.NetworkObjectId, out var networkObject))
+                {
+                    continue;
+                }
+
                 // Double check to make sure user did not remove the callback
                 if (networkObject.OnDeferredDespawnComplete != null)
                 {
@@ -2074,23 +2092,21 @@ internal void DeferredDespawnUpdate(NetworkTime serverTime)
             }
 
             // Parse backwards so we can remove objects as we parse through them
-            for (int i = DeferredDespawnObjects.Count - 1; i >= 0; i--)
+            for (int i = deferredDespawnCount - 1; i >= 0; i--)
             {
                 var deferredObjectEntry = DeferredDespawnObjects[i];
                 if (deferredObjectEntry.TickToDespawn >= currentTick)
                 {
                     continue;
                 }
 
-                if (!SpawnedObjects.ContainsKey(deferredObjectEntry.NetworkObjectId))
+                if (SpawnedObjects.TryGetValue(deferredObjectEntry.NetworkObjectId, out var networkObject))
                 {
-                    DeferredDespawnObjects.Remove(deferredObjectEntry);
-                    continue;
+                    // Local instance despawns the instance
+                    OnDespawnNonAuthorityObject(networkObject);
                 }
-                var networkObject = SpawnedObjects[deferredObjectEntry.NetworkObjectId];
-                // Local instance despawns the instance
-                OnDespawnObject(networkObject, deferredObjectEntry.DestroyGameObject);
-                DeferredDespawnObjects.Remove(deferredObjectEntry);
+
+                DeferredDespawnObjects.RemoveAt(i);
             }
         }
 

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

@@ -1,9 +1,6 @@
 {
   "dependencies": {
-    "com.unity.ai.navigation": "2.0.5",
-    "com.unity.multiplayer.center": "1.0.0",
-    "com.unity.netcode.gameobjects": "file:../../com.unity.netcode.gameobjects",
-    "com.unity.modules.accessibility": "1.0.0"
+    "com.unity.netcode.gameobjects": "file:../../com.unity.netcode.gameobjects"
   },
   "testables": [
     "com.unity.netcode.gameobjects"

minimalproject/Packages/manifest.json

@@ -4,7 +4,7 @@
     "com.unity.ai.navigation": "2.0.5",
     "com.unity.ide.rider": "3.0.31",
     "com.unity.multiplayer.center": "1.0.0",
-    "com.unity.multiplayer.tools": "https://github.com/Unity-Technologies/com.unity.multiplayer.tools.git#release/1.1.0",
+    "com.unity.multiplayer.tools": "2.2.3",
     "com.unity.netcode.gameobjects": "file:../../com.unity.netcode.gameobjects",
     "com.unity.test-framework": "1.4.6",
     "com.unity.test-framework.performance": "3.0.3",