-
Notifications
You must be signed in to change notification settings - Fork 459
fix!: MTT-1425 Resolving overflow issue with UTP Adapter #1300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
00f689e
fix: Fixing issue with adapter bugs
andrews-unity e9603b2
Merge branch 'develop' into fix/utp-adapter-fixes
andrews-unity bb6d4c0
Adding some error message resolution and cleanup serializedfields
andrews-unity a19209f
fix: Adding a ConnectionAddressData wraper because NetworkEndpoint is…
andrews-unity 9ba6a30
Merge branch 'develop' into fix/utp-adapter-fixes
andrews-unity af2d703
fix: Fix test
andrews-unity c9def32
Merge branch 'fix/utp-adapter-fixes' of github.com:Unity-Technologies…
andrews-unity ad4ecdf
Merge branch 'develop' into fix/utp-adapter-fixes
mattwalsh-unity 86ef032
Merge branch 'develop' into fix/utp-adapter-fixes
andrews-unity File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,49 @@ public interface INetworkStreamDriverConstructor | |
| void CreateDriver(UnityTransport transport, out NetworkDriver driver, out NetworkPipeline unreliableSequencedPipeline, out NetworkPipeline reliableSequencedPipeline, out NetworkPipeline reliableSequencedFragmentedPipeline); | ||
| } | ||
|
|
||
| public static class ErrorUtilities | ||
| { | ||
| private const string k_NetworkSuccess = "Success"; | ||
| private const string k_NetworkIdMismatch = "NetworkId is invalid, likely caused by stale connection {0}."; | ||
| private const string k_NetworkVersionMismatch = "NetworkVersion is invalid, likely caused by stale connection {0}."; | ||
| private const string k_NetworkStateMismatch = "Sending data while connecting on connectionId{0} is now allowed"; | ||
| private const string k_NetworkPacketOverflow = "Unable to allocate packet due to buffer overflow."; | ||
| private const string k_NetworkSendQueueFull = "Currently unable to queue packet as there is too many inflight packets."; | ||
| private const string k_NetworkHeaderInvalid = "Invalid Unity Transport Protocol header."; | ||
| private const string k_NetworkDriverParallelForErr = "The parallel network driver needs to process a single unique connection per job, processing a single connection multiple times in a parallel for is not supported."; | ||
| private const string k_NetworkSendHandleInvalid = "Invalid NetworkInterface Send Handle. Likely caused by pipeline send data corruption."; | ||
| private const string k_NetworkArgumentMismatch = "Invalid NetworkEndpoint Arguments."; | ||
|
|
||
| public static string ErrorToString(Networking.Transport.Error.StatusCode error, ulong connectionId) | ||
| { | ||
| switch (error) | ||
| { | ||
| case Networking.Transport.Error.StatusCode.Success: | ||
| return k_NetworkSuccess; | ||
| case Networking.Transport.Error.StatusCode.NetworkIdMismatch: | ||
| return string.Format(k_NetworkIdMismatch, connectionId); | ||
| case Networking.Transport.Error.StatusCode.NetworkVersionMismatch: | ||
| return string.Format(k_NetworkVersionMismatch, connectionId); | ||
| case Networking.Transport.Error.StatusCode.NetworkStateMismatch: | ||
| return string.Format(k_NetworkStateMismatch, connectionId); | ||
| case Networking.Transport.Error.StatusCode.NetworkPacketOverflow: | ||
| return k_NetworkPacketOverflow; | ||
| case Networking.Transport.Error.StatusCode.NetworkSendQueueFull: | ||
| return k_NetworkSendQueueFull; | ||
| case Networking.Transport.Error.StatusCode.NetworkHeaderInvalid: | ||
| return k_NetworkHeaderInvalid; | ||
| case Networking.Transport.Error.StatusCode.NetworkDriverParallelForErr: | ||
| return k_NetworkDriverParallelForErr; | ||
| case Networking.Transport.Error.StatusCode.NetworkSendHandleInvalid: | ||
| return k_NetworkSendHandleInvalid; | ||
| case Networking.Transport.Error.StatusCode.NetworkArgumentMismatch: | ||
| return k_NetworkArgumentMismatch; | ||
| } | ||
|
|
||
| return $"Unknown ErrorCode {Enum.GetName(typeof(Networking.Transport.Error.StatusCode), error)}"; | ||
| } | ||
| } | ||
|
|
||
| public class UnityTransport : NetworkTransport, INetworkStreamDriverConstructor | ||
| { | ||
| public enum ProtocolType | ||
|
|
@@ -34,23 +77,43 @@ private enum State | |
| Connected, | ||
| } | ||
|
|
||
| public const int MaximumMessageLength = 6 * 1024; | ||
| public const int InitialBatchQueueSize = 6 * 1024; | ||
| public const int InitialMaxPacketSize = NetworkParameterConstants.MTU; | ||
|
|
||
| private static ConnectionAddressData s_DefaultConnectionAddressData = new ConnectionAddressData() | ||
| { Address = "127.0.0.1", Port = 7777 }; | ||
|
|
||
| #pragma warning disable IDE1006 // Naming Styles | ||
| public static INetworkStreamDriverConstructor s_DriverConstructor; | ||
| #pragma warning restore IDE1006 // Naming Styles | ||
| public INetworkStreamDriverConstructor DriverConstructor => s_DriverConstructor != null ? s_DriverConstructor : this; | ||
|
|
||
| [Tooltip("Which protocol should be selected Relay/Non-Relay")] | ||
| [SerializeField] private ProtocolType m_ProtocolType; | ||
| [SerializeField] private int m_MessageBufferSize = MaximumMessageLength; | ||
| [SerializeField] private int m_ReciveQueueSize = 128; | ||
| [SerializeField] private int m_SendQueueSize = 128; | ||
|
|
||
| [Tooltip("The maximum size of the send queue for batching Netcode events")] | ||
| [SerializeField] private int m_SendQueueBatchSize = 4096; | ||
| [Tooltip("Maximum size in bytes for a given packet")] | ||
| [SerializeField] private int m_MaximumPacketSize = InitialMaxPacketSize; | ||
|
|
||
| [Tooltip("The maximum amount of packets that can be in the send/recv queues")] | ||
| [SerializeField] private int m_MaxPacketQueueSize = 128; | ||
|
|
||
| [Tooltip("The maximum size in bytes of the send queue for batching Netcode events")] | ||
| [SerializeField] private int m_SendQueueBatchSize = InitialBatchQueueSize; | ||
|
|
||
| [SerializeField] private string m_ServerAddress = "127.0.0.1"; | ||
| [SerializeField] private ushort m_ServerPort = 7777; | ||
| [Serializable] | ||
| public struct ConnectionAddressData | ||
| { | ||
| [SerializeField] public string Address; | ||
| [SerializeField] public int Port; | ||
|
|
||
| public static implicit operator NetworkEndPoint(ConnectionAddressData d) => | ||
| NetworkEndPoint.Parse(d.Address, (ushort)d.Port); | ||
|
|
||
| public static implicit operator ConnectionAddressData(NetworkEndPoint d) => | ||
| new ConnectionAddressData() { Address = d.Address.Split(':')[0], Port = d.Port }; | ||
| } | ||
|
|
||
| public ConnectionAddressData ConnectionData = s_DefaultConnectionAddressData; | ||
|
|
||
| private State m_State = State.Disconnected; | ||
| private NetworkDriver m_Driver; | ||
|
|
@@ -171,7 +234,7 @@ private bool ClientBindAndConnect() | |
| } | ||
| else | ||
| { | ||
| serverEndpoint = NetworkEndPoint.Parse(m_ServerAddress, m_ServerPort); | ||
| serverEndpoint = ConnectionData; | ||
| } | ||
|
|
||
| InitDriver(); | ||
|
|
@@ -273,8 +336,16 @@ public void SetRelayServerData(string ipv4Address, ushort port, byte[] allocatio | |
| /// </summary> | ||
| public void SetConnectionData(string ipv4Address, ushort port) | ||
| { | ||
| m_ServerAddress = ipv4Address; | ||
| m_ServerPort = port; | ||
| ConnectionData.Address = ipv4Address; | ||
| ConnectionData.Port = port; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Sets IP and Port information. This will be ignored if using the Unity Relay and you should call <see cref="SetRelayServerData"/> | ||
| /// </summary> | ||
| public void SetConnectionData(NetworkEndPoint endPoint) | ||
| { | ||
| ConnectionData = endPoint; | ||
| } | ||
|
|
||
| private bool StartRelayServer() | ||
|
|
@@ -372,25 +443,23 @@ private bool ProcessEvent() | |
|
|
||
| private unsafe void ReadData(int size, ref DataStreamReader reader, ref NetworkConnection networkConnection) | ||
| { | ||
| if (size > m_MessageBufferSize) | ||
| if (size > m_SendQueueBatchSize) | ||
| { | ||
| Debug.LogError("The received message does not fit into the message buffer"); | ||
| Debug.LogError($"The received message does not fit into the message buffer: {size} {m_SendQueueBatchSize}"); | ||
| } | ||
| else | ||
| { | ||
| unsafe | ||
| { | ||
| fixed (byte* buffer = &m_MessageBuffer[0]) | ||
| { | ||
| reader.ReadBytes(buffer, size); | ||
| } | ||
| using var data = new NativeArray<byte>(size, Allocator.Temp); | ||
| reader.ReadBytes(data); | ||
|
|
||
| InvokeOnTransportEvent(NetcodeNetworkEvent.Data, | ||
| ParseClientId(networkConnection), | ||
| new ArraySegment<byte>(data.ToArray(), 0, size), | ||
| Time.realtimeSinceStartup | ||
| ); | ||
| } | ||
|
|
||
| InvokeOnTransportEvent(NetcodeNetworkEvent.Data, | ||
| ParseClientId(networkConnection), | ||
| new ArraySegment<byte>(m_MessageBuffer, 0, size), | ||
| Time.realtimeSinceStartup | ||
| ); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -438,7 +507,6 @@ public override void DisconnectLocalClient() | |
|
|
||
| m_State = State.Disconnected; | ||
|
|
||
|
|
||
| // 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 | ||
|
|
@@ -491,23 +559,20 @@ public override void Initialize() | |
| { | ||
| Debug.Assert(sizeof(ulong) == UnsafeUtility.SizeOf<NetworkConnection>(), | ||
| "Netcode connection id size does not match UTP connection id size"); | ||
| Debug.Assert(m_MessageBufferSize > 5, "Message buffer size must be greater than 5"); | ||
| Debug.Assert(m_MaximumPacketSize > 5, "Message buffer size must be greater than 5"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. where this magical 5 is coming from and should it be a k_ constant value? |
||
|
|
||
| m_NetworkParameters = new List<INetworkParameter>(); | ||
|
|
||
| // If we want to be able to actually handle messages MaximumMessageLength bytes in | ||
| // size, we need to allow a bit more than that in FragmentationUtility since this needs | ||
| // to account for headers and such. 128 bytes is plenty enough for such overhead. | ||
| var maxFragmentationCapacity = MaximumMessageLength + 128; | ||
| m_NetworkParameters.Add(new FragmentationUtility.Parameters() { PayloadCapacity = maxFragmentationCapacity }); | ||
| m_NetworkParameters.Add(new FragmentationUtility.Parameters() { PayloadCapacity = m_SendQueueBatchSize }); | ||
| m_NetworkParameters.Add(new BaselibNetworkParameter() | ||
| { | ||
| maximumPayloadSize = (uint)m_MessageBufferSize, | ||
| receiveQueueCapacity = m_ReciveQueueSize, | ||
| sendQueueCapacity = m_SendQueueSize | ||
| maximumPayloadSize = (uint)m_MaximumPacketSize, | ||
| receiveQueueCapacity = m_MaxPacketQueueSize, | ||
| sendQueueCapacity = m_MaxPacketQueueSize | ||
| }); | ||
|
|
||
| m_MessageBuffer = new byte[m_MessageBufferSize]; | ||
| } | ||
|
|
||
| public override NetcodeNetworkEvent PollEvent(out ulong clientId, out ArraySegment<byte> payload, out float receiveTime) | ||
|
|
@@ -556,7 +621,6 @@ private unsafe void SendBatchedMessage(ulong clientId, ref NativeArray<byte> dat | |
| { | ||
| var payloadSize = data.Length + 1; // One extra byte to mark whether this message is batched or not | ||
| var result = m_Driver.BeginSend(pipeline, ParseClientId(clientId), out var writer, payloadSize); | ||
|
|
||
| if (result == 0) | ||
| { | ||
| if (data.IsCreated) | ||
|
|
@@ -573,13 +637,14 @@ private unsafe void SendBatchedMessage(ulong clientId, ref NativeArray<byte> dat | |
| } | ||
| } | ||
|
|
||
| Debug.LogError($"Error sending the message {result}"); | ||
| Debug.LogError($"Error sending the message: {ErrorUtilities.ErrorToString((Networking.Transport.Error.StatusCode)result, clientId)}"); | ||
| } | ||
|
|
||
| private unsafe void SendMessageInstantly(ulong clientId, ArraySegment<byte> data, NetworkPipeline pipeline) | ||
| { | ||
| var payloadSize = data.Count + 1 + 4; // 1 byte to indicate if the message is batched and 4 for the payload size | ||
| var result = m_Driver.BeginSend(pipeline, ParseClientId(clientId), out var writer, payloadSize); | ||
|
|
||
| if (result == 0) | ||
| { | ||
| if (data.Array != null) | ||
|
|
@@ -604,7 +669,7 @@ private unsafe void SendMessageInstantly(ulong clientId, ArraySegment<byte> data | |
| } | ||
| } | ||
|
|
||
| Debug.LogError($"Error sending the message {result}"); | ||
| Debug.LogError($"Error sending the message: {ErrorUtilities.ErrorToString((Networking.Transport.Error.StatusCode)result, clientId)}"); | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
@@ -657,7 +722,7 @@ public override bool StartServer() | |
| switch (m_ProtocolType) | ||
| { | ||
| case ProtocolType.UnityTransport: | ||
| return ServerBindAndListen(NetworkEndPoint.Parse(m_ServerAddress, m_ServerPort)); | ||
| return ServerBindAndListen(ConnectionData); | ||
| case ProtocolType.RelayUnityTransport: | ||
| return StartRelayServer(); | ||
| default: | ||
|
|
@@ -743,7 +808,6 @@ public void CreateDriver(UnityTransport transport, out NetworkDriver driver, out | |
| } | ||
| } | ||
|
|
||
|
|
||
| // -------------- Utility Types ------------------------------------------------------------------------------- | ||
|
|
||
| /// <summary> | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,7 +35,7 @@ public class DriverClient : MonoBehaviour | |
|
|
||
| private void Awake() | ||
| { | ||
| var maxCap = UnityTransport.MaximumMessageLength + 128; | ||
| var maxCap = UnityTransport.InitialBatchQueueSize + 128; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. another magic number here too... |
||
| var fragParams = new FragmentationUtility.Parameters() { PayloadCapacity = maxCap }; | ||
|
|
||
| m_Driver = NetworkDriver.Create(fragParams); | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not like this but its the way NetworkEndpoint encodes the address. If there is no ':' then it will just return nothing or whatever is before it.