diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 2d16bbc30e..d253135563 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -10,6 +10,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) @@ -30,6 +31,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 bd18473b27..25dd8967f1 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(); @@ -193,7 +197,7 @@ public override void OnGUI(Rect position, SerializedProperty property, GUIConten // The below can cause visual anomalies and/or throws an exception within the EditorGUI itself (index out of bounds of the array). and has // The visual anomaly is when you select one field it is set in the drop down but then the flags selection in the popup menu selects more items - // even though if you exit the popup menu the flag setting is correct. + // even though if you exit the popup menu the flag setting is correct. //var ownership = (NetworkObject.OwnershipStatus)EditorGUI.EnumFlagsField(position, label, (NetworkObject.OwnershipStatus)property.enumValueFlag); //property.enumValueFlag = (int)ownership; EditorGUI.EndDisabledGroup(); diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index 09119e8076..fe6bc4816d 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -11,7 +11,7 @@ namespace Unity.Netcode { /// - /// The connection event type set within to signify the type of connection event notification received. + /// The connection event type set within to signify the type of connection event notification received. /// /// /// is returned as a parameter of the event notification. @@ -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 4f8c56e1f4..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); } @@ -416,7 +412,7 @@ public void NetworkUpdate(NetworkUpdateStage updateStage) // Metrics update needs to be driven by NetworkConnectionManager's update to assure metrics are dispatched after the send queue is processed. MetricsManager.UpdateMetrics(); - // Handle sending any pending transport messages + // Handle sending any pending transport messages NetworkConfig.NetworkTransport.PostLateUpdate(); // TODO: Determine a better way to handle this diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs index 1606343cca..c449c9aadb 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs @@ -182,7 +182,7 @@ private static void PrefabStageOpened(PrefabStage prefabStage) /// /// InContext: Typically means a are in prefab edit mode for an in-scene placed network prefab instance. /// (currently no such thing as a network prefab with nested network prefab instances) - /// + /// /// InIsolation: Typically means we are in prefb edit mode for a prefab asset. /// /// @@ -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)) + { + 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 { - SetOwnershipLock(lockAction == OwnershipLockActions.SetAndLock); + // 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."); } } } diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index 9ce6595c5f..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) { @@ -771,7 +764,7 @@ internal NetworkObject InstantiateAndSpawnNoParameterChecks(NetworkObject networ } /// - /// Gets the right NetworkObject prefab instance to spawn. If a handler is registered or there is an override assigned to the + /// Gets the right NetworkObject prefab instance to spawn. If a handler is registered or there is an override assigned to the /// passed in globalObjectIdHash value, then that is what will be instantiated, spawned, and returned. /// internal NetworkObject GetNetworkObjectToSpawn(uint globalObjectIdHash, ulong ownerId, Vector3? position, Quaternion? rotation, bool isScenePlaced = false) @@ -803,8 +796,8 @@ internal NetworkObject GetNetworkObjectToSpawn(uint globalObjectIdHash, ulong ow case NetworkPrefabOverride.Hash: case NetworkPrefabOverride.Prefab: { - // When scene management is disabled and this is an in-scene placed NetworkObject, we want to always use the - // SourcePrefabToOverride and not any possible prefab override as a user might want to spawn overrides dynamically + // When scene management is disabled and this is an in-scene placed NetworkObject, we want to always use the + // SourcePrefabToOverride and not any possible prefab override as a user might want to spawn overrides dynamically // but might want to use the same source network prefab as an in-scene placed NetworkObject. // (When scene management is enabled, clients don't delete their in-scene placed NetworkObjects prior to dynamically // spawning them so the original prefab placed is preserved and this is not needed) @@ -998,7 +991,7 @@ internal NetworkObject CreateLocalNetworkObject(NetworkObject.SceneObject sceneO /// - NetworkObject when spawning a newly instantiated NetworkObject for the first time. /// - NetworkSceneManager after a server/session-owner has loaded a scene to locally spawn the newly instantiated in-scene placed NetworkObjects. /// - NetworkSpawnManager when spawning any already loaded in-scene placed NetworkObjects (client-server or session owner). - /// + /// /// Client-Server: /// Server is the only instance that invokes this method. /// @@ -1376,7 +1369,7 @@ internal void DespawnAndDestroyNetworkObjects() } } - // If spawned, then despawn and potentially destroy. + // If spawned, then despawn and potentially destroy. if (networkObjects[i].IsSpawned) { OnDespawnObject(networkObjects[i], shouldDestroy); @@ -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); } } } @@ -1805,7 +1802,7 @@ internal void DistributeNetworkObjects(ulong clientId) } - // DA-NGO CMB SERVICE NOTES: + // DA-NGO CMB SERVICE NOTES: // The most basic object distribution should be broken up into a table of spawned object types // where each type contains a list of each client's owned objects of that type that can be // distributed. @@ -1824,7 +1821,7 @@ internal void DistributeNetworkObjects(ulong clientId) var clientCount = NetworkManager.ConnectedClientsIds.Count; - // Cycle through each prefab type + // Cycle through each prefab type foreach (var objectTypeEntry in distributedNetworkObjects) { // Calculate the number of objects that should be distributed amongst the clients @@ -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}"); } @@ -1894,7 +1891,7 @@ internal void DistributeNetworkObjects(ulong clientId) objectTypeCount.Clear(); GetObjectDistribution(ref distributedNetworkObjects, ref objectTypeCount); builder.AppendLine($"Client Relative Distributed Object Count: (distribution follows)"); - // Cycle through each prefab type + // Cycle through each prefab type foreach (var objectTypeEntry in distributedNetworkObjects) { builder.AppendLine($"[GID: {objectTypeEntry.Key} | {objectTypeEntry.Value.First().Value.First().name}][Total Count: {objectTypeCount[objectTypeEntry.Key]}]"); 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;