diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 17abe14225..d2572fcc89 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -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) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/DestroyObjectMessage.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/DestroyObjectMessage.cs index cdcff74251..2db9b8226b 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/DestroyObjectMessage.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/DestroyObjectMessage.cs @@ -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); + } + + /// + /// Handles forwarding the when acting as the DA Host + /// + [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); + } } } + + /// + /// Handles adding to the deferred despawn queue when the indicates a deferred despawn + /// + [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); + } } } diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index 4807e3dfc0..aa277735c3 100644 --- a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.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(); #endif - var isConnectedCMBService = NetworkManager.CMBServiceConnection; var networkObjectsToSpawn = new List(); for (int i = 0; i < networkObjects.Length; i++) { @@ -1510,15 +1510,30 @@ internal void ServerSpawnSceneObjectsOnStartSweep() networkObjectsToSpawn.Clear(); } + /// + /// Called when destroying an object after receiving a . + /// Processes logic for how to destroy objects on the non-authority client. + /// + 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 /// associated NetworkObject /// when to despawn the NetworkObject /// if true, user script is to be invoked to determine when to despawn - 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,7 +2092,7 @@ 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) @@ -2082,15 +2100,13 @@ internal void DeferredDespawnUpdate(NetworkTime serverTime) 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); } }