diff --git a/Assets/Scripts/ConnectionManagement/ConnectionManager.cs b/Assets/Scripts/ConnectionManagement/ConnectionManager.cs index 6ebf40d62..b2b2fa42c 100644 --- a/Assets/Scripts/ConnectionManagement/ConnectionManager.cs +++ b/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 states = new() { m_Offline, m_ClientConnecting, m_ClientConnected, m_ClientReconnecting, m_DisconnectingWithReason, m_StartingHost, m_Hosting }; + List 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(); } - - /// - /// 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) - /// - 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); - } - - /// - /// Sends a DisconnectReason to all connected clients. This should only be done on the server, prior to disconnecting the clients. - /// - /// The reason for the upcoming disconnect. - public void SendServerToAllClientsSetDisconnectReason(ConnectStatus status) - { - var writer = new FastBufferWriter(sizeof(ConnectStatus), Allocator.Temp); - writer.WriteValueSafe(status); - NetworkManager.CustomMessagingManager.SendNamedMessageToAll(nameof(ReceiveServerToClientSetDisconnectReason_CustomMessage), writer); - } - - /// - /// Sends a DisconnectReason to the indicated client. This should only be done on the server, prior to disconnecting the client. - /// - /// id of the client to send to - /// The reason for the upcoming disconnect. - 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); - } } } diff --git a/Assets/Scripts/ConnectionManagement/ConnectionState/ClientConnectedState.cs b/Assets/Scripts/ConnectionManagement/ConnectionState/ClientConnectedState.cs index c4a7252aa..a87bce2ca 100644 --- a/Assets/Scripts/ConnectionManagement/ConnectionState/ClientConnectedState.cs +++ b/Assets/Scripts/ConnectionManagement/ConnectionState/ClientConnectedState.cs @@ -7,7 +7,7 @@ namespace Unity.BossRoom.ConnectionManagement { /// /// 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. /// class ClientConnectedState : ConnectionState { @@ -26,8 +26,18 @@ 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(disconnectReason); + m_ConnectStatusPublisher.Publish(connectStatus); + m_ConnectionManager.ChangeState(m_ConnectionManager.m_Offline); + } } public override void OnUserRequestedShutdown() @@ -35,11 +45,5 @@ 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); - } } } diff --git a/Assets/Scripts/ConnectionManagement/ConnectionState/ClientConnectingState.cs b/Assets/Scripts/ConnectionManagement/ConnectionState/ClientConnectingState.cs index 2b27fef74..d8b7106db 100644 --- a/Assets/Scripts/ConnectionManagement/ConnectionState/ClientConnectingState.cs +++ b/Assets/Scripts/ConnectionManagement/ConnectionState/ClientConnectingState.cs @@ -9,8 +9,7 @@ namespace Unity.BossRoom.ConnectionManagement { /// /// 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. /// 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(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) { diff --git a/Assets/Scripts/ConnectionManagement/ConnectionState/ClientReconnectingState.cs b/Assets/Scripts/ConnectionManagement/ConnectionState/ClientReconnectingState.cs index 4d0ad9e82..e6e47ff42 100644 --- a/Assets/Scripts/ConnectionManagement/ConnectionState/ClientReconnectingState.cs +++ b/Assets/Scripts/ConnectionManagement/ConnectionState/ClientReconnectingState.cs @@ -8,9 +8,10 @@ namespace Unity.BossRoom.ConnectionManagement { /// /// 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. /// 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(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(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); } } diff --git a/Assets/Scripts/ConnectionManagement/ConnectionState/ConnectionState.cs b/Assets/Scripts/ConnectionManagement/ConnectionState/ConnectionState.cs index f0b9c6a2c..44251e45a 100644 --- a/Assets/Scripts/ConnectionManagement/ConnectionState/ConnectionState.cs +++ b/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() { } diff --git a/Assets/Scripts/ConnectionManagement/ConnectionState/DisconnectingWithReasonState.cs b/Assets/Scripts/ConnectionManagement/ConnectionState/DisconnectingWithReasonState.cs deleted file mode 100644 index 96d6c8217..000000000 --- a/Assets/Scripts/ConnectionManagement/ConnectionState/DisconnectingWithReasonState.cs +++ /dev/null @@ -1,19 +0,0 @@ -namespace Unity.BossRoom.ConnectionManagement -{ - /// - /// Connection state corresponding to a client who received a message from the server with a disconnect reason. - /// Since our disconnect process runs in multiple steps host side, this state is the first step client side. This - /// state simply waits for the actual disconnect, then transitions to the offline state. - /// - class DisconnectingWithReasonState : OnlineState - { - public override void Enter() { } - - public override void Exit() { } - - public override void OnClientDisconnect(ulong _) - { - m_ConnectionManager.ChangeState(m_ConnectionManager.m_Offline); - } - } -} diff --git a/Assets/Scripts/ConnectionManagement/ConnectionState/DisconnectingWithReasonState.cs.meta b/Assets/Scripts/ConnectionManagement/ConnectionState/DisconnectingWithReasonState.cs.meta deleted file mode 100644 index 96cbc0b52..000000000 --- a/Assets/Scripts/ConnectionManagement/ConnectionState/DisconnectingWithReasonState.cs.meta +++ /dev/null @@ -1,3 +0,0 @@ -fileFormatVersion: 2 -guid: 8cdbdfce07de4da48006d8e16b95da48 -timeCreated: 1654886224 \ No newline at end of file diff --git a/Assets/Scripts/ConnectionManagement/ConnectionState/HostingState.cs b/Assets/Scripts/ConnectionManagement/ConnectionState/HostingState.cs index 3886de6ae..daf1bcd58 100644 --- a/Assets/Scripts/ConnectionManagement/ConnectionState/HostingState.cs +++ b/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); } /// @@ -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.Instance.IsDuplicateConnection(connectionPayload.playerId) ? ConnectStatus.LoggedInAgain : ConnectStatus.Success; } - - IEnumerator WaitToShutdown() - { - yield return null; - m_ConnectionManager.ChangeState(m_ConnectionManager.m_Offline); - } } } diff --git a/CHANGELOG.md b/CHANGELOG.md index 471c6bdd7..d90f7c8f0 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/Documentation/Images/BossRoomConnectionManager.png b/Documentation/Images/BossRoomConnectionManager.png index 31d384108..d5183f68d 100644 --- a/Documentation/Images/BossRoomConnectionManager.png +++ b/Documentation/Images/BossRoomConnectionManager.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:a22ea4fd5b2355272c50ab7d8e3b0f2c7e908f7aa545d36ec22b8ed690f31db1 -size 88312 +oid sha256:3c7a2f72a6c7fe4287598c77f923fb63c5059f8f95098503a775a160ead28f7c +size 66141 diff --git a/README.md b/README.md index c9b88dedb..bd22f4b11 100644 --- a/README.md +++ b/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)
[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)