From 0e9d664512349659f6da2f97b01519b247e550ac Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Tue, 28 Oct 2025 18:46:47 -0500 Subject: [PATCH 1/4] fix Fixing issue with duplicated disconnect reasons for all cases. --- .../Connection/NetworkConnectionManager.cs | 55 ++++++++++++++++--- .../Messaging/DisconnectReasonMessage.cs | 5 +- 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index 7c39673e3a..1685da74e6 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -103,11 +103,52 @@ public sealed class NetworkConnectionManager private static ProfilerMarker s_TransportDisconnect = new ProfilerMarker($"{nameof(NetworkManager)}.TransportDisconnect"); #endif + private string m_DisconnectReason; /// /// When disconnected from the server, the server may send a reason. If a reason was sent, this property will - /// tell client code what the reason was. It should be queried after the OnClientDisconnectCallback is called + /// provide disconnect information that will be followed by the server's disconnect reason. /// - public string DisconnectReason { get; internal set; } + /// + /// On a server or host, this value could no longer exist after all subscribed callbacks are invoked for the + /// client that disconnected. It is recommended to copy the message to some other property or field when + /// is invoked. + /// + public string DisconnectReason + { + get + { + // For in-frequent event driven invocations, a method within a getter + // is "generally ok". + return GetDisconnectReason(); + } + internal set + { + m_DisconnectReason = value; + } + } + + /// + /// Returns the conbined result of the locally applied and the + /// server applied . + /// - If both values are empty or null, then it returns . + /// - If either value is valid, then it returns that value. + /// - If both values are valid, then it returns followed by a + /// new line and then . + /// + /// A disconnect reason, if any. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal string GetDisconnectReason(string header = null) + { + var disconnectReason = string.IsNullOrEmpty(m_DisconnectReason) ? string.Empty : m_DisconnectReason; + var serverDisconnectReason = string.IsNullOrEmpty(ServerDisconnectReason) ? string.Empty : $"\n{ServerDisconnectReason}"; + var headerInfo = string.IsNullOrEmpty(header) ? string.Empty : header; + return $"{headerInfo}{disconnectReason}{serverDisconnectReason}"; + } + + /// + /// Updated by . + /// + internal string ServerDisconnectReason; /// /// The callback to invoke once a client connects. This callback is only ran on the server and on the local client that connects. @@ -537,17 +578,15 @@ internal void DataEventHandler(ulong transportClientId, ref ArraySegment p private void GenerateDisconnectInformation(ulong clientId, ulong transportClientId, string reason = null) { var header = $"[Disconnect Event][Client-{clientId}][TransportClientId-{transportClientId}]"; - var existingDisconnectReason = DisconnectReason; - var defaultMessage = Transport.DisconnectEventMessage; if (reason != null) { defaultMessage = $"{reason} {defaultMessage}"; } + // Just go ahead and set this whether client or server so any subscriptions to a disconnect event can check the DisconnectReason // to determine why the client disconnected - DisconnectReason = $"{header}[{Transport.DisconnectEvent}] {defaultMessage}"; - DisconnectReason = $"{DisconnectReason}\n{existingDisconnectReason}"; + m_DisconnectReason = $"{header}[{Transport.DisconnectEvent}] {defaultMessage}"; if (NetworkLog.CurrentLogLevel <= LogLevel.Developer) { @@ -1450,7 +1489,6 @@ internal void DisconnectClient(ulong clientId, string reason = null) var transportId = ClientIdToTransportId(clientId); if (transportId.Item2) { - DisconnectReason = string.Empty; GenerateDisconnectInformation(clientId, transportId.Item1, reason); } @@ -1476,7 +1514,8 @@ internal void Initialize(NetworkManager networkManager) TransportIdToClientIdMap.Clear(); ClientsToApprove.Clear(); NetworkObject.OrphanChildren.Clear(); - DisconnectReason = string.Empty; + m_DisconnectReason = string.Empty; + ServerDisconnectReason = string.Empty; NetworkManager = networkManager; MessageManager = networkManager.MessageManager; diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/DisconnectReasonMessage.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/DisconnectReasonMessage.cs index b26982ea1f..1e698df9a4 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/DisconnectReasonMessage.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/DisconnectReasonMessage.cs @@ -37,7 +37,10 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int public void Handle(ref NetworkContext context) { - ((NetworkManager)context.SystemOwner).ConnectionManager.DisconnectReason = Reason; + // Always apply the server-side generated disconnect reason to the server specific disconnect reason. + // This is combined with the additional disconnect information when getting NetworkManager.DisconnectReason + // (NetworkConnectionManager.DisconnectReason). + ((NetworkManager)context.SystemOwner).ConnectionManager.ServerDisconnectReason = Reason; } }; } From 7f30084a7b65a09c52c9b233e3e3b6fed6e856bd Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Tue, 28 Oct 2025 18:47:37 -0500 Subject: [PATCH 2/4] test - update Making adjustments to the validation logic checking to determine if a server's disconnect reason is included in the NetworkManager.DisconnectReaon property. --- .../Tests/Runtime/Messaging/DisconnectReasonTests.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/DisconnectReasonTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/DisconnectReasonTests.cs index e293be3e64..3d65334c86 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/DisconnectReasonTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/DisconnectReasonTests.cs @@ -57,9 +57,8 @@ public IEnumerator DisconnectReasonTest() yield return null; } - Assert.AreEqual(m_ClientNetworkManagers[0].DisconnectReason, "Bogus reason 1"); - Assert.AreEqual(m_ClientNetworkManagers[1].DisconnectReason, "Bogus reason 2"); - + Assert.True(m_ClientNetworkManagers[0].DisconnectReason.Contains("Bogus reason 1"), $"[Client-{m_ClientNetworkManagers[0].LocalClientId}] Disconnect reason should contain \"Bogus reason 1\" but is: {m_ClientNetworkManagers[0].DisconnectReason}"); + Assert.True(m_ClientNetworkManagers[1].DisconnectReason.Contains("Bogus reason 2"), $"[Client-{m_ClientNetworkManagers[0].LocalClientId}] Disconnect reason should contain \"Bogus reason 2\" but is: {m_ClientNetworkManagers[1].DisconnectReason}"); Debug.Assert(m_DisconnectCount == 2); } From b5f86875e56159208b77e6838912941123e035fe Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Tue, 28 Oct 2025 19:00:54 -0500 Subject: [PATCH 3/4] style Removing a trailing whitespace --- .../Runtime/Connection/NetworkConnectionManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index 1685da74e6..1c60bfa1c9 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -132,7 +132,7 @@ internal set /// server applied . /// - If both values are empty or null, then it returns . /// - If either value is valid, then it returns that value. - /// - If both values are valid, then it returns followed by a + /// - If both values are valid, then it returns followed by a /// new line and then . /// /// A disconnect reason, if any. From cf20cda3c60fc45c2c79d365a9e93e339edfd148 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Wed, 29 Oct 2025 01:00:11 -0500 Subject: [PATCH 4/4] test - update One more area that needs to check for the server reason but not be a 1:1 due to the additional disconnect information before it. --- .../Tests/Runtime/ConnectionApproval.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApproval.cs b/com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApproval.cs index fdf8cc3836..b630136c61 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApproval.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApproval.cs @@ -92,7 +92,7 @@ protected override void OnServerAndClientsCreated() private void Client_OnClientDisconnectCallback(ulong clientId) { m_ClientNetworkManagers[0].OnClientDisconnectCallback -= Client_OnClientDisconnectCallback; - m_ClientDisconnectReasonValidated = m_ClientNetworkManagers[0].LocalClientId == clientId && m_ClientNetworkManagers[0].DisconnectReason == k_InvalidToken; + m_ClientDisconnectReasonValidated = m_ClientNetworkManagers[0].LocalClientId == clientId && m_ClientNetworkManagers[0].DisconnectReason.Contains(k_InvalidToken); } private bool ClientAndHostValidated()