Skip to content

Commit 46424d1

Browse files
fix: NetcodeIntegrationTest updates and fixes (#3437)
## This PR Includes ### Initial setup changes - Minor improvements to the initial CMB server detection and adjustments to the initial configuration order of operations. - Session owner now starts and connects ahead of the other clients. ### Ignoring tests This PR includes updates that will ignore any test that: - Uses a client-server network topology - _No point in re-running these as they will have already run multiple times on multiple platforms before testing against the CMB server._ - Has overridden UseCmbService and is returning false. - _The test has yet to be converted and so no point in even attempting to run it._ - _This also includes the `TODO: [CmbServiceTests]` to help track all tests that need to be converted or reviewed to determine if they need to be converted._ - Is not derived from NetcodeIntegrationTest. - _Many of these (not all) don't need to run again, but they all have the `TODO: [CmbServiceTests]` to help track all tests that need to be converted or not._ - Does not need to be tested against the CMB Server (i.e. a test for some kind of functionality without actually starting a session and the like). _(This helps to reduce the time to complete the CMB Server integration tests)_ ### Marking/Tracking "for review" tests Assuring that every test that needs to be reviewed includes the `TODO: [CmbServiceTests]` using of the two ways: For classes that derive from `NetcodeIntegrationTest` the existing pattern used in #3423 was applied: ``` // TODO: [CmbServiceTests] Adapt to run with the service protected override bool UseCMBService() { return false; } ``` For classes that **do not** derive from `NetcodeIntegrationTest`, an added internal helper method was used: ``` [OneTimeSetUp] public void OneTimeSetup() { // TODO: [CmbServiceTests] if this test is deemed needed to test against the CMB server then update this test. NetcodeIntegrationTestHelpers.IgnoreIfServiceEnviromentVariableSet(); } ``` _(This PR includes some tests that includes one of the two script segments but was was reviewed and determined it didn't need to be run against a CMB server)_ ## Fixes Some issues were exposed in regards to the `NetworkClient` not being set to approved on all clients relative to each local `NetworkManager` instance when using the distributed authority network topology and connecting to a CMB server. ## Changelog NA - All internal testing and/or specific to testing. ## Testing and Documentation - Includes integration test adjustments. - Includes `NetcodeIntegrationTest` related adjustments. - Includes `NetcodeIntegrationTestHelpers` related adjustments. - No documentation changes or additions were necessary. ## Backport Does not require a backport since these changes are specific to updates in #3423 and over-all distributed authority integration testing.
1 parent c1f9445 commit 46424d1

File tree

55 files changed

+786
-407
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+786
-407
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,10 +273,12 @@ public void Handle(ref NetworkContext context)
273273
// Stop the client-side approval timeout coroutine since we are approved.
274274
networkManager.ConnectionManager.StopClientApprovalCoroutine();
275275

276-
networkManager.ConnectionManager.ConnectedClientIds.Clear();
277276
foreach (var clientId in ConnectedClientIds)
278277
{
279-
if (!networkManager.ConnectionManager.ConnectedClientIds.Contains(clientId))
278+
// DANGO-TODO: Revisit the entire connection sequence and determine why we would need to check both cases as we shouldn't have to =or= we could
279+
// try removing this after the Rust server connection sequence stuff is resolved. (Might be only needed if scene management is disabled)
280+
// If there is any disconnect between the connection sequence of Ids vs ConnectedClients, then add the client.
281+
if (!networkManager.ConnectionManager.ConnectedClientIds.Contains(clientId) || !networkManager.ConnectionManager.ConnectedClients.ContainsKey(clientId))
280282
{
281283
networkManager.ConnectionManager.AddClient(clientId);
282284
}

com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2595,30 +2595,26 @@ private void HandleSessionOwnerEvent(uint sceneEventId, ulong clientId)
25952595
case SceneEventType.SynchronizeComplete:
25962596
{
25972597
// At this point the client is considered fully "connected"
2598-
if ((NetworkManager.DistributedAuthorityMode && NetworkManager.LocalClient.IsSessionOwner) || !NetworkManager.DistributedAuthorityMode)
2598+
// Make sure we have a NetworkClient for this synchronized client
2599+
if (!NetworkManager.ConnectedClients.ContainsKey(clientId))
25992600
{
2600-
// Notify the local server that a client has finished synchronizing
2601-
OnSceneEvent?.Invoke(new SceneEvent()
2602-
{
2603-
SceneEventType = sceneEventData.SceneEventType,
2604-
SceneName = string.Empty,
2605-
ClientId = clientId
2606-
});
2607-
if (NetworkManager.ConnectedClients.ContainsKey(clientId))
2608-
{
2609-
NetworkManager.ConnectedClients[clientId].IsConnected = true;
2610-
}
2601+
NetworkManager.ConnectionManager.AddClient(clientId);
26112602
}
2612-
else
2603+
// Mark this client as being connected
2604+
NetworkManager.ConnectedClients[clientId].IsConnected = true;
2605+
2606+
// Notify the local server that a client has finished synchronizing
2607+
OnSceneEvent?.Invoke(new SceneEvent()
26132608
{
2614-
// Notify the local server that a client has finished synchronizing
2615-
OnSceneEvent?.Invoke(new SceneEvent()
2616-
{
2617-
SceneEventType = sceneEventData.SceneEventType,
2618-
SceneName = string.Empty,
2619-
ClientId = clientId
2620-
});
2609+
SceneEventType = sceneEventData.SceneEventType,
2610+
SceneName = string.Empty,
2611+
ClientId = clientId
2612+
});
26212613

2614+
// For non-authority clients in a distributed authority session, we show hidden objects,
2615+
// we distribute NetworkObjects, and then we end the scene event.
2616+
if (NetworkManager.DistributedAuthorityMode && !NetworkManager.LocalClient.IsSessionOwner)
2617+
{
26222618
// Show any NetworkObjects that are:
26232619
// - Hidden from the session owner
26242620
// - Owned by this client

com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -831,7 +831,7 @@ internal NetworkObject GetNetworkObjectToSpawn(uint globalObjectIdHash, ulong ow
831831
{
832832
if (NetworkLog.CurrentLogLevel <= LogLevel.Error)
833833
{
834-
NetworkLog.LogError($"Failed to create object locally. [{nameof(globalObjectIdHash)}={globalObjectIdHash}]. {nameof(NetworkPrefab)} could not be found. Is the prefab registered with {nameof(NetworkManager)}?");
834+
NetworkLog.LogError($"Failed to create object locally. [{nameof(globalObjectIdHash)}={globalObjectIdHash}]. {nameof(NetworkPrefab)} could not be found. Is the prefab registered with {NetworkManager.name}?");
835835
}
836836
}
837837
else

0 commit comments

Comments
 (0)