refactor!: delete SocketTask and make StartServer/Client calls non-async (#1239)

Author: 0xFA11

com.unity.netcode.adapter.utp/Runtime/UnityTransport.cs

@@ -1,5 +1,4 @@
 using System;
-using System.Collections;
 using System.Collections.Generic;
 using UnityEngine;
 using NetcodeNetworkEvent = Unity.Netcode.NetworkEvent;
@@ -57,6 +56,7 @@ private enum State
         private NetworkDriver m_Driver;
         private List<INetworkParameter> m_NetworkParameters;
         private byte[] m_MessageBuffer;
+        private NetworkConnection m_ServerConnection;
         private ulong m_ServerClientId;
 
         private NetworkPipeline m_UnreliableSequencedPipeline;
@@ -153,7 +153,7 @@ private NetworkPipeline SelectSendPipeline(NetworkDelivery delivery, int size)
             }
         }
 
-        private IEnumerator ClientBindAndConnect(SocketTask task)
+        private bool ClientBindAndConnect()
         {
             var serverEndpoint = default(NetworkEndPoint);
 
@@ -164,9 +164,7 @@ private IEnumerator ClientBindAndConnect(SocketTask task)
                 if (m_RelayServerData.Equals(default(RelayServerData)))
                 {
                     Debug.LogError("You must call SetRelayServerData() at least once before calling StartRelayServer.");
-                    task.IsDone = true;
-                    task.Success = false;
-                    yield break;
+                    return false;
                 }
 
                 m_NetworkParameters.Add(new RelayNetworkParameter { ServerData = m_RelayServerData });
@@ -178,66 +176,39 @@ private IEnumerator ClientBindAndConnect(SocketTask task)
 
             InitDriver();
 
-            if (m_Driver.Bind(NetworkEndPoint.AnyIpv4) != 0)
+            int result = m_Driver.Bind(NetworkEndPoint.AnyIpv4);
+            if (result != 0)
             {
                 Debug.LogError("Client failed to bind");
+                return false;
             }
-            else
-            {
-                while (!m_Driver.Bound)
-                {
-                    yield return null;
-                }
-
-                var serverConnection = m_Driver.Connect(serverEndpoint);
-                m_ServerClientId = ParseClientId(serverConnection);
 
-                while (m_Driver.GetConnectionState(serverConnection) == NetworkConnection.State.Connecting)
-                {
-                    yield return null;
-                }
-
-                if (m_Driver.GetConnectionState(serverConnection) == NetworkConnection.State.Connected)
-                {
-                    task.Success = true;
-                    m_State = State.Connected;
-                }
-                else
-                {
-                    Debug.LogError("Client failed to connect to server");
-                }
-            }
+            m_ServerConnection = m_Driver.Connect(serverEndpoint);
+            m_ServerClientId = ParseClientId(m_ServerConnection);
 
-            task.IsDone = true;
+            return true;
         }
 
-        private IEnumerator ServerBindAndListen(SocketTask task, NetworkEndPoint endPoint)
+        private bool ServerBindAndListen(NetworkEndPoint endPoint)
         {
             InitDriver();
 
-            if (m_Driver.Bind(endPoint) != 0)
+            int result = m_Driver.Bind(endPoint);
+            if (result != 0)
             {
                 Debug.LogError("Server failed to bind");
+                return false;
             }
-            else
-            {
-                while (!m_Driver.Bound)
-                {
-                    yield return null;
-                }
 
-                if (m_Driver.Listen() == 0)
-                {
-                    task.Success = true;
-                    m_State = State.Listening;
-                }
-                else
-                {
-                    Debug.LogError("Server failed to listen");
-                }
+            result = m_Driver.Listen();
+            if (result != 0)
+            {
+                Debug.LogError("Server failed to listen");
+                return false;
             }
 
-            task.IsDone = true;
+            m_State = State.Listening;
+            return true;
         }
 
         private static RelayAllocationId ConvertFromAllocationIdBytes(byte[] allocationIdBytes)
@@ -306,22 +277,19 @@ public void SetConnectionData(string ipv4Address, ushort port)
             m_ServerPort = port;
         }
 
-        private IEnumerator StartRelayServer(SocketTask task)
+        private bool StartRelayServer()
         {
             //This comparison is currently slow since RelayServerData does not implement a custom comparison operator that doesn't use
             //reflection, but this does not live in the context of a performance-critical loop, it runs once at initial connection time.
             if (m_RelayServerData.Equals(default(RelayServerData)))
             {
                 Debug.LogError("You must call SetRelayServerData() at least once before calling StartRelayServer.");
-                task.IsDone = true;
-                task.Success = false;
-                yield break;
+                return false;
             }
             else
             {
                 m_NetworkParameters.Add(new RelayNetworkParameter { ServerData = m_RelayServerData });
-
-                yield return ServerBindAndListen(task, NetworkEndPoint.AnyIpv4);
+                return ServerBindAndListen(NetworkEndPoint.AnyIpv4);
             }
         }
 
@@ -336,7 +304,7 @@ private bool AcceptConnection()
 
             InvokeOnTransportEvent(NetcodeNetworkEvent.Connect,
                 ParseClientId(connection),
-                default(ArraySegment<byte>),
+                default,
                 Time.realtimeSinceStartup);
 
             return true;
@@ -350,37 +318,53 @@ private bool ProcessEvent()
             switch (eventType)
             {
                 case TransportNetworkEvent.Type.Connect:
-                    InvokeOnTransportEvent(NetcodeNetworkEvent.Connect,
-                        ParseClientId(networkConnection),
-                        default(ArraySegment<byte>),
-                        Time.realtimeSinceStartup);
-                    return true;
+                    {
+                        InvokeOnTransportEvent(NetcodeNetworkEvent.Connect,
+                            ParseClientId(networkConnection),
+                            default(ArraySegment<byte>),
+                            Time.realtimeSinceStartup);
 
+                        m_State = State.Connected;
+                        return true;
+                    }
                 case TransportNetworkEvent.Type.Disconnect:
-                    InvokeOnTransportEvent(NetcodeNetworkEvent.Disconnect,
-                        ParseClientId(networkConnection),
-                        default(ArraySegment<byte>),
-                        Time.realtimeSinceStartup);
-                    return true;
+                    {
+                        InvokeOnTransportEvent(NetcodeNetworkEvent.Disconnect,
+                            ParseClientId(networkConnection),
+                            default(ArraySegment<byte>),
+                            Time.realtimeSinceStartup);
 
+                        if (m_ServerConnection.IsCreated)
+                        {
+                            m_ServerConnection = default;
+                            if (m_Driver.GetConnectionState(m_ServerConnection) == NetworkConnection.State.Connecting)
+                            {
+                                Debug.LogError("Client failed to connect to server");
+                            }
+                        }
+
+                        m_State = State.Disconnected;
+                        return true;
+                    }
                 case TransportNetworkEvent.Type.Data:
-                    var isBatched = reader.ReadByte();
-                    if (isBatched == 1)
                     {
-                        while (reader.GetBytesRead() < reader.Length)
+                        var isBatched = reader.ReadByte();
+                        if (isBatched == 1)
+                        {
+                            while (reader.GetBytesRead() < reader.Length)
+                            {
+                                var payloadSize = reader.ReadInt();
+                                ReadData(payloadSize, ref reader, ref networkConnection);
+                            }
+                        }
+                        else // If is not batched, then read the entire buffer at once
                         {
                             var payloadSize = reader.ReadInt();
                             ReadData(payloadSize, ref reader, ref networkConnection);
                         }
-                    }
-                    else // If is not batched, then read the entire buffer at once
-                    {
-                        var payloadSize = reader.ReadInt();
 
-                        ReadData(payloadSize, ref reader, ref networkConnection);
+                        return true;
                     }
-
-                    return true;
             }
 
             return false;
@@ -428,7 +412,6 @@ private void Update()
                     ;
                 }
             }
-
         }
 
         private void OnDestroy()
@@ -593,11 +576,9 @@ private unsafe void SendBatchedMessage(ulong clientId, ref NativeArray<byte> dat
             Debug.LogError($"Error sending the message {result}");
         }
 
-        private unsafe void SendMessageInstantly(ulong clientId, ArraySegment<byte> data,
-            NetworkPipeline pipeline)
+        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 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)
             {
@@ -656,37 +637,32 @@ private void SendBatchedMessageAndClearQueue(SendTarget sendTarget, SendQueue se
             sendQueue.Clear();
         }
 
-        public override SocketTasks StartClient()
+        public override bool StartClient()
         {
             if (m_Driver.IsCreated)
             {
-                return SocketTask.Fault.AsTasks();
+                return false;
             }
 
-            var task = SocketTask.Working;
-            StartCoroutine(ClientBindAndConnect(task));
-            return task.AsTasks();
+            return ClientBindAndConnect();
         }
 
-        public override SocketTasks StartServer()
+        public override bool StartServer()
         {
             if (m_Driver.IsCreated)
             {
-                return SocketTask.Fault.AsTasks();
+                return false;
             }
 
-            var task = SocketTask.Working;
             switch (m_ProtocolType)
             {
                 case ProtocolType.UnityTransport:
-                    StartCoroutine(ServerBindAndListen(task, NetworkEndPoint.Parse(m_ServerAddress, m_ServerPort)));
-                    break;
+                    return ServerBindAndListen(NetworkEndPoint.Parse(m_ServerAddress, m_ServerPort));
                 case ProtocolType.RelayUnityTransport:
-                    StartCoroutine(StartRelayServer(task));
-                    break;
+                    return StartRelayServer();
+                default:
+                    return false;
             }
-
-            return task.AsTasks();
         }
 
         public override void Shutdown()

com.unity.netcode.adapter.utp/Tests/Editor/UnityTransportTests.cs

@@ -12,8 +12,7 @@ public void BasicInitServer()
             UnityTransport transport = new GameObject().AddComponent<UnityTransport>();
             transport.Initialize();
 
-            var tasks = transport.StartServer();
-            Assert.False(tasks.IsDone && !tasks.Success);
+            Assert.True(transport.StartServer());
 
             transport.Shutdown();
         }
@@ -25,8 +24,7 @@ public void BasicInitClient()
             UnityTransport transport = new GameObject().AddComponent<UnityTransport>();
             transport.Initialize();
 
-            var tasks = transport.StartClient();
-            Assert.False(tasks.IsDone && !tasks.Success);
+            Assert.True(transport.StartClient());
 
             transport.Shutdown();
         }
@@ -39,8 +37,7 @@ public void NoRestartServer()
             transport.Initialize();
 
             transport.StartServer();
-            var tasks = transport.StartServer();
-            Assert.True(tasks.IsDone && !tasks.AnySuccess);
+            Assert.False(transport.StartServer());
 
             transport.Shutdown();
         }
@@ -53,8 +50,7 @@ public void NoRestartClient()
             transport.Initialize();
 
             transport.StartClient();
-            var tasks = transport.StartClient();
-            Assert.True(tasks.IsDone && !tasks.AnySuccess);
+            Assert.False(transport.StartClient());
 
             transport.Shutdown();
         }
@@ -64,15 +60,13 @@ public void NoRestartClient()
         public void NotBothServerAndClient()
         {
             UnityTransport transport;
-            SocketTasks tasks;
 
             // Start server then client.
             transport = new GameObject().AddComponent<UnityTransport>();
             transport.Initialize();
 
             transport.StartServer();
-            tasks = transport.StartClient();
-            Assert.True(tasks.IsDone && !tasks.AnySuccess);
+            Assert.False(transport.StartClient());
 
             transport.Shutdown();
 
@@ -81,12 +75,9 @@ public void NotBothServerAndClient()
             transport.Initialize();
 
             transport.StartClient();
-            tasks = transport.StartServer();
-            Assert.True(tasks.IsDone && !tasks.AnySuccess);
+            Assert.False(transport.StartServer());
 
             transport.Shutdown();
         }
     }
 }
-
-

com.unity.netcode.adapter.utp/Tests/Runtime/ConnectionTests.cs

@@ -92,7 +92,6 @@ public IEnumerator ConnectMultipleClients()
         [UnityTest]
         public IEnumerator ServerDisconnectSingleClient()
         {
-
             InitializeTransport(out m_Server, out m_ServerEvents);
             InitializeTransport(out m_Clients[0], out m_ClientsEvents[0]);
 
@@ -152,7 +151,6 @@ public IEnumerator ServerDisconnectMultipleClients()
         [UnityTest]
         public IEnumerator ClientDisconnectSingleClient()
         {
-
             InitializeTransport(out m_Server, out m_ServerEvents);
             InitializeTransport(out m_Clients[0], out m_ClientsEvents[0]);
 
@@ -164,8 +162,6 @@ public IEnumerator ClientDisconnectSingleClient()
             m_Clients[0].DisconnectLocalClient();
 
             yield return WaitForNetworkEvent(NetworkEvent.Disconnect, m_ServerEvents);
-
-            yield return null;
         }
 
         // Check client disconnection with multiple clients.