From c97f1be771173ddbbfe3d8402a8809f92ddf5b9a Mon Sep 17 00:00:00 2001 From: Simon Lemay Date: Mon, 17 Mar 2025 12:10:09 -0400 Subject: [PATCH 1/5] Remove internal NetworkDriver accessor --- .../Runtime/Transports/UTP/UnityTransport.cs | 3 +-- .../Tests/Editor/Transports/UnityTransportTests.cs | 2 +- .../Tests/Runtime/Metrics/PacketLossMetricsTests.cs | 4 ++-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs b/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs index 3bd74f7aab..c836c0cead 100644 --- a/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs +++ b/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs @@ -421,7 +421,6 @@ private struct PacketLossCache internal static event Action TransportInitialized; internal static event Action TransportDisposed; - internal NetworkDriver NetworkDriver => m_Driver; /// /// Provides access to the for this instance. @@ -501,7 +500,7 @@ private void InitDriver() out m_UnreliableSequencedFragmentedPipeline, out m_ReliableSequencedPipeline); - TransportInitialized?.Invoke(GetInstanceID(), NetworkDriver); + TransportInitialized?.Invoke(GetInstanceID(), m_Driver); } private void DisposeInternals() diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Transports/UnityTransportTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/Transports/UnityTransportTests.cs index 87ffbdb418..bd9d9e937e 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Transports/UnityTransportTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Editor/Transports/UnityTransportTests.cs @@ -189,7 +189,7 @@ public void UnityTransport_EmptySecurityStringsShouldThrow([Values("", null)] st networkManager.StartServer(); }); // Make sure StartServer failed - Assert.False(transport.NetworkDriver.IsCreated); + Assert.False(transport.GetNetworkDriver().IsCreated); Assert.False(networkManager.IsServer); Assert.False(networkManager.IsListening); } diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/PacketLossMetricsTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/PacketLossMetricsTests.cs index 2956f48fda..1f9a35390f 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/PacketLossMetricsTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Metrics/PacketLossMetricsTests.cs @@ -64,9 +64,9 @@ public IEnumerator TrackPacketLossAsClient() var clientNetworkManager = m_ClientNetworkManagers[0]; var clientTransport = (UnityTransport)clientNetworkManager.NetworkConfig.NetworkTransport; - clientTransport.NetworkDriver.CurrentSettings.TryGet(out var parameters); + clientTransport.GetNetworkDriver().CurrentSettings.TryGet(out var parameters); parameters.PacketDropPercentage = m_PacketLossRate; - clientTransport.NetworkDriver.ModifySimulatorStageParameters(parameters); + clientTransport.GetNetworkDriver().ModifySimulatorStageParameters(parameters); var waitForPacketLossMetric = new WaitForGaugeMetricValues((clientNetworkManager.NetworkMetrics as NetworkMetrics).Dispatcher, NetworkMetricTypes.PacketLoss, From 8d23970b59c928ef5d78b0f13364ddaf789175d0 Mon Sep 17 00:00:00 2001 From: Simon Lemay Date: Mon, 24 Mar 2025 17:03:35 -0400 Subject: [PATCH 2/5] Remove internal "state" from UnityTransport --- .../Runtime/Transports/UTP/UnityTransport.cs | 41 ++++++------------- .../Runtime/Transports/UnityTransportTests.cs | 4 +- 2 files changed, 15 insertions(+), 30 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs b/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs index c836c0cead..cf954050da 100644 --- a/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs +++ b/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs @@ -114,13 +114,6 @@ public enum ProtocolType RelayUnityTransport, } - private enum State - { - Disconnected, - Listening, - Connected, - } - /// /// The default maximum (receive) packet queue size /// @@ -454,7 +447,6 @@ public NetworkEndpoint GetLocalEndpoint() private PacketLossCache m_PacketLossCache = new PacketLossCache(); - private State m_State = State.Disconnected; private NetworkSettings m_NetworkSettings; private ulong m_ServerClientId; @@ -582,8 +574,7 @@ private bool ClientBindAndConnect() return false; } - var serverConnection = Connect(serverEndpoint); - m_ServerClientId = ParseClientId(serverConnection); + Connect(serverEndpoint); return true; } @@ -623,7 +614,6 @@ private bool ServerBindAndListen(NetworkEndpoint endPoint) return false; } - m_State = State.Listening; return true; } @@ -904,26 +894,20 @@ private bool ProcessEvent() default, m_RealTimeProvider.RealTimeSinceStartup); - m_State = State.Connected; + m_ServerClientId = clientId; return true; } case TransportNetworkEvent.Type.Disconnect: { - // Handle cases where we're a client receiving a Disconnect event. The - // meaning of the event depends on our current state. If we were connected - // then it means we got disconnected. If we were disconnected means that our - // connection attempt has failed. - if (m_State == State.Connected) - { - m_State = State.Disconnected; - m_ServerClientId = default; - } - else if (m_State == State.Disconnected) + // If we're a client and had not yet set the server client ID, it means + // our connection to the server failed to be established. Any other case + // means a clean disconnect that doesn't require logging. + if (!m_Driver.Listening && m_ServerClientId == default) { Debug.LogError("Failed to connect to server."); - m_ServerClientId = default; } + m_ServerClientId = default; m_ReliableReceiveQueues.Remove(clientId); ClearSendQueuesForClientId(clientId); @@ -1179,13 +1163,13 @@ private void FlushSendQueuesForClientId(ulong clientId) /// public override void DisconnectLocalClient() { - if (m_State == State.Connected) + if (m_ServerClientId != default) { FlushSendQueuesForClientId(m_ServerClientId); if (m_Driver.Disconnect(ParseClientId(m_ServerClientId)) == 0) { - m_State = State.Disconnected; + m_ServerClientId = default; m_ReliableReceiveQueues.Remove(m_ServerClientId); ClearSendQueuesForClientId(m_ServerClientId); @@ -1208,14 +1192,14 @@ public override void DisconnectLocalClient() public override void DisconnectRemoteClient(ulong clientId) { #if DEBUG - if (m_State != State.Listening) + if (!m_Driver.IsCreated) { Debug.LogWarning($"{nameof(DisconnectRemoteClient)} should only be called on a listening server!"); return; } #endif - if (m_State == State.Listening) + if (m_Driver.IsCreated) { FlushSendQueuesForClientId(clientId); @@ -1514,10 +1498,9 @@ public override void Shutdown() DisposeInternals(); m_ReliableReceiveQueues.Clear(); - m_State = State.Disconnected; // We must reset this to zero because UTP actually re-uses clientIds if there is a clean disconnect - m_ServerClientId = 0; + m_ServerClientId = default; } private void ConfigureSimulator() diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTests.cs index 4cd33fcc9b..83af69383d 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Transports/UnityTransportTests.cs @@ -311,6 +311,8 @@ public IEnumerator DisconnectOnReliableSendQueueOverflow() yield return WaitForNetworkEvent(NetworkEvent.Connect, m_Client1Events); + var serverClientId = m_Client1.ServerClientId; + m_Server.Shutdown(); var numSends = (maxSendQueueSize / 1024); @@ -322,7 +324,7 @@ public IEnumerator DisconnectOnReliableSendQueueOverflow() } LogAssert.Expect(LogType.Error, "Couldn't add payload of size 1024 to reliable send queue. " + - $"Closing connection {m_Client1.ServerClientId} as reliability guarantees can't be maintained."); + $"Closing connection {serverClientId} as reliability guarantees can't be maintained."); Assert.AreEqual(2, m_Client1Events.Count); Assert.AreEqual(NetworkEvent.Disconnect, m_Client1Events[1].Type); From 82d09dc49cd5e8809c01569ccc1dc6237cc0a141 Mon Sep 17 00:00:00 2001 From: Simon Lemay Date: Wed, 26 Mar 2025 13:32:16 -0400 Subject: [PATCH 3/5] Clean up using directives --- .../Runtime/Transports/UTP/UnityTransport.cs | 50 ++++++++++--------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs b/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs index cf954050da..1b7c285602 100644 --- a/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs +++ b/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs @@ -16,8 +16,10 @@ using Unity.Networking.Transport.TLS; using Unity.Networking.Transport.Utilities; using UnityEngine; -using NetcodeNetworkEvent = Unity.Netcode.NetworkEvent; -using TransportNetworkEvent = Unity.Networking.Transport.NetworkEvent; + +using NetcodeEvent = Unity.Netcode.NetworkEvent; +using TransportError = Unity.Networking.Transport.Error.StatusCode; +using TransportEvent = Unity.Networking.Transport.NetworkEvent.Type; namespace Unity.Netcode.Transports.UTP { @@ -60,7 +62,7 @@ public static class ErrorUtilities /// UTP error code. /// ID of the connection on which the error occurred. /// Human-readable error message. - public static string ErrorToString(Networking.Transport.Error.StatusCode error, ulong connectionId) + public static string ErrorToString(TransportError error, ulong connectionId) { return ErrorToString((int)error, connectionId); } @@ -72,19 +74,19 @@ internal static string ErrorToString(int error, ulong connectionId) internal static FixedString128Bytes ErrorToFixedString(int error, ulong connectionId) { - switch ((Networking.Transport.Error.StatusCode)error) + switch ((TransportError)error) { - case Networking.Transport.Error.StatusCode.Success: + case TransportError.Success: return k_NetworkSuccess; - case Networking.Transport.Error.StatusCode.NetworkIdMismatch: + case TransportError.NetworkIdMismatch: return FixedString.Format(k_NetworkIdMismatch, connectionId); - case Networking.Transport.Error.StatusCode.NetworkVersionMismatch: + case TransportError.NetworkVersionMismatch: return FixedString.Format(k_NetworkVersionMismatch, connectionId); - case Networking.Transport.Error.StatusCode.NetworkStateMismatch: + case TransportError.NetworkStateMismatch: return FixedString.Format(k_NetworkStateMismatch, connectionId); - case Networking.Transport.Error.StatusCode.NetworkPacketOverflow: + case TransportError.NetworkPacketOverflow: return k_NetworkPacketOverflow; - case Networking.Transport.Error.StatusCode.NetworkSendQueueFull: + case TransportError.NetworkSendQueueFull: return k_NetworkSendQueueFull; default: return FixedString.Format("Unknown error code {0}.", error); @@ -765,7 +767,7 @@ public void Execute() while (!Queue.IsEmpty) { var result = Driver.BeginSend(pipeline, connection, out var writer); - if (result != (int)Networking.Transport.Error.StatusCode.Success) + if (result != (int)TransportError.Success) { Debug.LogError($"Error sending message: {ErrorUtilities.ErrorToFixedString(result, clientId)}"); return; @@ -792,7 +794,7 @@ public void Execute() // and we'll retry sending them later). Otherwise log the error and remove the // message from the queue (we don't want to resend it again since we'll likely // just get the same error again). - if (result != (int)Networking.Transport.Error.StatusCode.NetworkSendQueueFull) + if (result != (int)TransportError.NetworkSendQueueFull) { Debug.LogError($"Error sending the message: {ErrorUtilities.ErrorToFixedString(result, clientId)}"); Queue.Consume(written); @@ -838,7 +840,7 @@ private bool AcceptConnection() return false; } - InvokeOnTransportEvent(NetcodeNetworkEvent.Connect, + InvokeOnTransportEvent(NetcodeEvent.Connect, ParseClientId(connection), default, m_RealTimeProvider.RealTimeSinceStartup); @@ -876,7 +878,7 @@ private void ReceiveMessages(ulong clientId, NetworkPipeline pipeline, DataStrea break; } - InvokeOnTransportEvent(NetcodeNetworkEvent.Data, clientId, message, m_RealTimeProvider.RealTimeSinceStartup); + InvokeOnTransportEvent(NetcodeEvent.Data, clientId, message, m_RealTimeProvider.RealTimeSinceStartup); } } @@ -887,9 +889,9 @@ private bool ProcessEvent() switch (eventType) { - case TransportNetworkEvent.Type.Connect: + case TransportEvent.Connect: { - InvokeOnTransportEvent(NetcodeNetworkEvent.Connect, + InvokeOnTransportEvent(NetcodeEvent.Connect, clientId, default, m_RealTimeProvider.RealTimeSinceStartup); @@ -897,7 +899,7 @@ private bool ProcessEvent() m_ServerClientId = clientId; return true; } - case TransportNetworkEvent.Type.Disconnect: + case TransportEvent.Disconnect: { // If we're a client and had not yet set the server client ID, it means // our connection to the server failed to be established. Any other case @@ -911,14 +913,14 @@ private bool ProcessEvent() m_ReliableReceiveQueues.Remove(clientId); ClearSendQueuesForClientId(clientId); - InvokeOnTransportEvent(NetcodeNetworkEvent.Disconnect, + InvokeOnTransportEvent(NetcodeEvent.Disconnect, clientId, default, m_RealTimeProvider.RealTimeSinceStartup); return true; } - case TransportNetworkEvent.Type.Data: + case TransportEvent.Data: { ReceiveMessages(clientId, pipeline, reader); return true; @@ -940,7 +942,7 @@ protected override void OnEarlyUpdate() Debug.LogError("Transport failure! Relay allocation needs to be recreated, and NetworkManager restarted. " + "Use NetworkManager.OnTransportFailure to be notified of such events programmatically."); - InvokeOnTransportEvent(NetcodeNetworkEvent.TransportFailure, 0, default, m_RealTimeProvider.RealTimeSinceStartup); + InvokeOnTransportEvent(NetcodeEvent.TransportFailure, 0, default, m_RealTimeProvider.RealTimeSinceStartup); return; } @@ -1177,7 +1179,7 @@ public override void DisconnectLocalClient() // If we successfully disconnect we dispatch a local disconnect message // this how uNET and other transports worked and so this is just keeping with the old behavior // should be also noted on the client this will call shutdown on the NetworkManager and the Transport - InvokeOnTransportEvent(NetcodeNetworkEvent.Disconnect, + InvokeOnTransportEvent(NetcodeEvent.Disconnect, m_ServerClientId, default, m_RealTimeProvider.RealTimeSinceStartup); @@ -1314,12 +1316,12 @@ public override void Initialize(NetworkManager networkManager = null) /// The incoming data payload /// The time the event was received, as reported by m_RealTimeProvider.RealTimeSinceStartup. /// Returns the event type - public override NetcodeNetworkEvent PollEvent(out ulong clientId, out ArraySegment payload, out float receiveTime) + public override NetcodeEvent PollEvent(out ulong clientId, out ArraySegment payload, out float receiveTime) { clientId = default; payload = default; receiveTime = default; - return NetcodeNetworkEvent.Nothing; + return NetcodeEvent.Nothing; } /// @@ -1382,7 +1384,7 @@ public override void Send(ulong clientId, ArraySegment payload, NetworkDel DisconnectRemoteClient(clientId); // DisconnectRemoteClient doesn't notify SDK of disconnection. - InvokeOnTransportEvent(NetcodeNetworkEvent.Disconnect, + InvokeOnTransportEvent(NetcodeEvent.Disconnect, clientId, default(ArraySegment), m_RealTimeProvider.RealTimeSinceStartup); From 90c16bc4f542558bba607b33bdecda708e83a086 Mon Sep 17 00:00:00 2001 From: Simon Lemay Date: Wed, 26 Mar 2025 15:42:01 -0400 Subject: [PATCH 4/5] Simplify ErrorUtilities --- .../Runtime/Transports/UTP/UnityTransport.cs | 87 ++++++++----------- 1 file changed, 35 insertions(+), 52 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs b/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs index 1b7c285602..7036a2316a 100644 --- a/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs +++ b/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs @@ -44,56 +44,6 @@ void CreateDriver( out NetworkPipeline reliableSequencedPipeline); } - /// - /// Helper utility class to convert error codes to human readable error messages. - /// - public static class ErrorUtilities - { - private static readonly FixedString128Bytes k_NetworkSuccess = "Success"; - private static readonly FixedString128Bytes k_NetworkIdMismatch = "Invalid connection ID {0}."; - private static readonly FixedString128Bytes k_NetworkVersionMismatch = "Connection ID is invalid. Likely caused by sending on stale connection {0}."; - private static readonly FixedString128Bytes k_NetworkStateMismatch = "Connection state is invalid. Likely caused by sending on connection {0} which is stale or still connecting."; - private static readonly FixedString128Bytes k_NetworkPacketOverflow = "Packet is too large to be allocated by the transport."; - private static readonly FixedString128Bytes k_NetworkSendQueueFull = "Unable to queue packet in the transport. Likely caused by send queue size ('Max Send Queue Size') being too small."; - - /// - /// Convert a UTP error code to human-readable error message. - /// - /// UTP error code. - /// ID of the connection on which the error occurred. - /// Human-readable error message. - public static string ErrorToString(TransportError error, ulong connectionId) - { - return ErrorToString((int)error, connectionId); - } - - internal static string ErrorToString(int error, ulong connectionId) - { - return ErrorToFixedString(error, connectionId).ToString(); - } - - internal static FixedString128Bytes ErrorToFixedString(int error, ulong connectionId) - { - switch ((TransportError)error) - { - case TransportError.Success: - return k_NetworkSuccess; - case TransportError.NetworkIdMismatch: - return FixedString.Format(k_NetworkIdMismatch, connectionId); - case TransportError.NetworkVersionMismatch: - return FixedString.Format(k_NetworkVersionMismatch, connectionId); - case TransportError.NetworkStateMismatch: - return FixedString.Format(k_NetworkStateMismatch, connectionId); - case TransportError.NetworkPacketOverflow: - return k_NetworkPacketOverflow; - case TransportError.NetworkSendQueueFull: - return k_NetworkSendQueueFull; - default: - return FixedString.Format("Unknown error code {0}.", error); - } - } - } - /// /// The Netcode for GameObjects NetworkTransport for UnityTransport. /// Note: This is highly recommended to use over UNet. @@ -769,7 +719,7 @@ public void Execute() var result = Driver.BeginSend(pipeline, connection, out var writer); if (result != (int)TransportError.Success) { - Debug.LogError($"Error sending message: {ErrorUtilities.ErrorToFixedString(result, clientId)}"); + Debug.LogError($"Send error on connection {clientId}: {ErrorUtilities.ErrorToFixedString(result)}"); return; } @@ -796,7 +746,7 @@ public void Execute() // just get the same error again). if (result != (int)TransportError.NetworkSendQueueFull) { - Debug.LogError($"Error sending the message: {ErrorUtilities.ErrorToFixedString(result, clientId)}"); + Debug.LogError($"Send error on connection {clientId}: {ErrorUtilities.ErrorToFixedString(result)}"); Queue.Consume(written); } @@ -1765,4 +1715,37 @@ public override int GetHashCode() } } } + + /// + /// Utility class to convert Unity Transport error codes to human-readable error messages. + /// + public static class ErrorUtilities + { + /// + /// Convert a Unity Transport error code to human-readable error message. + /// + /// Unity Transport error code. + /// ID of connection on which error occurred (unused). + /// Human-readable error message. + public static string ErrorToString(TransportError error, ulong connectionId) + { + return ErrorToFixedString((int)error).ToString(); + } + + internal static FixedString128Bytes ErrorToFixedString(int error) + { + switch ((TransportError)error) + { + case TransportError.NetworkVersionMismatch: + case TransportError.NetworkStateMismatch: + return "invalid connection state (likely stale/closed connection)"; + case TransportError.NetworkPacketOverflow: + return "packet is too large for the transport (likely need to increase MTU)"; + case TransportError.NetworkSendQueueFull: + return "send queue full (need to increase 'Max Send Queue Size' parameter)"; + default: + return FixedString.Format("unexpected error code {0}", error); + } + } + } } From 6392d0c9df9de7d94a5e63186bf13d85e35c110c Mon Sep 17 00:00:00 2001 From: Simon Lemay Date: Wed, 2 Apr 2025 17:04:21 -0400 Subject: [PATCH 5/5] Make the standards check happy --- .../Runtime/Transports/UTP/UnityTransport.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs b/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs index 7036a2316a..9fe803fcd1 100644 --- a/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs +++ b/com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs @@ -368,14 +368,14 @@ private struct PacketLossCache internal static event Action TransportDisposed; /// - /// Provides access to the for this instance. + /// Provides access to the for this instance. /// protected NetworkDriver m_Driver; /// - /// Gets a reference to the . + /// Gets a reference to the . /// - /// ref + /// ref public ref NetworkDriver GetNetworkDriver() { return ref m_Driver;