From 243f674ae8d49ee2509c8d2dbc52d0024078ed13 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Fri, 1 Dec 2023 11:48:31 -0600 Subject: [PATCH 01/19] fix Just return from OnClientDisconnectFromServer when the server is shutting down as there is no point in doing ownership cleanup or client disconnected notification messages at that point. --- .../Runtime/Connection/NetworkConnectionManager.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index d7c0ef4b05..e6806fb5a3 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -899,9 +899,8 @@ internal void OnClientDisconnectFromServer(ulong clientId) throw new Exception("[OnClientDisconnectFromServer] Was invoked by non-server instance!"); } - // If we are shutting down and this is the server or host disconnecting, then ignore - // clean up as everything that needs to be destroyed will be during shutdown. - if (NetworkManager.ShutdownInProgress && clientId == NetworkManager.ServerClientId) + // If we are shutting down, then ignore clean up as everything that needs to be destroyed will be during shutdown. + if (NetworkManager.ShutdownInProgress) { return; } From e1963eeadc09efa49e78e083755501a7f800828e Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Fri, 1 Dec 2023 17:07:39 -0600 Subject: [PATCH 02/19] Fix Approaching this fix a different way since we still should send out client disconnect notifications and such. --- .../Runtime/Connection/NetworkConnectionManager.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index e6806fb5a3..637c8d6d64 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -899,8 +899,10 @@ internal void OnClientDisconnectFromServer(ulong clientId) throw new Exception("[OnClientDisconnectFromServer] Was invoked by non-server instance!"); } - // If we are shutting down, then ignore clean up as everything that needs to be destroyed will be during shutdown. - if (NetworkManager.ShutdownInProgress) + // If we are shutting down and this is the server or host disconnecting, then ignore + // clean up as everything that needs to be destroyed will be during shutdown. + + if (NetworkManager.ShutdownInProgress && clientId == NetworkManager.ServerClientId) { return; } @@ -924,6 +926,7 @@ internal void OnClientDisconnectFromServer(ulong clientId) } } else + if (!NetworkManager.ShutdownInProgress) { playerObject.RemoveOwnership(); } @@ -942,7 +945,7 @@ internal void OnClientDisconnectFromServer(ulong clientId) } else { - // Handle changing ownership and prefab handlers + // Handle changing ownership and prefab handlers for (int i = clientOwnedObjects.Count - 1; i >= 0; i--) { var ownedObject = clientOwnedObjects[i]; @@ -960,6 +963,7 @@ internal void OnClientDisconnectFromServer(ulong clientId) } } else + if (!NetworkManager.ShutdownInProgress) { ownedObject.RemoveOwnership(); } From 221e853a309ba6ad601972f352e788a2bb2feacd Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Fri, 1 Dec 2023 17:55:29 -0600 Subject: [PATCH 03/19] fix Server or host was not sending disconnect when it shutdown. --- .../Connection/NetworkConnectionManager.cs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index 637c8d6d64..0d450c1473 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -1080,16 +1080,9 @@ internal void Initialize(NetworkManager networkManager) /// internal void Shutdown() { - LocalClient.IsApproved = false; - LocalClient.IsConnected = false; - ConnectedClients.Clear(); - ConnectedClientIds.Clear(); - ConnectedClientsList.Clear(); + if (LocalClient.IsServer) { - // make sure all messages are flushed before transport disconnect clients - MessageManager?.ProcessSendQueues(); - // Build a list of all client ids to be disconnected var disconnectedIds = new HashSet(); @@ -1125,6 +1118,9 @@ internal void Shutdown() { DisconnectRemoteClient(clientId); } + + // make sure all messages are flushed before transport disconnect clients + MessageManager?.ProcessSendQueues(); } else if (NetworkManager != null && NetworkManager.IsListening && LocalClient.IsClient) { @@ -1139,6 +1135,12 @@ internal void Shutdown() } } + LocalClient.IsApproved = false; + LocalClient.IsConnected = false; + ConnectedClients.Clear(); + ConnectedClientIds.Clear(); + ConnectedClientsList.Clear(); + if (NetworkManager != null && NetworkManager.NetworkConfig?.NetworkTransport != null) { NetworkManager.NetworkConfig.NetworkTransport.OnTransportEvent -= HandleNetworkEvent; From 80b731b7dc143a4f5b2a0e71c88a2c6e2b29c2ef Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 6 Dec 2023 16:34:50 -0600 Subject: [PATCH 04/19] update When a client disconnects itself or is disconnected by the server, it now returns its client identifier. Server or host shutdown sequence now includes a "soft shutdown" where the server-host will send out disconnection messages to all clients with the reason for the client being disconnected. --- .../Connection/NetworkConnectionManager.cs | 17 +++- .../Runtime/Core/NetworkManager.cs | 90 +++++++++++++++++-- 2 files changed, 96 insertions(+), 11 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index 0d450c1473..cd91b72b6a 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -347,7 +347,7 @@ internal ulong TransportIdCleanUp(ulong transportId) // When this happens, the client will not have an entry within the m_TransportIdToClientIdMap or m_ClientIdToTransportIdMap lookup tables so we exit early and just return 0 to be used for the disconnect event. if (!LocalClient.IsServer && !TransportIdToClientIdMap.ContainsKey(transportId)) { - return 0; + return NetworkManager.LocalClientId; } var clientId = TransportIdToClientId(transportId); @@ -476,12 +476,18 @@ internal void DisconnectEventHandler(ulong transportClientId) s_TransportDisconnect.Begin(); #endif var clientId = TransportIdCleanUp(transportClientId); - if (NetworkLog.CurrentLogLevel <= LogLevel.Developer) { NetworkLog.LogInfo($"Disconnect Event From {clientId}"); } + // If we are a client and we have gotten the ServerClientId back, then use our assigned local id as the client that was + // disconnected (either the user disconnected or the server disconnected, but the client that disconnected is the LocalClientId) + if (!NetworkManager.IsServer && clientId == NetworkManager.ServerClientId) + { + clientId = NetworkManager.LocalClientId; + } + // 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(); @@ -496,9 +502,12 @@ internal void DisconnectEventHandler(ulong transportClientId) { OnClientDisconnectFromServer(clientId); } - else + else // 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 and thus there is no one to send any messages to and this will cause an exception within UnityTransport as the client ID is no longer valid. + // 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 diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs index c994e4d8f3..4fd66e3bfb 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs @@ -73,13 +73,89 @@ public void NetworkUpdate(NetworkUpdateStage updateStage) if (m_ShuttingDown) { - ShutdownInternal(); + // Host-server will disconnect any connected clients prior to finalizing its shutdown + if (IsServer) + { + ProcessServerShutdown(); + } + else + { + // Clients just disconnect immediately + ShutdownInternal(); + } } } break; } } + /// + /// Used to provide a graceful shutdown sequence + /// + internal enum ServerShutdownStates + { + None, + WaitForClientDisconnects, + InternalShutdown, + ShuttingDown, + }; + + internal ServerShutdownStates ServerShutdownState; + private float m_ShutdownTimeout; + + /// + /// This is a "soft shutdown" where the host or server will disconnect + /// all clients, with a provided reasons, prior to invoking its final + /// internal shutdown. + /// + internal void ProcessServerShutdown() + { + var minClientCount = IsHost ? 2 : 1; + switch (ServerShutdownState) + { + case ServerShutdownStates.None: + { + if (ConnectedClients.Count >= minClientCount) + { + var hostServer = IsHost ? "host" : "server"; + var disconnectReason = $"Disconnected due to {hostServer} shutting down."; + for (int i = ConnectedClientsIds.Count - 1; i >= 0; i--) + { + var clientId = ConnectedClientsIds[i]; + if (clientId == ServerClientId) + { + continue; + } + ConnectionManager.DisconnectClient(clientId, disconnectReason); + } + ServerShutdownState = ServerShutdownStates.WaitForClientDisconnects; + m_ShutdownTimeout = Time.realtimeSinceStartup + 5.0f; + } + else + { + ServerShutdownState = ServerShutdownStates.InternalShutdown; + ProcessServerShutdown(); + } + break; + } + case ServerShutdownStates.WaitForClientDisconnects: + { + if (ConnectedClients.Count < minClientCount || m_ShutdownTimeout < Time.realtimeSinceStartup) + { + ServerShutdownState = ServerShutdownStates.InternalShutdown; + ProcessServerShutdown(); + } + break; + } + case ServerShutdownStates.InternalShutdown: + { + ServerShutdownState = ServerShutdownStates.ShuttingDown; + ShutdownInternal(); + break; + } + } + } + /// /// The client id used to represent the server /// @@ -704,6 +780,12 @@ public int MaximumFragmentedMessageSize internal void Initialize(bool server) { + // Make sure the ServerShutdownState is reset when initializing + if (server) + { + ServerShutdownState = ServerShutdownStates.None; + } + // Don't allow the user to start a network session if the NetworkManager is // still parented under another GameObject if (NetworkManagerCheckForParent(true)) @@ -970,7 +1052,6 @@ public bool StartHost() private void HostServerInitialize() { LocalClientId = ServerClientId; - //ConnectionManager.ConnectedClientIds.Add(ServerClientId); NetworkMetrics.SetConnectionId(LocalClientId); MessageManager.SetLocalClientId(LocalClientId); @@ -1051,11 +1132,6 @@ public void Shutdown(bool discardMessageQueue = false) MessageManager.StopProcessing = discardMessageQueue; } } - - if (NetworkConfig != null && NetworkConfig.NetworkTransport != null) - { - NetworkConfig.NetworkTransport.OnTransportEvent -= ConnectionManager.HandleNetworkEvent; - } } // Ensures that the NetworkManager is cleaned up before OnDestroy is run on NetworkObjects and NetworkBehaviours when unloading a scene with a NetworkManager From 6eea1eb03358debf8b2f83cbf927935a19eda23e Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 6 Dec 2023 16:35:49 -0600 Subject: [PATCH 05/19] update the test project exit button script is now a bit more intelligent with how it handles exiting and now logs the reason for disconnection. --- .../Assets/Scripts/ExitButtonScript.cs | 64 ++++++++++++++++++- 1 file changed, 62 insertions(+), 2 deletions(-) diff --git a/testproject/Assets/Scripts/ExitButtonScript.cs b/testproject/Assets/Scripts/ExitButtonScript.cs index 4e49c135b4..bd5dd5593a 100644 --- a/testproject/Assets/Scripts/ExitButtonScript.cs +++ b/testproject/Assets/Scripts/ExitButtonScript.cs @@ -7,14 +7,68 @@ public class ExitButtonScript : MonoBehaviour [SerializeField] private MenuReference m_SceneMenuToLoad; + [SerializeField] + private bool m_DestroyNetworkManagerOnExit = true; + + private void Start() + { + if (!NetworkManager.Singleton) + { + return; + } + + NetworkManager.Singleton.OnServerStopped += OnNetworkManagerStopped; + NetworkManager.Singleton.OnClientStopped += OnNetworkManagerStopped; + + NetworkManager.Singleton.OnServerStarted += OnNetworkManagerStarted; + NetworkManager.Singleton.OnClientStarted += OnNetworkManagerStarted; + } + + private void OnNetworkManagerStarted() + { + NetworkManager.Singleton.OnClientDisconnectCallback -= OnClientDisconnectCallback; + NetworkManager.Singleton.OnClientDisconnectCallback += OnClientDisconnectCallback; + } + + private void OnClientDisconnectCallback(ulong clientId) + { + var disconnectedReason = "Disconnected from session."; + if (NetworkManager.Singleton && !string.IsNullOrEmpty(NetworkManager.Singleton.DisconnectReason)) + { + disconnectedReason = $"{NetworkManager.Singleton.DisconnectReason}"; + } + Debug.Log($"[Client-{clientId}] {disconnectedReason}"); + } + /// /// A very basic way to exit back to the first scene in the build settings /// public void OnExitScene() { - if (NetworkManager.Singleton) + if (!NetworkManager.Singleton) + { + LoadExitScene(); + return; + } + + if (NetworkManager.Singleton.IsListening) + { + if (!NetworkManager.Singleton.ShutdownInProgress) + { + + NetworkManager.Singleton.Shutdown(); + } + } + else + { + LoadExitScene(); + } + } + + private void LoadExitScene() + { + if (m_DestroyNetworkManagerOnExit) { - NetworkManager.Singleton.Shutdown(); Destroy(NetworkManager.Singleton.gameObject); } @@ -26,6 +80,12 @@ public void OnExitScene() { SceneManager.LoadSceneAsync(0, LoadSceneMode.Single); } + } + private void OnNetworkManagerStopped(bool obj) + { + NetworkManager.Singleton.OnServerStopped -= OnNetworkManagerStopped; + NetworkManager.Singleton.OnClientStopped -= OnNetworkManagerStopped; + LoadExitScene(); } } From d81a796c29ca1977fb7f7a9a21ead453c927574f Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 6 Dec 2023 16:36:47 -0600 Subject: [PATCH 06/19] fix - testproject fixing exceptions when certain missing properties are not set (primarily buttons or toggles) --- testproject/Assets/Scripts/ConnectionModeScript.cs | 9 ++++++--- testproject/Assets/Tests/Manual/Scripts/StatsDisplay.cs | 5 ++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/testproject/Assets/Scripts/ConnectionModeScript.cs b/testproject/Assets/Scripts/ConnectionModeScript.cs index f69e37aefb..70bc46a1d9 100644 --- a/testproject/Assets/Scripts/ConnectionModeScript.cs +++ b/testproject/Assets/Scripts/ConnectionModeScript.cs @@ -256,18 +256,21 @@ private void StartClient() { NetworkManager.Singleton.StartClient(); OnNotifyConnectionEventClient?.Invoke(); - if (m_ConnectionModeButtons != null) + if (m_ConnectionModeButtons) { m_ConnectionModeButtons.SetActive(false); NetworkManager.Singleton.OnClientStopped += OnClientStopped; } - m_DisconnectClientButton?.SetActive(true); + if (m_DisconnectClientButton) + { + m_DisconnectClientButton.SetActive(true); + } } private void OnClientStopped(bool obj) { - if (m_ConnectionModeButtons != null) + if (m_ConnectionModeButtons) { NetworkManager.Singleton.OnClientStopped -= OnClientStopped; m_ConnectionModeButtons.SetActive(true); diff --git a/testproject/Assets/Tests/Manual/Scripts/StatsDisplay.cs b/testproject/Assets/Tests/Manual/Scripts/StatsDisplay.cs index 460066554a..4d45807013 100644 --- a/testproject/Assets/Tests/Manual/Scripts/StatsDisplay.cs +++ b/testproject/Assets/Tests/Manual/Scripts/StatsDisplay.cs @@ -54,7 +54,10 @@ public override void OnNetworkSpawn() } else { - m_ClientServerToggle?.SetActive(true); + if (m_ClientServerToggle) + { + m_ClientServerToggle.SetActive(false); + } UpdateButton(); } From bc6efb8a1f4c3b6218f678a4197c65513f2926e2 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 6 Dec 2023 18:16:44 -0600 Subject: [PATCH 07/19] test updating tests for changes. --- .../Runtime/PeerDisconnectCallbackTests.cs | 5 ++-- .../Tests/Runtime/StopStartRuntimeTests.cs | 24 ++++++++++++++----- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/PeerDisconnectCallbackTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/PeerDisconnectCallbackTests.cs index aea5149053..227b72ef79 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/PeerDisconnectCallbackTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/PeerDisconnectCallbackTests.cs @@ -176,9 +176,8 @@ public IEnumerator TestPeerDisconnectCallback([Values] ClientDisconnectType clie } } - // If disconnected client-side, only server will receive it. - // If disconnected server-side, one for server, one for self - Assert.AreEqual(clientDisconnectType == ClientDisconnectType.ClientDisconnectsFromServer ? 1 : 2, m_ClientDisconnectCount); + // If disconnected, the server and the client that disconnected will be notified + Assert.AreEqual(2, m_ClientDisconnectCount); // Host receives peer disconnect, dedicated server does not Assert.AreEqual(m_UseHost ? 3 : 2, m_PeerDisconnectCount); } diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/StopStartRuntimeTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/StopStartRuntimeTests.cs index 16fa90e7a7..c754c4ff83 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/StopStartRuntimeTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/StopStartRuntimeTests.cs @@ -16,15 +16,19 @@ protected override void OnOneTimeSetup() base.OnOneTimeSetup(); } + + private bool m_ServerStopped; [UnityTest] public IEnumerator WhenShuttingDownAndRestarting_SDKRestartsSuccessfullyAndStaysRunning() { // shutdown the server + m_ServerNetworkManager.OnServerStopped += OnServerStopped; m_ServerNetworkManager.Shutdown(); - // wait 1 frame because shutdowns are delayed - var nextFrameNumber = Time.frameCount + 1; - yield return new WaitUntil(() => Time.frameCount >= nextFrameNumber); + // wait until the OnServerStopped is invoked + m_ServerStopped = false; + yield return WaitForConditionOrTimeOut(() => m_ServerStopped); + AssertOnTimeout("Timed out waiting for the server to stop!"); // Verify the shutdown occurred Assert.IsFalse(m_ServerNetworkManager.IsServer); @@ -45,15 +49,23 @@ public IEnumerator WhenShuttingDownAndRestarting_SDKRestartsSuccessfullyAndStays Assert.IsTrue(m_ServerNetworkManager.IsListening); } + private void OnServerStopped(bool obj) + { + m_ServerNetworkManager.OnServerStopped -= OnServerStopped; + m_ServerStopped = true; + } + [UnityTest] public IEnumerator WhenShuttingDownTwiceAndRestarting_SDKRestartsSuccessfullyAndStaysRunning() { // shutdown the server + m_ServerNetworkManager.OnServerStopped += OnServerStopped; m_ServerNetworkManager.Shutdown(); - // wait 1 frame because shutdowns are delayed - var nextFrameNumber = Time.frameCount + 1; - yield return new WaitUntil(() => Time.frameCount >= nextFrameNumber); + // wait until the OnServerStopped is invoked + m_ServerStopped = false; + yield return WaitForConditionOrTimeOut(() => m_ServerStopped); + AssertOnTimeout("Timed out waiting for the server to stop!"); // Verify the shutdown occurred Assert.IsFalse(m_ServerNetworkManager.IsServer); From 1b905f303175791e7fe6ddd56aff5852d183aef4 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 6 Dec 2023 19:17:59 -0600 Subject: [PATCH 08/19] fix Don't allow a host or server to disconnect its local client. --- .../Runtime/Connection/NetworkConnectionManager.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index cd91b72b6a..2f40dfa348 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -1045,6 +1045,12 @@ internal void DisconnectClient(ulong clientId, string reason = null) throw new NotServerException($"Only server can disconnect remote clients. Please use `{nameof(Shutdown)}()` instead."); } + if (clientId == NetworkManager.ServerClientId) + { + Debug.LogWarning($"Disconnecting the local server-host client is not allowed. Use NetworkManager.Shutdown instead."); + return; + } + if (!string.IsNullOrEmpty(reason)) { var disconnectReason = new DisconnectReasonMessage From 5fafb0f0e2d8d860e8615450101bf862e2daf651 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 6 Dec 2023 19:18:11 -0600 Subject: [PATCH 09/19] update Adding changelog entries --- com.unity.netcode.gameobjects/CHANGELOG.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 25cf88150c..6029f718c0 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -21,6 +21,12 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed +- Fixed issue where a client disconnected by a server-host would not receive a local notification. (#2789) +- Fixed issue where a server-host could shutdown during a relay connection but periodically the transport disconnect message sent to any connected clients could be dropped. (#2789) +- Fixed issue where a host could disconnect its local client but remain running as a server. (#2789) +- Fixed issue where `OnClientDisconnectedCallback` was not being invoked under certain conditions. (#2789) +- Fixed issue where `OnClientDisconnectedCallback` was always returning 0 as the client identifier. (#2789) +- Fixed issue where if a host or server shutdown while a client owned NetworkObjects (other than the player) it would throw an exception. - Fixed issue where a teleport state could potentially be overridden by a previous unreliable delta state. (#2777) - Fixed issue where `NetworkTransform` was using the `NetworkManager.ServerTime.Tick` as opposed to `NetworkManager.NetworkTickSystem.ServerTime.Tick` during the authoritative side's tick update where it performed a delta state check. (#2777) - Fixed issue where a parented in-scene placed NetworkObject would be destroyed upon a client or server exiting a network session but not unloading the original scene in which the NetworkObject was placed. (#2737) @@ -28,7 +34,8 @@ Additional documentation and release notes are available at [Multiplayer Documen - Fixed issue where a `NetworkTransform` instance with interpolation enabled would result in wide visual motion gaps (stuttering) under above normal latency conditions and a 1-5% or higher packet are drop rate. (#2713) ### Changed - +- Changed the server or host shutdown so it will now perform a "soft shutdown" when `NetworkManager.Shutdown` is invoked. This will send a disconnect notification to all connected clients and the server-host will wait for all connected clients to disconnect or timeout after a 5 second period before completing the shutdown process. (#2789) +- Changed `OnClientDisconnectedCallback` will now return the assigned client identifier on the local client side if the client was approved and assigned one prior to being disconnected. (#2789) - Changed `NetworkTransform.SetState` (and related methods) now are cumulative during a fractional tick period and sent on the next pending tick. (#2777) - `NetworkManager.ConnectedClientsIds` is now accessible on the client side and will contain the list of all clients in the session, including the host client if the server is operating in host mode (#2762) - Changed `NetworkSceneManager` to return a `SceneEventProgress` status and not throw exceptions for methods invoked when scene management is disabled and when a client attempts to access a `NetworkSceneManager` method by a client. (#2735) From 45e2aca85313bae01ec0fe773fb118ab765bcad6 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 6 Dec 2023 19:28:16 -0600 Subject: [PATCH 10/19] style removing unused namespace. --- .../Tests/Runtime/StopStartRuntimeTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/StopStartRuntimeTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/StopStartRuntimeTests.cs index c754c4ff83..8d52f1fc63 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/StopStartRuntimeTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/StopStartRuntimeTests.cs @@ -1,7 +1,6 @@ using System.Collections; using NUnit.Framework; using Unity.Netcode.TestHelpers.Runtime; -using UnityEngine; using UnityEngine.TestTools; namespace Unity.Netcode.RuntimeTests From a1cb81978c2a7bd8933bcb86de61806bf7d03a64 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 6 Dec 2023 20:57:45 -0600 Subject: [PATCH 11/19] fix Fixes issue with NetworkVariable and NetworkList throwing exceptions upon shutdown. --- .../Runtime/NetworkVariable/NetworkVariableBase.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs index c72813ee7e..27d4b644c5 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs @@ -100,7 +100,16 @@ protected void MarkNetworkBehaviourDirty() "Are you modifying a NetworkVariable before the NetworkObject is spawned?"); return; } - + if (m_NetworkBehaviour.NetworkManager.ShutdownInProgress) + { + if (m_NetworkBehaviour.NetworkManager.LogLevel <= LogLevel.Developer) + { + Debug.LogWarning($"NetworkVariable is written to, but is happenging during the NetworkManager shutdown! " + + "Are you modifying a NetworkVariable within a NetworkBehaviour.OnDestroy or NetworkBehaviour.OnDespawn method? " + + "If so, avoid trying to set NetworkVariables during despawn or destroy."); + } + return; + } m_NetworkBehaviour.NetworkManager.BehaviourUpdater.AddForUpdate(m_NetworkBehaviour.NetworkObject); } From fff05964454f1c85db2a2c1b530493c090636aef Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 6 Dec 2023 20:58:26 -0600 Subject: [PATCH 12/19] Test Validates that setting NetworkVariables or NetworkList values during OnNetworkDespawn does not throw an exception. --- .../Tests/Runtime/NetworkVariableTests.cs | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariableTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariableTests.cs index 02afdc8dec..e1ceff24f3 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariableTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariableTests.cs @@ -2769,4 +2769,71 @@ public IEnumerator TestInheritedFields() Assert.IsFalse(s_GlobalTimeoutHelper.TimedOut, nameof(CheckTestObjectComponentValuesOnAll)); } } + + public class NetvarDespawnShutdown : NetworkBehaviour + { + private NetworkVariable m_IntNetworkVariable = new NetworkVariable(); + private NetworkList m_IntList; + + private void Awake() + { + m_IntList = new NetworkList(); + } + + public override void OnNetworkDespawn() + { + if (IsServer && m_IntList != null) + { + m_IntNetworkVariable.Value = 5; + for (int i = 0; i < 10; i++) + { + m_IntList.Add(i); + } + } + base.OnNetworkDespawn(); + } + } + + /// + /// Validates that setting values for NetworkVariable or NetworkList during the + /// OnNetworkDespawn method will not cause an exception to occur. + /// + public class NetworkVariableModifyOnNetworkDespawn : NetcodeIntegrationTest + { + protected override int NumberOfClients => 1; + + private GameObject m_TestPrefab; + + protected override void OnServerAndClientsCreated() + { + m_TestPrefab = CreateNetworkObjectPrefab("NetVarDespawn"); + m_TestPrefab.AddComponent(); + base.OnServerAndClientsCreated(); + } + + private bool OnClientSpawnedTestPrefab(ulong networkObjectId) + { + var clientId = m_ClientNetworkManagers[0].LocalClientId; + if (!s_GlobalNetworkObjects.ContainsKey(clientId)) + { + return false; + } + + if (!s_GlobalNetworkObjects[clientId].ContainsKey(networkObjectId)) + { + return false; + } + + return true; + } + + [UnityTest] + public IEnumerator ModifyNetworkVariableOrListOnNetworkDespawn() + { + var instance = SpawnObject(m_TestPrefab, m_ServerNetworkManager); + yield return WaitForConditionOrTimeOut(() => OnClientSpawnedTestPrefab(instance.GetComponent().NetworkObjectId)); + m_ServerNetworkManager.Shutdown(); + // As long as no excetptions occur, the test passes. + } + } } From 92eeca5d6e0136c4e8d4bf436463f0bdea857702 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 6 Dec 2023 21:02:19 -0600 Subject: [PATCH 13/19] update adding change log entry --- com.unity.netcode.gameobjects/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 6029f718c0..b34bf63f11 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -26,7 +26,8 @@ Additional documentation and release notes are available at [Multiplayer Documen - Fixed issue where a host could disconnect its local client but remain running as a server. (#2789) - Fixed issue where `OnClientDisconnectedCallback` was not being invoked under certain conditions. (#2789) - Fixed issue where `OnClientDisconnectedCallback` was always returning 0 as the client identifier. (#2789) -- Fixed issue where if a host or server shutdown while a client owned NetworkObjects (other than the player) it would throw an exception. +- Fixed issue where if a host or server shutdown while a client owned NetworkObjects (other than the player) it would throw an exception. (#2789) +- Fixed issue where setting values on a `NetworkVariable` or `NetworkList` within `OnNetworkDespawn` during a shutdown sequence would throw an exception. (#2789) - Fixed issue where a teleport state could potentially be overridden by a previous unreliable delta state. (#2777) - Fixed issue where `NetworkTransform` was using the `NetworkManager.ServerTime.Tick` as opposed to `NetworkManager.NetworkTickSystem.ServerTime.Tick` during the authoritative side's tick update where it performed a delta state check. (#2777) - Fixed issue where a parented in-scene placed NetworkObject would be destroyed upon a client or server exiting a network session but not unloading the original scene in which the NetworkObject was placed. (#2737) From 75038bbd4a177da3a7508ad325066c1c8d5c4a40 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 6 Dec 2023 21:12:26 -0600 Subject: [PATCH 14/19] style fixing warning message about setting network variable/list values during a shutdown. --- .../Runtime/NetworkVariable/NetworkVariableBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs index 27d4b644c5..7ae29e84d3 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs @@ -104,7 +104,7 @@ protected void MarkNetworkBehaviourDirty() { if (m_NetworkBehaviour.NetworkManager.LogLevel <= LogLevel.Developer) { - Debug.LogWarning($"NetworkVariable is written to, but is happenging during the NetworkManager shutdown! " + + Debug.LogWarning($"NetworkVariable is written to during the NetworkManager shutdown! " + "Are you modifying a NetworkVariable within a NetworkBehaviour.OnDestroy or NetworkBehaviour.OnDespawn method? " + "If so, avoid trying to set NetworkVariables during despawn or destroy."); } From 5199a5a72c1efe3888ec8543a8a5b05ce66baf05 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 6 Dec 2023 21:14:55 -0600 Subject: [PATCH 15/19] test Removing unrequired null check --- .../Tests/Runtime/NetworkVariableTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariableTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariableTests.cs index e1ceff24f3..8c17d1bb3e 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariableTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariableTests.cs @@ -2782,7 +2782,7 @@ private void Awake() public override void OnNetworkDespawn() { - if (IsServer && m_IntList != null) + if (IsServer) { m_IntNetworkVariable.Value = 5; for (int i = 0; i < 10; i++) From 9c87381c969bac4245fe0f3bbbf1ca1146c0d228 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Fri, 8 Dec 2023 13:50:50 -0600 Subject: [PATCH 16/19] style remove the cr/lf after some else if statments --- .../Runtime/Connection/NetworkConnectionManager.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index 2f40dfa348..0f98ece31d 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -934,8 +934,7 @@ internal void OnClientDisconnectFromServer(ulong clientId) NetworkManager.SpawnManager.DespawnObject(playerObject, true); } } - else - if (!NetworkManager.ShutdownInProgress) + else if (!NetworkManager.ShutdownInProgress) { playerObject.RemoveOwnership(); } @@ -971,8 +970,7 @@ internal void OnClientDisconnectFromServer(ulong clientId) Object.Destroy(ownedObject.gameObject); } } - else - if (!NetworkManager.ShutdownInProgress) + else if (!NetworkManager.ShutdownInProgress) { ownedObject.RemoveOwnership(); } From 58c5846326cac472e7946bf8645671491c837d2f Mon Sep 17 00:00:00 2001 From: Kitty Draper Date: Fri, 8 Dec 2023 14:31:21 -0600 Subject: [PATCH 17/19] Minor formatting changes. --- com.unity.netcode.gameobjects/Components/NetworkAnimator.cs | 6 ++---- .../Manual/InSceneObjectParentingTests/ChildObjectScript.cs | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/com.unity.netcode.gameobjects/Components/NetworkAnimator.cs b/com.unity.netcode.gameobjects/Components/NetworkAnimator.cs index 538b4b9c88..79030b7be7 100644 --- a/com.unity.netcode.gameobjects/Components/NetworkAnimator.cs +++ b/com.unity.netcode.gameobjects/Components/NetworkAnimator.cs @@ -833,8 +833,7 @@ private void CheckForStateChange(int layer) stateChangeDetected = true; //Debug.Log($"[Cross-Fade] To-Hash: {nt.fullPathHash} | TI-Duration: ({tt.duration}) | TI-Norm: ({tt.normalizedTime}) | From-Hash: ({m_AnimationHash[layer]}) | SI-FPHash: ({st.fullPathHash}) | SI-Norm: ({st.normalizedTime})"); } - else - if (!tt.anyState && tt.fullPathHash != m_TransitionHash[layer]) + else if (!tt.anyState && tt.fullPathHash != m_TransitionHash[layer]) { // first time in this transition for this layer m_TransitionHash[layer] = tt.fullPathHash; @@ -1053,8 +1052,7 @@ private unsafe void WriteParameters(ref FastBufferWriter writer) BytePacker.WriteValuePacked(writer, valueBool); } } - else - if (cacheValue.Type == AnimationParamEnumWrapper.AnimatorControllerParameterFloat) + else if (cacheValue.Type == AnimationParamEnumWrapper.AnimatorControllerParameterFloat) { var valueFloat = m_Animator.GetFloat(hash); fixed (void* value = cacheValue.Value) diff --git a/testproject/Assets/Tests/Manual/InSceneObjectParentingTests/ChildObjectScript.cs b/testproject/Assets/Tests/Manual/InSceneObjectParentingTests/ChildObjectScript.cs index 0eb38e85fb..e3413a4ec1 100644 --- a/testproject/Assets/Tests/Manual/InSceneObjectParentingTests/ChildObjectScript.cs +++ b/testproject/Assets/Tests/Manual/InSceneObjectParentingTests/ChildObjectScript.cs @@ -71,8 +71,7 @@ public override void OnNetworkObjectParentChanged(NetworkObject parentNetworkObj transform.localRotation = m_OriginalLocalRotation; transform.localScale = m_OriginalLocalScale; } - else - if (parentNetworkObject == null && m_LastParent) + else if (parentNetworkObject == null && m_LastParent) { // This example will drop the object at the current world position transform.position = m_LastParent.transform.position; From 68a93f859e7230df2196af4b267c82faed3413e4 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Fri, 8 Dec 2023 14:48:50 -0600 Subject: [PATCH 18/19] update Adjusting warning message language. Adjusting when the send queue is processed. --- .../Runtime/Connection/NetworkConnectionManager.cs | 12 +++++++----- .../Runtime/NetworkVariable/NetworkVariableBase.cs | 3 +-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index 2f40dfa348..0ea8428fcc 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -994,7 +994,10 @@ internal void OnClientDisconnectFromServer(ulong clientId) ConnectedClientIds.Remove(clientId); var message = new ClientDisconnectedMessage { ClientId = clientId }; - NetworkManager.MessageManager.SendMessage(ref message, NetworkDelivery.ReliableFragmentedSequenced, ConnectedClientIds); + MessageManager?.SendMessage(ref message, NetworkDelivery.ReliableFragmentedSequenced, ConnectedClientIds); + + // Send the message immediately + MessageManager?.ProcessSendQueues(); } // If the client ID transport map exists @@ -1095,7 +1098,6 @@ internal void Initialize(NetworkManager networkManager) /// internal void Shutdown() { - if (LocalClient.IsServer) { // Build a list of all client ids to be disconnected @@ -1133,12 +1135,12 @@ internal void Shutdown() { DisconnectRemoteClient(clientId); } - - // make sure all messages are flushed before transport disconnect clients - MessageManager?.ProcessSendQueues(); } else if (NetworkManager != null && NetworkManager.IsListening && LocalClient.IsClient) { + // make sure all messages are flushed before transport disconnect clients + MessageManager?.ProcessSendQueues(); + // Client only, send disconnect and if transport throws and exception, log the exception and continue the shutdown sequence (or forever be shutting down) try { diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs index 7ae29e84d3..927687c692 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs @@ -105,8 +105,7 @@ protected void MarkNetworkBehaviourDirty() if (m_NetworkBehaviour.NetworkManager.LogLevel <= LogLevel.Developer) { Debug.LogWarning($"NetworkVariable is written to during the NetworkManager shutdown! " + - "Are you modifying a NetworkVariable within a NetworkBehaviour.OnDestroy or NetworkBehaviour.OnDespawn method? " + - "If so, avoid trying to set NetworkVariables during despawn or destroy."); + "Are you modifying a NetworkVariable within a NetworkBehaviour.OnDestroy or NetworkBehaviour.OnDespawn method?"); } return; } From 5ebc986303f49005b9955db9231323f5f1d76fd6 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Fri, 8 Dec 2023 17:27:10 -0600 Subject: [PATCH 19/19] update reverting back moving the MessageManager?.ProcessSendQueues(); within Shutdown and removing the migration of that out of OnClientDisconnectFromServer. --- .../Runtime/Connection/NetworkConnectionManager.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index fba9182162..d73759874a 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -993,9 +993,6 @@ internal void OnClientDisconnectFromServer(ulong clientId) ConnectedClientIds.Remove(clientId); var message = new ClientDisconnectedMessage { ClientId = clientId }; MessageManager?.SendMessage(ref message, NetworkDelivery.ReliableFragmentedSequenced, ConnectedClientIds); - - // Send the message immediately - MessageManager?.ProcessSendQueues(); } // If the client ID transport map exists @@ -1133,10 +1130,13 @@ internal void Shutdown() { DisconnectRemoteClient(clientId); } + + // make sure all messages are flushed before transport disconnects clients + MessageManager?.ProcessSendQueues(); } else if (NetworkManager != null && NetworkManager.IsListening && LocalClient.IsClient) { - // make sure all messages are flushed before transport disconnect clients + // make sure all messages are flushed before disconnecting MessageManager?.ProcessSendQueues(); // Client only, send disconnect and if transport throws and exception, log the exception and continue the shutdown sequence (or forever be shutting down)