diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index c3c3d9f73e..5c5772143f 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -14,6 +14,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Changed +- Improved performance around the NetworkBehaviour component. (#3687) ### Deprecated diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs index d86f72f2ac..332694af7d 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs @@ -74,6 +74,11 @@ internal enum __RpcExecStage internal FastBufferWriter __beginSendServerRpc(uint rpcMethodId, ServerRpcParams serverRpcParams, RpcDelivery rpcDelivery) #pragma warning restore IDE1006 // restore naming rule violation check { + if (m_NetworkObject == null && !IsSpawned) + { + throw new RpcException("The NetworkBehaviour must be spawned before calling this method."); + } + return new FastBufferWriter(k_RpcMessageDefaultSize, Allocator.Temp, k_RpcMessageMaximumSize); } @@ -142,7 +147,7 @@ internal void __endSendServerRpc(ref FastBufferWriter bufferWriter, uint rpcMeth { NetworkManager.NetworkMetrics.TrackRpcSent( NetworkManager.ServerClientId, - NetworkObject, + m_NetworkObject, rpcMethodName, __getTypeName(), rpcWriteSize); @@ -155,6 +160,11 @@ internal void __endSendServerRpc(ref FastBufferWriter bufferWriter, uint rpcMeth internal FastBufferWriter __beginSendClientRpc(uint rpcMethodId, ClientRpcParams clientRpcParams, RpcDelivery rpcDelivery) #pragma warning restore IDE1006 // restore naming rule violation check { + if (m_NetworkObject == null && !IsSpawned) + { + throw new RpcException("The NetworkBehaviour must be spawned before calling this method."); + } + return new FastBufferWriter(k_RpcMessageDefaultSize, Allocator.Temp, k_RpcMessageMaximumSize); } @@ -206,7 +216,7 @@ internal void __endSendClientRpc(ref FastBufferWriter bufferWriter, uint rpcMeth continue; } // Check to make sure we are sending to only observers, if not log an error. - if (networkManager.LogLevel >= LogLevel.Error && !NetworkObject.Observers.Contains(targetClientId)) + if (networkManager.LogLevel >= LogLevel.Error && !m_NetworkObject.Observers.Contains(targetClientId)) { NetworkLog.LogError(GenerateObserverErrorMessage(clientRpcParams, targetClientId)); } @@ -223,7 +233,7 @@ internal void __endSendClientRpc(ref FastBufferWriter bufferWriter, uint rpcMeth continue; } // Check to make sure we are sending to only observers, if not log an error. - if (networkManager.LogLevel >= LogLevel.Error && !NetworkObject.Observers.Contains(targetClientId)) + if (networkManager.LogLevel >= LogLevel.Error && !m_NetworkObject.Observers.Contains(targetClientId)) { NetworkLog.LogError(GenerateObserverErrorMessage(clientRpcParams, targetClientId)); } @@ -232,7 +242,7 @@ internal void __endSendClientRpc(ref FastBufferWriter bufferWriter, uint rpcMeth } else { - var observerEnumerator = NetworkObject.Observers.GetEnumerator(); + var observerEnumerator = m_NetworkObject.Observers.GetEnumerator(); while (observerEnumerator.MoveNext()) { // Skip over the host @@ -274,7 +284,7 @@ internal void __endSendClientRpc(ref FastBufferWriter bufferWriter, uint rpcMeth { networkManager.NetworkMetrics.TrackRpcSent( targetClientId, - NetworkObject, + m_NetworkObject, rpcMethodName, __getTypeName(), rpcWriteSize); @@ -286,7 +296,7 @@ internal void __endSendClientRpc(ref FastBufferWriter bufferWriter, uint rpcMeth { networkManager.NetworkMetrics.TrackRpcSent( targetClientId, - NetworkObject, + m_NetworkObject, rpcMethodName, __getTypeName(), rpcWriteSize); @@ -294,12 +304,12 @@ internal void __endSendClientRpc(ref FastBufferWriter bufferWriter, uint rpcMeth } else { - var observerEnumerator = NetworkObject.Observers.GetEnumerator(); + var observerEnumerator = m_NetworkObject.Observers.GetEnumerator(); while (observerEnumerator.MoveNext()) { networkManager.NetworkMetrics.TrackRpcSent( observerEnumerator.Current, - NetworkObject, + m_NetworkObject, rpcMethodName, __getTypeName(), rpcWriteSize); @@ -315,6 +325,10 @@ internal void __endSendClientRpc(ref FastBufferWriter bufferWriter, uint rpcMeth internal FastBufferWriter __beginSendRpc(uint rpcMethodId, RpcParams rpcParams, RpcAttribute.RpcAttributeParams attributeParams, SendTo defaultTarget, RpcDelivery rpcDelivery) #pragma warning restore IDE1006 // restore naming rule violation check { + if (m_NetworkObject == null && !IsSpawned) + { + throw new RpcException("The NetworkBehaviour must be spawned before calling this method."); + } if (attributeParams.RequireOwnership && !IsOwner) { throw new RpcException("This RPC can only be sent by its owner."); @@ -546,6 +560,11 @@ internal bool IsBehaviourEditable() ((networkManager.DistributedAuthorityMode && m_NetworkObject.IsOwner) || (!networkManager.DistributedAuthorityMode && networkManager.IsServer)); } + internal void SetNetworkObject(NetworkObject networkObject) + { + m_NetworkObject = networkObject; + } + // TODO: this needs an overhaul. It's expensive, it's ja little naive in how it looks for networkObject in // its parent and worst, it creates a puzzle if you are a NetworkBehaviour wanting to see if you're live or not // (e.g. editor code). All you want to do is find out if NetworkManager is null, but to do that you @@ -635,43 +654,32 @@ protected NetworkBehaviour GetNetworkBehaviour(ushort behaviourId) /// internal void UpdateNetworkProperties() { - var networkObject = NetworkObject; - // Set NetworkObject dependent properties - if (networkObject != null) - { - var networkManager = NetworkManager; - // Set identification related properties - NetworkObjectId = networkObject.NetworkObjectId; - IsLocalPlayer = networkObject.IsLocalPlayer; - - // This is "OK" because GetNetworkBehaviourOrderIndex uses the order of - // NetworkObject.ChildNetworkBehaviours which is set once when first - // accessed. - NetworkBehaviourId = networkObject.GetNetworkBehaviourOrderIndex(this); - - // Set ownership related properties - IsOwnedByServer = networkObject.IsOwnedByServer; - IsOwner = networkObject.IsOwner; - OwnerClientId = networkObject.OwnerClientId; - - // Set NetworkManager dependent properties - if (networkManager != null) - { - IsHost = networkManager.IsListening && networkManager.IsHost; - IsClient = networkManager.IsListening && networkManager.IsClient; - IsServer = networkManager.IsListening && networkManager.IsServer; - LocalClient = networkManager.LocalClient; - HasAuthority = networkObject.HasAuthority; - ServerIsHost = networkManager.IsListening && networkManager.ServerIsHost; - } - } - else // Shouldn't happen, but if so then set the properties to their default value; + var networkObject = m_NetworkObject; + var networkManager = NetworkManager; + + // Set identification related properties + NetworkObjectId = networkObject.NetworkObjectId; + IsLocalPlayer = networkObject.IsLocalPlayer; + + // This is "OK" because GetNetworkBehaviourOrderIndex uses the order of + // NetworkObject.ChildNetworkBehaviours which is set once when first + // accessed. + NetworkBehaviourId = networkObject.GetNetworkBehaviourOrderIndex(this); + + // Set ownership related properties + IsOwnedByServer = networkObject.IsOwnedByServer; + IsOwner = networkObject.IsOwner; + OwnerClientId = networkObject.OwnerClientId; + + // Set NetworkManager dependent properties + if (networkManager != null) { - OwnerClientId = NetworkObjectId = default; - IsOwnedByServer = IsOwner = IsHost = IsClient = IsServer = ServerIsHost = default; - NetworkBehaviourId = default; - LocalClient = default; - HasAuthority = default; + IsHost = networkManager.IsListening && networkManager.IsHost; + IsClient = networkManager.IsListening && networkManager.IsClient; + IsServer = networkManager.IsListening && networkManager.IsServer; + LocalClient = networkManager.LocalClient; + HasAuthority = networkObject.HasAuthority; + ServerIsHost = networkManager.IsListening && networkManager.ServerIsHost; } } @@ -752,8 +760,11 @@ public virtual void OnNetworkDespawn() { } /// public virtual void OnNetworkPreDespawn() { } - internal void NetworkPreSpawn(ref NetworkManager networkManager) + internal void NetworkPreSpawn(ref NetworkManager networkManager, NetworkObject networkObject) { + m_NetworkObject = networkObject; + UpdateNetworkProperties(); + try { OnNetworkPreSpawn(ref networkManager); @@ -767,12 +778,10 @@ internal void NetworkPreSpawn(ref NetworkManager networkManager) internal void InternalOnNetworkSpawn() { IsSpawned = true; + // Initialize the NetworkVariables so they are accessible in OnNetworkSpawn; InitializeVariables(); UpdateNetworkProperties(); - } - internal void VisibleOnNetworkSpawn() - { try { OnNetworkSpawn(); @@ -782,9 +791,10 @@ internal void VisibleOnNetworkSpawn() Debug.LogException(e); } + // Initialize again in case the user's OnNetworkSpawn changed something InitializeVariables(); - if (NetworkObject.HasAuthority) + if (m_NetworkObject.HasAuthority) { // Since we just spawned the object and since user code might have modified their NetworkVariable, esp. // NetworkList, we need to mark the object as free of updates. @@ -872,11 +882,10 @@ public virtual void OnGainedOwnership() { } internal void InternalOnGainedOwnership() { - UpdateNetworkProperties(); // New owners need to assure any NetworkVariables they have write permissions // to are updated so the previous and original values are aligned with the // current value (primarily for collections). - if (OwnerClientId == NetworkManager.LocalClientId) + if (IsOwner) { UpdateNetworkVariableOnOwnershipChanged(); } @@ -907,12 +916,6 @@ internal void InternalOnOwnershipChanged(ulong previous, ulong current) /// public virtual void OnLostOwnership() { } - internal void InternalOnLostOwnership() - { - UpdateNetworkProperties(); - OnLostOwnership(); - } - /// /// Gets called when the parent NetworkObject of this NetworkBehaviour's NetworkObject has changed. /// @@ -1104,7 +1107,7 @@ internal void NetworkVariableUpdate(ulong targetClientId, bool forceSend = false // Getting these ahead of time actually improves performance var networkManager = NetworkManager; - var networkObject = NetworkObject; + var networkObject = m_NetworkObject; var behaviourIndex = networkObject.GetNetworkBehaviourOrderIndex(this); var messageManager = networkManager.MessageManager; var connectionManager = networkManager.ConnectionManager; @@ -1190,7 +1193,7 @@ private bool CouldHaveDirtyNetworkVariables() } // If it's dirty but can't be sent yet, we have to keep monitoring it until one of the // conditions blocking its send changes. - NetworkManager.BehaviourUpdater.AddForUpdate(NetworkObject); + NetworkManager.BehaviourUpdater.AddForUpdate(m_NetworkObject); } } @@ -1551,12 +1554,12 @@ internal virtual void InternalOnDestroy() public virtual void OnDestroy() { InternalOnDestroy(); - if (NetworkObject != null && NetworkObject.IsSpawned && IsSpawned) + if (m_NetworkObject != null && m_NetworkObject.IsSpawned && IsSpawned) { // If the associated NetworkObject is still spawned then this // NetworkBehaviour will be removed from the NetworkObject's // ChildNetworkBehaviours list. - NetworkObject.OnNetworkBehaviourDestroyed(this); + m_NetworkObject.OnNetworkBehaviourDestroyed(this); } // this seems odd to do here, but in fact especially in tests we can find ourselves @@ -1575,6 +1578,8 @@ public virtual void OnDestroy() { NetworkVariableFields[i].Dispose(); } + + m_NetworkObject = null; } } } diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs index 50101deeb7..090b63f60a 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs @@ -1999,46 +1999,44 @@ public void ChangeOwnership(ulong newOwnerClientId) NetworkManager.SpawnManager.ChangeOwnership(this, newOwnerClientId, HasAuthority); } - internal void InvokeBehaviourOnLostOwnership() + /// + /// Invokes the and events. + /// is called to update the ownership in-between the two callbacks. + /// + internal void InvokeBehaviourOnOwnershipChanged(ulong originalOwnerClientId, ulong newOwnerClientId) { - // Always update the ownership table in distributed authority mode - if (NetworkManager.DistributedAuthorityMode) - { - NetworkManager.SpawnManager.UpdateOwnershipTable(this, OwnerClientId, true); - } - else // Server already handles this earlier, hosts should ignore and only client owners should update - if (!NetworkManager.IsServer) - { - NetworkManager.SpawnManager.UpdateOwnershipTable(this, OwnerClientId, true); - } - for (int i = 0; i < ChildNetworkBehaviours.Count; i++) - { - ChildNetworkBehaviours[i].InternalOnLostOwnership(); - } - } + var distributedAuthorityMode = NetworkManager.DistributedAuthorityMode; + var isServer = NetworkManager.IsServer; + var isPreviousOwner = originalOwnerClientId == NetworkManager.LocalClientId; + var isNewOwner = newOwnerClientId == NetworkManager.LocalClientId; - internal void InvokeBehaviourOnGainedOwnership() - { - // Always update the ownership table in distributed authority mode - if (NetworkManager.DistributedAuthorityMode) - { - NetworkManager.SpawnManager.UpdateOwnershipTable(this, OwnerClientId); - } - else // Server already handles this earlier, hosts should ignore and only client owners should update - if (!NetworkManager.IsServer && NetworkManager.LocalClientId == OwnerClientId) + if (distributedAuthorityMode || isPreviousOwner) { - NetworkManager.SpawnManager.UpdateOwnershipTable(this, OwnerClientId); + NetworkManager.SpawnManager.UpdateOwnershipTable(this, originalOwnerClientId, true); } - for (int i = 0; i < ChildNetworkBehaviours.Count; i++) + foreach (var childBehaviour in ChildNetworkBehaviours) { - if (ChildNetworkBehaviours[i].gameObject.activeInHierarchy) + childBehaviour.UpdateNetworkProperties(); + if (distributedAuthorityMode || isServer || isPreviousOwner) { - ChildNetworkBehaviours[i].InternalOnGainedOwnership(); + childBehaviour.OnLostOwnership(); } - else + } + + NetworkManager.SpawnManager.UpdateOwnershipTable(this, newOwnerClientId); + + if (distributedAuthorityMode || isServer || isNewOwner) + { + foreach (var childBehaviour in ChildNetworkBehaviours) { - Debug.LogWarning($"{ChildNetworkBehaviours[i].gameObject.name} is disabled! Netcode for GameObjects does not support disabled NetworkBehaviours! The {ChildNetworkBehaviours[i].GetType().Name} component was skipped during ownership assignment!"); + if (!childBehaviour.gameObject.activeInHierarchy) + { + Debug.LogWarning($"{childBehaviour.gameObject.name} is disabled! Netcode for GameObjects does not support disabled NetworkBehaviours! The {childBehaviour.GetType().Name} component was skipped during ownership assignment!"); + continue; + } + + childBehaviour.InternalOnGainedOwnership(); } } } @@ -2541,7 +2539,7 @@ internal void InvokeBehaviourNetworkPreSpawn() { if (ChildNetworkBehaviours[i].gameObject.activeInHierarchy) { - ChildNetworkBehaviours[i].NetworkPreSpawn(ref networkManager); + ChildNetworkBehaviours[i].NetworkPreSpawn(ref networkManager, this); } } } @@ -2550,23 +2548,15 @@ internal void InvokeBehaviourNetworkSpawn() { NetworkManager.SpawnManager.UpdateOwnershipTable(this, OwnerClientId); - for (int i = 0; i < ChildNetworkBehaviours.Count; i++) + foreach (var childBehaviour in ChildNetworkBehaviours) { - if (ChildNetworkBehaviours[i].gameObject.activeInHierarchy) - { - ChildNetworkBehaviours[i].InternalOnNetworkSpawn(); - } - else - { - Debug.LogWarning($"{ChildNetworkBehaviours[i].gameObject.name} is disabled! Netcode for GameObjects does not support spawning disabled NetworkBehaviours! The {ChildNetworkBehaviours[i].GetType().Name} component was skipped during spawn!"); - } - } - for (int i = 0; i < ChildNetworkBehaviours.Count; i++) - { - if (ChildNetworkBehaviours[i].gameObject.activeInHierarchy) + if (!childBehaviour.gameObject.activeInHierarchy) { - ChildNetworkBehaviours[i].VisibleOnNetworkSpawn(); + Debug.LogWarning($"{childBehaviour.gameObject.name} is disabled! Netcode for GameObjects does not support spawning disabled NetworkBehaviours! The {childBehaviour.GetType().Name} component was skipped during spawn!"); + continue; } + + childBehaviour.InternalOnNetworkSpawn(); } } @@ -2636,31 +2626,39 @@ internal List ChildNetworkBehaviours var networkBehaviours = GetComponentsInChildren(true); for (int i = 0; i < networkBehaviours.Length; i++) { - if (networkBehaviours[i].NetworkObject == this) + // Find the first parent NetworkObject of this child + // if it's not ourselves, this childBehaviour belongs to a different NetworkObject. + var networkObj = networkBehaviours[i].GetComponentInParent(); + if (networkObj != this) + { + continue; + } + + // Set ourselves as the NetworkObject that this behaviour belongs to and add it to the child list + networkBehaviours[i].SetNetworkObject(this); + m_ChildNetworkBehaviours.Add(networkBehaviours[i]); + + var type = networkBehaviours[i].GetType(); + if (type == typeof(NetworkTransform) || type.IsInstanceOfType(typeof(NetworkTransform)) || type.IsSubclassOf(typeof(NetworkTransform))) { - m_ChildNetworkBehaviours.Add(networkBehaviours[i]); - var type = networkBehaviours[i].GetType(); - if (type == typeof(NetworkTransform) || type.IsInstanceOfType(typeof(NetworkTransform)) || type.IsSubclassOf(typeof(NetworkTransform))) + if (NetworkTransforms == null) { - if (NetworkTransforms == null) - { - NetworkTransforms = new List(); - } - var networkTransform = networkBehaviours[i] as NetworkTransform; - networkTransform.IsNested = i != 0 && networkTransform.gameObject != gameObject; - NetworkTransforms.Add(networkTransform); + NetworkTransforms = new List(); } + var networkTransform = networkBehaviours[i] as NetworkTransform; + networkTransform.IsNested = i != 0 && networkTransform.gameObject != gameObject; + NetworkTransforms.Add(networkTransform); + } #if COM_UNITY_MODULES_PHYSICS || COM_UNITY_MODULES_PHYSICS2D - else if (type.IsSubclassOf(typeof(NetworkRigidbodyBase))) + else if (type.IsSubclassOf(typeof(NetworkRigidbodyBase))) + { + if (NetworkRigidbodies == null) { - if (NetworkRigidbodies == null) - { - NetworkRigidbodies = new List(); - } - NetworkRigidbodies.Add(networkBehaviours[i] as NetworkRigidbodyBase); + NetworkRigidbodies = new List(); } -#endif + NetworkRigidbodies.Add(networkBehaviours[i] as NetworkRigidbodyBase); } +#endif } return m_ChildNetworkBehaviours; @@ -3284,7 +3282,13 @@ internal static NetworkObject AddSceneObject(in SceneObject sceneObject, FastBuf } // Spawn the NetworkObject - networkManager.SpawnManager.SpawnNetworkObjectLocally(networkObject, sceneObject, sceneObject.DestroyWithScene); + if (networkObject.IsSpawned) + { + throw new SpawnStateException($"[{networkObject.name}] Object-{networkObject.NetworkObjectId} is already spawned!"); + } + + // Do not invoke Pre spawn here (SynchronizeNetworkBehaviours needs to be invoked prior to this) + networkManager.SpawnManager.SpawnNetworkObjectLocallyCommon(networkObject, sceneObject.NetworkObjectId, sceneObject.IsSceneObject, sceneObject.IsPlayerObject, sceneObject.OwnerClientId, sceneObject.DestroyWithScene); if (sceneObject.SyncObservers) { diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ChangeOwnershipMessage.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ChangeOwnershipMessage.cs index c3b9fd559f..85cfce4f58 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ChangeOwnershipMessage.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ChangeOwnershipMessage.cs @@ -203,6 +203,7 @@ private void HandleOwnershipChange(ref NetworkContext context) { var networkManager = (NetworkManager)context.SystemOwner; var networkObject = networkManager.SpawnManager.SpawnedObjects[NetworkObjectId]; + var distributedAuthorityMode = networkManager.DistributedAuthorityMode; // Sanity check that we are not sending duplicated change ownership messages if (networkObject.OwnerClientId == OwnerClientId) @@ -215,49 +216,18 @@ private void HandleOwnershipChange(ref NetworkContext context) var originalOwner = networkObject.OwnerClientId; networkObject.OwnerClientId = OwnerClientId; - // If in distributed authority mode - if (networkManager.DistributedAuthorityMode) + if (distributedAuthorityMode) { networkObject.Ownership = (NetworkObject.OwnershipStatus)OwnershipFlags; - - networkObject.InvokeBehaviourOnLostOwnership(); - - // Always update the network properties in distributed authority mode - foreach (var child in networkObject.ChildNetworkBehaviours) - { - child.UpdateNetworkProperties(); - } - - networkObject.InvokeBehaviourOnGainedOwnership(); } - else - { - // We are initial owner - if (originalOwner == networkManager.LocalClientId) - { - networkObject.InvokeBehaviourOnLostOwnership(); - } - // For all other clients that are neither the former or current owner, update the behaviours' properties - if (OwnerClientId != networkManager.LocalClientId && originalOwner != networkManager.LocalClientId) - { - foreach (var childBehaviour in networkObject.ChildNetworkBehaviours) - { - childBehaviour.UpdateNetworkProperties(); - } - } - - // We are new owner - if (OwnerClientId == networkManager.LocalClientId) - { - networkObject.InvokeBehaviourOnGainedOwnership(); - } + // Notify lost ownership, update the ownership, then notify gained ownership for the network behaviours + networkObject.InvokeBehaviourOnOwnershipChanged(originalOwner, OwnerClientId); - if (originalOwner == networkManager.LocalClientId) - { - // Fully synchronize NetworkVariables with either read or write ownership permissions. - networkObject.SynchronizeOwnerNetworkVariables(originalOwner, networkObject.PreviousOwnerId); - } + if (!distributedAuthorityMode && originalOwner == networkManager.LocalClientId) + { + // Fully synchronize NetworkVariables with either read or write ownership permissions. + networkObject.SynchronizeOwnerNetworkVariables(originalOwner, networkObject.PreviousOwnerId); } // Always invoke ownership change notifications @@ -265,7 +235,7 @@ private void HandleOwnershipChange(ref NetworkContext context) // If this change was requested, then notify that the request was approved (doing this last so all ownership // changes have already been applied if the callback is invoked) - if (networkManager.DistributedAuthorityMode && networkManager.LocalClientId == OwnerClientId) + if (distributedAuthorityMode && networkManager.LocalClientId == OwnerClientId) { if (ChangeMessageType is ChangeType.RequestApproved) { diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index 69376d8ed4..af5025404b 100644 --- a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs @@ -551,14 +551,8 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool // Assign the new owner networkObject.OwnerClientId = clientId; - // Always notify locally on the server when ownership is lost - networkObject.InvokeBehaviourOnLostOwnership(); - - // Authority adds entries for all client ownership - UpdateOwnershipTable(networkObject, networkObject.OwnerClientId); - - // Always notify locally on the server when a new owner is assigned - networkObject.InvokeBehaviourOnGainedOwnership(); + // Notify lost ownership, update the ownership, then notify gained ownership for the network behaviours + networkObject.InvokeBehaviourOnOwnershipChanged(originalOwner, clientId); // If we are the original owner, then we want to synchronize owner read & write NetworkVariables. if (originalOwner == NetworkManager.LocalClientId) @@ -583,12 +577,6 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool // If we are connected to the CMB service or not the DAHost (i.e. pure DA-Clients only) if (NetworkManager.CMBServiceConnection || !NetworkManager.DAHost) { - // Always update the network properties in distributed authority mode for the client gaining ownership - for (int i = 0; i < networkObject.ChildNetworkBehaviours.Count; i++) - { - networkObject.ChildNetworkBehaviours[i].UpdateNetworkProperties(); - } - // Populate valid target client identifiers that should receive this change in ownership message. message.ClientIds = NetworkManager.ConnectedClientsIds.Where((c) => !IsObjectVisibilityPending(c, ref networkObject) && networkObject.IsNetworkVisibleTo(c)).ToArray(); message.ClientIdCount = message.ClientIds.Length; @@ -780,7 +768,7 @@ public NetworkObject InstantiateAndSpawn(NetworkObject networkPrefab, ulong owne /// internal NetworkObject InstantiateAndSpawnNoParameterChecks(NetworkObject networkPrefab, ulong ownerClientId = NetworkManager.ServerClientId, bool destroyWithScene = false, bool isPlayerObject = false, bool forceOverride = false, Vector3 position = default, Quaternion rotation = default) { - var networkObject = networkPrefab; + NetworkObject networkObject; // - Host and clients always instantiate the override if one exists. // - Server instantiates the original prefab unless: // -- forceOverride is set to true =or= @@ -821,66 +809,62 @@ internal NetworkObject InstantiateAndSpawnNoParameterChecks(NetworkObject networ /// internal NetworkObject GetNetworkObjectToSpawn(uint globalObjectIdHash, ulong ownerId, Vector3? position, Quaternion? rotation, bool isScenePlaced = false, byte[] instantiationData = null) { - NetworkObject networkObject = null; // If the prefab hash has a registered INetworkPrefabInstanceHandler derived class if (NetworkManager.PrefabHandler.ContainsHandler(globalObjectIdHash)) { // Let the handler spawn the NetworkObject - networkObject = NetworkManager.PrefabHandler.HandleNetworkPrefabSpawn(globalObjectIdHash, ownerId, position ?? default, rotation ?? default, instantiationData); - networkObject.NetworkManagerOwner = NetworkManager; + var prefabHandlerObject = NetworkManager.PrefabHandler.HandleNetworkPrefabSpawn(globalObjectIdHash, ownerId, position ?? default, rotation ?? default, instantiationData); + prefabHandlerObject.NetworkManagerOwner = NetworkManager; + return prefabHandlerObject; } - else + + // See if there is a valid registered NetworkPrefabOverrideLink associated with the provided prefabHash + var networkPrefabReference = (GameObject)null; + var inScenePlacedWithNoSceneManagement = !NetworkManager.NetworkConfig.EnableSceneManagement && isScenePlaced; + + if (NetworkManager.NetworkConfig.Prefabs.NetworkPrefabOverrideLinks.ContainsKey(globalObjectIdHash)) { - // See if there is a valid registered NetworkPrefabOverrideLink associated with the provided prefabHash - var networkPrefabReference = (GameObject)null; - var inScenePlacedWithNoSceneManagement = !NetworkManager.NetworkConfig.EnableSceneManagement && isScenePlaced; + var networkPrefab = NetworkManager.NetworkConfig.Prefabs.NetworkPrefabOverrideLinks[globalObjectIdHash]; - if (NetworkManager.NetworkConfig.Prefabs.NetworkPrefabOverrideLinks.ContainsKey(globalObjectIdHash)) + switch (networkPrefab.Override) { - var networkPrefab = NetworkManager.NetworkConfig.Prefabs.NetworkPrefabOverrideLinks[globalObjectIdHash]; - - switch (networkPrefab.Override) - { - default: - case NetworkPrefabOverride.None: - networkPrefabReference = networkPrefab.Prefab; - break; - case NetworkPrefabOverride.Hash: - case NetworkPrefabOverride.Prefab: + default: + case NetworkPrefabOverride.None: + networkPrefabReference = networkPrefab.Prefab; + break; + 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 + // 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) + if (inScenePlacedWithNoSceneManagement) { - // 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) - if (inScenePlacedWithNoSceneManagement) - { - networkPrefabReference = networkPrefab.SourcePrefabToOverride ? networkPrefab.SourcePrefabToOverride : networkPrefab.Prefab; - } - else - { - networkPrefabReference = NetworkManager.NetworkConfig.Prefabs.NetworkPrefabOverrideLinks[globalObjectIdHash].OverridingTargetPrefab; - } - break; + networkPrefabReference = networkPrefab.SourcePrefabToOverride ? networkPrefab.SourcePrefabToOverride : networkPrefab.Prefab; } - } + else + { + networkPrefabReference = NetworkManager.NetworkConfig.Prefabs.NetworkPrefabOverrideLinks[globalObjectIdHash].OverridingTargetPrefab; + } + break; + } } + } - // If not, then there is an issue (user possibly didn't register the prefab properly?) - if (networkPrefabReference == null) - { - if (NetworkLog.CurrentLogLevel <= LogLevel.Error) - { - NetworkLog.LogError($"Failed to create object locally. [{nameof(globalObjectIdHash)}={globalObjectIdHash}]. {nameof(NetworkPrefab)} could not be found. Is the prefab registered with {NetworkManager.name}?"); - } - } - else + // If not, then there is an issue (user possibly didn't register the prefab properly?) + if (networkPrefabReference == null) + { + if (NetworkLog.CurrentLogLevel <= LogLevel.Error) { - // Create prefab instance while applying any pre-assigned position and rotation values - networkObject = InstantiateNetworkPrefab(networkPrefabReference, globalObjectIdHash, position, rotation); + NetworkLog.LogError($"Failed to create object locally. [{nameof(globalObjectIdHash)}={globalObjectIdHash}]. {nameof(NetworkPrefab)} could not be found. Is the prefab registered with {NetworkManager.name}?"); } + return null; } - return networkObject; + + // Create prefab instance while applying any pre-assigned position and rotation values + return InstantiateNetworkPrefab(networkPrefabReference, globalObjectIdHash, position, rotation); } /// @@ -1078,6 +1062,7 @@ internal void SpawnNetworkObjectLocally(NetworkObject networkObject, ulong netwo } } // Invoke NetworkBehaviour.OnPreSpawn methods + networkObject.NetworkManagerOwner = NetworkManager; networkObject.InvokeBehaviourNetworkPreSpawn(); // DANGO-TODO: It would be nice to allow users to specify which clients are observers prior to spawning @@ -1118,39 +1103,11 @@ internal void SpawnNetworkObjectLocally(NetworkObject networkObject, ulong netwo } } } - SpawnNetworkObjectLocallyCommon(networkObject, networkId, sceneObject, playerObject, ownerClientId, destroyWithScene); - - // Invoke NetworkBehaviour.OnPostSpawn methods - networkObject.InvokeBehaviourNetworkPostSpawn(); - } - - /// - /// This is only invoked to instantiate a serialized NetworkObject via - /// - /// - /// - /// IMPORTANT: Pre spawn methods need to be invoked from within . - /// - internal void SpawnNetworkObjectLocally(NetworkObject networkObject, in NetworkObject.SceneObject sceneObject, bool destroyWithScene) - { - if (networkObject == null) - { - throw new ArgumentNullException(nameof(networkObject), "Cannot spawn null object"); - } - - if (networkObject.IsSpawned) - { - throw new SpawnStateException($"[{networkObject.name}] Object-{networkObject.NetworkObjectId} is already spawned!"); - } - - // Do not invoke Pre spawn here (SynchronizeNetworkBehaviours needs to be invoked prior to this) - SpawnNetworkObjectLocallyCommon(networkObject, sceneObject.NetworkObjectId, sceneObject.IsSceneObject, sceneObject.IsPlayerObject, sceneObject.OwnerClientId, destroyWithScene); - // It is ok to invoke NetworkBehaviour.OnPostSpawn methods - networkObject.InvokeBehaviourNetworkPostSpawn(); + SpawnNetworkObjectLocallyCommon(networkObject, networkId, sceneObject, playerObject, ownerClientId, destroyWithScene); } - private void SpawnNetworkObjectLocallyCommon(NetworkObject networkObject, ulong networkId, bool sceneObject, bool playerObject, ulong ownerClientId, bool destroyWithScene) + internal void SpawnNetworkObjectLocallyCommon(NetworkObject networkObject, ulong networkId, bool sceneObject, bool playerObject, ulong ownerClientId, bool destroyWithScene) { if (SpawnedObjects.ContainsKey(networkId)) { @@ -1255,6 +1212,9 @@ private void SpawnNetworkObjectLocallyCommon(NetworkObject networkObject, ulong { networkObject.PrefabGlobalObjectIdHash = networkObject.InScenePlacedSourceGlobalObjectIdHash; } + + // It is now ok to invoke NetworkBehaviour.OnPostSpawn methods + networkObject.InvokeBehaviourNetworkPostSpawn(); } internal Dictionary NetworkObjectsToSynchronizeSceneChanges = new Dictionary(); diff --git a/com.unity.netcode.gameobjects/Tests/Editor/NetworkBehaviourTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/NetworkBehaviourEditorTests.cs similarity index 76% rename from com.unity.netcode.gameobjects/Tests/Editor/NetworkBehaviourTests.cs rename to com.unity.netcode.gameobjects/Tests/Editor/NetworkBehaviourEditorTests.cs index 29c827923d..67bc886ae7 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/NetworkBehaviourTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Editor/NetworkBehaviourEditorTests.cs @@ -4,7 +4,7 @@ namespace Unity.Netcode.EditorTests { - internal class NetworkBehaviourTests + internal class NetworkBehaviourEditorTests { [Test] public void HasNetworkObjectTest() @@ -12,6 +12,9 @@ public void HasNetworkObjectTest() var gameObject = new GameObject(nameof(HasNetworkObjectTest)); var networkBehaviour = gameObject.AddComponent(); + // Ensure GetTypeName returns the correct value + Assert.AreEqual(nameof(EmptyNetworkBehaviour), networkBehaviour.__getTypeName()); + Assert.That(networkBehaviour.HasNetworkObject, Is.False); var networkObject = gameObject.AddComponent(); @@ -47,33 +50,33 @@ public void AccessNetworkObjectTest() } [Test] - public void GivenClassDerivesFromNetworkBehaviour_GetTypeNameReturnsCorrectValue() + public void AccessNetworkObjectTestInDerivedClassWithOverrideFunctions() { - var gameObject = new GameObject(nameof(GivenClassDerivesFromNetworkBehaviour_GetTypeNameReturnsCorrectValue)); - var networkBehaviour = gameObject.AddComponent(); - - Assert.AreEqual(nameof(EmptyNetworkBehaviour), networkBehaviour.__getTypeName()); - } - - [Test] - public void GivenClassDerivesFromNetworkBehaviourDerivedClass_GetTypeNameReturnsCorrectValue() - { - var gameObject = new GameObject(nameof(GivenClassDerivesFromNetworkBehaviourDerivedClass_GetTypeNameReturnsCorrectValue)); + var gameObject = new GameObject(nameof(AccessNetworkObjectTestInDerivedClassWithOverrideFunctions)); var networkBehaviour = gameObject.AddComponent(); Assert.AreEqual(nameof(DerivedNetworkBehaviour), networkBehaviour.__getTypeName()); + + var networkObject = gameObject.AddComponent(); + + Assert.That(networkBehaviour.NetworkObject, Is.EqualTo(networkObject)); + + Object.DestroyImmediate(networkObject); + + Assert.That(networkBehaviour.NetworkObject, Is.Null); + + // Cleanup + Object.DestroyImmediate(gameObject); } // Note: in order to repro https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/issues/1078 // this child class must be defined before its parent to assure it is processed first by ILPP internal class DerivedNetworkBehaviour : EmptyNetworkBehaviour { - } internal class EmptyNetworkBehaviour : NetworkBehaviour { - } } } diff --git a/com.unity.netcode.gameobjects/Tests/Editor/NetworkBehaviourTests.cs.meta b/com.unity.netcode.gameobjects/Tests/Editor/NetworkBehaviourEditorTests.cs.meta similarity index 100% rename from com.unity.netcode.gameobjects/Tests/Editor/NetworkBehaviourTests.cs.meta rename to com.unity.netcode.gameobjects/Tests/Editor/NetworkBehaviourEditorTests.cs.meta diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Unity.Netcode.Editor.Tests.asmdef b/com.unity.netcode.gameobjects/Tests/Editor/Unity.Netcode.Editor.Tests.asmdef index 976fa7be17..f45952ca0e 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Unity.Netcode.Editor.Tests.asmdef +++ b/com.unity.netcode.gameobjects/Tests/Editor/Unity.Netcode.Editor.Tests.asmdef @@ -12,7 +12,8 @@ "Unity.Networking.Transport", "Unity.Mathematics", "UnityEngine.TestRunner", - "UnityEditor.TestRunner" + "UnityEditor.TestRunner", + "Unity.Netcode.Runtime.Tests" ], "includePlatforms": [ "Editor" diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkBehaviourTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkBehaviourTests.cs new file mode 100644 index 0000000000..3d97aae216 --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkBehaviourTests.cs @@ -0,0 +1,63 @@ +using System.Collections; +using NUnit.Framework; +using Unity.Netcode.TestHelpers.Runtime; +using UnityEngine; +using UnityEngine.TestTools; + +namespace Unity.Netcode.RuntimeTests +{ + internal class NetworkBehaviourTests : NetcodeIntegrationTest + { + protected override int NumberOfClients => 1; + + private GameObject m_NetworkObjectPrefab; + + protected override void OnServerAndClientsCreated() + { + m_NetworkObjectPrefab = CreateNetworkObjectPrefab("NetworkObject"); + m_NetworkObjectPrefab.AddComponent(); + + base.OnServerAndClientsCreated(); + } + + [UnityTest] + public IEnumerator GetNetworkObjectAndNetworkBehaviourFromNetworkBehaviourTest() + { + var authority = GetAuthorityNetworkManager(); + var gameObject = SpawnObject(m_NetworkObjectPrefab, authority); + + yield return WaitForSpawnedOnAllOrTimeOut(gameObject); + AssertOnTimeout("Timed out waiting for NetworkBehaviour to spawn on all clients."); + + var networkBehaviour = gameObject.GetComponent(); + + Assert.That(networkBehaviour, Is.Not.Null); + var networkObject = gameObject.GetComponent(); + Assert.That(networkObject, Is.EqualTo(networkBehaviour.NetworkObject)); + + foreach (var manager in m_NetworkManagers) + { + Assert.True(manager.SpawnManager.SpawnedObjects.TryGetValue(networkObject.NetworkObjectId, out var localObject)); + + var localBehaviour = localObject.GetComponent(); + Assert.That(localBehaviour.NetworkObjectWithID(networkObject.NetworkObjectId), Is.EqualTo(localObject)); + Assert.That(localBehaviour.GetBehaviourAtId(networkBehaviour.NetworkBehaviourId), Is.EqualTo(localBehaviour)); + } + } + + internal class TestNetworkBehaviour : NetworkBehaviour + { + // Use protected class so it doesn't look unused, and so it's tested. + public NetworkBehaviour GetBehaviourAtId(ushort networkBehaviourId) + { + return GetNetworkBehaviour(networkBehaviourId); + } + + public NetworkObject NetworkObjectWithID(ulong networkObjectId) + { + return GetNetworkObject(networkObjectId); + } + } + } + +} diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkBehaviourTests.cs.meta b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkBehaviourTests.cs.meta new file mode 100644 index 0000000000..8e05b830f9 --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkBehaviourTests.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: ed87a422d8f2451aae8cde742fd26802 +timeCreated: 1758549972 \ No newline at end of file diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs index bb3f29bd7a..fedc6a05da 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs @@ -1939,7 +1939,19 @@ bool ValidateObjectSpawnedOnAllClients(StringBuilder errorLog) /// An for use in Unity coroutines. protected IEnumerator WaitForSpawnedOnAllOrTimeOut(NetworkObject networkObject, TimeoutHelper timeOutHelper = null) { - var networkObjectId = networkObject.GetComponent().NetworkObjectId; + var networkObjectId = networkObject.NetworkObjectId; + yield return WaitForSpawnedOnAllOrTimeOut(networkObjectId, timeOutHelper); + } + + /// + /// Waits until the given NetworkObject is spawned on all clients or a timeout occurs. + /// + /// The containing a to wait for. + /// An optional to control the timeout period. If null, the default timeout is used. + /// An for use in Unity coroutines. + protected IEnumerator WaitForSpawnedOnAllOrTimeOut(GameObject gameObject, TimeoutHelper timeOutHelper = null) + { + var networkObjectId = gameObject.GetComponent().NetworkObjectId; yield return WaitForSpawnedOnAllOrTimeOut(networkObjectId, timeOutHelper); }