diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 1bffdbce19..18ea7b717e 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -18,6 +18,8 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed +- Fixed issue when using a distributed authority network topology and many clients attempt to connect simultaneously the session owner could max-out the maximum in-flight reliable messages allowed, start dropping packets, and some of the connecting clients would fail to fully synchronize. (#3350) +- Fixed issue when using a distributed authority network topology and scene management was disabled clients would not be able to spawn any new network prefab instances until synchronization was complete. (#3350) - Fixed issue where the `MaximumInterpolationTime` could not be modified from within the inspector view or runtime. (#3337) - Fixed `ChangeOwnership` changing ownership to clients that are not observers. This also happened with automated object distribution. (#3323) - Fixed issue where `AnticipatedNetworkVariable` previous value returned by `AnticipatedNetworkVariable.OnAuthoritativeValueChanged` is updated correctly on the non-authoritative side. (#3306) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ClientConnectedMessage.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ClientConnectedMessage.cs index 0234bc8b67..2d01389cd0 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ClientConnectedMessage.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ClientConnectedMessage.cs @@ -73,6 +73,8 @@ public void Handle(ref NetworkContext context) /// DANGO-TODO: Determine if this needs to be removed once the service handles object distribution networkManager.RedistributeToClients = true; networkManager.ClientsToRedistribute.Add(ClientId); + + // TODO: We need a client synchronized message or something like that here } } } diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs index 4f9e0f5453..cb1de9356f 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs @@ -270,6 +270,13 @@ public void Handle(ref NetworkContext context) // Only if scene management is disabled do we handle NetworkObject synchronization at this point if (!networkManager.NetworkConfig.EnableSceneManagement) { + /// Mark the client being connected before running through the spawning synchronization so we + /// can assure that if a user attempts to spawn something when an already spawned NetworkObject + /// is spawned (during the initial synchronization just below) it will not error out complaining + /// about the player not being connected. + /// The check for this is done within + networkManager.IsConnectedClient = true; + // DANGO-TODO: This is a temporary fix for no DA CMB scene event handling. // We will either use this same concept or provide some way for the CMB state plugin to handle it. if (networkManager.DistributedAuthorityMode && networkManager.LocalClient.IsSessionOwner) @@ -292,9 +299,6 @@ public void Handle(ref NetworkContext context) NetworkObject.AddSceneObject(sceneObject, m_ReceivedSceneObjectData, networkManager); } - // Mark the client being connected - networkManager.IsConnectedClient = true; - if (networkManager.AutoSpawnPlayerPrefabClientSide) { networkManager.ConnectionManager.CreateAndSpawnPlayer(OwnerClientId); @@ -315,14 +319,14 @@ public void Handle(ref NetworkContext context) if (networkManager.DistributedAuthorityMode && networkManager.CMBServiceConnection && networkManager.LocalClient.IsSessionOwner && networkManager.NetworkConfig.EnableSceneManagement) { // Mark the client being connected - networkManager.IsConnectedClient = true; + networkManager.IsConnectedClient = networkManager.ConnectionManager.LocalClient.IsApproved; networkManager.SceneManager.IsRestoringSession = GetIsSessionRestor(); if (!networkManager.SceneManager.IsRestoringSession) { // Synchronize the service with the initial session owner's loaded scenes and spawned objects - networkManager.SceneManager.SynchronizeNetworkObjects(NetworkManager.ServerClientId); + networkManager.SceneManager.SynchronizeNetworkObjects(NetworkManager.ServerClientId, true); // Spawn any in-scene placed NetworkObjects networkManager.SpawnManager.ServerSpawnSceneObjectsOnStartSweep(); @@ -334,9 +338,9 @@ public void Handle(ref NetworkContext context) } // Synchronize the service with the initial session owner's loaded scenes and spawned objects - networkManager.SceneManager.SynchronizeNetworkObjects(NetworkManager.ServerClientId); + networkManager.SceneManager.SynchronizeNetworkObjects(NetworkManager.ServerClientId, true); - // With scene management enabled and since the session owner doesn't send a Synchronize scene event synchronize itself, + // With scene management enabled and since the session owner doesn't send a scene event synchronize to itself, // we need to notify the session owner that everything should be synchronized/spawned at this time. networkManager.SpawnManager.NotifyNetworkObjectsSynchronized(); diff --git a/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs b/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs index 1d87687e6d..331d5ff112 100644 --- a/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs @@ -1986,6 +1986,14 @@ private void OnClientLoadedScene(uint sceneEventId, Scene scene) /// internal Func ExcludeSceneFromSychronization; + /// + /// This is used for distributed authority sessions only and assures that + /// when many clients attempt to connect at the same time they will be + /// handled sequentially so as to not saturate the session owner's maximum + /// reliable messages. + /// + internal List ClientConnectionQueue = new List(); + /// /// Server Side: /// This is used for players that have just had their connection approved and will assure they are synchronized @@ -1994,8 +2002,30 @@ private void OnClientLoadedScene(uint sceneEventId, Scene scene) /// synchronized. /// /// newly joined client identifier - internal void SynchronizeNetworkObjects(ulong clientId) + /// true only when invoked on a newly connected and approved client. + internal void SynchronizeNetworkObjects(ulong clientId, bool synchronizingService = false) { + // If we are connected to a live service hosted session and we are not doing the initial synchronization for the service... + if (NetworkManager.CMBServiceConnection && !synchronizingService) + { + // then as long as this is a newly connecting client add it to the connecting client queue. + // Otherwise, if this is not a newly connecting client (i.e. it is already in the queue), then go ahead and synchronize + // that client. + if (!ClientConnectionQueue.Contains(clientId)) + { + ClientConnectionQueue.Add(clientId); + // If we are already synchronizing one or more clients, exit early. This client will be synchronized later. + if (ClientConnectionQueue.Count > 1) + { + if (NetworkManager.LogLevel <= LogLevel.Developer) + { + Debug.Log($"Deferring Client-{clientId} synchrnization."); + } + return; + } + } + } + // Update the clients NetworkManager.SpawnManager.UpdateObservedNetworkObjects(clientId); @@ -2623,6 +2653,44 @@ private void HandleSessionOwnerEvent(uint sceneEventId, ulong clientId) // DANGO-EXP TODO: Remove this once service distributes objects NetworkManager.SpawnManager.DistributeNetworkObjects(clientId); EndSceneEvent(sceneEventId); + + // Exit early if not a distributed authority session or this is a DAHost + // (DAHost has a unique connection per client, so no need to queue synchronization) + if (!NetworkManager.DistributedAuthorityMode || NetworkManager.DAHost) + { + return; + } + + // Otherwise, this is a session owner that could have pending clients to synchronize + if (NetworkManager.DistributedAuthorityMode && NetworkManager.CMBServiceConnection) + { + // Remove the client that just synchronized + ClientConnectionQueue.Remove(clientId); + + // If we have pending clients to synchronize, then make sure they are still connected + while (ClientConnectionQueue.Count > 0) + { + // If the next client is no longer connected then remove it from the list + if (!NetworkManager.ConnectedClientsIds.Contains(ClientConnectionQueue[0])) + { + ClientConnectionQueue.RemoveAt(0); + } + else + { + break; + } + } + + // If we still have any pending clients waiting, then synchronize the next one + if (ClientConnectionQueue.Count > 0) + { + if (NetworkManager.LogLevel <= LogLevel.Developer) + { + Debug.Log($"Synchronizing Client-{ClientConnectionQueue[0]}..."); + } + SynchronizeNetworkObjects(ClientConnectionQueue[0]); + } + } break; } default: diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index 7b390547a4..0c8b8d74bd 100644 --- a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs @@ -1815,7 +1815,7 @@ internal void Shutdown() /// /// the table to populate /// the total number of the specific object type to distribute - internal void GetObjectDistribution(ref Dictionary>> objectByTypeAndOwner, ref Dictionary objectTypeCount) + internal void GetObjectDistribution(ulong clientId, ref Dictionary>> objectByTypeAndOwner, ref Dictionary objectTypeCount) { // DANGO-TODO-MVP: Remove this once the service handles object distribution var onlyIncludeOwnedObjects = NetworkManager.CMBServiceConnection; @@ -1844,6 +1844,11 @@ internal void GetObjectDistribution(ref Dictionary(); // Get all spawned objects by type and then by client owner that are spawned and can be distributed - GetObjectDistribution(ref distributedNetworkObjects, ref objectTypeCount); + GetObjectDistribution(clientId, ref distributedNetworkObjects, ref objectTypeCount); var clientCount = NetworkManager.ConnectedClientsIds.Count; @@ -1951,7 +1956,6 @@ internal void DistributeNetworkObjects(ulong clientId) var maxDistributeCount = Mathf.Max(ownerList.Value.Count - objPerClient, 1); var distributed = 0; - // For now when we have more players then distributed NetworkObjects that // a specific client owns, just assign half of the NetworkObjects to the new client var offsetCount = Mathf.Max((int)Math.Round((float)(ownerList.Value.Count / objPerClient)), 1); @@ -1964,11 +1968,6 @@ internal void DistributeNetworkObjects(ulong clientId) { if ((i % offsetCount) == 0) { - while (!ownerList.Value[i].Observers.Contains(clientId)) - { - i++; - } - var children = ownerList.Value[i].GetComponentsInChildren(); // Since the ownerList.Value[i] has to be distributable, then transfer all child NetworkObjects // with the same owner clientId and are marked as distributable also to the same client to keep @@ -2011,7 +2010,7 @@ internal void DistributeNetworkObjects(ulong clientId) var builder = new StringBuilder(); distributedNetworkObjects.Clear(); objectTypeCount.Clear(); - GetObjectDistribution(ref distributedNetworkObjects, ref objectTypeCount); + GetObjectDistribution(clientId, ref distributedNetworkObjects, ref objectTypeCount); builder.AppendLine($"Client Relative Distributed Object Count: (distribution follows)"); // Cycle through each prefab type foreach (var objectTypeEntry in distributedNetworkObjects) @@ -2026,7 +2025,6 @@ internal void DistributeNetworkObjects(ulong clientId) } Debug.Log(builder.ToString()); } - } internal struct DeferredDespawnObject diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/SpawnDuringSynchronizationTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/SpawnDuringSynchronizationTests.cs new file mode 100644 index 0000000000..6c91112259 --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/SpawnDuringSynchronizationTests.cs @@ -0,0 +1,147 @@ +using System.Collections; +using System.Collections.Generic; +using System.Text; +using NUnit.Framework; +using Unity.Netcode.TestHelpers.Runtime; +using UnityEngine; +using UnityEngine.TestTools; + +namespace Unity.Netcode.RuntimeTests +{ + [TestFixture(NetworkBehaviourSpawnTimes.OnNetworkSpawn, SceneManagement.Enabled)] + [TestFixture(NetworkBehaviourSpawnTimes.OnNetworkPostSpawn, SceneManagement.Enabled)] + [TestFixture(NetworkBehaviourSpawnTimes.OnSynchronized, SceneManagement.Enabled)] + [TestFixture(NetworkBehaviourSpawnTimes.OnNetworkSpawn, SceneManagement.Disabled)] + [TestFixture(NetworkBehaviourSpawnTimes.OnNetworkPostSpawn, SceneManagement.Disabled)] + [TestFixture(NetworkBehaviourSpawnTimes.OnSynchronized, SceneManagement.Disabled)] + internal class SpawnDuringSynchronizationTests : NetcodeIntegrationTest + { + protected override int NumberOfClients => 2; + + private List m_AllNetworkManagers = new List(); + private StringBuilder m_ErrorLog = new StringBuilder(); + + private NetworkBehaviourSpawnTimes m_SpawnTime; + private bool m_EnableSceneManagement; + internal enum NetworkBehaviourSpawnTimes + { + OnNetworkSpawn, + OnNetworkPostSpawn, + OnSynchronized + } + + internal enum SceneManagement + { + Enabled, + Disabled, + } + + internal class SpawnTestComponent : NetworkBehaviour + { + public GameObject PrefabToSpawn; + public NetworkObject SpawnedObject { get; private set; } + public NetworkBehaviourSpawnTimes SpawnTime; + + private void SpawnObject(NetworkBehaviourSpawnTimes spawnTime) + { + if (IsOwner && SpawnTime == spawnTime) + { + SpawnedObject = NetworkObject.InstantiateAndSpawn(PrefabToSpawn, NetworkManager, OwnerClientId); + } + } + + public override void OnNetworkSpawn() + { + SpawnObject(NetworkBehaviourSpawnTimes.OnNetworkSpawn); + base.OnNetworkSpawn(); + } + + protected override void OnNetworkPostSpawn() + { + SpawnObject(NetworkBehaviourSpawnTimes.OnNetworkPostSpawn); + base.OnNetworkPostSpawn(); + } + + protected override void OnNetworkSessionSynchronized() + { + SpawnObject(NetworkBehaviourSpawnTimes.OnSynchronized); + base.OnNetworkSessionSynchronized(); + } + } + + + public SpawnDuringSynchronizationTests(NetworkBehaviourSpawnTimes spawnTime, SceneManagement sceneManagement) : base(NetworkTopologyTypes.DistributedAuthority, HostOrServer.DAHost) + { + m_SpawnTime = spawnTime; + m_EnableSceneManagement = sceneManagement == SceneManagement.Enabled; + } + + protected override void OnCreatePlayerPrefab() + { + var spawnTestComponent = m_PlayerPrefab.AddComponent(); + spawnTestComponent.SpawnTime = m_SpawnTime; + spawnTestComponent.NetworkObject.SetOwnershipStatus(NetworkObject.OwnershipStatus.None); + base.OnCreatePlayerPrefab(); + } + + protected override void OnServerAndClientsCreated() + { + var spawnTestComponent = m_PlayerPrefab.GetComponent(); + spawnTestComponent.PrefabToSpawn = CreateNetworkObjectPrefab("ObjToSpawn"); + spawnTestComponent.PrefabToSpawn.GetComponent().SetOwnershipStatus(NetworkObject.OwnershipStatus.Transferable); + if (!UseCMBService()) + { + m_ServerNetworkManager.NetworkConfig.EnableSceneManagement = m_EnableSceneManagement; + } + foreach (var networkManager in m_ClientNetworkManagers) + { + networkManager.NetworkConfig.EnableSceneManagement = m_EnableSceneManagement; + } + base.OnServerAndClientsCreated(); + } + + private bool AllClientsSpawnedObject() + { + m_ErrorLog.Clear(); + var spawnTestComponent = (SpawnTestComponent)null; + foreach (var networkManager in m_AllNetworkManagers) + { + spawnTestComponent = networkManager.LocalClient.PlayerObject.GetComponent(); + if (spawnTestComponent.SpawnedObject == null || !spawnTestComponent.SpawnedObject.IsSpawned) + { + m_ErrorLog.AppendLine($"{networkManager.name}'s player failed to spawn the network prefab!"); + break; + } + foreach (var networkManagerToCheck in m_AllNetworkManagers) + { + if (networkManagerToCheck == networkManager) + { + continue; + } + if (!networkManagerToCheck.SpawnManager.SpawnedObjects.ContainsKey(spawnTestComponent.NetworkObjectId)) + { + m_ErrorLog.AppendLine($"{networkManager.name}'s player failed to spawn the network prefab!"); + } + } + } + return m_ErrorLog.Length == 0; + } + + /// + /// Validates that a client can spawn network prefabs during OnNetworkSpawn, OnNetworkPostSpawn, and OnNetworkSessionSynchronized. + /// + [UnityTest] + public IEnumerator SpawnDuringSynchronization() + { + m_AllNetworkManagers.Clear(); + m_AllNetworkManagers.AddRange(m_ClientNetworkManagers); + if (!UseCMBService()) + { + m_AllNetworkManagers.Add(m_ServerNetworkManager); + } + + yield return WaitForConditionOrTimeOut(AllClientsSpawnedObject); + AssertOnTimeout(m_ErrorLog.ToString()); + } + } +} diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/SpawnDuringSynchronizationTests.cs.meta b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/SpawnDuringSynchronizationTests.cs.meta new file mode 100644 index 0000000000..415940bde8 --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/SpawnDuringSynchronizationTests.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 3535334da619ee944a73ca56e2e76d22 \ No newline at end of file