Skip to content

Commit 1d47660

Browse files
EmandMEmandM
committed
fix: CMB Service not disconnecting
1 parent 46c84f9 commit 1d47660

File tree

3 files changed

+91
-62
lines changed

3 files changed

+91
-62
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,8 @@ internal void HandleNetworkEvent(NetworkEvent networkEvent, ulong transportClien
492492
/// </remarks>
493493
private ulong m_LocalClientTransportId;
494494

495+
internal ulong LocalClientTransportId => m_LocalClientTransportId;
496+
495497
/// <summary>
496498
/// Handles a <see cref="NetworkEvent.Connect"/> event.
497499
/// </summary>
@@ -596,7 +598,7 @@ internal void DisconnectEventHandler(ulong transportClientId)
596598
var (clientId, isConnectedClient) = TransportIdToClientId(transportClientId);
597599

598600
// If the client is not registered and we are the server
599-
if (!isConnectedClient && NetworkManager.IsServer)
601+
if (!isConnectedClient)
600602
{
601603
// Then exit early
602604
return;

‎com.unity.netcode.gameobjects/Runtime/Messaging/Messages/DisconnectReasonMessage.cs‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
using System.Collections;
2+
13
namespace Unity.Netcode
24
{
35
internal struct DisconnectReasonMessage : INetworkMessage
@@ -37,10 +39,25 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
3739

3840
public void Handle(ref NetworkContext context)
3941
{
42+
var networkManager = (NetworkManager)context.SystemOwner;
4043
// Always apply the server-side generated disconnect reason to the server specific disconnect reason.
4144
// This is combined with the additional disconnect information when getting NetworkManager.DisconnectReason
4245
// (NetworkConnectionManager.DisconnectReason).
43-
((NetworkManager)context.SystemOwner).ConnectionManager.ServerDisconnectReason = Reason;
46+
networkManager.ConnectionManager.ServerDisconnectReason = Reason;
47+
48+
49+
if (networkManager.NetworkConfig.UseCMBService)
50+
{
51+
networkManager.StartCoroutine(HandleDisconnectAfterReason(networkManager));
52+
}
53+
}
54+
55+
private IEnumerator HandleDisconnectAfterReason(NetworkManager networkManager)
56+
{
57+
yield return new WaitForFixedUpdate();
58+
59+
var connectionManager = networkManager.ConnectionManager;
60+
connectionManager.DisconnectEventHandler(connectionManager.LocalClientTransportId);
4461
}
4562
};
4663
}

‎com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/SessionVersionConnectionRequest.cs‎

Lines changed: 70 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System.Collections;
22
using NUnit.Framework;
33
using Unity.Netcode.TestHelpers.Runtime;
4+
using UnityEngine;
45
using UnityEngine.TestTools;
56

67
namespace Unity.Netcode.RuntimeTests
@@ -9,52 +10,36 @@ internal class SessionVersionConnectionRequest : NetcodeIntegrationTest
910
{
1011
protected override int NumberOfClients => 0;
1112

12-
// TODO: [CmbServiceTests] Adapt to run with the service
13-
protected override bool UseCMBService()
14-
{
15-
return false;
16-
}
13+
// Use a specific version for the CMB tests
14+
// The CMB service has more detailed versioning logic. Not all lower versions are invalid to connect with the higher version.
15+
// This version will not connect with the version lower.
16+
private const int k_ValidCMBVersion = 5;
1717

1818
public SessionVersionConnectionRequest() : base(NetworkTopologyTypes.DistributedAuthority, HostOrServer.DAHost) { }
1919

2020
private bool m_UseValidSessionVersion;
2121
private bool m_ClientWasDisconnected;
22-
private NetworkManager m_ClientNetworkManager;
22+
private bool m_CanStartClients;
23+
24+
// Don't start automatically when using the CMB Service
25+
// We want to customize the SessionVersion of the session owner before they connect
26+
protected override bool CanStartServerAndClients() => !m_UseCmbService || m_CanStartClients;
2327

2428
/// <summary>
2529
/// Callback used to mock the scenario where a client has an invalid session version
2630
/// </summary>
27-
/// <returns><see cref="SessionConfig"/></returns>
28-
private SessionConfig GetInavlidSessionConfig()
31+
private SessionConfig GetInvalidSessionConfig()
2932
{
3033
var authority = GetAuthorityNetworkManager();
3134
return new SessionConfig(authority.SessionConfig.SessionVersion - 1);
3235
}
3336

34-
/// <summary>
35-
/// Overriding this method allows us to configure the newly instantiated client's
36-
/// NetworkManager prior to it being started.
37-
/// </summary>
38-
/// <param name="networkManager">the newly instantiated NetworkManager</param>
39-
protected override void OnNewClientCreated(NetworkManager networkManager)
40-
{
41-
m_ClientWasDisconnected = false;
42-
m_ClientNetworkManager = networkManager;
43-
m_ClientNetworkManager.OnClientDisconnectCallback += OnClientDisconnectCallback;
44-
if (!m_UseValidSessionVersion)
45-
{
46-
networkManager.OnGetSessionConfig = GetInavlidSessionConfig;
47-
}
48-
base.OnNewClientCreated(networkManager);
49-
}
50-
5137
/// <summary>
5238
/// Tracks if the client was disconnected or not
5339
/// </summary>
5440
private void OnClientDisconnectCallback(ulong clientId)
5541
{
5642
m_ClientWasDisconnected = true;
57-
m_ClientNetworkManager.OnClientDisconnectCallback -= OnClientDisconnectCallback;
5843
}
5944

6045
/// <summary>
@@ -69,51 +54,76 @@ protected override bool ShouldWaitForNewClientToConnect(NetworkManager networkMa
6954
{
7055
return m_UseValidSessionVersion;
7156
}
72-
73-
internal enum SessionVersionType
74-
{
75-
Valid,
76-
Invalid,
77-
}
78-
7957
/// <summary>
8058
/// Validates that when the client's session config version is valid a client will be
8159
/// allowed to connect and when it is not valid the client will be disconnected.
8260
/// </summary>
83-
/// <remarks>
84-
/// This is just a mock of the service logic to validate everything on the NGO side is
85-
/// working correctly.
86-
/// </remarks>
87-
/// <param name="useValidSessionVersion">true = use valid session version | false = use invalid session version</param>
8861
[UnityTest]
89-
public IEnumerator ValidateSessionVersion([Values] SessionVersionType type)
62+
public IEnumerator ValidateSessionVersion()
9063
{
91-
// Test client being disconnected due to invalid session version
92-
m_UseValidSessionVersion = type == SessionVersionType.Valid;
93-
yield return CreateAndStartNewClient();
94-
yield return s_DefaultWaitForTick;
95-
if (!m_UseValidSessionVersion)
64+
if (m_UseCmbService)
9665
{
97-
yield return WaitForConditionOrTimeOut(() => m_ClientWasDisconnected);
98-
AssertOnTimeout("Client was not disconnected when it should have been!");
99-
Assert.True(m_ClientNetworkManager.DisconnectReason.Contains(ConnectionRequestMessage.InvalidSessionVersionMessage), "Client did not receive the correct invalid session version message!");
66+
var authority = GetAuthorityNetworkManager();
67+
authority.OnGetSessionConfig = () => new SessionConfig(k_ValidCMBVersion);
68+
m_CanStartClients = true;
69+
yield return StartServerAndClients();
10070
}
101-
else
71+
72+
/*
73+
* Test client being disconnected due to invalid session version
74+
*/
75+
m_UseValidSessionVersion = false;
76+
77+
// Create and setup client to use invalid session config
78+
var invalidClient = CreateNewClient();
79+
invalidClient.OnClientDisconnectCallback += OnClientDisconnectCallback;
80+
invalidClient.OnGetSessionConfig = GetInvalidSessionConfig;
81+
82+
// Start client and wait for disconnect callback
83+
m_ClientWasDisconnected = false;
84+
yield return StartClient(invalidClient);
85+
Assert.True(invalidClient.IsListening);
86+
yield return s_DefaultWaitForTick;
87+
88+
var timeoutHelper = new TimeoutHelper(30f);
89+
yield return WaitForConditionOrTimeOut(() => !invalidClient.IsListening, timeoutHelper);
90+
AssertOnTimeout("Client is still listening when it should have been disconnected!", timeoutHelper);
91+
92+
yield return WaitForConditionOrTimeOut(() => m_ClientWasDisconnected);
93+
AssertOnTimeout("Client was not disconnected when it should have been!");
94+
95+
var expectedReason = m_UseCmbService ? "incompatible ngo c# package versions for feature" : ConnectionRequestMessage.InvalidSessionVersionMessage;
96+
Assert.That(invalidClient.DisconnectReason, Does.Contain(expectedReason), $"Client did not receive the correct invalid session version message! Received: {invalidClient.DisconnectReason}");
97+
98+
// Clean up invalid client
99+
invalidClient.OnClientDisconnectCallback -= OnClientDisconnectCallback;
100+
yield return StopOneClient(invalidClient, true);
101+
102+
/*
103+
* Test a later client with a valid version
104+
* They should connect as normal
105+
*/
106+
m_UseValidSessionVersion = true;
107+
108+
// Create and setup client to use invalid session config
109+
var lateJoin = CreateNewClient();
110+
lateJoin.OnClientDisconnectCallback += OnClientDisconnectCallback;
111+
if (m_UseCmbService)
102112
{
103-
Assert.False(m_ClientWasDisconnected, "Client was disconnected when it was expected to connect!");
104-
Assert.True(m_ClientNetworkManager.IsConnectedClient, "Client did not connect properly using the correct session version!");
113+
lateJoin.OnGetSessionConfig = () => new SessionConfig(k_ValidCMBVersion);
105114
}
106-
}
107115

108-
/// <summary>
109-
/// Invoked at the end of each integration test pass.
110-
/// Primarily used to clean up for the next pass.
111-
/// </summary>
112-
protected override IEnumerator OnTearDown()
113-
{
114-
m_ClientNetworkManager.OnClientDisconnectCallback -= OnClientDisconnectCallback;
115-
m_ClientNetworkManager = null;
116-
yield return base.OnTearDown();
116+
// Start client and wait for disconnect callback
117+
m_ClientWasDisconnected = false;
118+
yield return StartClient(lateJoin);
119+
yield return s_DefaultWaitForTick;
120+
121+
Assert.False(m_ClientWasDisconnected, "Client was disconnected when it was expected to connect!");
122+
Assert.True(lateJoin.IsConnectedClient, "Client did not connect properly using the correct session version!");
123+
Assert.That(GetAuthorityNetworkManager().ConnectedClientsIds, Has.Member(lateJoin.LocalClientId), "Newly joined client should be in connected list!");
124+
125+
// Clean up
126+
lateJoin.OnClientDisconnectCallback -= OnClientDisconnectCallback;
117127
}
118128
}
119129
}

0 commit comments

Comments
 (0)