refactor!: connection approval

com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs

@@ -60,6 +60,8 @@ internal static string PrefabDebugHelper(NetworkPrefab networkPrefab)
 
         private NetworkPrefabHandler m_PrefabHandler;
 
+        internal Dictionary<ulong, ConnectionApprovalResponse> ClientsToApprove = new Dictionary<ulong, ConnectionApprovalResponse>();
+
         public NetworkPrefabHandler PrefabHandler
         {
             get
@@ -367,13 +369,15 @@ public IReadOnlyList<ulong> ConnectedClientsIds
         /// <param name="PlayerPrefabHash">The prefabHash to use for the client. If createPlayerObject is false, this is ignored. If playerPrefabHash is null, the default player prefab is used.</param>
         /// <param name="Position">The position to spawn the client at. If null, the prefab position is used.</param>
         /// <param name="Rotation">The rotation to spawn the client with. If null, the prefab position is used.</param>
-        public struct ConnectionApprovalResponse
+        /// <param name="Pending">If the Approval decision cannot be made immediately, the client code can set Pending to true, keep a reference to the ConnectionApprovalResponse object and write to it later. Client code must exercise care to setting all the members to the value it wants before marking Pending to false, to indicate completion.</param>
+        public class ConnectionApprovalResponse
         {
             public bool Approved;
             public bool CreatePlayerObject;
             public uint? PlayerPrefabHash;
             public Vector3? Position;
             public Quaternion? Rotation;
+            public bool Pending;
         }
 
         /// <summary>
@@ -390,7 +394,7 @@ public struct ConnectionApprovalRequest
         /// <summary>
         /// The callback to invoke during connection approval. Allows client code to decide whether or not to allow incoming client connection
         /// </summary>
-        public Func<ConnectionApprovalRequest, ConnectionApprovalResponse> ConnectionApprovalCallback
+        public Action<ConnectionApprovalRequest, ConnectionApprovalResponse> ConnectionApprovalCallback
         {
             get => m_ConnectionApprovalCallback;
             set
@@ -406,7 +410,7 @@ public Func<ConnectionApprovalRequest, ConnectionApprovalResponse> ConnectionApp
             }
         }
 
-        private Func<ConnectionApprovalRequest, ConnectionApprovalResponse> m_ConnectionApprovalCallback;
+        private Action<ConnectionApprovalRequest, ConnectionApprovalResponse> m_ConnectionApprovalCallback;
 
         /// <summary>
         /// The current NetworkConfig
@@ -941,6 +945,7 @@ private void ClearClients()
             m_ConnectedClientIds.Clear();
             LocalClient = null;
             NetworkObject.OrphanChildren.Clear();
+            ClientsToApprove.Clear();
         }
 
         /// <summary>
@@ -1048,7 +1053,8 @@ public bool StartHost()
 
             if (NetworkConfig.ConnectionApproval && ConnectionApprovalCallback != null)
             {
-                var response = ConnectionApprovalCallback(new ConnectionApprovalRequest { Payload = NetworkConfig.ConnectionData, ClientNetworkId = ServerClientId });
+                var response = new ConnectionApprovalResponse();
+                ConnectionApprovalCallback(new ConnectionApprovalRequest { Payload = NetworkConfig.ConnectionData, ClientNetworkId = ServerClientId }, response);
                 if (!response.Approved)
                 {
                     if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
@@ -1403,13 +1409,45 @@ public void NetworkUpdate(NetworkUpdateStage updateStage)
             }
         }
 
+        private void ProcessPendingApprovals()
+        {
+            List<ulong> senders = null;
+
+            foreach (var responsePair in ClientsToApprove)
+            {
+                var response = responsePair.Value;
+                var senderId = responsePair.Key;
+
+                if (!response.Pending)
+                {
+                    HandleConnectionApproval(senderId, response);
+
+                    if (senders == null)
+                    {
+                        senders = new List<ulong>();
+                    }
+                    senders.Add(senderId);
+                }
+            }
+
+            if (senders != null)
+            {
+                foreach (var sender in senders)
+                {
+                    ClientsToApprove.Remove(sender);
+                }
+            }
+        }
+
         private void OnNetworkEarlyUpdate()
         {
             if (!IsListening)
             {
                 return;
             }
 
+            ProcessPendingApprovals();
+
 #if DEVELOPMENT_BUILD || UNITY_EDITOR
             s_TransportPoll.Begin();
 #endif

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

@@ -101,13 +101,15 @@ public void Handle(ref NetworkContext context)
             {
                 // Note: Delegate creation allocates.
                 // Note: ToArray() also allocates. :(
-                var response = networkManager.ConnectionApprovalCallback(
+                var response = new NetworkManager.ConnectionApprovalResponse();
+                networkManager.ClientsToApprove[senderId] = response;
+
+                networkManager.ConnectionApprovalCallback(
                     new NetworkManager.ConnectionApprovalRequest
                     {
                         Payload = ConnectionData,
                         ClientNetworkId = senderId
-                    });
-                networkManager.HandleConnectionApproval(senderId, response);
+                    }, response);
             }
             else
             {

com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApproval.cs

@@ -47,9 +47,8 @@ public IEnumerator ConnectionApproval()
             Assert.True(m_IsValidated);
         }
 
-        private NetworkManager.ConnectionApprovalResponse NetworkManagerObject_ConnectionApprovalCallback(NetworkManager.ConnectionApprovalRequest request)
+        private void NetworkManagerObject_ConnectionApprovalCallback(NetworkManager.ConnectionApprovalRequest request, NetworkManager.ConnectionApprovalResponse response)
         {
-            var response = new NetworkManager.ConnectionApprovalResponse();
             var stringGuid = Encoding.UTF8.GetString(request.Payload);
             if (m_ValidationToken.ToString() == stringGuid)
             {
@@ -61,8 +60,6 @@ private NetworkManager.ConnectionApprovalResponse NetworkManagerObject_Connectio
             response.Position = null;
             response.Rotation = null;
             response.PlayerPrefabHash = null;
-
-            return response;
         }
 
         [TearDown]

testproject/Assets/Tests/Manual/Scripts/ConnectionApprovalComponent.cs

@@ -153,9 +153,8 @@ public void OnDisconnectClient()
         /// </summary>
         /// <param name="request">The connection approval request</param>
         /// <returns>ConnectionApprovalResult with the approval decision, with parameters</returns>
-        private NetworkManager.ConnectionApprovalResponse ConnectionApprovalCallback(NetworkManager.ConnectionApprovalRequest request)
+        private void ConnectionApprovalCallback(NetworkManager.ConnectionApprovalRequest request, NetworkManager.ConnectionApprovalResponse response)
         {
-            var response = new NetworkManager.ConnectionApprovalResponse();
             string approvalToken = Encoding.ASCII.GetString(request.Payload);
             var isTokenValid = approvalToken == m_ApprovalToken;
             if (m_SimulateFailure && m_SimulateFailure.isOn && IsServer && request.ClientNetworkId != NetworkManager.LocalClientId)
@@ -193,8 +192,6 @@ private NetworkManager.ConnectionApprovalResponse ConnectionApprovalCallback(Net
 
                 m_ConnectionMessageToDisplay.gameObject.SetActive(true);
             }
-
-            return response;
         }
 
         /// <summary>

testproject/Assets/Tests/Runtime/MultiClientConnectionApproval.cs

@@ -19,15 +19,18 @@ public class MultiClientConnectionApproval
 
         private GameObject m_PlayerPrefab;
         private GameObject m_PlayerPrefabOverride;
+        private bool m_DelayedApproval;
+        private List<NetworkManager.ConnectionApprovalResponse> m_ResponseToSet = new List<NetworkManager.ConnectionApprovalResponse>();
 
         /// <summary>
         /// Tests connection approval and connection approval failure
         /// </summary>
         /// <returns></returns>
         [UnityTest]
-        public IEnumerator ConnectionApproval()
+        public IEnumerator ConnectionApproval([Values(true, false)] bool delayedApproval)
         {
             m_ConnectionToken = "ThisIsTheRightPassword";
+            m_DelayedApproval = delayedApproval;
             return ConnectionApprovalHandler(3, 1);
         }
 
@@ -130,6 +133,22 @@ private IEnumerator ConnectionApprovalHandler(int numClients, int failureTestCou
                 Assert.Fail("Failed to start instances");
             }
 
+            if (m_DelayedApproval)
+            {
+                // This is necessary so that clients gets the time to attempt connecting and fill the pending approval responses
+                var nextFrameNumber = Time.frameCount + 10;
+                yield return new WaitUntil(() => Time.frameCount >= nextFrameNumber);
+
+                foreach (var response in m_ResponseToSet)
+                {
+                    // perform delayed approval
+                    // The response class has already been filled, when created in ConnectionApprovalCallback()
+                    yield return new WaitForSeconds(0.2f);
+                    response.Pending = false;
+                }
+                m_ResponseToSet.Clear();
+            }
+
             // [Client-Side] Wait for a connection to the server
             yield return NetcodeIntegrationTestHelpers.WaitForClientsConnected(clientsAdjustedList.ToArray(), null, 512);
 
@@ -173,12 +192,17 @@ private IEnumerator ConnectionApprovalHandler(int numClients, int failureTestCou
         /// <param name="connectionData">the NetworkConfig.ConnectionData sent from the client being approved</param>
         /// <param name="clientId">the client id being approved</param>
         /// <param name="callback">the callback invoked to handle approval</param>
-        private NetworkManager.ConnectionApprovalResponse ConnectionApprovalCallback(NetworkManager.ConnectionApprovalRequest request)
+        private void ConnectionApprovalCallback(NetworkManager.ConnectionApprovalRequest request, NetworkManager.ConnectionApprovalResponse response)
         {
-            var response = new NetworkManager.ConnectionApprovalResponse();
             string approvalToken = Encoding.ASCII.GetString(request.Payload);
             var isApproved = approvalToken == m_ConnectionToken;
 
+            if (m_DelayedApproval)
+            {
+                response.Pending = true;
+                m_ResponseToSet.Add(response);
+            }
+
             if (isApproved)
             {
                 m_SuccessfulConnections++;
@@ -204,8 +228,6 @@ private NetworkManager.ConnectionApprovalResponse ConnectionApprovalCallback(Net
                 response.Rotation = null;
                 response.PlayerPrefabHash = m_PrefabOverrideGlobalObjectIdHash;
             }
-
-            return response;
         }