fix: Synchronize despawn to client (#3514)

<!-- Replace this block with what this PR does and why. Describe what
you'd like reviewers to know, how you applied the engineering
principles, and any interesting tradeoffs made. Delete bullet points
below that don't apply, and update the changelog section as appropriate.
-->

<!-- Add short version of the JIRA ticket to the PR title (e.g. "feat:
new shiny feature [MTT-123]") -->
[MTTB-1377](https://jira.unity3d.com/browse/MTTB-1377)

## Changelog

- Fixed: Send the DestroyGameObject parameter to the client.

## Testing and Documentation

- Includes unit tests.

<!-- Uncomment and mark items off with a * if this PR deprecates any
API:
### Deprecated API
- [ ] An `[Obsolete]` attribute was added along with a `(RemovedAfter
yyyy-mm-dd)` entry.
- [ ] An [api updater] was added.
- [ ] Deprecation of the API is explained in the CHANGELOG.
- [ ] The users can understand why this API was removed and what they
should use instead.
-->

## Backport

<!-- If this is a backport:
 - Add the following to the PR title: "\[Backport\] ..." .
 - Link to the original PR.
If this needs a backport - state this here
If a backport is not needed please provide the reason why.
If the "Backports" section is not present it will lead to a CI test
failure.
-->

No backport needed as this is 2.x functionality.

---------

Co-authored-by: Noel Stephens <[email protected]>

Author: EmandM

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -16,6 +16,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed synchronizing the destroyGameObject parameter to clients for InScenePlaced network objects. (#3514)
 - Fixed distributed authority related issue where enabling the `NetworkObject.DestroyWithScene` would cause errors when a destroying non-authority instances due to loading (single mode) or unloading scene events. (#3500)
 
 ### Changed

com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs

@@ -7,6 +7,7 @@
 using Unity.Collections.LowLevel.Unsafe;
 using Unity.Profiling;
 using UnityEngine;
+using Object = UnityEngine.Object;
 
 namespace Unity.Netcode
 {
@@ -1126,36 +1127,15 @@ internal void OnClientDisconnectFromServer(ulong clientId)
                 {
                     if (!playerObject.DontDestroyWithOwner)
                     {
-                        // If a player NetworkObject is being despawned, make sure to remove all children if they are marked to not be destroyed
-                        // with the owner.
-                        if (NetworkManager.DAHost)
-                        {
-                            // Remove any children from the player object if they are not going to be destroyed with the owner
-                            var childNetworkObjects = playerObject.GetComponentsInChildren<NetworkObject>();
-                            foreach (var child in childNetworkObjects)
-                            {
-                                // TODO: We have always just removed all children, but we might think about changing this to preserve the nested child hierarchy.
-                                if (child.DontDestroyWithOwner && child.transform.transform.parent)
-                                {
-                                    // If we are here, then we are running in DAHost mode and have the authority to remove the child from its parent
-                                    child.AuthorityAppliedParenting = true;
-                                    child.TryRemoveParentCachedWorldPositionStays();
-                                }
-                            }
-                        }
-
-                        if (NetworkManager.PrefabHandler.ContainsHandler(playerObject.GlobalObjectIdHash))
-                        {
-                            // Despawn but don't destroy. DA Host will act like the service and send despawn notifications.
-                            NetworkManager.SpawnManager.DespawnObject(playerObject, false, NetworkManager.DistributedAuthorityMode);
-                            // Let the prefab handler determine if it will be destroyed
-                            NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(playerObject);
-                        }
-                        else if (playerObject.IsSpawned)
+                        if (playerObject.IsSpawned)
                         {
                             // Call despawn to assure NetworkBehaviour.OnNetworkDespawn is invoked on the server-side (when the client side disconnected).
                             // This prevents the issue (when just destroying the GameObject) where any NetworkBehaviour component(s) destroyed before the NetworkObject would not have OnNetworkDespawn invoked.
-                            NetworkManager.SpawnManager.DespawnObject(playerObject, true, NetworkManager.DistributedAuthorityMode);
+                            NetworkManager.SpawnManager.DespawnObject(playerObject, true, true);
+                        }
+                        else
+                        {
+                            Object.Destroy(playerObject.gameObject);
                         }
                     }
                     else if (!NetworkManager.ShutdownInProgress)
@@ -1179,18 +1159,13 @@ internal void OnClientDisconnectFromServer(ulong clientId)
                         // If destroying with owner, then always despawn and destroy (or defer destroying to prefab handler)
                         if (!ownedObject.DontDestroyWithOwner)
                         {
-                            if (NetworkManager.PrefabHandler.ContainsHandler(ownedObject.GlobalObjectIdHash))
+                            if (ownedObject.IsSpawned)
                             {
-                                if (ownedObject.IsSpawned)
-                                {
-                                    // Don't destroy (prefab handler will determine this, but always notify
-                                    NetworkManager.SpawnManager.DespawnObject(ownedObject, false, true);
-                                }
-                                NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(ownedObject);
+                                NetworkManager.SpawnManager.DespawnObject(ownedObject, true, true);
                             }
                             else
                             {
-                                NetworkManager.SpawnManager.DespawnObject(ownedObject, true, true);
+                                Object.Destroy(ownedObject.gameObject);
                             }
                         }
                         else if (!NetworkManager.ShutdownInProgress)

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

@@ -6,17 +6,13 @@ namespace Unity.Netcode
     internal struct DestroyObjectMessage : INetworkMessage, INetworkSerializeByMemcpy
     {
         private const int k_OptimizeDestroyObjectMessage = 1;
-        public int Version => k_OptimizeDestroyObjectMessage;
+        private const int k_AllowDestroyGameInPlaced = 2;
+        public int Version => k_AllowDestroyGameInPlaced;
 
         private const string k_Name = "DestroyObjectMessage";
 
         public ulong NetworkObjectId;
 
-        /// <summary>
-        /// Used to communicate whether to destroy the associated game object.
-        /// Should be false if the object is InScenePlaced and true otherwise
-        /// </summary>
-        public bool DestroyGameObject;
         private byte m_DestroyFlags;
 
         internal int DeferredDespawnTick;
@@ -25,32 +21,29 @@ internal struct DestroyObjectMessage : INetworkMessage, INetworkSerializeByMemcp
 
         internal bool IsDistributedAuthority;
 
-        private const byte k_ClientTargetedDestroy = 0x01;
-        private const byte k_DeferredDespawn = 0x02;
-
         internal bool IsTargetedDestroy
         {
-            get => GetFlag(k_ClientTargetedDestroy);
+            get => ByteUtility.GetBit(m_DestroyFlags, 0);
 
-            set => SetFlag(value, k_ClientTargetedDestroy);
+            set => ByteUtility.SetBit(ref m_DestroyFlags, 0, value);
         }
 
         private bool IsDeferredDespawn
         {
-            get => GetFlag(k_DeferredDespawn);
+            get => ByteUtility.GetBit(m_DestroyFlags, 1);
 
-            set => SetFlag(value, k_DeferredDespawn);
+            set => ByteUtility.SetBit(ref m_DestroyFlags, 1, value);
         }
 
-        private bool GetFlag(int flag)
+        /// <summary>
+        /// Used to communicate whether to destroy the associated game object.
+        /// Should be false if the object is InScenePlaced and true otherwise
+        /// </summary>
+        public bool DestroyGameObject
         {
-            return (m_DestroyFlags & flag) != 0;
-        }
+            get => ByteUtility.GetBit(m_DestroyFlags, 2);
 
-        private void SetFlag(bool set, byte flag)
-        {
-            if (set) { m_DestroyFlags = (byte)(m_DestroyFlags | flag); }
-            else { m_DestroyFlags = (byte)(m_DestroyFlags & ~flag); }
+            set => ByteUtility.SetBit(ref m_DestroyFlags, 2, value);
         }
 
         public void Serialize(FastBufferWriter writer, int targetVersion)
@@ -74,6 +67,10 @@ public void Serialize(FastBufferWriter writer, int targetVersion)
                     BytePacker.WriteValueBitPacked(writer, DeferredDespawnTick);
                 }
             }
+            else if (targetVersion >= k_AllowDestroyGameInPlaced)
+            {
+                writer.WriteByteSafe(m_DestroyFlags);
+            }
 
             if (targetVersion < k_OptimizeDestroyObjectMessage)
             {
@@ -103,10 +100,15 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
                     ByteUnpacker.ReadValueBitPacked(reader, out DeferredDespawnTick);
                 }
             }
+            else if (receivedMessageVersion >= k_AllowDestroyGameInPlaced)
+            {
+                reader.ReadByteSafe(out m_DestroyFlags);
+            }
 
             if (receivedMessageVersion < k_OptimizeDestroyObjectMessage)
             {
-                reader.ReadValueSafe(out DestroyGameObject);
+                reader.ReadValueSafe(out bool destroyGameObject);
+                DestroyGameObject = destroyGameObject;
             }
 
             if (networkManager.SpawnManager.SpawnedObjects.ContainsKey(NetworkObjectId))
@@ -169,7 +171,7 @@ public void Handle(ref NetworkContext context)
             }
 
             // Otherwise just despawn the NetworkObject right now
-            networkManager.SpawnManager.OnDespawnNonAuthorityObject(networkObject);
+            networkManager.SpawnManager.OnDespawnNonAuthorityObject(networkObject, DestroyGameObject);
             networkManager.NetworkMetrics.TrackObjectDestroyReceived(context.SenderId, networkObject, context.MessageSize);
         }
 
@@ -208,7 +210,7 @@ private void HandleDeferredDespawn(ref NetworkManager networkManager, ref Networ
         {
             networkObject.DeferredDespawnTick = DeferredDespawnTick;
             var hasCallback = networkObject.OnDeferredDespawnComplete != null;
-            networkManager.SpawnManager.DeferDespawnNetworkObject(NetworkObjectId, DeferredDespawnTick, hasCallback);
+            networkManager.SpawnManager.DeferDespawnNetworkObject(NetworkObjectId, DeferredDespawnTick, hasCallback, DestroyGameObject);
         }
     }
 }

com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariablePermission.cs

@@ -3,6 +3,10 @@ namespace Unity.Netcode
     /// <summary>
     /// The permission types for reading a var
     /// </summary>
+    /// <remarks>
+    /// Only relevant when using the client/server network topology.
+    /// In distributed authority mode everyone can always read.
+    /// </remarks>
     public enum NetworkVariableReadPermission
     {
         /// <summary>

com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs

@@ -1280,13 +1280,7 @@ public SceneEventProgressStatus UnloadScene(Scene scene)
                 }
             }
 
-            sceneEventProgress.LoadSceneMode = LoadSceneMode.Additive;
-
-            // Any NetworkObjects marked to not be destroyed with a scene and reside within the scene about to be unloaded
-            // should be migrated temporarily into the DDOL, once the scene is unloaded they will be migrated into the
-            // currently active scene.
             var networkManager = NetworkManager;
-            SceneManagerHandler.MoveObjectsFromSceneToDontDestroyOnLoad(ref networkManager, scene);
 
             var sceneEventData = BeginSceneEvent();
             sceneEventData.SceneEventProgressId = sceneEventProgress.Guid;
@@ -1297,10 +1291,16 @@ public SceneEventProgressStatus UnloadScene(Scene scene)
 
             // This will be the message we send to everyone when this scene event sceneEventProgress is complete
             sceneEventProgress.SceneEventType = SceneEventType.UnloadEventCompleted;
+            sceneEventProgress.LoadSceneMode = LoadSceneMode.Additive;
 
             sceneEventProgress.SceneEventId = sceneEventData.SceneEventId;
             sceneEventProgress.OnSceneEventCompleted = OnSceneUnloaded;
 
+            // Any NetworkObjects marked to not be destroyed with a scene and reside within the scene about to be unloaded
+            // should be migrated temporarily into the DDOL, once the scene is unloaded they will be migrated into the
+            // currently active scene.
+            SceneManagerHandler.MoveObjectsFromSceneToDontDestroyOnLoad(ref networkManager, scene);
+
             if (!RemoveServerClientSceneHandle(sceneEventData.SceneHandle, scene.handle))
             {
                 Debug.LogError($"Failed to remove {SceneNameFromHash(sceneEventData.SceneHash)} scene handles [Server ({sceneEventData.SceneHandle})][Local({scene.handle})]");
@@ -2831,6 +2831,27 @@ internal bool IsSceneEventInProgress()
             return false;
         }
 
+        internal bool IsSceneUnloading(NetworkObject networkObject)
+        {
+            if (!NetworkManager.NetworkConfig.EnableSceneManagement)
+            {
+                return false;
+            }
+
+            foreach (var sceneEventEntry in SceneEventProgressTracking)
+            {
+                var name = SceneNameFromHash(sceneEventEntry.Value.SceneHash);
+                var scene = SceneManager.GetSceneByName(name);
+                var sameScene = name == networkObject.gameObject.scene.name && scene.handle == networkObject.SceneOriginHandle;
+
+                if (!sceneEventEntry.Value.HasTimedOut() && sceneEventEntry.Value.IsUnloading() && sceneEventEntry.Value.Status == SceneEventProgressStatus.Started && sameScene)
+                {
+                    return true;
+                }
+            }
+            return false;
+        }
+
         /// <summary>
         /// Handles notifying clients when a NetworkObject has been migrated into a new scene
         /// </summary>

com.unity.netcode.gameobjects/Runtime/SceneManagement/SceneEventProgress.cs

@@ -223,6 +223,14 @@ internal void ClientFinishedSceneEvent(ulong clientId)
             }
         }
 
+        /// <summary>
+        /// Returns whether the SceneEventType is related to an unloading event.
+        /// </summary>
+        internal bool IsUnloading()
+        {
+            return SceneEventType is SceneEventType.Unload or SceneEventType.UnloadComplete or SceneEventType.UnloadEventCompleted;
+        }
+
         /// <summary>
         /// Determines if the scene event has finished for both
         /// client(s) and server.