Skip to content

chore: updating to ngo 1.2.0 #791

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 27 commits into from
Dec 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
a03f3e0
replacing custom messaging with new disconnect reason (v1)
LPLafontaineB Dec 7, 2022
7ccd612
Revert "replacing custom messaging with new disconnect reason (v1)"
LPLafontaineB Dec 7, 2022
b4e5da8
replacing custom messaging with new disconnect reason (v2)
LPLafontaineB Dec 7, 2022
ae29efd
setting SceneLoader's DontDestroyWithOwner to true
LPLafontaineB Dec 7, 2022
25d7d83
introducing temp var
LPLafontaineB Dec 7, 2022
6817630
deleting DisconnectingWithReason connection state
LPLafontaineB Dec 7, 2022
2b5ec1e
Revert "setting SceneLoader's DontDestroyWithOwner to true"
LPLafontaineB Dec 8, 2022
4a4f84a
Making sure we don't try to disconnect ourselves as the host when shu…
LPLafontaineB Dec 8, 2022
2b885d1
updating index entry
LPLafontaineB Dec 8, 2022
4480c3e
Adding changelog entry
LPLafontaineB Dec 8, 2022
47e9a7d
clarifying handling of disconnect reason in reconnectingState
LPLafontaineB Dec 8, 2022
ab45bf6
updating state machine diagram
LPLafontaineB Dec 9, 2022
d340e40
updating to ngo 1.2.0
LPLafontaineB Dec 9, 2022
0248547
Adding changelog entry
LPLafontaineB Dec 9, 2022
0fd0c37
Removing workaround
SamuelBellomo Oct 24, 2022
713abb4
Adding changelog entry for workaround removal
LPLafontaineB Dec 9, 2022
3920cc1
Revert "Removing workaround"
LPLafontaineB Dec 12, 2022
07294e7
removing workaround in tests waiting two frames before disconnecting …
LPLafontaineB Dec 12, 2022
c5004ff
updating changelog
LPLafontaineB Dec 12, 2022
e3b2bc0
Merge branch 'chore/update-to-1.2' into feature/disconnect-reason
LPLafontaineB Dec 12, 2022
b0bde56
fixing for loop index issue
LPLafontaineB Dec 12, 2022
b66bd2c
updating descriptions of connection state classes
LPLafontaineB Dec 12, 2022
2166a68
added info to changelog about ngo PR allowing us to remove workaround
LPLafontaineB Dec 12, 2022
a27132d
moving changelog entry to Changed section instead of Cleanup
LPLafontaineB Dec 12, 2022
48fffdf
adding entry to readme index for disconnecting every client with reason
LPLafontaineB Dec 12, 2022
73da2be
simplifying for loop
LPLafontaineB Dec 12, 2022
1a36c2d
Merge pull request #790 from Unity-Technologies/feature/disconnect-re…
LPLafontaineB Dec 12, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 1 addition & 40 deletions Assets/Scripts/ConnectionManagement/ConnectionManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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()
{
Expand All @@ -74,7 +76,6 @@ internal async Task ConnectClientAsync()
}

SceneLoaderWrapper.Instance.AddOnSceneEventCallback();
m_ConnectionManager.RegisterCustomMessages();
}
catch (Exception e)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() { }
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
}
}
3 changes: 0 additions & 3 deletions Assets/Tests/Runtime/ConnectionManagementTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -468,9 +468,6 @@ public IEnumerator ClientCancellingWhileConnectingToListeningServer_ConnectionCa
m_ClientConnectionManagers[i].StartClientIp($"client{i}", "127.0.0.1", 9998);
}

yield return null;
yield return null;

m_ClientConnectionManagers[0].RequestShutdown();

yield return WaitForClientsConnectedOrTimeOut();
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ Additional documentation and release notes are available at [Multiplayer Documen
## [unreleased] - yyyy-mm-dd
### 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

Expand Down
4 changes: 2 additions & 2 deletions Documentation/Images/BossRoomConnectionManager.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion Packages/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"com.unity.learn.iet-framework": "2.2.2",
"com.unity.memoryprofiler": "0.5.0-preview.1",
"com.unity.multiplayer.tools": "1.1.0",
"com.unity.netcode.gameobjects": "1.1.0",
"com.unity.netcode.gameobjects": "1.2.0",
"com.unity.performance.profile-analyzer": "1.1.1",
"com.unity.postprocessing": "3.2.2",
"com.unity.render-pipelines.universal": "12.1.7",
Expand Down
2 changes: 1 addition & 1 deletion Packages/packages-lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@
"url": "https://packages.unity.com"
},
"com.unity.netcode.gameobjects": {
"version": "1.1.0",
"version": "1.2.0",
"depth": 0,
"source": "registry",
"dependencies": {
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down