diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index accae93b14..ff93d501ee 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -11,6 +11,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Added +- Added `NetworkObject.OwnershipStatus.SessionOwner` to allow Network Objects to be distributable and only owned by the Session Owner. This flag will override all other `OwnershipStatus` flags. (#3175) - Added `UnityTransport.GetEndpoint` method to provide a way to obtain `NetworkEndpoint` information of a connection via client identifier. (#3130) - Added `NetworkTransport.OnEarlyUpdate` and `NetworkTransport.OnPostLateUpdate` methods to provide more control over handling transport related events at the start and end of each frame. (#3113) @@ -31,6 +32,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Changed +- In-scene placed `NetworkObject`s have been made distributable when balancing object distribution after a connection event. (#3175) - Optimised `NetworkVariable` and `NetworkTransform` related packets when in Distributed Authority mode. - The Debug Simulator section of the Unity Transport component was removed. This section was not functional anymore and users are now recommended to use the more featureful [Network Simulator](https://docs-multiplayer.unity3d.com/tools/current/tools-network-simulator/) tool from the Multiplayer Tools package instead. (#3121) diff --git a/com.unity.netcode.gameobjects/Editor/NetworkObjectEditor.cs b/com.unity.netcode.gameobjects/Editor/NetworkObjectEditor.cs index 6adb7b8223..0860fd9c92 100644 --- a/com.unity.netcode.gameobjects/Editor/NetworkObjectEditor.cs +++ b/com.unity.netcode.gameobjects/Editor/NetworkObjectEditor.cs @@ -115,6 +115,10 @@ public override void OnInspectorGUI() EditorGUI.BeginChangeCheck(); serializedObject.UpdateIfRequiredOrScript(); DrawPropertiesExcluding(serializedObject, k_HiddenFields); + if (m_NetworkObject.IsOwnershipSessionOwner) + { + m_NetworkObject.Ownership = NetworkObject.OwnershipStatus.SessionOwner; + } serializedObject.ApplyModifiedProperties(); EditorGUI.EndChangeCheck(); diff --git a/com.unity.netcode.gameobjects/Runtime/Configuration/NetworkConfig.cs b/com.unity.netcode.gameobjects/Runtime/Configuration/NetworkConfig.cs index 79035a895f..a9668084bd 100644 --- a/com.unity.netcode.gameobjects/Runtime/Configuration/NetworkConfig.cs +++ b/com.unity.netcode.gameobjects/Runtime/Configuration/NetworkConfig.cs @@ -13,6 +13,14 @@ namespace Unity.Netcode [Serializable] public class NetworkConfig { + // Clamp spawn time outs to prevent dropping messages during scene events + // Note: The legacy versions of NGO defaulted to 1s which was too low. As + // well, the SpawnTimeOut is now being clamped to within this recommended + // range both via UI and when NetworkManager is validated. + internal const float MinSpawnTimeout = 10.0f; + // Clamp spawn time outs to no more than 1 hour (really that is a bit high) + internal const float MaxSpawnTimeout = 3600.0f; + /// /// The protocol version. Different versions doesn't talk to each other. /// @@ -132,6 +140,8 @@ public class NetworkConfig /// The amount of time a message will be held (deferred) if the destination NetworkObject needed to process the message doesn't exist yet. If the NetworkObject is not spawned within this time period, all deferred messages for that NetworkObject will be dropped. /// [Tooltip("The amount of time a message will be held (deferred) if the destination NetworkObject needed to process the message doesn't exist yet. If the NetworkObject is not spawned within this time period, all deferred messages for that NetworkObject will be dropped.")] + + [Range(MinSpawnTimeout, MaxSpawnTimeout)] public float SpawnTimeout = 10f; /// @@ -176,6 +186,21 @@ public class NetworkConfig [Tooltip("Enable (default) if you want to profile network messages with development builds and defaults to being disabled in release builds. When disabled, network messaging profiling will be disabled in development builds.")] public bool NetworkProfilingMetrics = true; + /// + /// Invoked by when it is validated. + /// + /// + /// Used to check for potential legacy values that have already been serialized and/or + /// runtime modifications to a property outside of the recommended range. + /// For each property checked below, provide a brief description of the reason. + /// + internal void OnValidate() + { + // Legacy NGO versions defaulted this value to 1 second that has since been determiend + // any range less than 10 seconds can lead to dropped messages during scene events. + SpawnTimeout = Mathf.Clamp(SpawnTimeout, MinSpawnTimeout, MaxSpawnTimeout); + } + /// /// Returns a base64 encoded version of the configuration /// diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index e297375ec1..fe6bc4816d 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -1212,7 +1212,7 @@ internal void OnClientDisconnectFromServer(ulong clientId) { // Only NetworkObjects that have the OwnershipStatus.Distributable flag set and no parent // (ownership is transferred to all children) will have their ownership redistributed. - if (ownedObject.IsOwnershipDistributable && ownedObject.GetCachedParent() == null) + if (ownedObject.IsOwnershipDistributable && ownedObject.GetCachedParent() == null && !ownedObject.IsOwnershipSessionOwner) { if (ownedObject.IsOwnershipLocked) { @@ -1249,6 +1249,11 @@ internal void OnClientDisconnectFromServer(ulong clientId) childObject.SetOwnershipLock(false); } + // Ignore session owner marked objects + if (childObject.IsOwnershipSessionOwner) + { + continue; + } NetworkManager.SpawnManager.ChangeOwnership(childObject, targetOwner, true); if (EnableDistributeLogging) { diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs index 75db75b9f2..9b3ef047a0 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs @@ -225,11 +225,7 @@ internal void SetSessionOwner(ulong sessionOwner) foreach (var networkObjectEntry in SpawnManager.SpawnedObjects) { var networkObject = networkObjectEntry.Value; - if (networkObject.IsSceneObject == null || !networkObject.IsSceneObject.Value) - { - continue; - } - if (networkObject.OwnerClientId != LocalClientId) + if (networkObject.IsOwnershipSessionOwner && LocalClient.IsSessionOwner) { SpawnManager.ChangeOwnership(networkObject, LocalClientId, true); } @@ -955,6 +951,9 @@ internal void OnValidate() return; // May occur when the component is added } + // Do a validation pass on NetworkConfig properties + NetworkConfig.OnValidate(); + if (GetComponentInChildren() != null) { if (NetworkLog.CurrentLogLevel <= LogLevel.Normal) diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs index 2ff4e27eb3..c449c9aadb 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs @@ -439,6 +439,13 @@ public void DeferDespawn(int tickOffset, bool destroy = true) /// public bool IsOwnershipDistributable => Ownership.HasFlag(OwnershipStatus.Distributable); + /// + /// When true, the can only be owned by the current Session Owner. + /// To set during runtime, use to ensure the session owner owns the object. + /// Once the session owner owns the object, then use . + /// + public bool IsOwnershipSessionOwner => Ownership.HasFlag(OwnershipStatus.SessionOwner); + /// /// Returns true if the is has ownership locked. /// When locked, the cannot be redistributed nor can it be transferred by another client. @@ -481,7 +488,8 @@ public void DeferDespawn(int tickOffset, bool destroy = true) /// : If nothing is set, then ownership is considered "static" and cannot be redistributed, requested, or transferred (i.e. a Player would have this). /// : When set, this instance will be automatically redistributed when a client joins (if not locked or no request is pending) or leaves. /// : When set, a non-owner can obtain ownership immediately (without requesting and as long as it is not locked). - /// : When set, When set, a non-owner must request ownership from the owner (will always get locked once ownership is transferred). + /// : When set, a non-owner must request ownership from the owner (will always get locked once ownership is transferred). + /// : When set, only the current session owner may have ownership over this object. /// // Ranges from 1 to 8 bits [Flags] @@ -491,6 +499,7 @@ public enum OwnershipStatus Distributable = 1 << 0, Transferable = 1 << 1, RequestRequired = 1 << 2, + SessionOwner = 1 << 3, } /// @@ -549,7 +558,7 @@ public bool SetOwnershipLock(bool lockOwnership = true) } // If we don't have the Transferable flag set and it is not a player object, then it is the same as having a static lock on ownership - if (!IsOwnershipTransferable && !IsPlayerObject) + if (!(IsOwnershipTransferable || IsPlayerObject) || IsOwnershipSessionOwner) { NetworkLog.LogWarning($"Trying to add or remove ownership lock on [{name}] which does not have the {nameof(OwnershipStatus.Transferable)} flag set!"); return false; @@ -582,13 +591,15 @@ public bool SetOwnershipLock(bool lockOwnership = true) /// : The requires an ownership request via . /// : The is already processing an ownership request and ownership cannot be acquired at this time. /// : The does not have the flag set and ownership cannot be acquired. + /// : The has the flag set and ownership cannot be acquired. /// public enum OwnershipPermissionsFailureStatus { Locked, RequestRequired, RequestInProgress, - NotTransferrable + NotTransferrable, + SessionOwnerOnly } /// @@ -610,6 +621,7 @@ public enum OwnershipPermissionsFailureStatus /// : The flag is not set on this /// : The current owner has locked ownership which means requests are not available at this time. /// : There is already a known request in progress. You can scan for ownership changes and try upon + /// : This object is marked as SessionOwnerOnly and therefore cannot be requested /// a change in ownership or just try again after a specific period of time or no longer attempt to request ownership. /// public enum OwnershipRequestStatus @@ -619,6 +631,7 @@ public enum OwnershipRequestStatus RequestRequiredNotSet, Locked, RequestInProgress, + SessionOwnerOnly, } /// @@ -631,6 +644,7 @@ public enum OwnershipRequestStatus /// : The flag is not set on this /// : The current owner has locked ownership which means requests are not available at this time. /// : There is already a known request in progress. You can scan for ownership changes and try upon + /// : This object can only belong the the session owner and so cannot be requested /// a change in ownership or just try again after a specific period of time or no longer attempt to request ownership. /// /// @@ -660,6 +674,12 @@ public OwnershipRequestStatus RequestOwnership() return OwnershipRequestStatus.RequestInProgress; } + // Exit early if it has the SessionOwner flag + if (IsOwnershipSessionOwner) + { + return OwnershipRequestStatus.SessionOwnerOnly; + } + // Otherwise, send the request ownership message var changeOwnership = new ChangeOwnershipMessage { @@ -716,7 +736,7 @@ internal void OwnershipRequest(ulong clientRequestingOwnership) { response = OwnershipRequestResponseStatus.RequestInProgress; } - else if (!IsOwnershipRequestRequired && !IsOwnershipTransferable) + else if (!(IsOwnershipRequestRequired || IsOwnershipTransferable) || IsOwnershipSessionOwner) { response = OwnershipRequestResponseStatus.CannotRequest; } @@ -836,6 +856,12 @@ public enum OwnershipLockActions /// public bool SetOwnershipStatus(OwnershipStatus status, bool clearAndSet = false, OwnershipLockActions lockAction = OwnershipLockActions.None) { + if (status.HasFlag(OwnershipStatus.SessionOwner) && !NetworkManager.LocalClient.IsSessionOwner) + { + NetworkLog.LogWarning("Only the session owner is allowed to set the ownership status to session owner only."); + return false; + } + // If it already has the flag do nothing if (!clearAndSet && Ownership.HasFlag(status)) { @@ -847,13 +873,25 @@ public bool SetOwnershipStatus(OwnershipStatus status, bool clearAndSet = false, Ownership = OwnershipStatus.None; } - // Faster to just OR a None status than to check - // if it is !None before "OR'ing". - Ownership |= status; - - if (lockAction != OwnershipLockActions.None) + if (status.HasFlag(OwnershipStatus.SessionOwner)) { - SetOwnershipLock(lockAction == OwnershipLockActions.SetAndLock); + Ownership = OwnershipStatus.SessionOwner; + } + else if (Ownership.HasFlag(OwnershipStatus.SessionOwner)) + { + NetworkLog.LogWarning("No other ownership statuses may be set while SessionOwner is set."); + return false; + } + else + { + // Faster to just OR a None status than to check + // if it is !None before "OR'ing". + Ownership |= status; + + if (lockAction != OwnershipLockActions.None) + { + SetOwnershipLock(lockAction == OwnershipLockActions.SetAndLock); + } } SendOwnershipStatusUpdate(); @@ -1629,7 +1667,7 @@ internal void SpawnInternal(bool destroyWithScene, ulong ownerClientId, bool pla // DANGO-TODO: Review over don't destroy with owner being set but DistributeOwnership not being set if (NetworkManager.LogLevel == LogLevel.Developer) { - NetworkLog.LogWarning("DANGO-TODO: Review over don't destroy with owner being set but DistributeOwnership not being set. For now, if the NetworkObject does not destroy with the owner it will automatically set DistributeOwnership."); + NetworkLog.LogWarning("DANGO-TODO: Review over don't destroy with owner being set but DistributeOwnership not being set. For now, if the NetworkObject does not destroy with the owner it will set ownership to SessionOwner."); } } } @@ -2007,12 +2045,14 @@ public bool TrySetParent(NetworkObject parent, bool worldPositionStays = true) internal bool InternalTrySetParent(NetworkObject parent, bool worldPositionStays = true) { - if (parent != null && (IsSpawned ^ parent.IsSpawned)) + if (parent != null && (IsSpawned ^ parent.IsSpawned) && NetworkManager != null && !NetworkManager.ShutdownInProgress) { - if (NetworkManager != null && !NetworkManager.ShutdownInProgress) + if (NetworkManager.LogLevel <= LogLevel.Developer) { - return false; + var nameOfNotSpawnedObject = IsSpawned ? $" the parent ({parent.name})" : $"the child ({name})"; + NetworkLog.LogWarning($"Parenting failed because {nameOfNotSpawnedObject} is not spawned!"); } + return false; } m_CachedWorldPositionStays = worldPositionStays; diff --git a/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs b/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs index acffc411f8..8996dfcfd7 100644 --- a/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs @@ -1903,10 +1903,12 @@ private void OnSessionOwnerLoadedScene(uint sceneEventId, Scene scene) SendSceneEventData(sceneEventData.SceneEventId, NetworkManager.ConnectedClientsIds.Where(c => c != sessionOwner).ToArray()); m_IsSceneEventActive = false; + + sceneEventData.SceneEventType = SceneEventType.LoadComplete; //First, notify local server that the scene was loaded OnSceneEvent?.Invoke(new SceneEvent() { - SceneEventType = SceneEventType.LoadComplete, + SceneEventType = sceneEventData.SceneEventType, LoadSceneMode = sceneEventData.LoadSceneMode, SceneName = SceneNameFromHash(sceneEventData.SceneHash), ClientId = NetworkManager.LocalClientId, diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index 85ff9a89e2..ed0b0348b7 100644 --- a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs @@ -422,27 +422,8 @@ internal void RemoveOwnership(NetworkObject networkObject) { if (NetworkManager.DistributedAuthorityMode && !NetworkManager.ShutdownInProgress) { - if (networkObject.IsOwnershipDistributable || networkObject.IsOwnershipTransferable) - { - if (networkObject.IsOwner || NetworkManager.DAHost) - { - NetworkLog.LogWarning("DANGO-TODO: Determine if removing ownership should make the CMB Service redistribute ownership or if this just isn't a valid thing in DAMode."); - return; - } - else - { - NetworkLog.LogError($"Only the owner is allowed to remove ownership in distributed authority mode!"); - return; - } - } - else - { - if (!NetworkManager.DAHost) - { - Debug.LogError($"Only {nameof(NetworkObject)}s with {nameof(NetworkObject.IsOwnershipDistributable)} or {nameof(NetworkObject.IsOwnershipTransferable)} set can perform ownership changes!"); - } - return; - } + Debug.LogError($"Removing ownership is invalid in Distributed Authority Mode. Use {nameof(ChangeOwnership)} instead."); + return; } ChangeOwnership(networkObject, NetworkManager.ServerClientId, true); } @@ -474,6 +455,18 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool if (NetworkManager.DistributedAuthorityMode) { + // Ensure only the session owner can change ownership (i.e. acquire) and that the session owner is not trying to assign a non-session owner client + // ownership of a NetworkObject with SessionOwner permissions. + if (networkObject.IsOwnershipSessionOwner && (!NetworkManager.LocalClient.IsSessionOwner || clientId != NetworkManager.CurrentSessionOwner)) + { + if (NetworkManager.LogLevel <= LogLevel.Developer) + { + NetworkLog.LogErrorServer($"[{networkObject.name}][Session Owner Only] You cannot change ownership of a {nameof(NetworkObject)} that has the {NetworkObject.OwnershipStatus.SessionOwner} flag set!"); + } + networkObject.OnOwnershipPermissionsFailure?.Invoke(NetworkObject.OwnershipPermissionsFailureStatus.SessionOwnerOnly); + return; + } + // If are not authorized and this is not an approved ownership change, then check to see if we can change ownership if (!isAuthorized && !isRequestApproval) { @@ -1746,6 +1739,11 @@ internal void GetObjectDistribution(ref Dictionary>()); + objectByTypeAndOwner.Add(globalOjectIdHash, new Dictionary>()); } // Sub-divide each type by owner - if (!objectByTypeAndOwner[networkObject.GlobalObjectIdHash].ContainsKey(networkObject.OwnerClientId)) + if (!objectByTypeAndOwner[globalOjectIdHash].ContainsKey(networkObject.OwnerClientId)) { - objectByTypeAndOwner[networkObject.GlobalObjectIdHash].Add(networkObject.OwnerClientId, new List()); + objectByTypeAndOwner[globalOjectIdHash].Add(networkObject.OwnerClientId, new List()); } // Add to the client's spawned object list - objectByTypeAndOwner[networkObject.GlobalObjectIdHash][networkObject.OwnerClientId].Add(networkObject); + objectByTypeAndOwner[globalOjectIdHash][networkObject.OwnerClientId].Add(networkObject); } } } @@ -1872,7 +1869,7 @@ internal void DistributeNetworkObjects(ulong clientId) if ((i % offsetCount) == 0) { ChangeOwnership(ownerList.Value[i], clientId, true); - if (EnableDistributeLogging) + //if (EnableDistributeLogging) { Debug.Log($"[Client-{ownerList.Key}][NetworkObjectId-{ownerList.Value[i].NetworkObjectId} Distributed to Client-{clientId}"); } diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs index beccbdd9db..dcf44f7d8f 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs @@ -128,7 +128,7 @@ public IEnumerator ValidateOwnershipPermissionsTest() yield return WaitForConditionOrTimeOut(ValidateObjectSpawnedOnAllClients); AssertOnTimeout($"[Failed To Spawn] {firstInstance.name}: \n {m_ErrorLog}"); - // Validate the base non-assigned persmissions value for all instances are the same. + // Validate the base non-assigned permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); AssertOnTimeout($"[Permissions Mismatch] {firstInstance.name}: \n {m_ErrorLog}"); @@ -141,9 +141,15 @@ public IEnumerator ValidateOwnershipPermissionsTest() foreach (var permissionObject in Enum.GetValues(typeof(NetworkObject.OwnershipStatus))) { var permission = (NetworkObject.OwnershipStatus)permissionObject; + // Adding the SessionOwner flag here should fail as this NetworkObject is not owned by the Session Owner + if (permission == NetworkObject.OwnershipStatus.SessionOwner) + { + Assert.IsFalse(firstInstance.SetOwnershipStatus(permission), $"[Add][IncorrectPermissions] Setting {NetworkObject.OwnershipStatus.SessionOwner} is not valid when the client is not the Session Owner: \n {m_ErrorLog}!"); + continue; + } // Add the status firstInstance.SetOwnershipStatus(permission); - // Validate the persmissions value for all instances are the same. + // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); AssertOnTimeout($"[Add][Permissions Mismatch] {firstInstance.name}: \n {m_ErrorLog}"); @@ -153,7 +159,7 @@ public IEnumerator ValidateOwnershipPermissionsTest() continue; } firstInstance.RemoveOwnershipStatus(permission); - // Validate the persmissions value for all instances are the same. + // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); AssertOnTimeout($"[Remove][Permissions Mismatch] {firstInstance.name}: \n {m_ErrorLog}"); } @@ -163,7 +169,7 @@ public IEnumerator ValidateOwnershipPermissionsTest() firstInstance.SetOwnershipStatus(multipleFlags, true); Assert.IsTrue(firstInstance.HasOwnershipStatus(multipleFlags), $"[Set][Multi-flag Failure] Expected: {(ushort)multipleFlags} but was {(ushort)firstInstance.Ownership}!"); - // Validate the persmissions value for all instances are the same. + // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); AssertOnTimeout($"[Set Multiple][Permissions Mismatch] {firstInstance.name}: \n {m_ErrorLog}"); @@ -175,7 +181,7 @@ public IEnumerator ValidateOwnershipPermissionsTest() // Validate that the Distributable flag is still set Assert.IsTrue(firstInstance.HasOwnershipStatus(NetworkObject.OwnershipStatus.Distributable), $"[Remove][Multi-flag Failure] Expected: {(ushort)NetworkObject.OwnershipStatus.Distributable} but was {(ushort)firstInstance.Ownership}!"); - // Validate the persmissions value for all instances are the same. + // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); AssertOnTimeout($"[Set Multiple][Permissions Mismatch] {firstInstance.name}: \n {m_ErrorLog}"); @@ -186,7 +192,7 @@ public IEnumerator ValidateOwnershipPermissionsTest() // Clear the flags, set the permissions to transferrable, and lock ownership in one pass. firstInstance.SetOwnershipStatus(NetworkObject.OwnershipStatus.Transferable, true, NetworkObject.OwnershipLockActions.SetAndLock); - // Validate the persmissions value for all instances are the same. + // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); AssertOnTimeout($"[Reset][Permissions Mismatch] {firstInstance.name}: \n {m_ErrorLog}"); @@ -199,7 +205,7 @@ public IEnumerator ValidateOwnershipPermissionsTest() $" status is {secondInstanceHelper.OwnershipPermissionsFailureStatus}!"); firstInstance.SetOwnershipLock(false); - // Validate the persmissions value for all instances are the same. + // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); AssertOnTimeout($"[Unlock][Permissions Mismatch] {firstInstance.name}: \n {m_ErrorLog}"); @@ -208,7 +214,7 @@ public IEnumerator ValidateOwnershipPermissionsTest() // Now try to acquire ownership secondInstance.ChangeOwnership(m_ClientNetworkManagers[1].LocalClientId); - // Validate the persmissions value for all instances are the same + // Validate the permissions value for all instances are the same yield return WaitForConditionOrTimeOut(() => secondInstance.IsOwner); AssertOnTimeout($"[Acquire Ownership Failed] Client-{m_ClientNetworkManagers[1].LocalClientId} failed to get ownership!"); @@ -220,7 +226,7 @@ public IEnumerator ValidateOwnershipPermissionsTest() // Clear the flags, set the permissions to RequestRequired, and lock ownership in one pass. secondInstance.SetOwnershipStatus(NetworkObject.OwnershipStatus.RequestRequired, true); - // Validate the persmissions value for all instances are the same. + // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); AssertOnTimeout($"[Unlock][Permissions Mismatch] {secondInstance.name}: \n {m_ErrorLog}"); @@ -238,7 +244,7 @@ public IEnumerator ValidateOwnershipPermissionsTest() // Start with a request for the client we expect to be given ownership var requestStatus = firstInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{firstInstance.NetworkManager.LocalClientId} was unabled to send a request for ownership because: {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{firstInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); // Get the 3rd client to send a request at the "relatively" same time var thirdInstance = m_ClientNetworkManagers[2].SpawnManager.SpawnedObjects[networkObjectId]; @@ -248,7 +254,7 @@ public IEnumerator ValidateOwnershipPermissionsTest() requestStatus = thirdInstance.RequestOwnership(); // We expect the 3rd client's request should be able to be sent at this time as well (i.e. creates the race condition between two clients) - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{m_ServerNetworkManager.LocalClientId} was unabled to send a request for ownership because: {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{m_ServerNetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); // We expect the first requesting client to be given ownership yield return WaitForConditionOrTimeOut(() => firstInstance.IsOwner); @@ -263,7 +269,7 @@ public IEnumerator ValidateOwnershipPermissionsTest() yield return WaitForConditionOrTimeOut(() => thirdInstanceHelper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.RequestInProgress); AssertOnTimeout($"[Request In Progress Failed] Client-{thirdInstanceHelper.NetworkManager.LocalClientId} did not get the right request denied reponse!"); - // Validate the persmissions value for all instances are the same. + // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); AssertOnTimeout($"[Unlock][Permissions Mismatch] {firstInstance.name}: \n {m_ErrorLog}"); @@ -278,11 +284,11 @@ public IEnumerator ValidateOwnershipPermissionsTest() // Send out a request from three clients at the same time // The first one sent (and received for this test) gets ownership requestStatus = secondInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{secondInstance.NetworkManager.LocalClientId} was unabled to send a request for ownership because: {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{secondInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); requestStatus = thirdInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{thirdInstance.NetworkManager.LocalClientId} was unabled to send a request for ownership because: {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{thirdInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); requestStatus = fourthInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{fourthInstance.NetworkManager.LocalClientId} was unabled to send a request for ownership because: {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{fourthInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); // The 2nd and 3rd client should be denied and the 4th client should be approved yield return WaitForConditionOrTimeOut(() => @@ -296,7 +302,7 @@ public IEnumerator ValidateOwnershipPermissionsTest() yield return WaitForConditionOrTimeOut(() => ValidateAllInstancesAreOwnedByClient(secondInstance.NetworkManager.LocalClientId)); AssertOnTimeout($"[Ownership Mismatch] {secondInstance.name}: \n {m_ErrorLog}"); - // Validate the persmissions value for all instances are the same. + // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); AssertOnTimeout($"[Unlock][Permissions Mismatch] {secondInstance.name}: \n {m_ErrorLog}"); @@ -314,22 +320,84 @@ public IEnumerator ValidateOwnershipPermissionsTest() // Send out a request from all three clients requestStatus = firstInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{firstInstance.NetworkManager.LocalClientId} was unabled to send a request for ownership because: {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{firstInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); requestStatus = thirdInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{thirdInstance.NetworkManager.LocalClientId} was unabled to send a request for ownership because: {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{thirdInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); requestStatus = fourthInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{fourthInstance.NetworkManager.LocalClientId} was unabled to send a request for ownership because: {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{fourthInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); requestStatus = daHostInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{daHostInstance.NetworkManager.LocalClientId} was unabled to send a request for ownership because: {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{daHostInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); - // The server and the 2nd client should be denied and the third client should be approved + // Only the client marked as ClientToAllowOwnership (daHost) should be approved. All others should be denied. yield return WaitForConditionOrTimeOut(() => (firstInstanceHelper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.Denied) && (thirdInstanceHelper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.Denied) && (fourthInstanceHelper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.Denied) && (daHostInstanceHelper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.Approved) ); - AssertOnTimeout($"[Targeted Owner] Client-{daHostInstance.NetworkManager.LocalClientId} did not get the right request reponse: {daHostInstanceHelper.OwnershipRequestResponseStatus} Expecting: {NetworkObject.OwnershipRequestResponseStatus.Approved}!"); + AssertOnTimeout($"[Targeted Owner] Client-{daHostInstance.NetworkManager.LocalClientId} did not get the right request response: {daHostInstanceHelper.OwnershipRequestResponseStatus} Expecting: {NetworkObject.OwnershipRequestResponseStatus.Approved}!"); + + /////////////////////////////////////////////// + // Test OwnershipStatus.SessionOwner: + /////////////////////////////////////////////// + + OwnershipPermissionsTestHelper.CurrentOwnedInstance = daHostInstance; + m_ObjectToValidate = OwnershipPermissionsTestHelper.CurrentOwnedInstance; + + // Add multiple statuses + daHostInstance.SetOwnershipStatus(NetworkObject.OwnershipStatus.Transferable | NetworkObject.OwnershipStatus.SessionOwner); + // Validate the permissions value for all instances are the same. + yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); + AssertOnTimeout($"[Add][Permissions Mismatch] {daHostInstance.name}: \n {m_ErrorLog}"); + + // Trying to set SessionOwner flag should override any other flags. + Assert.IsFalse(daHostInstance.HasOwnershipStatus(NetworkObject.OwnershipStatus.Transferable), $"[Set][SessionOwner flag Failure] Expected: {NetworkObject.OwnershipStatus.Transferable} not to be set!"); + + // Add another status. Should fail as SessionOwner should be exclusive + daHostInstance.SetOwnershipStatus(NetworkObject.OwnershipStatus.Distributable); + Assert.IsFalse(daHostInstance.HasOwnershipStatus(NetworkObject.OwnershipStatus.Distributable), $"[Add][SessionOwner flag Failure] Expected: {NetworkObject.OwnershipStatus.Transferable} not to be set!"); + + // Request ownership of the SessionOwner flag instance + requestStatus = firstInstance.RequestOwnership(); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestRequiredNotSet, $"Client-{firstInstance.NetworkManager.LocalClientId} should not be able to send a request for ownership because object is marked as owned by the session owner. {requestStatus}!"); + + // Set ownership directly on local object. This will allow the request to be sent + firstInstance.Ownership = NetworkObject.OwnershipStatus.RequestRequired; + requestStatus = firstInstance.RequestOwnership(); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{firstInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); + + // Request should be denied with CannotRequest + yield return WaitForConditionOrTimeOut(() => firstInstanceHelper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.CannotRequest); + AssertOnTimeout($"[Targeted Owner] Client-{firstInstance.NetworkManager.LocalClientId} did not get the right request response: {daHostInstanceHelper.OwnershipRequestResponseStatus} Expecting: {NetworkObject.OwnershipRequestResponseStatus.CannotRequest}!"); + + // Try changing the ownership explicitly + // Get the cloned daHostInstance instance on a client side + var clientInstance = m_ClientNetworkManagers[2].SpawnManager.SpawnedObjects[daHostInstance.NetworkObjectId]; + + // Get the client instance of the OwnershipPermissionsTestHelper component + var clientInstanceHelper = clientInstance.GetComponent(); + + // Have the client attempt to change ownership + clientInstance.ChangeOwnership(m_ClientNetworkManagers[2].LocalClientId); + + // Verify the client side gets a permission failure status of NetworkObject.OwnershipPermissionsFailureStatus.SessionOwnerOnly + Assert.IsTrue(clientInstanceHelper.OwnershipPermissionsFailureStatus == NetworkObject.OwnershipPermissionsFailureStatus.SessionOwnerOnly, + $"Expected {clientInstance.name} to return {NetworkObject.OwnershipPermissionsFailureStatus.SessionOwnerOnly} but its permission failure" + + $" status is {clientInstanceHelper.OwnershipPermissionsFailureStatus}!"); + + // Have the session owner attempt to change ownership to a non-session owner + daHostInstance.ChangeOwnership(m_ClientNetworkManagers[2].LocalClientId); + + // Verify the session owner cannot assign a SessionOwner permission NetworkObject to a non-sessionowner client + Assert.IsTrue(daHostInstanceHelper.OwnershipPermissionsFailureStatus == NetworkObject.OwnershipPermissionsFailureStatus.SessionOwnerOnly, + $"Expected {daHostInstance.name} to return {NetworkObject.OwnershipPermissionsFailureStatus.SessionOwnerOnly} but its permission failure" + + $" status is {daHostInstanceHelper.OwnershipPermissionsFailureStatus}!"); + + // Remove status + daHostInstance.RemoveOwnershipStatus(NetworkObject.OwnershipStatus.SessionOwner); + // Validate the permissions value for all instances are the same. + yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); + AssertOnTimeout($"[Remove][Permissions Mismatch] {daHostInstance.name}: \n {m_ErrorLog}"); } internal class OwnershipPermissionsTestHelper : NetworkBehaviour diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs index bda9d15fcb..6c1fe6bba7 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs @@ -127,7 +127,11 @@ public IEnumerator TestOwnershipCallbacks([Values] OwnershipChecks ownershipChec } // Verifies that removing the ownership when the default (server) is already set does not cause a Key Not Found Exception - m_ServerNetworkManager.SpawnManager.RemoveOwnership(m_OwnershipNetworkObject); + // Distributed authority does not allow remove ownership (users are instructed to use change ownership) + if (!m_DistributedAuthority) + { + m_ServerNetworkManager.SpawnManager.RemoveOwnership(m_OwnershipNetworkObject); + } var serverObject = m_ServerNetworkManager.SpawnManager.SpawnedObjects[ownershipNetworkObjectId]; var clientObject = m_ClientNetworkManagers[0].SpawnManager.SpawnedObjects[ownershipNetworkObjectId]; @@ -237,7 +241,12 @@ bool WaitForClientsToSpawnNetworkObject() Assert.False(s_GlobalTimeoutHelper.TimedOut, "Timed out waiting for all clients to change ownership!"); // Verifies that removing the ownership when the default (server) is already set does not cause a Key Not Found Exception - m_ServerNetworkManager.SpawnManager.RemoveOwnership(m_OwnershipNetworkObject); + // Distributed authority does not allow remove ownership (users are instructed to use change ownership) + if (!m_DistributedAuthority) + { + m_ServerNetworkManager.SpawnManager.RemoveOwnership(m_OwnershipNetworkObject); + } + var serverObject = m_ServerNetworkManager.SpawnManager.SpawnedObjects[ownershipNetworkObjectId]; Assert.That(serverObject, Is.Not.Null); var clientObject = (NetworkObject)null;