From e8c05b07e652820bab5a12f467b84a3e8e922022 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Albin=20Cor=C3=A9n?= Date: Tue, 14 Sep 2021 23:00:16 +0200 Subject: [PATCH 1/3] fix: Fixed remote disconnects not properly cleaning up --- .../Runtime/Core/NetworkManager.cs | 13 +---- .../Tests/Runtime/DisconnectTests.cs | 50 +++++++++++++++++++ .../Tests/Runtime/DisconnectTests.cs.meta | 3 ++ 3 files changed, 55 insertions(+), 11 deletions(-) create mode 100644 com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs create mode 100644 com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs.meta diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs index 70833997f6..d069326cea 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs @@ -1294,21 +1294,12 @@ public void DisconnectClient(ulong clientId) throw new NotServerException("Only server can disconnect remote clients. Use StopClient instead."); } - ConnectedClients.Remove(clientId); - PendingClients.Remove(clientId); - - for (int i = ConnectedClientsList.Count - 1; i > -1; i--) - { - if (ConnectedClientsList[i].ClientId == clientId) - { - ConnectedClientsList.RemoveAt(i); - } - } + OnClientDisconnectFromServer(clientId); NetworkConfig.NetworkTransport.DisconnectRemoteClient(clientId); } - internal void OnClientDisconnectFromServer(ulong clientId) + private void OnClientDisconnectFromServer(ulong clientId) { PendingClients.Remove(clientId); diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs new file mode 100644 index 0000000000..398e7d6057 --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs @@ -0,0 +1,50 @@ +using System.Collections; +using System.Collections.Generic; +using System.Linq; +using NUnit.Framework; +using UnityEngine; +using UnityEngine.TestTools; + +namespace Unity.Netcode.RuntimeTests +{ + public class DisconnectTests + { + [UnityTest] + public IEnumerator RemoteDisconnectPlayerObjectCleanup() + { + // create server and client instances + MultiInstanceHelpers.Create(1, out NetworkManager server, out NetworkManager[] clients); + + // create prefab + var gameObject = new GameObject("PlayerObject"); + var networkObject = gameObject.AddComponent(); + networkObject.DontDestroyWithOwner = true; + MultiInstanceHelpers.MakeNetworkObjectTestPrefab(networkObject); + + server.NetworkConfig.PlayerPrefab = gameObject; + + for (int i = 0; i < clients.Length; i++) + { + clients[i].NetworkConfig.PlayerPrefab = gameObject; + } + + // start server and connect clients + MultiInstanceHelpers.Start(false, server, clients); + + // wait for connection on client side + yield return MultiInstanceHelpers.Run(MultiInstanceHelpers.WaitForClientsConnected(clients)); + + // wait for connection on server side + yield return MultiInstanceHelpers.Run(MultiInstanceHelpers.WaitForClientConnectedToServer(server)); + + // disconnect the remote client + server.DisconnectClient(clients[0].LocalClientId); + + // ensure the object was destroyed + Assert.That(server.SpawnManager.SpawnedObjects.Any(x => x.Value.IsLocalPlayer && x.Value.OwnerClientId == clients[0].LocalClientId)); + + // cleanup + MultiInstanceHelpers.Destroy(); + } + } +} diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs.meta b/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs.meta new file mode 100644 index 0000000000..5528fb9435 --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: b05b4daca3854ff6b01a1f002d433dd6 +timeCreated: 1631652586 \ No newline at end of file From 274544961b72bca9a00e0dfb74d909ed58ca043e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Albin=20Cor=C3=A9n?= Date: Tue, 14 Sep 2021 23:02:46 +0200 Subject: [PATCH 2/3] Test fix --- com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs index 398e7d6057..fe2b7fd4dc 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs @@ -1,5 +1,4 @@ using System.Collections; -using System.Collections.Generic; using System.Linq; using NUnit.Framework; using UnityEngine; @@ -41,7 +40,7 @@ public IEnumerator RemoteDisconnectPlayerObjectCleanup() server.DisconnectClient(clients[0].LocalClientId); // ensure the object was destroyed - Assert.That(server.SpawnManager.SpawnedObjects.Any(x => x.Value.IsLocalPlayer && x.Value.OwnerClientId == clients[0].LocalClientId)); + Assert.That(server.SpawnManager.SpawnedObjects.Any(x => x.Value.IsPlayerObject && x.Value.OwnerClientId == clients[0].LocalClientId)); // cleanup MultiInstanceHelpers.Destroy(); From b9dd6b8bc3d5373d112e44e78b6dc5cac143af59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Albin=20Cor=C3=A9n?= Date: Tue, 14 Sep 2021 23:22:40 +0200 Subject: [PATCH 3/3] Fixed test --- .../Tests/Runtime/DisconnectTests.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs index fe2b7fd4dc..45fb045a08 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs @@ -39,8 +39,12 @@ public IEnumerator RemoteDisconnectPlayerObjectCleanup() // disconnect the remote client server.DisconnectClient(clients[0].LocalClientId); + // wait 1 frame because destroys are delayed + var nextFrameNumber = Time.frameCount + 1; + yield return new WaitUntil(() => Time.frameCount >= nextFrameNumber); + // ensure the object was destroyed - Assert.That(server.SpawnManager.SpawnedObjects.Any(x => x.Value.IsPlayerObject && x.Value.OwnerClientId == clients[0].LocalClientId)); + Assert.False(server.SpawnManager.SpawnedObjects.Any(x => x.Value.IsPlayerObject && x.Value.OwnerClientId == clients[0].LocalClientId)); // cleanup MultiInstanceHelpers.Destroy();