Remove internal "state" from UnityTransport

Author: simon-lemay-unity

com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs

@@ -114,13 +114,6 @@ public enum ProtocolType
             RelayUnityTransport,
         }
 
-        private enum State
-        {
-            Disconnected,
-            Listening,
-            Connected,
-        }
-
         /// <summary>
         /// The default maximum (receive) packet queue size
         /// </summary>
@@ -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)
         /// </summary>
         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()

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);