From 51b72188a7e131d1cf8383ee846232c311a1ba22 Mon Sep 17 00:00:00 2001 From: EmandM Date: Thu, 12 Dec 2024 17:09:53 -0500 Subject: [PATCH 01/16] Initial pass on SessionOwner ownership flag --- .../Editor/NetworkObjectEditor.cs | 6 +- .../Connection/NetworkConnectionManager.cs | 2 +- .../Runtime/Core/NetworkManager.cs | 8 +- .../Runtime/Core/NetworkObject.cs | 36 ++++++--- .../Runtime/Spawning/NetworkSpawnManager.cs | 58 ++++++-------- testproject/Packages/packages-lock.json | 80 +++++++++---------- .../com.unity.services.core/Settings.json | 0 7 files changed, 99 insertions(+), 91 deletions(-) create mode 100644 testproject/ProjectSettings/Packages/com.unity.services.core/Settings.json 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..ca539860d6 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) { diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs index 6a7d71f159..f3a1583632 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 && networkObject.OwnerClientId != LocalClientId) { 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 5d94f4fd3c..8f8f76a10c 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. /// /// @@ -438,6 +438,7 @@ public void DeferDespawn(int tickOffset, bool destroy = true) /// - If the has an ownership request in progress but the target client is no longer connected, then it will be redistributed. /// public bool IsOwnershipDistributable => Ownership.HasFlag(OwnershipStatus.Distributable); + public bool IsOwnershipSessionOwner => Ownership.HasFlag(OwnershipStatus.SessionOwner); /// /// Returns true if the is has ownership locked. @@ -482,6 +483,7 @@ public void DeferDespawn(int tickOffset, bool destroy = true) /// : 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, When set, a non-owner must request ownership from the owner (will always get locked once ownership is transferred). /// // Ranges from 1 to 8 bits [Flags] @@ -491,6 +493,7 @@ public enum OwnershipStatus Distributable = 1 << 0, Transferable = 1 << 1, RequestRequired = 1 << 2, + SessionOwner = 1 << 3, } /// @@ -588,7 +591,8 @@ public enum OwnershipPermissionsFailureStatus Locked, RequestRequired, RequestInProgress, - NotTransferrable + NotTransferrable, + SessionOwnerOnly } /// @@ -716,7 +720,7 @@ internal void OwnershipRequest(ulong clientRequestingOwnership) { response = OwnershipRequestResponseStatus.RequestInProgress; } - else if (!IsOwnershipRequestRequired && !IsOwnershipTransferable) + else if (!(IsOwnershipRequestRequired || IsOwnershipTransferable) || IsOwnershipSessionOwner) { response = OwnershipRequestResponseStatus.CannotRequest; } @@ -836,6 +840,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 +857,21 @@ 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 (status.HasFlag(OwnershipStatus.SessionOwner)) + { - if (lockAction != OwnershipLockActions.None) + Ownership = OwnershipStatus.SessionOwner; + } + 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 +1647,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..abc0834fcd 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); } @@ -477,6 +458,15 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool // If are not authorized and this is not an approved ownership change, then check to see if we can change ownership if (!isAuthorized && !isRequestApproval) { + if (networkObject.IsOwnershipSessionOwner) + { + 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 (networkObject.IsOwnershipLocked) { if (NetworkManager.LogLevel <= LogLevel.Developer) @@ -771,7 +761,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 +793,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 +988,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 +1366,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 +1736,11 @@ internal void GetObjectDistribution(ref Dictionary Date: Thu, 12 Dec 2024 17:42:14 -0500 Subject: [PATCH 02/16] feat: Add SessionOwner OwnershipStatus flag --- .../Editor/NetworkObjectEditor.cs | 6 +- .../Connection/NetworkConnectionManager.cs | 4 +- .../Runtime/Core/NetworkManager.cs | 8 +-- .../Runtime/Core/NetworkObject.cs | 54 +++++++++++++---- .../Runtime/Spawning/NetworkSpawnManager.cs | 58 ++++++++----------- 5 files changed, 77 insertions(+), 53 deletions(-) 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..937271209e 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) { diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs index 6a7d71f159..f3a1583632 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 && networkObject.OwnerClientId != LocalClientId) { 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 5d94f4fd3c..52a1f6016d 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. @@ -482,6 +489,7 @@ public void DeferDespawn(int tickOffset, bool destroy = true) /// : 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, 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. /// /// @@ -643,7 +657,7 @@ public OwnershipRequestStatus RequestOwnership() } // Exit early if it doesn't have the RequestRequired flag - if (!IsOwnershipRequestRequired) + if (!IsOwnershipRequestRequired || IsOwnershipSessionOwner) { return OwnershipRequestStatus.RequestRequiredNotSet; } @@ -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,21 @@ 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 (status.HasFlag(OwnershipStatus.SessionOwner)) + { - if (lockAction != OwnershipLockActions.None) + Ownership = OwnershipStatus.SessionOwner; + } + 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(); diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index 9ce6595c5f..abc0834fcd 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); } @@ -477,6 +458,15 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool // If are not authorized and this is not an approved ownership change, then check to see if we can change ownership if (!isAuthorized && !isRequestApproval) { + if (networkObject.IsOwnershipSessionOwner) + { + 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 (networkObject.IsOwnershipLocked) { if (NetworkManager.LogLevel <= LogLevel.Developer) @@ -771,7 +761,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 +793,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 +988,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 +1366,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 +1736,11 @@ internal void GetObjectDistribution(ref Dictionary Date: Thu, 12 Dec 2024 16:48:11 -0600 Subject: [PATCH 03/16] update removing additional session owner accessor. --- com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs index 9b7d6c6635..8adf160fa7 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs @@ -438,7 +438,6 @@ public void DeferDespawn(int tickOffset, bool destroy = true) /// - If the has an ownership request in progress but the target client is no longer connected, then it will be redistributed. /// public bool IsOwnershipDistributable => Ownership.HasFlag(OwnershipStatus.Distributable); - public bool IsOwnershipSessionOwner => Ownership.HasFlag(OwnershipStatus.SessionOwner); /// /// When true, the can only be owned by the current Session Owner. From d2ab5db42932ea0d1b49b0c69d4acc217b3321cb Mon Sep 17 00:00:00 2001 From: EmandM Date: Thu, 12 Dec 2024 19:59:59 -0500 Subject: [PATCH 04/16] Add OwnershipPermissions tests --- .../Runtime/Core/NetworkObject.cs | 8 +- .../Runtime/Spawning/NetworkSpawnManager.cs | 20 ++-- .../OwnershipPermissionsTests.cs | 95 ++++++++++++++----- 3 files changed, 90 insertions(+), 33 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs index 8adf160fa7..e015cb2f0a 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs @@ -657,7 +657,7 @@ public OwnershipRequestStatus RequestOwnership() } // Exit early if it doesn't have the RequestRequired flag - if (!IsOwnershipRequestRequired || IsOwnershipSessionOwner) + if (!IsOwnershipRequestRequired) { return OwnershipRequestStatus.RequestRequiredNotSet; } @@ -875,9 +875,13 @@ public bool SetOwnershipStatus(OwnershipStatus status, bool clearAndSet = false, 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 { // Faster to just OR a None status than to check diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index abc0834fcd..4554d39a7c 100644 --- a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs @@ -455,18 +455,20 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool if (NetworkManager.DistributedAuthorityMode) { - // If are not authorized and this is not an approved ownership change, then check to see if we can change ownership - if (!isAuthorized && !isRequestApproval) + // Ensure we are not changing the ownership of an object marked as IsSessionOwner + if (networkObject.IsOwnershipSessionOwner) { - if (networkObject.IsOwnershipSessionOwner) + if (NetworkManager.LogLevel <= LogLevel.Developer) { - 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; + 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) + { if (networkObject.IsOwnershipLocked) { if (NetworkManager.LogLevel <= LogLevel.Developer) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs index beccbdd9db..fe531d32ee 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,67 @@ 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 + daHostInstance.ChangeOwnership(m_ClientNetworkManagers[0].LocalClientId); + 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 From 3c3b35444200da308dd07e1dd2e7b591fa0842d5 Mon Sep 17 00:00:00 2001 From: EmandM Date: Thu, 12 Dec 2024 20:08:49 -0500 Subject: [PATCH 05/16] fix client connect --- .../Runtime/Spawning/NetworkSpawnManager.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index 4554d39a7c..f2d15a63f2 100644 --- a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs @@ -1738,11 +1738,6 @@ internal void GetObjectDistribution(ref Dictionary Date: Thu, 12 Dec 2024 20:32:58 -0500 Subject: [PATCH 06/16] Revert "fix client connect" This reverts commit 3c3b35444200da308dd07e1dd2e7b591fa0842d5. --- .../Runtime/Spawning/NetworkSpawnManager.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index f2d15a63f2..4554d39a7c 100644 --- a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs @@ -1738,6 +1738,11 @@ internal void GetObjectDistribution(ref Dictionary Date: Thu, 12 Dec 2024 19:41:53 -0600 Subject: [PATCH 07/16] update object distribution for in-scene placed NetworkObjects needs to use the InScenePlacedSourceGlobalObjectIdHash when building an object distribution list. --- .../Runtime/Spawning/NetworkSpawnManager.cs | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index f2d15a63f2..7002a49d77 100644 --- a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs @@ -1753,12 +1753,15 @@ 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); } } } @@ -1864,7 +1868,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}"); } From 76619d4ff6e480e468f5943060b2e9a741f9ae3b Mon Sep 17 00:00:00 2001 From: EmandM Date: Thu, 12 Dec 2024 20:48:28 -0500 Subject: [PATCH 08/16] Add changelog --- com.unity.netcode.gameobjects/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 99197d550f..426fddb7ba 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 +- Changed object distribution when new clients join to distribute in-scene placed `NetworkObject`s. (#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) From 2bb940fc294a7ebe28c34ded10899431a075f597 Mon Sep 17 00:00:00 2001 From: EmandM Date: Thu, 12 Dec 2024 20:49:55 -0500 Subject: [PATCH 09/16] Remove unnecessary change --- testproject/Packages/packages-lock.json | 80 ++++++++++++------------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/testproject/Packages/packages-lock.json b/testproject/Packages/packages-lock.json index 2894198a3d..7b289e9221 100644 --- a/testproject/Packages/packages-lock.json +++ b/testproject/Packages/packages-lock.json @@ -1,17 +1,17 @@ { "dependencies": { "com.unity.addressables": { - "version": "2.1.0", + "version": "2.0.8", "depth": 0, "source": "registry", "dependencies": { - "com.unity.profiling.core": "1.0.2", + "com.unity.scriptablebuildpipeline": "2.1.2", "com.unity.modules.assetbundle": "1.0.0", - "com.unity.modules.jsonserialize": "1.0.0", "com.unity.modules.imageconversion": "1.0.0", + "com.unity.modules.jsonserialize": "1.0.0", "com.unity.modules.unitywebrequest": "1.0.0", - "com.unity.scriptablebuildpipeline": "2.1.3", - "com.unity.modules.unitywebrequestassetbundle": "1.0.0" + "com.unity.modules.unitywebrequestassetbundle": "1.0.0", + "com.unity.profiling.core": "1.0.2" }, "url": "https://packages.unity.com" }, @@ -25,7 +25,7 @@ "url": "https://packages.unity.com" }, "com.unity.burst": { - "version": "1.8.18", + "version": "1.8.13", "depth": 2, "source": "registry", "dependencies": { @@ -35,20 +35,20 @@ "url": "https://packages.unity.com" }, "com.unity.collab-proxy": { - "version": "2.4.3", + "version": "2.3.1", "depth": 0, "source": "registry", "dependencies": {}, "url": "https://packages.unity.com" }, "com.unity.collections": { - "version": "2.5.1", + "version": "2.4.0", "depth": 2, "source": "registry", "dependencies": { - "com.unity.burst": "1.8.17", - "com.unity.test-framework": "1.4.5", + "com.unity.burst": "1.8.12", "com.unity.nuget.mono-cecil": "1.11.4", + "com.unity.test-framework": "1.4.3", "com.unity.test-framework.performance": "3.0.3" }, "url": "https://packages.unity.com" @@ -91,7 +91,7 @@ "source": "local", "dependencies": { "com.unity.nuget.mono-cecil": "1.11.4", - "com.unity.transport": "2.3.0" + "com.unity.transport": "2.2.1" } }, "com.unity.nuget.mono-cecil": { @@ -125,32 +125,32 @@ "url": "https://packages.unity.com" }, "com.unity.scriptablebuildpipeline": { - "version": "2.1.4", + "version": "2.1.2", "depth": 1, "source": "registry", "dependencies": {}, "url": "https://packages.unity.com" }, "com.unity.services.authentication": { - "version": "3.3.3", + "version": "3.3.1", "depth": 0, "source": "registry", "dependencies": { - "com.unity.ugui": "1.0.0", - "com.unity.services.core": "1.13.0", "com.unity.nuget.newtonsoft-json": "3.2.1", - "com.unity.modules.unitywebrequest": "1.0.0" + "com.unity.services.core": "1.12.5", + "com.unity.modules.unitywebrequest": "1.0.0", + "com.unity.ugui": "1.0.0" }, "url": "https://packages.unity.com" }, "com.unity.services.core": { - "version": "1.13.0", + "version": "1.12.5", "depth": 0, "source": "registry", "dependencies": { - "com.unity.modules.androidjni": "1.0.0", + "com.unity.modules.unitywebrequest": "1.0.0", "com.unity.nuget.newtonsoft-json": "3.2.1", - "com.unity.modules.unitywebrequest": "1.0.0" + "com.unity.modules.androidjni": "1.0.0" }, "url": "https://packages.unity.com" }, @@ -159,11 +159,11 @@ "depth": 1, "source": "registry", "dependencies": { - "com.unity.collections": "1.2.4", "com.unity.services.core": "1.12.4", - "com.unity.nuget.newtonsoft-json": "3.0.2", "com.unity.modules.unitywebrequest": "1.0.0", - "com.unity.services.authentication": "2.0.0" + "com.unity.nuget.newtonsoft-json": "3.0.2", + "com.unity.services.authentication": "2.0.0", + "com.unity.collections": "1.2.4" }, "url": "https://packages.unity.com" }, @@ -172,38 +172,38 @@ "depth": 0, "source": "registry", "dependencies": { - "com.unity.transport": "1.3.0", - "com.unity.services.qos": "1.1.0", "com.unity.services.core": "1.4.0", - "com.unity.nuget.newtonsoft-json": "3.0.2", - "com.unity.modules.unitywebrequest": "1.0.0", "com.unity.services.authentication": "2.0.0", - "com.unity.modules.unitywebrequestwww": "1.0.0", + "com.unity.services.qos": "1.1.0", + "com.unity.modules.unitywebrequest": "1.0.0", + "com.unity.modules.unitywebrequestassetbundle": "1.0.0", "com.unity.modules.unitywebrequestaudio": "1.0.0", "com.unity.modules.unitywebrequesttexture": "1.0.0", - "com.unity.modules.unitywebrequestassetbundle": "1.0.0" + "com.unity.modules.unitywebrequestwww": "1.0.0", + "com.unity.nuget.newtonsoft-json": "3.0.2", + "com.unity.transport": "1.3.0" }, "url": "https://packages.unity.com" }, "com.unity.sysroot": { - "version": "2.0.10", + "version": "2.0.7", "depth": 1, "source": "registry", "dependencies": {}, "url": "https://packages.unity.com" }, "com.unity.sysroot.linux-x86_64": { - "version": "2.0.9", + "version": "2.0.6", "depth": 1, "source": "registry", "dependencies": { - "com.unity.sysroot": "2.0.10" + "com.unity.sysroot": "2.0.7" }, "url": "https://packages.unity.com" }, "com.unity.test-framework": { - "version": "1.4.5", - "depth": 1, + "version": "1.4.4", + "depth": 0, "source": "registry", "dependencies": { "com.unity.ext.nunit": "2.0.3", @@ -223,34 +223,34 @@ "url": "https://packages.unity.com" }, "com.unity.timeline": { - "version": "1.8.7", + "version": "1.8.6", "depth": 0, "source": "registry", "dependencies": { - "com.unity.modules.audio": "1.0.0", "com.unity.modules.director": "1.0.0", "com.unity.modules.animation": "1.0.0", + "com.unity.modules.audio": "1.0.0", "com.unity.modules.particlesystem": "1.0.0" }, "url": "https://packages.unity.com" }, "com.unity.toolchain.win-x86_64-linux-x86_64": { - "version": "2.0.9", + "version": "2.0.6", "depth": 0, "source": "registry", "dependencies": { - "com.unity.sysroot": "2.0.10", - "com.unity.sysroot.linux-x86_64": "2.0.9" + "com.unity.sysroot": "2.0.7", + "com.unity.sysroot.linux-x86_64": "2.0.6" }, "url": "https://packages.unity.com" }, "com.unity.transport": { - "version": "2.3.0", + "version": "2.2.1", "depth": 1, "source": "registry", "dependencies": { - "com.unity.burst": "1.8.12", "com.unity.collections": "2.2.1", + "com.unity.burst": "1.8.8", "com.unity.mathematics": "1.3.1" }, "url": "https://packages.unity.com" From 1b1cd667b700d9293ffca4800798aebae9fa35e5 Mon Sep 17 00:00:00 2001 From: EmandM Date: Thu, 12 Dec 2024 20:51:08 -0500 Subject: [PATCH 10/16] Remove Settings.json --- .../Packages/com.unity.services.core/Settings.json | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 testproject/ProjectSettings/Packages/com.unity.services.core/Settings.json diff --git a/testproject/ProjectSettings/Packages/com.unity.services.core/Settings.json b/testproject/ProjectSettings/Packages/com.unity.services.core/Settings.json deleted file mode 100644 index e69de29bb2..0000000000 From ad4ff7bc0e1defd97947936c77a70523bfd18f63 Mon Sep 17 00:00:00 2001 From: EmandM Date: Thu, 12 Dec 2024 20:54:38 -0500 Subject: [PATCH 11/16] Reword CHANGELOG --- com.unity.netcode.gameobjects/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 426fddb7ba..9659524647 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -31,7 +31,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Changed -- Changed object distribution when new clients join to distribute in-scene placed `NetworkObject`s. (#3175) +- 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) From e8b2a0a392dd41df2c7716c2d24455db6e5e8a8c Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Thu, 12 Dec 2024 21:48:12 -0600 Subject: [PATCH 12/16] fix DAHost should not distribute session owner permission NetworkObjects. When client is promoted to session owner, for now newly promoted client takes ownership of NetworkObjects that have the SessionOwner permission set. Only prevent non-session owners from taking ownership of a NetworkObject with the SessionOwner permission set. --- .../Runtime/Connection/NetworkConnectionManager.cs | 5 +++++ com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs | 2 +- .../Runtime/Spawning/NetworkSpawnManager.cs | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index 937271209e..fe6bc4816d 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -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 f3a1583632..a90226fe08 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs @@ -225,7 +225,7 @@ internal void SetSessionOwner(ulong sessionOwner) foreach (var networkObjectEntry in SpawnManager.SpawnedObjects) { var networkObject = networkObjectEntry.Value; - if (networkObject.IsOwnershipSessionOwner && networkObject.OwnerClientId != LocalClientId) + if (networkObject.IsOwnershipSessionOwner && LocalClient.IsSessionOwner) { SpawnManager.ChangeOwnership(networkObject, LocalClientId, true); } diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index 782b126619..9a2d1e5870 100644 --- a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs @@ -456,7 +456,7 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool if (NetworkManager.DistributedAuthorityMode) { // Ensure we are not changing the ownership of an object marked as IsSessionOwner - if (networkObject.IsOwnershipSessionOwner) + if (networkObject.IsOwnershipSessionOwner && !NetworkManager.LocalClient.IsSessionOwner) { if (NetworkManager.LogLevel <= LogLevel.Developer) { From c8a26595b3b9a21051b952a2fecd3f839232b49a Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Thu, 12 Dec 2024 21:53:17 -0600 Subject: [PATCH 13/16] test fix Avoid the RemoveOwnership client-server only method. --- .../NetworkObject/NetworkObjectOwnershipTests.cs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs index bda9d15fcb..05fd5b3ac3 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs @@ -1,7 +1,7 @@ +using NUnit.Framework; using System.Collections; using System.Collections.Generic; using System.Linq; -using NUnit.Framework; using Unity.Netcode.TestHelpers.Runtime; using UnityEngine; using UnityEngine.TestTools; @@ -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; From 07a0a61ed6804b0061c3e90d69fe8a86fa2711b7 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Thu, 12 Dec 2024 22:16:04 -0600 Subject: [PATCH 14/16] style Visual studio code cleanup likes to sort by alpha... fixing for our standards. --- .../Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs index 05fd5b3ac3..6c1fe6bba7 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs @@ -1,7 +1,7 @@ -using NUnit.Framework; using System.Collections; using System.Collections.Generic; using System.Linq; +using NUnit.Framework; using Unity.Netcode.TestHelpers.Runtime; using UnityEngine; using UnityEngine.TestTools; From 75fb258fb3c11d10805c3217810a7c105c7bdcc5 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Thu, 12 Dec 2024 23:30:40 -0600 Subject: [PATCH 15/16] update Adding check for session owner trying to change ownership to a non-session owner client. --- .../Runtime/Spawning/NetworkSpawnManager.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index 9a2d1e5870..ed0b0348b7 100644 --- a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs @@ -455,8 +455,9 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool if (NetworkManager.DistributedAuthorityMode) { - // Ensure we are not changing the ownership of an object marked as IsSessionOwner - if (networkObject.IsOwnershipSessionOwner && !NetworkManager.LocalClient.IsSessionOwner) + // 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) { From 0b8fa5c205a261a49cfbcd4db4eea76ed4d992bd Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Thu, 12 Dec 2024 23:31:53 -0600 Subject: [PATCH 16/16] test Adding an additional validation that a non-session owner cannot change ownership and that a session owner cannot change ownership to a non-session owner when the NetworkObject in question has the SessionOwner permissions set. --- .../OwnershipPermissionsTests.cs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs index fe531d32ee..dcf44f7d8f 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs @@ -371,7 +371,24 @@ public IEnumerator ValidateOwnershipPermissionsTest() 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 - daHostInstance.ChangeOwnership(m_ClientNetworkManagers[0].LocalClientId); + // 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}!");