refactor: disconnect reason [MTT-2984] [MTT-5200]

Assets/Scripts/ConnectionManagement/ConnectionManager.cs

@@ -76,7 +76,6 @@ public class ConnectionManager : MonoBehaviour
         internal readonly ClientConnectingState m_ClientConnecting = new ClientConnectingState();
         internal readonly ClientConnectedState m_ClientConnected = new ClientConnectedState();
         internal readonly ClientReconnectingState m_ClientReconnecting = new ClientReconnectingState();
-        internal readonly DisconnectingWithReasonState m_DisconnectingWithReason = new DisconnectingWithReasonState();
         internal readonly StartingHostState m_StartingHost = new StartingHostState();
         internal readonly HostingState m_Hosting = new HostingState();
 
@@ -87,7 +86,7 @@ void Awake()
 
         void Start()
         {
-            List<ConnectionState> states = new() { m_Offline, m_ClientConnecting, m_ClientConnected, m_ClientReconnecting, m_DisconnectingWithReason, m_StartingHost, m_Hosting };
+            List<ConnectionState> states = new() { m_Offline, m_ClientConnecting, m_ClientConnected, m_ClientReconnecting, m_StartingHost, m_Hosting };
             foreach (var connectionState in states)
             {
                 m_Resolver.Inject(connectionState);
@@ -172,43 +171,5 @@ public void RequestShutdown()
         {
             m_CurrentState.OnUserRequestedShutdown();
         }
-
-        /// <summary>
-        /// Registers the message handler for custom named messages. This should only be done once StartClient has been
-        /// called (start client will initialize NetworkSceneManager and CustomMessagingManager)
-        /// </summary>
-        public void RegisterCustomMessages()
-        {
-            NetworkManager.CustomMessagingManager.RegisterNamedMessageHandler(nameof(ReceiveServerToClientSetDisconnectReason_CustomMessage), ReceiveServerToClientSetDisconnectReason_CustomMessage);
-        }
-
-        void ReceiveServerToClientSetDisconnectReason_CustomMessage(ulong clientID, FastBufferReader reader)
-        {
-            reader.ReadValueSafe(out ConnectStatus status);
-            m_CurrentState.OnDisconnectReasonReceived(status);
-        }
-
-        /// <summary>
-        /// Sends a DisconnectReason to all connected clients. This should only be done on the server, prior to disconnecting the clients.
-        /// </summary>
-        /// <param name="status"> The reason for the upcoming disconnect.</param>
-        public void SendServerToAllClientsSetDisconnectReason(ConnectStatus status)
-        {
-            var writer = new FastBufferWriter(sizeof(ConnectStatus), Allocator.Temp);
-            writer.WriteValueSafe(status);
-            NetworkManager.CustomMessagingManager.SendNamedMessageToAll(nameof(ReceiveServerToClientSetDisconnectReason_CustomMessage), writer);
-        }
-
-        /// <summary>
-        /// Sends a DisconnectReason to the indicated client. This should only be done on the server, prior to disconnecting the client.
-        /// </summary>
-        /// <param name="clientID"> id of the client to send to </param>
-        /// <param name="status"> The reason for the upcoming disconnect.</param>
-        public void SendServerToClientSetDisconnectReason(ulong clientID, ConnectStatus status)
-        {
-            var writer = new FastBufferWriter(sizeof(ConnectStatus), Allocator.Temp);
-            writer.WriteValueSafe(status);
-            NetworkManager.CustomMessagingManager.SendNamedMessage(nameof(ReceiveServerToClientSetDisconnectReason_CustomMessage), clientID, writer);
-        }
     }
 }

Assets/Scripts/ConnectionManagement/ConnectionState/ClientConnectedState.cs

@@ -7,7 +7,7 @@ namespace Unity.BossRoom.ConnectionManagement
 {
     /// <summary>
     /// Connection state corresponding to a connected client. When being disconnected, transitions to the
-    /// ClientReconnecting state. When receiving a disconnect reason, transitions to the DisconnectingWithReason state.
+    /// ClientReconnecting state if no reason is given, or to the Offline state.
     /// </summary>
     class ClientConnectedState : ConnectionState
     {
@@ -26,20 +26,24 @@ public override void Exit() { }
 
         public override void OnClientDisconnect(ulong _)
         {
-            m_ConnectStatusPublisher.Publish(ConnectStatus.Reconnecting);
-            m_ConnectionManager.ChangeState(m_ConnectionManager.m_ClientReconnecting);
+            var disconnectReason = m_ConnectionManager.NetworkManager.DisconnectReason;
+            if (string.IsNullOrEmpty(disconnectReason))
+            {
+                m_ConnectStatusPublisher.Publish(ConnectStatus.Reconnecting);
+                m_ConnectionManager.ChangeState(m_ConnectionManager.m_ClientReconnecting);
+            }
+            else
+            {
+                var connectStatus = JsonUtility.FromJson<ConnectStatus>(disconnectReason);
+                m_ConnectStatusPublisher.Publish(connectStatus);
+                m_ConnectionManager.ChangeState(m_ConnectionManager.m_Offline);
+            }
         }
 
         public override void OnUserRequestedShutdown()
         {
             m_ConnectStatusPublisher.Publish(ConnectStatus.UserRequestedDisconnect);
             m_ConnectionManager.ChangeState(m_ConnectionManager.m_Offline);
         }
-
-        public override void OnDisconnectReasonReceived(ConnectStatus disconnectReason)
-        {
-            m_ConnectStatusPublisher.Publish(disconnectReason);
-            m_ConnectionManager.ChangeState(m_ConnectionManager.m_DisconnectingWithReason);
-        }
     }
 }

Assets/Scripts/ConnectionManagement/ConnectionState/ClientConnectingState.cs

@@ -9,8 +9,7 @@ namespace Unity.BossRoom.ConnectionManagement
 {
     /// <summary>
     /// Connection state corresponding to when a client is attempting to connect to a server. Starts the client when
-    /// entering. If successful, transitions to the ClientConnected state. If not, transitions to the Offline state. If
-    /// given a disconnect reason first, transitions to the DisconnectingWithReason state.
+    /// entering. If successful, transitions to the ClientConnected state. If not, transitions to the Offline state.
     /// </summary>
     class ClientConnectingState : OnlineState
     {
@@ -49,16 +48,19 @@ public override void OnClientDisconnect(ulong _)
 
         protected void StartingClientFailedAsync()
         {
-            m_ConnectStatusPublisher.Publish(ConnectStatus.StartClientFailed);
+            var disconnectReason = m_ConnectionManager.NetworkManager.DisconnectReason;
+            if (string.IsNullOrEmpty(disconnectReason))
+            {
+                m_ConnectStatusPublisher.Publish(ConnectStatus.StartClientFailed);
+            }
+            else
+            {
+                var connectStatus = JsonUtility.FromJson<ConnectStatus>(disconnectReason);
+                m_ConnectStatusPublisher.Publish(connectStatus);
+            }
             m_ConnectionManager.ChangeState(m_ConnectionManager.m_Offline);
         }
 
-        public override void OnDisconnectReasonReceived(ConnectStatus disconnectReason)
-        {
-            m_ConnectStatusPublisher.Publish(disconnectReason);
-            m_ConnectionManager.ChangeState(m_ConnectionManager.m_DisconnectingWithReason);
-        }
-
 
         internal async Task ConnectClientAsync()
         {
@@ -74,7 +76,6 @@ internal async Task ConnectClientAsync()
                 }
 
                 SceneLoaderWrapper.Instance.AddOnSceneEventCallback();
-                m_ConnectionManager.RegisterCustomMessages();
             }
             catch (Exception e)
             {

Assets/Scripts/ConnectionManagement/ConnectionState/ClientReconnectingState.cs

@@ -8,9 +8,10 @@ namespace Unity.BossRoom.ConnectionManagement
 {
     /// <summary>
     /// Connection state corresponding to a client attempting to reconnect to a server. It will try to reconnect a
-    /// number of times defined by k_NbReconnectAttempts. If it succeeds, it will transition to the ClientConnected
-    /// state. If not, it will transition to the Offline state. If given a disconnect reason first, depending on the
-    /// reason given, may transition to the DisconnectingWithReason state.
+    /// number of times defined by the ConnectionManager's NbReconnectAttempts property. If it succeeds, it will
+    /// transition to the ClientConnected state. If not, it will transition to the Offline state. If given a disconnect
+    /// reason first, depending on the reason given, may not try to reconnect again and transition directly to the
+    /// Offline state.
     /// </summary>
     class ClientReconnectingState : ClientConnectingState
     {
@@ -47,27 +48,44 @@ public override void OnClientConnected(ulong _)
 
         public override void OnClientDisconnect(ulong _)
         {
+            var disconnectReason = m_ConnectionManager.NetworkManager.DisconnectReason;
             if (m_NbAttempts < m_ConnectionManager.NbReconnectAttempts)
             {
-                m_ReconnectCoroutine = m_ConnectionManager.StartCoroutine(ReconnectCoroutine());
+                if (string.IsNullOrEmpty(disconnectReason))
+                {
+                    m_ReconnectCoroutine = m_ConnectionManager.StartCoroutine(ReconnectCoroutine());
+                }
+                else
+                {
+                    var connectStatus = JsonUtility.FromJson<ConnectStatus>(disconnectReason);
+                    m_ConnectStatusPublisher.Publish(connectStatus);
+                    switch (connectStatus)
+                    {
+                        case ConnectStatus.UserRequestedDisconnect:
+                        case ConnectStatus.HostEndedSession:
+                        case ConnectStatus.ServerFull:
+                        case ConnectStatus.IncompatibleBuildType:
+                            m_ConnectionManager.ChangeState(m_ConnectionManager.m_Offline);
+                            break;
+                        default:
+                            m_ReconnectCoroutine = m_ConnectionManager.StartCoroutine(ReconnectCoroutine());
+                            break;
+                    }
+                }
             }
             else
             {
-                m_ConnectStatusPublisher.Publish(ConnectStatus.GenericDisconnect);
-                m_ConnectionManager.ChangeState(m_ConnectionManager.m_Offline);
-            }
-        }
+                if (string.IsNullOrEmpty(disconnectReason))
+                {
+                    m_ConnectStatusPublisher.Publish(ConnectStatus.GenericDisconnect);
+                }
+                else
+                {
+                    var connectStatus = JsonUtility.FromJson<ConnectStatus>(disconnectReason);
+                    m_ConnectStatusPublisher.Publish(connectStatus);
+                }
 
-        public override void OnDisconnectReasonReceived(ConnectStatus disconnectReason)
-        {
-            m_ConnectStatusPublisher.Publish(disconnectReason);
-            switch (disconnectReason)
-            {
-                case ConnectStatus.UserRequestedDisconnect:
-                case ConnectStatus.HostEndedSession:
-                case ConnectStatus.ServerFull:
-                    m_ConnectionManager.ChangeState(m_ConnectionManager.m_DisconnectingWithReason);
-                    break;
+                m_ConnectionManager.ChangeState(m_ConnectionManager.m_Offline);
             }
         }
 

Assets/Scripts/ConnectionManagement/ConnectionState/ConnectionState.cs

@@ -36,8 +36,6 @@ public virtual void StartHostLobby(string playerName) { }
 
         public virtual void OnUserRequestedShutdown() { }
 
-        public virtual void OnDisconnectReasonReceived(ConnectStatus disconnectReason) { }
-
         public virtual void ApprovalCheck(NetworkManager.ConnectionApprovalRequest request, NetworkManager.ConnectionApprovalResponse response) { }
 
         public virtual void OnTransportFailure() { }

Assets/Scripts/ConnectionManagement/ConnectionState/HostingState.cs

@@ -71,9 +71,16 @@ public override void OnClientDisconnect(ulong clientId)
 
         public override void OnUserRequestedShutdown()
         {
-            m_ConnectionManager.SendServerToAllClientsSetDisconnectReason(ConnectStatus.HostEndedSession);
-            // Wait before shutting down to make sure clients receive that message before they are disconnected
-            m_ConnectionManager.StartCoroutine(WaitToShutdown());
+            var reason = JsonUtility.ToJson(ConnectStatus.HostEndedSession);
+            for (var i = m_ConnectionManager.NetworkManager.ConnectedClientsIds.Count - 1; i >= 0; i--)
+            {
+                var id = m_ConnectionManager.NetworkManager.ConnectedClientsIds[i];
+                if (id != m_ConnectionManager.NetworkManager.LocalClientId)
+                {
+                    m_ConnectionManager.NetworkManager.DisconnectClient(id, reason);
+                }
+            }
+            m_ConnectionManager.ChangeState(m_ConnectionManager.m_Offline);
         }
 
         /// <summary>
@@ -119,21 +126,8 @@ public override void ApprovalCheck(NetworkManager.ConnectionApprovalRequest requ
                 return;
             }
 
-            // In order for clients to not just get disconnected with no feedback, the server needs to tell the client why it disconnected it.
-            // This could happen after an auth check on a service or because of gameplay reasons (server full, wrong build version, etc)
-            // Since network objects haven't synced yet (still in the approval process), we need to send a custom message to clients, wait for
-            // UTP to update a frame and flush that message, then give our response to NetworkManager's connection approval process, with a denied approval.
-            IEnumerator WaitToDenyApproval()
-            {
-                response.Pending = true; // give some time for server to send connection status message to clients
-                response.Approved = false;
-                m_ConnectionManager.SendServerToClientSetDisconnectReason(clientId, gameReturnStatus);
-                yield return null; // wait a frame so UTP can flush it's messages on next update
-                response.Pending = false; // connection approval process can be finished.
-            }
-
-            m_ConnectionManager.SendServerToClientSetDisconnectReason(clientId, gameReturnStatus);
-            m_ConnectionManager.StartCoroutine(WaitToDenyApproval());
+            response.Approved = false;
+            response.Reason = JsonUtility.ToJson(gameReturnStatus);
             if (m_LobbyServiceFacade.CurrentUnityLobby != null)
             {
                 m_LobbyServiceFacade.RemovePlayerFromLobbyAsync(connectionPayload.playerId, m_LobbyServiceFacade.CurrentUnityLobby.Id);
@@ -155,11 +149,5 @@ ConnectStatus GetConnectStatus(ConnectionPayload connectionPayload)
             return SessionManager<SessionPlayerData>.Instance.IsDuplicateConnection(connectionPayload.playerId) ?
                 ConnectStatus.LoggedInAgain : ConnectStatus.Success;
         }
-
-        IEnumerator WaitToShutdown()
-        {
-            yield return null;
-            m_ConnectionManager.ChangeState(m_ConnectionManager.m_Offline);
-        }
     }
 }

CHANGELOG.md

@@ -10,6 +10,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Changed
 * Bumped RNSM to 1.1.0: Switched x axis units to seconds instead of frames now that it's available. This means adjusting the sample count to a lower value as well to 30 seconds, since the x axis was moving too slowly. (#788)
 * Updated Boss Room to NGO 1.2.0 (#791).
+  * Removed a workaround in our tests waiting for two frames before shutting down a client that is attempting to connect to a server. (see https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/pull/2261)
+* Replaced the workaround using custom messages to send a disconnect reason to clients with the new DisconnectReason feature in NGO. (#790)
 
 ## [2.0.3] - 2022-12-05
 

README.md

@@ -178,7 +178,8 @@ Running the game over internet currently requires setting up a relay.
 * Win state - [Assets/Scripts/Gameplay/GameState/PersistentGameState.cs](Assets/Scripts/Gameplay/GameState/PersistentGameState.cs)
 
 ### Connectivity
-* Connection approval return  value with custom messaging - WaitToDenyApproval() in [Assets/Scripts/ConnectionManagement/ConnectionState/HostingState.cs ](Assets/Scripts/ConnectionManagement/ConnectionState/HostingState.cs)
+* Disconnecting every client with reason - OnUserRequestedShutdown() in [Assets/Scripts/ConnectionManagement/ConnectionState/HostingState.cs ](Assets/Scripts/ConnectionManagement/ConnectionState/HostingState.cs)
+* Connection approval with reason sent to the client when denied - ApprovalCheck() in [Assets/Scripts/ConnectionManagement/ConnectionState/HostingState.cs ](Assets/Scripts/ConnectionManagement/ConnectionState/HostingState.cs)
 * Connection state machine - [Assets/Scripts/ConnectionManagement/ConnectionManager.cs ](Assets/Scripts/ConnectionManagement/ConnectionManager.cs) <br> [Assets/Scripts/ConnectionManagement/ConnectionState/](Assets/Scripts/ConnectionManagement/ConnectionState/)
 * Session manager - [Packages/com.unity.multiplayer.samples.coop/Utilities/Net/SessionManager.cs ](Packages/com.unity.multiplayer.samples.coop/Utilities/Net/SessionManager.cs)
 * RTT stats - [Assets/Scripts/Utils/NetworkOverlay/NetworkStats.cs](Assets/Scripts/Utils/NetworkOverlay/NetworkStats.cs)