diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 88c351890a..708faa0bbd 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -49,6 +49,8 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed +- Ensure `NetworkBehaviour.IsSessionOwner` is correctly set when a new session owner is promoted. (#3817) +- Reset extended ownership flags on `NetworkObject` despawn. (#3817) - Fixed issues with the "Client-server quickstart for Netcode for GameObjects" script having static methods and properties. (#3787) - Fixed issue where a warning message was being logged upon a client disconnecting from a server when the log level is set to developer. (#3786) - Fixed issue where the server or host would no longer have access to the transport id to client id table when processing a transport level client disconnect event. (#3786) diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs index af911b4385..9910668f59 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs @@ -515,7 +515,7 @@ public NetworkManager NetworkManager /// /// Gets whether the client is the distributed authority mode session owner. /// - public bool IsSessionOwner { get; private set; } + public bool IsSessionOwner { get; internal set; } /// /// Gets whether the server (local or remote) is a host. diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs index dee822ec4f..faa97b87ff 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs @@ -216,19 +216,21 @@ internal void HandleRedistributionToClients() internal void SetSessionOwner(ulong sessionOwner) { - var previousSessionOwner = CurrentSessionOwner; CurrentSessionOwner = sessionOwner; - LocalClient.IsSessionOwner = LocalClientId == sessionOwner; - if (LocalClient.IsSessionOwner) + var isSessionOwner = LocalClientId == sessionOwner; + LocalClient.IsSessionOwner = isSessionOwner; + + foreach (var networkObject in SpawnManager.SpawnedObjects.Values) { - foreach (var networkObjectEntry in SpawnManager.SpawnedObjects) + if (isSessionOwner) { - var networkObject = networkObjectEntry.Value; if (networkObject.IsOwnershipSessionOwner && networkObject.OwnerClientId != LocalClientId) { SpawnManager.ChangeOwnership(networkObject, LocalClientId, true); } } + + networkObject.InvokeSessionOwnerPromoted(isSessionOwner); } OnSessionOwnerPromoted?.Invoke(sessionOwner); diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs index 953781d008..311a3df650 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs @@ -2002,6 +2002,7 @@ internal void ResetOnDespawn() IsSpawned = false; DeferredDespawnTick = 0; m_LatestParent = null; + RemoveOwnershipExtended(OwnershipStatusExtended.Locked | OwnershipStatusExtended.Requested); } /// @@ -2078,6 +2079,19 @@ internal void InvokeOwnershipChanged(ulong previous, ulong next) } } + internal void InvokeSessionOwnerPromoted(bool isSessionOwner) + { + if (!IsSpawned) + { + return; + } + + foreach (var childBehaviour in ChildNetworkBehaviours) + { + childBehaviour.IsSessionOwner = isSessionOwner; + } + } + internal void InvokeBehaviourOnNetworkObjectParentChanged(NetworkObject parentNetworkObject) { for (int i = 0; i < ChildNetworkBehaviours.Count; i++) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/ExtendedNetworkShowAndHideTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/ExtendedNetworkShowAndHideTests.cs index 321e8703f1..ebe3b54e47 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/ExtendedNetworkShowAndHideTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/ExtendedNetworkShowAndHideTests.cs @@ -1,4 +1,7 @@ using System.Collections; +using System.Collections.Generic; +using System.Linq; +using System.Text; using NUnit.Framework; using Unity.Netcode.TestHelpers.Runtime; using UnityEngine; @@ -6,28 +9,25 @@ namespace Unity.Netcode.RuntimeTests { - [TestFixture(HostOrServer.DAHost, true)] - [TestFixture(HostOrServer.DAHost, false)] + internal enum SceneManagementSetting + { + EnableSceneManagement, + DisableSceneManagement + } + + [TestFixture(SceneManagementSetting.EnableSceneManagement)] + [TestFixture(SceneManagementSetting.DisableSceneManagement)] internal class ExtendedNetworkShowAndHideTests : NetcodeIntegrationTest { protected override int NumberOfClients => 3; - private bool m_EnableSceneManagement; + private readonly bool m_EnableSceneManagement; private GameObject m_ObjectToSpawn; - private NetworkObject m_SpawnedObject; - private NetworkManager m_ClientToHideFrom; - private NetworkManager m_LateJoinClient; - private NetworkManager m_SpawnOwner; - - // TODO: [CmbServiceTests] Adapt to run with the service (can't control who becomes the session owner, needs a logic rework) - /// - protected override bool UseCMBService() - { - return false; - } + private List m_SpawnedObjects = new(); + private Dictionary m_ObjectHiddenFromClient = new(); - public ExtendedNetworkShowAndHideTests(HostOrServer hostOrServer, bool enableSceneManagement) : base(hostOrServer) + public ExtendedNetworkShowAndHideTests(SceneManagementSetting sceneManagement) : base(HostOrServer.DAHost) { - m_EnableSceneManagement = enableSceneManagement; + m_EnableSceneManagement = sceneManagement == SceneManagementSetting.EnableSceneManagement; } protected override void OnServerAndClientsCreated() @@ -38,48 +38,93 @@ protected override void OnServerAndClientsCreated() } m_ObjectToSpawn = CreateNetworkObjectPrefab("TestObject"); + m_ObjectToSpawn.AddComponent(); m_ObjectToSpawn.SetActive(false); base.OnServerAndClientsCreated(); } - private bool AllClientsSpawnedObject() + private bool AllObjectsHiddenFromTheirClient(StringBuilder errorLog) { - foreach (var client in m_NetworkManagers) + var allHidden = true; + foreach (var (clientToHideFrom, objectToHide) in m_ObjectHiddenFromClient) { - if (!s_GlobalNetworkObjects.ContainsKey(client.LocalClientId)) + if (clientToHideFrom.SpawnManager.SpawnedObjects.ContainsKey(objectToHide.NetworkObjectId)) { - return false; - } - if (!s_GlobalNetworkObjects[client.LocalClientId].ContainsKey(m_SpawnedObject.NetworkObjectId)) - { - return false; + errorLog.AppendLine($"Object-{objectToHide.NetworkObjectId} is still visible to Client-{clientToHideFrom.LocalClientId}"); + allHidden = false; } } - return true; + + return allHidden; } - private bool IsClientPromotedToSessionOwner() + private bool IsClientPromotedToSessionOwner(StringBuilder errorLog) { + var valid = true; + var currentSessionOwner = GetAuthorityNetworkManager(); foreach (var client in m_NetworkManagers) { - if (!client.IsConnectedClient) + if (currentSessionOwner == client) { - continue; + if (!client.LocalClient.IsSessionOwner) + { + errorLog.AppendLine($"Session owner is Client-{client.LocalClientId} but they are not marked as session owner."); + valid = false; + } } - if (client.CurrentSessionOwner != m_ClientToHideFrom.LocalClientId) + else if (client.LocalClient.IsSessionOwner) { - return false; + errorLog.AppendLine($"Client-{client.LocalClientId} is incorrectly marked as Session Owner."); + valid = false; + } + + if (client.CurrentSessionOwner != currentSessionOwner.LocalClientId) + { + errorLog.AppendLine($"Client-{client.LocalClientId} has the incorrect session owner id (Has: {client.CurrentSessionOwner}, expected: {currentSessionOwner.LocalClientId})."); + valid = false; } } - return true; + return valid; } - protected override void OnNewClientCreated(NetworkManager networkManager) + private bool AllObjectsSpawnedExceptForHidden(StringBuilder errorLog) { - m_LateJoinClient = networkManager; - networkManager.NetworkConfig.EnableSceneManagement = m_EnableSceneManagement; - base.OnNewClientCreated(networkManager); + var valid = true; + foreach (var client in m_NetworkManagers) + { + foreach (var spawnedObject in m_SpawnedObjects) + { + var isVisible = client.SpawnManager.SpawnedObjects.ContainsKey(spawnedObject.NetworkObjectId); + + if (m_ObjectHiddenFromClient.TryGetValue(client, out var hiddenObject) && hiddenObject == spawnedObject) + { + if (isVisible) + { + errorLog.AppendLine($"Client-{client.LocalClientId} can see object-{spawnedObject.NetworkObjectId} that should be hidden."); + valid = false; + continue; + } + } + else if (!isVisible) + { + errorLog.AppendLine($"Client-{client.LocalClientId} hasn't spawned object-{spawnedObject.NetworkObjectId}"); + valid = false; + } + + if (isVisible) + { + var clientObject = client.SpawnManager.SpawnedObjects[spawnedObject.NetworkObjectId]; + var behaviour = clientObject.GetComponent(); + if (behaviour.IsSessionOwner != client.LocalClient.IsSessionOwner) + { + errorLog.AppendLine($"Client-{client.LocalClientId} network behaviour has incorrect session owner value. Should be {client.LocalClient.IsSessionOwner}, is {behaviour.IsSessionOwner}"); + valid = false; + } + } + } + } + return valid; } /// @@ -95,41 +140,89 @@ public IEnumerator HiddenObjectPromotedSessionOwnerNewClientSynchronizes() { // Get the test relative session owner var sessionOwner = GetAuthorityNetworkManager(); - m_SpawnOwner = m_UseCmbService ? m_ClientNetworkManagers[1] : m_ClientNetworkManagers[0]; - m_ClientToHideFrom = m_UseCmbService ? m_ClientNetworkManagers[NumberOfClients - 1] : m_ClientNetworkManagers[1]; + + // Spawn objects for the non-session owner clients m_ObjectToSpawn.SetActive(true); + foreach (var client in m_NetworkManagers) + { + if (client == sessionOwner) + { + Assert.IsTrue(client.LocalClient.IsSessionOwner); + continue; + } + + var instance = SpawnObject(m_ObjectToSpawn, client).GetComponent(); + m_SpawnedObjects.Add(instance); + } + + yield return WaitForSpawnedOnAllOrTimeOut(m_SpawnedObjects); + AssertOnTimeout("[InitialSpawn] Not all clients spawned all objects"); + + // Hide one spawned object from each client + var setOfInstances = m_SpawnedObjects.ToHashSet(); + foreach (var clientToHideFrom in m_NetworkManagers) + { + // Session owner doesn't need to have an object hidden from them + if (clientToHideFrom == sessionOwner) + { + continue; + } + + // Find an object that this client doesn't own that isn't already hidden from another client + var toHide = setOfInstances.Last(obj => obj.OwnerClientId != clientToHideFrom.LocalClientId); + toHide.NetworkHide(clientToHideFrom.LocalClientId); + setOfInstances.Remove(toHide); + m_ObjectHiddenFromClient.Add(clientToHideFrom, toHide); - // Spawn the object with a non-session owner client - m_SpawnedObject = SpawnObject(m_ObjectToSpawn, m_SpawnOwner).GetComponent(); - yield return WaitForConditionOrTimeOut(AllClientsSpawnedObject); - AssertOnTimeout($"Not all clients spawned and instance of {m_SpawnedObject.name}"); + Assert.IsFalse(toHide.GetComponent().IsSessionOwner, "No object should have been spawned owned by the session owner"); + } - // Hide the spawned object from the to be promoted session owner - m_SpawnedObject.NetworkHide(m_ClientToHideFrom.LocalClientId); + Assert.That(m_ObjectHiddenFromClient.Count, Is.EqualTo(NumberOfClients), "Test should hide one object per non-authority client"); + Assert.That(setOfInstances, Is.Empty, "Not all objects have been hidden frm someone."); - yield return WaitForConditionOrTimeOut(() => !m_ClientToHideFrom.SpawnManager.SpawnedObjects.ContainsKey(m_SpawnedObject.NetworkObjectId)); - AssertOnTimeout($"{m_SpawnedObject.name} was not hidden from Client-{m_ClientToHideFrom.LocalClientId}!"); + yield return WaitForConditionOrTimeOut(AllObjectsHiddenFromTheirClient); + AssertOnTimeout("Not all objects have been hidden from someone!"); // Promoted a new session owner (DAHost promotes while CMB Session we disconnect the current session owner) if (!m_UseCmbService) { - m_ServerNetworkManager.PromoteSessionOwner(m_ClientToHideFrom.LocalClientId); + var nonAuthority = GetNonAuthorityNetworkManager(); + m_ServerNetworkManager.PromoteSessionOwner(nonAuthority.LocalClientId); + yield return s_DefaultWaitForTick; } else { - sessionOwner.Shutdown(); + yield return StopOneClient(sessionOwner); } // Wait for the new session owner to be promoted and for all clients to acknowledge the promotion yield return WaitForConditionOrTimeOut(IsClientPromotedToSessionOwner); - AssertOnTimeout($"Client-{m_ClientToHideFrom.LocalClientId} was not promoted as session owner on all client instances!"); + AssertOnTimeout($"No client was promoted as session owner on all client instances!"); + + var newSessionOwner = GetAuthorityNetworkManager(); + VerboseDebug($"Client-{newSessionOwner.LocalClientId} was promoted as session owner on all client instances!"); + Assert.That(newSessionOwner, Is.Not.EqualTo(sessionOwner), "The current session owner be different from the original session owner."); + Assert.That(m_ObjectHiddenFromClient, Does.ContainKey(newSessionOwner), "An object should be hidden from the newly promoted session owner"); // Connect a new client instance - yield return CreateAndStartNewClient(); + var newClient = CreateNewClient(); + newClient.NetworkConfig.EnableSceneManagement = m_EnableSceneManagement; + yield return StartClient(newClient); // Assure the newly connected client is synchronized with the NetworkObject hidden from the newly promoted session owner - yield return WaitForConditionOrTimeOut(() => m_LateJoinClient.SpawnManager.SpawnedObjects.ContainsKey(m_SpawnedObject.NetworkObjectId)); - AssertOnTimeout($"Client-{m_LateJoinClient.LocalClientId} never spawned {nameof(NetworkObject)} {m_SpawnedObject.name}!"); + yield return WaitForConditionOrTimeOut(AllObjectsSpawnedExceptForHidden); + AssertOnTimeout("[LateJoinClient] Not all objects spawned correctly"); + } + + protected override IEnumerator OnTearDown() + { + m_SpawnedObjects.Clear(); + m_ObjectHiddenFromClient.Clear(); + return base.OnTearDown(); + } + + private class EmptyBehaviour : NetworkBehaviour + { } } } diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs index 9da821c930..cc090c22e0 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs @@ -499,6 +499,108 @@ private IEnumerator ValidateRequestAndPermissionsChangeRaceCondition(NetworkObje AssertOnTimeout($"[{newStatus}][Set RequestRequired][Permissions mismatch] {firstClient.name}"); } + [UnityTest] + public IEnumerator ResetExtendedOwnershipFlagsOnRespawn() + { + var extendedFlags = (NetworkObject.OwnershipStatusExtended[])Enum.GetValues(typeof(NetworkObject.OwnershipStatusExtended)); + Assert.That(extendedFlags.Length, Is.LessThan(NumberOfClients), "This test should have at least one more non-authority client than extended flags!"); + + var spawnedObjects = new List(); + while (spawnedObjects.Count <= extendedFlags.Length) + { + var ownerClient = GetNonAuthorityNetworkManager(spawnedObjects.Count); + var spawnedObject = SpawnObject(m_PermissionsObject, ownerClient).GetComponent(); + spawnedObject.SetOwnershipStatus(NetworkObject.OwnershipStatus.RequestRequired, true); + spawnedObjects.Add(spawnedObject); + } + + yield return WaitForSpawnedOnAllOrTimeOut(spawnedObjects); + AssertOnTimeout("[InitialSpawn] Not all objects are spawned on all clients!"); + + for (int i = 0; i < extendedFlags.Length; i++) + { + var flag = extendedFlags[i]; + var obj = spawnedObjects[i]; + obj.AddOwnershipExtended(flag); + obj.SendOwnershipStatusUpdate(); + + m_ObjectToValidate = obj; + yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); + AssertOnTimeout($"[{flag}] Not all clients have set the permission on the object!"); + } + + // Despawn the objects + foreach (var networkObject in spawnedObjects) + { + networkObject.Despawn(false); + } + + yield return WaitForDespawnedOnAllOrTimeOut(spawnedObjects); + AssertOnTimeout("Not all clients have despawned all objects!"); + + // Respawn the objects + foreach (var networkObject in spawnedObjects) + { + networkObject.Spawn(); + } + + yield return WaitForSpawnedOnAllOrTimeOut(spawnedObjects); + AssertOnTimeout("[ReSpawn] Not all objects are spawned on all clients!"); + + // Requesting ownership should work as expected + var nonAuthorityIndex = 1; // Start the index at one more than the initial ownership + var objectToOwner = new Dictionary(); + foreach (var networkObject in spawnedObjects) + { + var nextOwner = GetNonAuthorityNetworkManager(nonAuthorityIndex++); + Assert.That(nextOwner.LocalClientId, Is.Not.EqualTo(networkObject.OwnerClientId)); + Assert.IsTrue(nextOwner.SpawnManager.SpawnedObjects.TryGetValue(networkObject.NetworkObjectId, out var nextOwnerInstance)); + Assert.That(nextOwnerInstance, Is.Not.Null); + + var requestStatus = nextOwnerInstance.RequestOwnership(); + Assert.That(requestStatus, Is.EqualTo(NetworkObject.OwnershipRequestStatus.RequestSent)); + + objectToOwner.Add(networkObject, nextOwner); + } + + yield return WaitForConditionOrTimeOut(errorLog => + { + foreach (var client in m_NetworkManagers) + { + foreach (var (networkObject, expectedOwner) in objectToOwner) + { + if (!client.SpawnManager.SpawnedObjects.TryGetValue(networkObject.NetworkObjectId, out var clientInstance)) + { + errorLog.AppendLine($"Object-{networkObject.NetworkObjectId} is not spawned on client-{client.LocalClientId}"); + return false; + } + + if (clientInstance.OwnerClientId != expectedOwner.LocalClientId) + { + errorLog.AppendLine($"Client-{client.LocalClientId} has the incorrect owner for Object-{networkObject.NetworkObjectId}"); + return false; + } + + if (clientInstance.IsOwner != (client == expectedOwner)) + { + errorLog.AppendLine($"Client-{client.LocalClientId} has the incorrect IsOwner for Object-{networkObject.NetworkObjectId}"); + return false; + } + } + } + + return true; + }); + AssertOnTimeout("Some ownership requests failed!"); + + foreach (var networkObject in spawnedObjects) + { + m_ObjectToValidate = networkObject; + yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); + AssertOnTimeout($"[Object-{networkObject.NetworkObjectId}] Not all clients have set the permission on the object!"); + } + } + internal class OwnershipPermissionsTestHelper : NetworkBehaviour { public static NetworkObject CurrentOwnedInstance; diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs index 9f6ab62ddf..da86d8a527 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs @@ -255,13 +255,18 @@ public enum HostOrServer /// The instance that is the current authority protected NetworkManager GetAuthorityNetworkManager() { - if (m_UseCmbService) + if (m_DistributedAuthority) { // If we haven't even started any NetworkManager, then return the first instance // since it will be the session owner. if (!NetcodeIntegrationTestHelpers.IsStarted) { - return m_NetworkManagers[0]; + return m_UseCmbService ? m_NetworkManagers[0] : m_ServerNetworkManager; + } + + if (!m_UseCmbService && m_ServerNetworkManager.LocalClient.IsSessionOwner) + { + return m_ServerNetworkManager; } foreach (var client in m_NetworkManagers) @@ -1969,6 +1974,60 @@ protected IEnumerator WaitForSpawnedOnAllOrTimeOut(GameObject gameObject, Timeou yield return WaitForSpawnedOnAllOrTimeOut(networkObjectId, timeOutHelper); } + /// + /// Waits until all given NetworkObjects are spawned on all clients or a timeout occurs. + /// + /// The list of s to wait for. + /// An optional to control the timeout period. If null, the default timeout is used. + /// An for use in Unity coroutines. + protected IEnumerator WaitForSpawnedOnAllOrTimeOut(List networkObjects, TimeoutHelper timeOutHelper = null) + { + bool ValidateObjectsSpawnedOnAllClients(StringBuilder errorLog) + { + foreach (var client in m_NetworkManagers) + { + foreach (var networkObject in networkObjects) + { + if (!client.SpawnManager.SpawnedObjects.ContainsKey(networkObject.NetworkObjectId)) + { + errorLog.Append($"Client-{client.LocalClientId} has not spawned Object-{networkObject.NetworkObjectId}!"); + return false; + } + } + } + return true; + } + + yield return WaitForConditionOrTimeOut(ValidateObjectsSpawnedOnAllClients, timeOutHelper); + } + + /// + /// Waits until all given NetworkObjects are spawned on all clients or a timeout occurs. + /// + /// The list of s to wait for. + /// An optional to control the timeout period. If null, the default timeout is used. + /// An for use in Unity coroutines. + protected IEnumerator WaitForDespawnedOnAllOrTimeOut(List networkObjects, TimeoutHelper timeOutHelper = null) + { + bool ValidateObjectsDespawnedOnAllClients(StringBuilder errorLog) + { + foreach (var client in m_NetworkManagers) + { + foreach (var networkObject in networkObjects) + { + if (client.SpawnManager.SpawnedObjects.TryGetValue(networkObject.NetworkObjectId, out NetworkObject clientObj) && clientObj.IsSpawned) + { + errorLog.Append($"Object-{networkObject.NetworkObjectId} is still spawned on Client-{client.LocalClientId}!"); + return false; + } + } + } + return true; + } + + yield return WaitForConditionOrTimeOut(ValidateObjectsDespawnedOnAllClients, timeOutHelper); + } + /// /// Validates that all remote clients (i.e. non-server) detect they are connected /// to the server and that the server reflects the appropriate number of clients