From ef9e1c1309b6b38370dd2c022f95cf2d4bc113a2 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Wed, 25 Sep 2024 16:23:26 -0500 Subject: [PATCH] fix: client side disconnect incorrect client count on host-server side [MTTB-135] (Up-Port) (#3075) * fix Fixed issue with the client count not being correct on the host or server side when a client disconnects itself from a session. * test validation tests for the changes/updates to this pr. * update updating the changelog. --- com.unity.netcode.gameobjects/CHANGELOG.md | 2 + .../Connection/NetworkConnectionManager.cs | 42 ++++++++++++------- .../Runtime/ConnectionApprovalTimeoutTests.cs | 2 +- .../Tests/Runtime/DisconnectTests.cs | 3 ++ 4 files changed, 33 insertions(+), 16 deletions(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 5b6d8f5049..fd680daedf 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -12,6 +12,8 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed +- Fixed issue with the client count not being correct on the host or server side when a client disconnects itself from a session. (#3075) + ### Changed ## [2.0.0] - 2024-09-12 diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index b5fff5404d..a77c91b6c4 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -105,8 +105,12 @@ internal void InvokeOnClientConnectedCallback(ulong clientId) continue; } - peerClientIds[idx] = peerId; - ++idx; + // This assures if the server has not timed out prior to the client synchronizing that it doesn't exceed the allocated peer count. + if (peerClientIds.Length > idx) + { + peerClientIds[idx] = peerId; + ++idx; + } } try @@ -496,24 +500,32 @@ internal void DisconnectEventHandler(ulong transportClientId) // Process the incoming message queue so that we get everything from the server disconnecting us or, if we are the server, so we got everything from that client. MessageManager.ProcessIncomingMessageQueue(); - InvokeOnClientDisconnectCallback(clientId); - - if (LocalClient.IsHost) - { - InvokeOnPeerDisconnectedCallback(clientId); - } - if (LocalClient.IsServer) { + // We need to process the disconnection before notifying OnClientDisconnectFromServer(clientId); + + // Now notify the client has disconnected + InvokeOnClientDisconnectCallback(clientId); + + if (LocalClient.IsHost) + { + InvokeOnPeerDisconnectedCallback(clientId); + } } - else // As long as we are not in the middle of a shutdown - if (!NetworkManager.ShutdownInProgress) + else { - // We must pass true here and not process any sends messages as we are no longer connected. - // Otherwise, attempting to process messages here can cause an exception within UnityTransport - // as the client ID is no longer valid. - NetworkManager.Shutdown(true); + // Notify local client of disconnection + InvokeOnClientDisconnectCallback(clientId); + + // As long as we are not in the middle of a shutdown + if (!NetworkManager.ShutdownInProgress) + { + // We must pass true here and not process any sends messages as we are no longer connected. + // Otherwise, attempting to process messages here can cause an exception within UnityTransport + // as the client ID is no longer valid. + NetworkManager.Shutdown(true); + } } #if DEVELOPMENT_BUILD || UNITY_EDITOR s_TransportDisconnect.End(); diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApprovalTimeoutTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApprovalTimeoutTests.cs index d324bad7ef..db1210face 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApprovalTimeoutTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApprovalTimeoutTests.cs @@ -82,7 +82,7 @@ protected override IEnumerator OnStartedServerAndClients() public IEnumerator ValidateApprovalTimeout() { // Just delay for a second - yield return new WaitForSeconds(1); + yield return new WaitForSeconds(k_TestTimeoutPeriod * 0.25f); // Verify we haven't received the time out message yet NetcodeLogAssert.LogWasNotReceived(LogType.Log, m_ExpectedLogMessage); diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs index c4997fe9fd..af001655ae 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs @@ -180,6 +180,9 @@ public IEnumerator ClientPlayerDisconnected([Values] ClientDisconnectType client Assert.IsTrue(m_DisconnectedEvent[m_ServerNetworkManager].ClientId == m_ClientId, $"Expected ClientID {m_ClientId} but found ClientID {m_DisconnectedEvent[m_ServerNetworkManager].ClientId} for the server {nameof(NetworkManager)} disconnect event entry!"); Assert.IsTrue(m_DisconnectedEvent.ContainsKey(m_ClientNetworkManagers[0]), $"Could not find the client {nameof(NetworkManager)} disconnect event entry!"); Assert.IsTrue(m_DisconnectedEvent[m_ClientNetworkManagers[0]].ClientId == m_ClientId, $"Expected ClientID {m_ClientId} but found ClientID {m_DisconnectedEvent[m_ServerNetworkManager].ClientId} for the client {nameof(NetworkManager)} disconnect event entry!"); + Assert.IsTrue(m_ServerNetworkManager.ConnectedClientsIds.Count == 1, $"Expected connected client identifiers count to be 1 but it was {m_ServerNetworkManager.ConnectedClientsIds.Count}!"); + Assert.IsTrue(m_ServerNetworkManager.ConnectedClients.Count == 1, $"Expected connected client identifiers count to be 1 but it was {m_ServerNetworkManager.ConnectedClients.Count}!"); + Assert.IsTrue(m_ServerNetworkManager.ConnectedClientsList.Count == 1, $"Expected connected client identifiers count to be 1 but it was {m_ServerNetworkManager.ConnectedClientsList.Count}!"); } if (m_OwnerPersistence == OwnerPersistence.DestroyWithOwner)