From b929360f20273a412105b2a321dd8b631cc95677 Mon Sep 17 00:00:00 2001 From: josiemessa Date: Mon, 28 Jun 2021 16:27:00 +0100 Subject: [PATCH 1/3] Change function signature of OnDespawnObject to accept network object rather than ID --- .../Runtime/Core/NetworkManager.cs | 4 +-- .../Runtime/Core/NetworkObject.cs | 4 +-- .../Messaging/InternalMessageHandler.cs | 7 +++- .../Runtime/Spawning/NetworkSpawnManager.cs | 36 +++++++++++-------- 4 files changed, 31 insertions(+), 20 deletions(-) diff --git a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkManager.cs b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkManager.cs index 207ba51fb0..ed9f94b79f 100644 --- a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkManager.cs +++ b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkManager.cs @@ -1507,7 +1507,7 @@ internal void OnClientDisconnectFromServer(ulong clientId) if (PrefabHandler.ContainsHandler(ConnectedClients[clientId].PlayerObject.GlobalObjectIdHash)) { PrefabHandler.HandleNetworkPrefabDestroy(ConnectedClients[clientId].PlayerObject); - SpawnManager.OnDespawnObject(ConnectedClients[clientId].PlayerObject.NetworkObjectId, false); + SpawnManager.OnDespawnObject(ConnectedClients[clientId].PlayerObject, false); } else { @@ -1525,7 +1525,7 @@ internal void OnClientDisconnectFromServer(ulong clientId) if (PrefabHandler.ContainsHandler(ConnectedClients[clientId].OwnedObjects[i].GlobalObjectIdHash)) { PrefabHandler.HandleNetworkPrefabDestroy(ConnectedClients[clientId].OwnedObjects[i]); - SpawnManager.OnDespawnObject(ConnectedClients[clientId].OwnedObjects[i].NetworkObjectId, false); + SpawnManager.OnDespawnObject(ConnectedClients[clientId].OwnedObjects[i], false); } else { diff --git a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkObject.cs b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkObject.cs index 1194c87ae0..c166c978ac 100644 --- a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkObject.cs +++ b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkObject.cs @@ -405,9 +405,9 @@ public static void NetworkHide(List networkObjects, ulong clientI private void OnDestroy() { - if (NetworkManager != null && NetworkManager.SpawnManager != null && NetworkManager.SpawnManager.SpawnedObjects.ContainsKey(NetworkObjectId)) + if (NetworkManager != null && NetworkManager.SpawnManager != null && NetworkManager.SpawnManager.SpawnedObjects.TryGetValue(NetworkObjectId, out var networkObject)) { - NetworkManager.SpawnManager.OnDespawnObject(NetworkObjectId, false); + NetworkManager.SpawnManager.OnDespawnObject(networkObject, false); } } diff --git a/com.unity.multiplayer.mlapi/Runtime/Messaging/InternalMessageHandler.cs b/com.unity.multiplayer.mlapi/Runtime/Messaging/InternalMessageHandler.cs index 82a8be7f1e..9e1f24b240 100644 --- a/com.unity.multiplayer.mlapi/Runtime/Messaging/InternalMessageHandler.cs +++ b/com.unity.multiplayer.mlapi/Runtime/Messaging/InternalMessageHandler.cs @@ -190,7 +190,12 @@ public void HandleDestroyObject(ulong clientId, Stream stream) using (var reader = PooledNetworkReader.Get(stream)) { ulong networkId = reader.ReadUInt64Packed(); - NetworkManager.SpawnManager.OnDespawnObject(networkId, true); + if (!NetworkManager.SpawnManager.SpawnedObjects.TryGetValue(networkId, out NetworkObject networkObject)) + { + return; + } + + NetworkManager.SpawnManager.OnDespawnObject(networkObject, true); } } diff --git a/com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs index 593bdd521e..f3c16675ac 100644 --- a/com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs @@ -460,7 +460,7 @@ internal void DespawnObject(NetworkObject networkObject, bool destroyObject = fa throw new NotServerException("Only server can despawn objects"); } - OnDespawnObject(networkObject.NetworkObjectId, destroyObject); + OnDespawnObject(networkObject, destroyObject); } // Makes scene objects ready to be reused @@ -498,7 +498,7 @@ internal void ServerDestroySpawnedSceneObjects() if (NetworkManager.PrefabHandler != null && NetworkManager.PrefabHandler.ContainsHandler(sobj)) { NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(sobj); - OnDespawnObject(sobj.NetworkObjectId, false); + OnDespawnObject(sobj, false); } else { @@ -522,7 +522,7 @@ internal void DestroyNonSceneObjects() { NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(networkObjects[i]); - OnDespawnObject(networkObjects[i].NetworkObjectId, false); + OnDespawnObject(networkObjects[i], false); } else { @@ -546,7 +546,7 @@ internal void DestroySceneObjects() if (NetworkManager.PrefabHandler.ContainsHandler(networkObjects[i])) { NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(networkObjects[i]); - OnDespawnObject(networkObjects[i].NetworkObjectId, false); + OnDespawnObject(networkObjects[i], false); } else { @@ -607,31 +607,37 @@ internal void ClientCollectSoftSyncSceneObjectSweep(NetworkObject[] networkObjec } } - internal void OnDespawnObject(ulong networkObjectId, bool destroyGameObject) + internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObject) { if (NetworkManager == null) { return; } + if (networkObject == null) + { + return; + } + // Removal of spawned object - if (!SpawnedObjects.TryGetValue(networkObjectId, out NetworkObject networkObject)) + if (!SpawnedObjects.ContainsKey(networkObject.NetworkObjectId)) { - Debug.LogWarning($"Trying to destroy object {networkObjectId} but it doesn't seem to exist anymore!"); + Debug.LogWarning($"Trying to destroy object {networkObject.NetworkObjectId} but it doesn't seem to exist anymore!"); return; } + // Move child NetworkObjects to the root when parent NetworkObject is destroyed foreach (var spawnedNetObj in SpawnedObjectsList) { var (isReparented, latestParent) = spawnedNetObj.GetNetworkParenting(); - if (isReparented && latestParent == networkObjectId) + if (isReparented && latestParent == networkObject.NetworkObjectId) { spawnedNetObj.gameObject.transform.parent = null; if (NetworkLog.CurrentLogLevel <= LogLevel.Normal) { - NetworkLog.LogWarning($"{nameof(NetworkObject)} #{spawnedNetObj.NetworkObjectId} moved to the root because its parent {nameof(NetworkObject)} #{networkObjectId} is destroyed"); + NetworkLog.LogWarning($"{nameof(NetworkObject)} #{spawnedNetObj.NetworkObjectId} moved to the root because its parent {nameof(NetworkObject)} #{networkObject.NetworkObjectId} is destroyed"); } } } @@ -641,7 +647,7 @@ internal void OnDespawnObject(ulong networkObjectId, bool destroyGameObject) //Someone owns it. for (int i = networkClient.OwnedObjects.Count - 1; i > -1; i--) { - if (networkClient.OwnedObjects[i].NetworkObjectId == networkObjectId) + if (networkClient.OwnedObjects[i].NetworkObjectId == networkObject.NetworkObjectId) { networkClient.OwnedObjects.RemoveAt(i); } @@ -657,7 +663,7 @@ internal void OnDespawnObject(ulong networkObjectId, bool destroyGameObject) { ReleasedNetworkObjectIds.Enqueue(new ReleasedNetworkId() { - NetworkId = networkObjectId, + NetworkId = networkObject.NetworkObjectId, ReleaseTime = Time.unscaledTime }); } @@ -673,13 +679,13 @@ internal void OnDespawnObject(ulong networkObjectId, bool destroyGameObject) var buffer = PooledNetworkBuffer.Get(); using (var writer = PooledNetworkWriter.Get(buffer)) { - writer.WriteUInt64Packed(networkObjectId); + writer.WriteUInt64Packed(networkObject.NetworkObjectId); var queueItem = new RpcFrameQueueItem { UpdateStage = NetworkUpdateStage.PostLateUpdate, QueueItemType = RpcQueueContainer.QueueItemType.DestroyObject, - NetworkId = networkObjectId, + NetworkId = networkObject.NetworkObjectId, NetworkBuffer = buffer, NetworkChannel = NetworkChannel.Internal, ClientNetworkIds = NetworkManager.ConnectedClientsList.Select(c => c.ClientId).ToArray() @@ -699,7 +705,7 @@ internal void OnDespawnObject(ulong networkObjectId, bool destroyGameObject) if (NetworkManager.PrefabHandler.ContainsHandler(networkObject)) { NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(networkObject); - OnDespawnObject(networkObjectId, false); + OnDespawnObject(networkObject, false); } else { @@ -710,7 +716,7 @@ internal void OnDespawnObject(ulong networkObjectId, bool destroyGameObject) // for some reason, we can get down here and SpawnedObjects for this // networkId will no longer be here, even as we check this at the start // of the function - if (SpawnedObjects.Remove(networkObjectId)) + if (SpawnedObjects.Remove(networkObject.NetworkObjectId)) { SpawnedObjectsList.Remove(networkObject); } From 8d0d6bbeb54bd91224dce72645c8d11d492dc384 Mon Sep 17 00:00:00 2001 From: josiemessa Date: Mon, 28 Jun 2021 16:27:00 +0100 Subject: [PATCH 2/3] chore: Change function signature of OnDespawnObject to accept network object rather than ID --- .../Runtime/Core/NetworkManager.cs | 4 +-- .../Runtime/Core/NetworkObject.cs | 4 +-- .../Messaging/InternalMessageHandler.cs | 7 +++- .../Runtime/Spawning/NetworkSpawnManager.cs | 36 +++++++++++-------- 4 files changed, 31 insertions(+), 20 deletions(-) diff --git a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkManager.cs b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkManager.cs index 207ba51fb0..ed9f94b79f 100644 --- a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkManager.cs +++ b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkManager.cs @@ -1507,7 +1507,7 @@ internal void OnClientDisconnectFromServer(ulong clientId) if (PrefabHandler.ContainsHandler(ConnectedClients[clientId].PlayerObject.GlobalObjectIdHash)) { PrefabHandler.HandleNetworkPrefabDestroy(ConnectedClients[clientId].PlayerObject); - SpawnManager.OnDespawnObject(ConnectedClients[clientId].PlayerObject.NetworkObjectId, false); + SpawnManager.OnDespawnObject(ConnectedClients[clientId].PlayerObject, false); } else { @@ -1525,7 +1525,7 @@ internal void OnClientDisconnectFromServer(ulong clientId) if (PrefabHandler.ContainsHandler(ConnectedClients[clientId].OwnedObjects[i].GlobalObjectIdHash)) { PrefabHandler.HandleNetworkPrefabDestroy(ConnectedClients[clientId].OwnedObjects[i]); - SpawnManager.OnDespawnObject(ConnectedClients[clientId].OwnedObjects[i].NetworkObjectId, false); + SpawnManager.OnDespawnObject(ConnectedClients[clientId].OwnedObjects[i], false); } else { diff --git a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkObject.cs b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkObject.cs index 1194c87ae0..c166c978ac 100644 --- a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkObject.cs +++ b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkObject.cs @@ -405,9 +405,9 @@ public static void NetworkHide(List networkObjects, ulong clientI private void OnDestroy() { - if (NetworkManager != null && NetworkManager.SpawnManager != null && NetworkManager.SpawnManager.SpawnedObjects.ContainsKey(NetworkObjectId)) + if (NetworkManager != null && NetworkManager.SpawnManager != null && NetworkManager.SpawnManager.SpawnedObjects.TryGetValue(NetworkObjectId, out var networkObject)) { - NetworkManager.SpawnManager.OnDespawnObject(NetworkObjectId, false); + NetworkManager.SpawnManager.OnDespawnObject(networkObject, false); } } diff --git a/com.unity.multiplayer.mlapi/Runtime/Messaging/InternalMessageHandler.cs b/com.unity.multiplayer.mlapi/Runtime/Messaging/InternalMessageHandler.cs index 82a8be7f1e..9e1f24b240 100644 --- a/com.unity.multiplayer.mlapi/Runtime/Messaging/InternalMessageHandler.cs +++ b/com.unity.multiplayer.mlapi/Runtime/Messaging/InternalMessageHandler.cs @@ -190,7 +190,12 @@ public void HandleDestroyObject(ulong clientId, Stream stream) using (var reader = PooledNetworkReader.Get(stream)) { ulong networkId = reader.ReadUInt64Packed(); - NetworkManager.SpawnManager.OnDespawnObject(networkId, true); + if (!NetworkManager.SpawnManager.SpawnedObjects.TryGetValue(networkId, out NetworkObject networkObject)) + { + return; + } + + NetworkManager.SpawnManager.OnDespawnObject(networkObject, true); } } diff --git a/com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs index 593bdd521e..f3c16675ac 100644 --- a/com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs @@ -460,7 +460,7 @@ internal void DespawnObject(NetworkObject networkObject, bool destroyObject = fa throw new NotServerException("Only server can despawn objects"); } - OnDespawnObject(networkObject.NetworkObjectId, destroyObject); + OnDespawnObject(networkObject, destroyObject); } // Makes scene objects ready to be reused @@ -498,7 +498,7 @@ internal void ServerDestroySpawnedSceneObjects() if (NetworkManager.PrefabHandler != null && NetworkManager.PrefabHandler.ContainsHandler(sobj)) { NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(sobj); - OnDespawnObject(sobj.NetworkObjectId, false); + OnDespawnObject(sobj, false); } else { @@ -522,7 +522,7 @@ internal void DestroyNonSceneObjects() { NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(networkObjects[i]); - OnDespawnObject(networkObjects[i].NetworkObjectId, false); + OnDespawnObject(networkObjects[i], false); } else { @@ -546,7 +546,7 @@ internal void DestroySceneObjects() if (NetworkManager.PrefabHandler.ContainsHandler(networkObjects[i])) { NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(networkObjects[i]); - OnDespawnObject(networkObjects[i].NetworkObjectId, false); + OnDespawnObject(networkObjects[i], false); } else { @@ -607,31 +607,37 @@ internal void ClientCollectSoftSyncSceneObjectSweep(NetworkObject[] networkObjec } } - internal void OnDespawnObject(ulong networkObjectId, bool destroyGameObject) + internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObject) { if (NetworkManager == null) { return; } + if (networkObject == null) + { + return; + } + // Removal of spawned object - if (!SpawnedObjects.TryGetValue(networkObjectId, out NetworkObject networkObject)) + if (!SpawnedObjects.ContainsKey(networkObject.NetworkObjectId)) { - Debug.LogWarning($"Trying to destroy object {networkObjectId} but it doesn't seem to exist anymore!"); + Debug.LogWarning($"Trying to destroy object {networkObject.NetworkObjectId} but it doesn't seem to exist anymore!"); return; } + // Move child NetworkObjects to the root when parent NetworkObject is destroyed foreach (var spawnedNetObj in SpawnedObjectsList) { var (isReparented, latestParent) = spawnedNetObj.GetNetworkParenting(); - if (isReparented && latestParent == networkObjectId) + if (isReparented && latestParent == networkObject.NetworkObjectId) { spawnedNetObj.gameObject.transform.parent = null; if (NetworkLog.CurrentLogLevel <= LogLevel.Normal) { - NetworkLog.LogWarning($"{nameof(NetworkObject)} #{spawnedNetObj.NetworkObjectId} moved to the root because its parent {nameof(NetworkObject)} #{networkObjectId} is destroyed"); + NetworkLog.LogWarning($"{nameof(NetworkObject)} #{spawnedNetObj.NetworkObjectId} moved to the root because its parent {nameof(NetworkObject)} #{networkObject.NetworkObjectId} is destroyed"); } } } @@ -641,7 +647,7 @@ internal void OnDespawnObject(ulong networkObjectId, bool destroyGameObject) //Someone owns it. for (int i = networkClient.OwnedObjects.Count - 1; i > -1; i--) { - if (networkClient.OwnedObjects[i].NetworkObjectId == networkObjectId) + if (networkClient.OwnedObjects[i].NetworkObjectId == networkObject.NetworkObjectId) { networkClient.OwnedObjects.RemoveAt(i); } @@ -657,7 +663,7 @@ internal void OnDespawnObject(ulong networkObjectId, bool destroyGameObject) { ReleasedNetworkObjectIds.Enqueue(new ReleasedNetworkId() { - NetworkId = networkObjectId, + NetworkId = networkObject.NetworkObjectId, ReleaseTime = Time.unscaledTime }); } @@ -673,13 +679,13 @@ internal void OnDespawnObject(ulong networkObjectId, bool destroyGameObject) var buffer = PooledNetworkBuffer.Get(); using (var writer = PooledNetworkWriter.Get(buffer)) { - writer.WriteUInt64Packed(networkObjectId); + writer.WriteUInt64Packed(networkObject.NetworkObjectId); var queueItem = new RpcFrameQueueItem { UpdateStage = NetworkUpdateStage.PostLateUpdate, QueueItemType = RpcQueueContainer.QueueItemType.DestroyObject, - NetworkId = networkObjectId, + NetworkId = networkObject.NetworkObjectId, NetworkBuffer = buffer, NetworkChannel = NetworkChannel.Internal, ClientNetworkIds = NetworkManager.ConnectedClientsList.Select(c => c.ClientId).ToArray() @@ -699,7 +705,7 @@ internal void OnDespawnObject(ulong networkObjectId, bool destroyGameObject) if (NetworkManager.PrefabHandler.ContainsHandler(networkObject)) { NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(networkObject); - OnDespawnObject(networkObjectId, false); + OnDespawnObject(networkObject, false); } else { @@ -710,7 +716,7 @@ internal void OnDespawnObject(ulong networkObjectId, bool destroyGameObject) // for some reason, we can get down here and SpawnedObjects for this // networkId will no longer be here, even as we check this at the start // of the function - if (SpawnedObjects.Remove(networkObjectId)) + if (SpawnedObjects.Remove(networkObject.NetworkObjectId)) { SpawnedObjectsList.Remove(networkObject); } From 6ac42106e30b87823dd6bd3c49ad7d543a630e2b Mon Sep 17 00:00:00 2001 From: josiemessa Date: Tue, 29 Jun 2021 11:16:48 +0100 Subject: [PATCH 3/3] Improve error messages --- .../Runtime/Messaging/InternalMessageHandler.cs | 3 +++ .../Runtime/Spawning/NetworkSpawnManager.cs | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/com.unity.multiplayer.mlapi/Runtime/Messaging/InternalMessageHandler.cs b/com.unity.multiplayer.mlapi/Runtime/Messaging/InternalMessageHandler.cs index 9e1f24b240..b92a1f9ded 100644 --- a/com.unity.multiplayer.mlapi/Runtime/Messaging/InternalMessageHandler.cs +++ b/com.unity.multiplayer.mlapi/Runtime/Messaging/InternalMessageHandler.cs @@ -192,6 +192,9 @@ public void HandleDestroyObject(ulong clientId, Stream stream) ulong networkId = reader.ReadUInt64Packed(); if (!NetworkManager.SpawnManager.SpawnedObjects.TryGetValue(networkId, out NetworkObject networkObject)) { + // This is the same check and log message that happens inside OnDespawnObject, but we have to do it here + // while we still have access to the network ID, otherwise the log message will be less useful. + Debug.LogWarning($"Trying to destroy object {networkId} but it doesn't seem to exist anymore!"); return; } diff --git a/com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs index f3c16675ac..10b08966f0 100644 --- a/com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs @@ -614,8 +614,10 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec return; } + // We have to do this check first as subsequent checks assume we can access NetworkObjectId. if (networkObject == null) { + Debug.LogWarning($"Trying to destroy network object but it is null"); return; } @@ -626,7 +628,6 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec return; } - // Move child NetworkObjects to the root when parent NetworkObject is destroyed foreach (var spawnedNetObj in SpawnedObjectsList) {