fix: g-2501 Connection disapproval results in spam logging 10 seconds later (#2514)

* fix: g-2501 Connection disapproval results in spam logging 10 seconds later

* fix

Minor adjustments to get this PR passing the connection approval timeout tests.

* update

Reverting accidental change in ProcessPendingApprovals that shouldn't have been changed.

---------

Co-authored-by: NoelStephensUnity <[email protected]>

Author: jeffreyrainy

com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs

@@ -83,25 +83,58 @@ public sealed class NetworkConnectionManager
 
         internal IReadOnlyDictionary<ulong, PendingClient> PendingClients => m_PendingClients;
 
+        internal Coroutine LocalClientApprovalCoroutine;
+
+        /// <summary>
+        /// Client-Side:
+        /// Starts the client-side approval timeout coroutine
+        /// </summary>
+        /// <param name="clientId"></param>
+        internal void StartClientApprovalCoroutine(ulong clientId)
+        {
+            LocalClientApprovalCoroutine = NetworkManager.StartCoroutine(ApprovalTimeout(clientId));
+        }
+
+        /// <summary>
+        /// Client-Side:
+        /// Stops the client-side approval timeout when it is approved.
+        /// <see cref="ConnectionApprovedMessage.Handle(ref NetworkContext)"/>
+        /// </summary>
+        internal void StopClientApprovalCoroutine()
+        {
+            if (LocalClientApprovalCoroutine != null)
+            {
+                NetworkManager.StopCoroutine(LocalClientApprovalCoroutine);
+                LocalClientApprovalCoroutine = null;
+            }
+        }
+
         /// <summary>
+        /// Server-Side:
         /// Handles the issue with populating NetworkManager.PendingClients
         /// </summary>
         internal void AddPendingClient(ulong clientId)
         {
             m_PendingClients.Add(clientId, new PendingClient()
             {
                 ClientId = clientId,
-                ConnectionState = PendingClient.State.PendingConnection
+                ConnectionState = PendingClient.State.PendingConnection,
+                ApprovalCoroutine = NetworkManager.StartCoroutine(ApprovalTimeout(clientId))
             });
 
             NetworkManager.PendingClients.Add(clientId, PendingClients[clientId]);
         }
 
         /// <summary>
+        /// Server-Side:
         /// Handles the issue with depopulating NetworkManager.PendingClients
         /// </summary>
         internal void RemovePendingClient(ulong clientId)
         {
+            if (m_PendingClients.ContainsKey(clientId) && m_PendingClients[clientId].ApprovalCoroutine != null)
+            {
+                NetworkManager.StopCoroutine(m_PendingClients[clientId].ApprovalCoroutine);
+            }
             m_PendingClients.Remove(clientId);
             NetworkManager.PendingClients.Remove(clientId);
         }
@@ -272,8 +305,6 @@ internal void ConnectEventHandler(ulong transportClientId)
                 }
 
                 AddPendingClient(clientId);
-
-                NetworkManager.StartCoroutine(ApprovalTimeout(clientId));
             }
             else
             {
@@ -283,7 +314,7 @@ internal void ConnectEventHandler(ulong transportClientId)
                 }
 
                 SendConnectionRequest();
-                NetworkManager.StartCoroutine(ApprovalTimeout(clientId));
+                StartClientApprovalCoroutine(clientId);
             }
 
 #if DEVELOPMENT_BUILD || UNITY_EDITOR
@@ -545,7 +576,7 @@ internal void HandleConnectionApproval(ulong ownerClientId, NetworkManager.Conne
             LocalClient.IsApproved = response.Approved;
             if (response.Approved)
             {
-                // Inform new client it got approved
+                // The client was approved, stop the server-side approval time out coroutine
                 RemovePendingClient(ownerClientId);
 
                 var client = AddClient(ownerClientId);
@@ -818,6 +849,8 @@ internal void OnClientDisconnectFromServer(ulong clientId)
             }
 
             // Assure the client id is no longer in the pending clients list
+            // and stop the server-side client approval timeout since the client
+            // is no longer connected.
             RemovePendingClient(clientId);
 
             // Handle cleaning up the server-side client send queue

com.unity.netcode.gameobjects/Runtime/Connection/PendingClient.cs

@@ -1,10 +1,15 @@
+using UnityEngine;
+
 namespace Unity.Netcode
 {
     /// <summary>
+    /// Server-Side Only:
     /// A class representing a client that is currently in the process of connecting
     /// </summary>
     public class PendingClient
     {
+        internal Coroutine ApprovalCoroutine = null;
+
         /// <summary>
         /// The ClientId of the client
         /// </summary>

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs

@@ -120,6 +120,8 @@ public void Handle(ref NetworkContext context)
             networkManager.ConnectionManager.LocalClient.SetRole(false, true, networkManager);
             networkManager.ConnectionManager.LocalClient.IsApproved = true;
             networkManager.ConnectionManager.LocalClient.ClientId = OwnerClientId;
+            // Stop the client-side approval timeout coroutine since we are approved.
+            networkManager.ConnectionManager.StopClientApprovalCoroutine();
 
             // Only if scene management is disabled do we handle NetworkObject synchronization at this point
             if (!networkManager.NetworkConfig.EnableSceneManagement)