diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index ed55793aba..573e77b99b 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -13,6 +13,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Changed +- Remove exceptions and replaced Debug usage by NetcodeLog on `NetworkSpawnManager`. (#3933) - Improve performance of `NetworkBehaviour`. (#3915) - Improve performance of `NetworkTransform`. (#3907) - Improve performance of `NetworkRigidbodyBase`. (#3906) @@ -26,6 +27,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed +- Early return when `ChangeOwnership` is called while not spawned. (#3933) - Fixed issue where attempts to use `NetworkLog` when there is no `NetworkManager` instance would result in an exception. (#3917) ### Security diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs index bd518b3048..9d8ef26d06 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs @@ -297,7 +297,7 @@ internal void OnValidate() if (globalId.identifierType != k_SceneObjectType) { // This should never happen, but in the event it does throw and error. - Debug.LogError($"[{gameObject.name}] is detected as an in-scene placed object but its identifier is of type {globalId.identifierType}! **Report this error**"); + NetworkLog.LogError($"[{gameObject.name}] is detected as an in-scene placed object but its identifier is of type {globalId.identifierType}! **Report this error**"); } // If this is a prefab instance, then we want to mark it as having been updated in order for the udpated GlobalObjectIdHash value to be saved. @@ -1778,7 +1778,10 @@ internal void SpawnInternal(bool destroyWithScene, ulong ownerClientId, bool pla { if (NetworkManagerOwner.LocalClient == null || !NetworkManagerOwner.IsConnectedClient || !NetworkManagerOwner.ConnectionManager.LocalClient.IsApproved) { - Debug.LogError($"Cannot spawn {name} until the client is fully connected to the session!"); + if (NetworkManagerOwner.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError($"Cannot spawn {name} until the client is fully connected to the session!"); + } return; } if (NetworkManagerOwner.NetworkConfig.EnableSceneManagement) @@ -1856,7 +1859,11 @@ public static NetworkObject InstantiateAndSpawn(GameObject networkPrefab, Networ var networkObject = networkPrefab.GetComponent(); if (networkObject == null) { - Debug.LogError($"The {nameof(NetworkPrefab)} {networkPrefab.name} does not have a {nameof(NetworkObject)} component!"); + if (networkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError($"The {nameof(NetworkPrefab)} {networkPrefab.name} does not have a {nameof(NetworkObject)} component!"); + } + return null; } return networkObject.InstantiateAndSpawn(networkManager, ownerClientId, destroyWithScene, isPlayerObject, forceOverride, position, rotation); @@ -1878,13 +1885,19 @@ public NetworkObject InstantiateAndSpawn(NetworkManager networkManager, ulong ow { if (networkManager == null) { - Debug.LogError(NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NetworkManagerNull]); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError(NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NetworkManagerNull]); + } return null; } if (!networkManager.IsListening) { - Debug.LogError(NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NoActiveSession]); + if (networkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError(NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NoActiveSession]); + } return null; } @@ -1892,20 +1905,29 @@ public NetworkObject InstantiateAndSpawn(NetworkManager networkManager, ulong ow // We only need to check for authority when running in client-server mode if (!networkManager.IsServer && !networkManager.DistributedAuthorityMode) { - Debug.LogError(NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NotAuthority]); + if (networkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError(NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NotAuthority]); + } return null; } if (networkManager.ShutdownInProgress) { - Debug.LogWarning(NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.InvokedWhenShuttingDown]); + if (networkManager.LogLevel <= LogLevel.Normal) + { + NetworkLog.LogWarning(NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.InvokedWhenShuttingDown]); + } return null; } // Verify it is actually a valid prefab if (!networkManager.NetworkConfig.Prefabs.Contains(gameObject)) { - Debug.LogError(NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NotRegisteredNetworkPrefab]); + if (networkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError(NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NotRegisteredNetworkPrefab]); + } return null; } @@ -2061,7 +2083,11 @@ internal void InvokeBehaviourOnOwnershipChanged(ulong originalOwnerClientId, ulo { if (!childBehaviour.gameObject.activeInHierarchy) { - Debug.LogWarning($"[{name}] {childBehaviour.gameObject.name} is disabled! Netcode for GameObjects does not support disabled NetworkBehaviours! The {childBehaviour.GetType().Name} component was skipped during ownership assignment!"); + if (NetworkManagerOwner.LogLevel <= LogLevel.Normal) + { + NetworkLog.LogWarning($"[{name}] {childBehaviour.gameObject.name} is disabled! Netcode for GameObjects does not support disabled NetworkBehaviours! The {childBehaviour.GetType().Name} component was skipped during ownership assignment!"); + } + continue; } @@ -2080,7 +2106,10 @@ internal void InvokeOwnershipChanged(ulong previous, ulong next) } else { - Debug.LogWarning($"[{name}] {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 (NetworkManagerOwner.LogLevel <= LogLevel.Normal) + { + NetworkLog.LogWarning($"[{name}] {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!"); + } } } } @@ -2291,7 +2320,7 @@ private void OnTransformParentChanged() return; } transform.parent = m_CachedParent; - if (NetworkManagerOwner.LogLevel <= LogLevel.Error) + if (networkManager.LogLevel <= LogLevel.Error) { NetworkLog.LogError($"[{name}] {nameof(networkManager)} is not listening, start a server or host before re-parenting."); } @@ -2311,7 +2340,7 @@ private void OnTransformParentChanged() else { transform.parent = m_CachedParent; - if (NetworkManagerOwner.LogLevel <= LogLevel.Error) + if (networkManager.LogLevel <= LogLevel.Error) { NetworkLog.LogErrorServer($"[{name}] {nameof(NetworkObject)} can only be re-parented after being spawned!"); } @@ -2326,9 +2355,9 @@ private void OnTransformParentChanged() if (!isParentingAuthority) { transform.parent = m_CachedParent; - if (networkManager.LogLevel <= LogLevel.Error) + if (NetworkManagerOwner.LogLevel <= LogLevel.Error) { - if (networkManager.DistributedAuthorityMode) + if (NetworkManagerOwner.DistributedAuthorityMode) { NetworkLog.LogError($"[{name}][Not Owner] Only the owner-authority of child {gameObject.name}'s {nameof(NetworkObject)} component can re-parent it!"); } @@ -2354,7 +2383,7 @@ private void OnTransformParentChanged() } return; } - else if (!parentObject.IsSpawned) + if (!parentObject.IsSpawned) { transform.parent = m_CachedParent; AuthorityAppliedParenting = false; @@ -2399,20 +2428,20 @@ private void OnTransformParentChanged() } // If we're not the server, we should tell the server about this parent change - if (!networkManager.IsServer) + if (!NetworkManagerOwner.IsServer) { // Don't send a message in DA mode if we're the only observers of this object (we're the only authority). - if (networkManager.DistributedAuthorityMode && Observers.Count <= 1) + if (NetworkManagerOwner.DistributedAuthorityMode && Observers.Count <= 1) { return; } - networkManager.ConnectionManager.SendMessage(ref message, MessageDeliveryType.DefaultDelivery, NetworkManager.ServerClientId); + NetworkManagerOwner.ConnectionManager.SendMessage(ref message, MessageDeliveryType.DefaultDelivery, NetworkManager.ServerClientId); return; } // Otherwise we are a Server (client-server or DAHost). Send to all observers - foreach (var clientId in networkManager.ConnectionManager.ConnectedClientIds) + foreach (var clientId in NetworkManagerOwner.ConnectionManager.ConnectedClientIds) { if (clientId == NetworkManager.ServerClientId) { @@ -2420,7 +2449,7 @@ private void OnTransformParentChanged() } if (Observers.Contains(clientId)) { - networkManager.ConnectionManager.SendMessage(ref message, MessageDeliveryType.DefaultDelivery, clientId); + NetworkManagerOwner.ConnectionManager.SendMessage(ref message, MessageDeliveryType.DefaultDelivery, clientId); } } } @@ -2494,10 +2523,10 @@ internal bool ApplyNetworkParenting(bool removeParent = false, bool ignoreNotSpa } } - // If we are removing the parent or our latest parent is not set, then remove the parent + // If we are removing the parent or our latest parent is not set, then remove the parent. // removeParent is only set when: // - The server-side NetworkObject.OnTransformParentChanged is invoked and the parent is being removed - // - The client-side when handling a ParentSyncMessage + // - The client-side is handling a ParentSyncMessage // When clients are synchronizing only the m_LatestParent.HasValue will not have a value if there is no parent // or a parent was removed prior to the client connecting (i.e. in-scene placed NetworkObjects) if (removeParent || !m_LatestParent.HasValue) @@ -2597,7 +2626,10 @@ internal void InvokeBehaviourNetworkSpawn() { if (!childBehaviour.gameObject.activeInHierarchy) { - Debug.LogWarning($"{GenerateDisabledNetworkBehaviourWarning(childBehaviour)}"); + if (NetworkManager.LogLevel <= LogLevel.Normal) + { + NetworkLog.LogWarning($"{GenerateDisabledNetworkBehaviourWarning(childBehaviour)}"); + } continue; } childBehaviour.NetworkSpawn(); @@ -3356,7 +3388,11 @@ internal static NetworkObject Deserialize(in SerializedObject serializedObject, // Ensure that the buffer is completely reset if (reader.Position != endOfSynchronizationData) { - Debug.LogWarning($"[Size mismatch] Expected: {endOfSynchronizationData} Currently At: {reader.Position}!"); + if (networkManager.LogLevel <= LogLevel.Normal) + { + NetworkLog.LogWarning($"[Size mismatch] Expected: {endOfSynchronizationData} Currently At: {reader.Position}!"); + } + reader.Seek(endOfSynchronizationData); } } diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index bf4eacbc1b..928133e5f0 100644 --- a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs @@ -75,11 +75,11 @@ private void AddPlayerObject(NetworkObject playerObject) { if (!playerObject.IsPlayerObject) { - if (NetworkManager.LogLevel == LogLevel.Normal) + if (NetworkManager.LogLevel <= LogLevel.Normal) { NetworkLog.LogError($"Attempting to register a {nameof(NetworkObject)} as a player object but {nameof(NetworkObject.IsPlayerObject)} is not set!"); - return; } + return; } var cmbService = NetworkManager.CMBServiceConnection; @@ -148,11 +148,11 @@ private void RemovePlayerObject(NetworkObject playerObject, bool destroyingObjec { if (!playerObject.IsPlayerObject) { - if (NetworkManager.LogLevel == LogLevel.Normal) + if (NetworkManager.LogLevel <= LogLevel.Normal) { NetworkLog.LogError($"Attempting to deregister a {nameof(NetworkObject)} as a player object but {nameof(NetworkObject.IsPlayerObject)} is not set!"); - return; } + return; } playerObject.IsPlayerObject = false; m_PlayerObjects.Remove(playerObject); @@ -211,8 +211,10 @@ internal bool RemoveObjectFromShowingTo(NetworkObject networkObject, ulong clien // probably overkill, but deals with multiple entries while (ObjectsToShowToClient[clientId].Contains(networkObject)) { - Debug.LogWarning( - "Object was shown and hidden from the same client in the same Network frame. As a result, the client will _not_ receive a NetworkSpawn"); + if (NetworkManager.LogLevel > LogLevel.Normal) + { + NetworkLog.LogWarning($"Object was shown and hidden from the same client in the same Network frame. As a result, the client will _not_ receive a NetworkSpawn"); + } ObjectsToShowToClient[clientId].Remove(networkObject); ret = true; } @@ -277,7 +279,11 @@ internal void UpdateOwnershipTable(NetworkObject networkObject, ulong newOwner, else { // Really, as long as UpdateOwnershipTable is invoked when ownership is gained or lost this should never happen - throw new Exception($"Client-ID {previousOwner} had a partial {nameof(m_ObjectToOwnershipTable)} entry! Potentially corrupted {nameof(OwnershipToObjectsTable)}?"); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError($"Client-ID {previousOwner} had a partial {nameof(m_ObjectToOwnershipTable)} entry! Potentially corrupted {nameof(OwnershipToObjectsTable)}?"); + } + return; } } @@ -375,7 +381,11 @@ public NetworkObject GetPlayerNetworkObject(ulong clientId) { if (!NetworkManager.IsServer && NetworkManager.LocalClientId != clientId) { - throw new NotServerException("Only the server can find player objects from other clients."); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogErrorServer($"{clientId} Only the server can find player objects from other clients."); + } + return null; } if (TryGetNetworkClient(clientId, out NetworkClient networkClient)) { @@ -389,7 +399,6 @@ public NetworkObject GetPlayerNetworkObject(ulong clientId) return m_PlayerObjectsTable[clientId].First(); } } - return null; } @@ -432,7 +441,10 @@ internal void RemoveOwnership(NetworkObject networkObject) { if (NetworkManager.DistributedAuthorityMode && !NetworkManager.ShutdownInProgress) { - Debug.LogError($"Removing ownership is invalid in Distributed Authority Mode. Use {nameof(ChangeOwnership)} instead."); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError($"Removing ownership is invalid in Distributed Authority Mode. Use {nameof(ChangeOwnership)} instead."); + } return; } ChangeOwnership(networkObject, NetworkManager.ServerClientId, true); @@ -443,11 +455,20 @@ internal void RemoveOwnership(NetworkObject networkObject) internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool isAuthorized, bool isRequestApproval = false) { + if (!networkObject.IsSpawned) + { + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogErrorServer($"[{networkObject.name}] Cannot change ownership while not spawned."); + } + return; + } + if (clientId == networkObject.OwnerClientId) { if (NetworkManager.LogLevel <= LogLevel.Developer) { - Debug.LogWarning($"[{nameof(NetworkSpawnManager)}][{nameof(ChangeOwnership)}] Attempting to change ownership to Client-{clientId} when the owner is already {networkObject.OwnerClientId}! (Ignoring)"); + NetworkLog.LogWarning($"[{nameof(NetworkSpawnManager)}][{nameof(ChangeOwnership)}] Attempting to change ownership to Client-{clientId} when the owner is already {networkObject.OwnerClientId}! (Ignoring)"); } return; } @@ -526,17 +547,16 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool } else if (!isAuthorized) { - throw new NotServerException("Only the server can change ownership"); - } - - if (!networkObject.IsSpawned) - { - throw new SpawnStateException("Object is not spawned"); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError("Only the server can change ownership! (ignoring)"); + } + return; } if (!networkObject.Observers.Contains(clientId)) { - if (NetworkManager.LogLevel == LogLevel.Developer) + if (NetworkManager.LogLevel <= LogLevel.Developer) { NetworkLog.LogWarningServer($"[Invalid Owner] Cannot send Ownership change as client-{clientId} cannot see {networkObject.name}! Use {nameof(NetworkObject.NetworkShow)} first."); } @@ -737,7 +757,10 @@ public NetworkObject InstantiateAndSpawn(NetworkObject networkPrefab, ulong owne { if (networkPrefab == null) { - Debug.LogError(InstantiateAndSpawnErrors[InstantiateAndSpawnErrorTypes.NetworkPrefabNull]); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError(InstantiateAndSpawnErrors[InstantiateAndSpawnErrorTypes.NetworkPrefabNull]); + } return null; } @@ -745,20 +768,29 @@ public NetworkObject InstantiateAndSpawn(NetworkObject networkPrefab, ulong owne // We only need to check for authority when running in client-server mode if (!NetworkManager.IsServer && !NetworkManager.DistributedAuthorityMode) { - Debug.LogError(InstantiateAndSpawnErrors[InstantiateAndSpawnErrorTypes.NotAuthority]); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError(InstantiateAndSpawnErrors[InstantiateAndSpawnErrorTypes.NotAuthority]); + } return null; } if (NetworkManager.ShutdownInProgress) { - Debug.LogWarning(InstantiateAndSpawnErrors[InstantiateAndSpawnErrorTypes.InvokedWhenShuttingDown]); + if (NetworkManager.LogLevel <= LogLevel.Normal) + { + NetworkLog.LogWarning(InstantiateAndSpawnErrors[InstantiateAndSpawnErrorTypes.InvokedWhenShuttingDown]); + } return null; } // Verify it is actually a valid prefab if (!NetworkManager.NetworkConfig.Prefabs.Contains(networkPrefab.gameObject)) { - Debug.LogError(InstantiateAndSpawnErrors[InstantiateAndSpawnErrorTypes.NotRegisteredNetworkPrefab]); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError(InstantiateAndSpawnErrors[InstantiateAndSpawnErrorTypes.NotRegisteredNetworkPrefab]); + } return null; } @@ -787,7 +819,10 @@ internal NetworkObject InstantiateAndSpawnNoParameterChecks(NetworkObject networ if (networkObject == null) { - Debug.LogError($"Failed to instantiate and spawn {networkPrefab.name}!"); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError($"Failed to instantiate and spawn {networkPrefab.name}!"); + } return null; } @@ -1043,7 +1078,10 @@ internal void AuthorityLocalSpawn([NotNull] NetworkObject networkObject, ulong n { if (networkObject.IsSpawned) { - Debug.LogError($"{networkObject.name} is already spawned!"); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError($"{networkObject.name} is already spawned!"); + } return; } @@ -1052,7 +1090,10 @@ internal void AuthorityLocalSpawn([NotNull] NetworkObject networkObject, ulong n var networkObjectChildren = networkObject.GetComponentsInChildren(); if (networkObjectChildren.Length > 1) { - Debug.LogError("Spawning NetworkObjects with nested NetworkObjects is only supported for scene objects. Child NetworkObjects will not be spawned over the network!"); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError("Spawning NetworkObjects with nested NetworkObjects is only supported for scene objects. Child NetworkObjects will not be spawned over the network!"); + } } } @@ -1095,7 +1136,10 @@ internal void AuthorityLocalSpawn([NotNull] NetworkObject networkObject, ulong n // Itentionally checking as opposed to just assigning in order to generate notification. if (!networkObject.Observers.Contains(ownerClientId)) { - Debug.LogError($"Client-{ownerClientId} is the owner of {networkObject.name} but is not an observer! Adding owner, but there is a bug in observer synchronization!"); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError($"Client-{ownerClientId} is the owner of {networkObject.name} but is not an observer! Adding owner, but there is a bug in observer synchronization!"); + } networkObject.AddObserver(ownerClientId); } } @@ -1121,7 +1165,10 @@ internal void NonAuthorityLocalSpawn([NotNull] NetworkObject networkObject, in N { if (networkObject.IsSpawned) { - Debug.LogError($"[{networkObject.name}] Object-{networkObject.NetworkObjectId} is already spawned!"); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError($"[{networkObject.name}] Object-{networkObject.NetworkObjectId} is already spawned!"); + } return; } @@ -1139,12 +1186,18 @@ internal void SpawnNetworkObjectLocallyCommon(NetworkObject networkObject, ulong { if (networkObject.NetworkManagerOwner == null) { - Debug.LogError("NetworkManagerOwner should not be null!"); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError($"{networkObject.name}'s NetworkManagerOwner should not be null!"); + } } if (SpawnedObjects.ContainsKey(networkId)) { - Debug.LogWarning($"[{NetworkManager.name}] Trying to spawn {networkObject.name} with a {nameof(NetworkObject.NetworkObjectId)} of {networkId} but it is already in the spawned list!"); + if (NetworkManager.LogLevel <= LogLevel.Normal) + { + NetworkLog.LogWarning($"[{NetworkManager.name}] Trying to spawn {networkObject.name} with a {nameof(NetworkObject.NetworkObjectId)} of {networkId} but it is already in the spawned list!"); + } return; } @@ -1311,7 +1364,11 @@ internal void SendSpawnCallForObserverUpdate(ulong[] newObservers, NetworkObject { if (!NetworkManager.DistributedAuthorityMode) { - throw new Exception("[SendSpawnCallForObserverUpdate] Invoking a distributed authority only method when distributed authority is not enabled!"); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogErrorServer($"[SendSpawnCallForObserverUpdate] Invoking a distributed authority only method on {networkObject.name} when distributed authority is not enabled!"); + } + return; } var message = new CreateObjectMessage @@ -1352,7 +1409,10 @@ internal void DespawnObject(NetworkObject networkObject, bool destroyObject = fa { if (!NetworkManager.IsServer && !NetworkManager.DistributedAuthorityMode) { - NetworkLog.LogErrorServer("Only server can despawn objects"); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogErrorServer("Only server can despawn objects"); + } return; } @@ -1360,7 +1420,10 @@ internal void DespawnObject(NetworkObject networkObject, bool destroyObject = fa { if (!NetworkManager.DAHost || NetworkManager.DAHost && !authorityOverride) { - NetworkLog.LogErrorServer($"In distributed authority mode, only the owner of the NetworkObject can despawn it! Local Client is ({NetworkManager.LocalClientId}) while the owner is ({networkObject.OwnerClientId})"); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogErrorServer($"In distributed authority mode, only the owner of the NetworkObject can despawn it! Local Client is ({NetworkManager.LocalClientId}) while the owner is ({networkObject.OwnerClientId})"); + } return; } } @@ -1536,7 +1599,10 @@ internal void OnDespawnNonAuthorityObject([NotNull] NetworkObject networkObject, { if (networkObject.HasAuthority) { - NetworkLog.LogError($"OnDespawnNonAuthorityObject called on object {networkObject.NetworkObjectId} when is current client {NetworkManager.LocalClientId} has authority on this object."); + if (NetworkManager.LogLevel <= LogLevel.Error) + { + NetworkLog.LogError($"[OnDespawnNonAuthorityObject] called on object {networkObject.NetworkObjectId} when its current client {NetworkManager.LocalClientId} has authority on this object."); + } } if (networkObject.IsSceneObject == false) @@ -1564,7 +1630,10 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec // We have to do this check first as subsequent checks assume we can access NetworkObjectId. if (!networkObject) { - NetworkLog.LogWarning("Trying to destroy network object but it is null"); + if (NetworkManager.LogLevel <= LogLevel.Normal) + { + NetworkLog.LogWarning("Trying to destroy network object but it is null"); + } return; } @@ -1573,7 +1642,10 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec { if (!NetworkManager.ShutdownInProgress && !NetworkManager.SceneManager.IsSceneEventInProgress()) { - NetworkLog.LogWarning($"Trying to destroy object {networkObject.NetworkObjectId} but it doesn't seem to exist anymore!"); + if (NetworkManager.LogLevel <= LogLevel.Normal) + { + NetworkLog.LogWarning($"Trying to destroy object {networkObject.NetworkObjectId} but it doesn't seem to exist anymore!"); + } } return; } @@ -1587,7 +1659,10 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec { if (destroyGameObject && networkObject.IsSceneObject == true && !NetworkManager.SceneManager.IsSceneUnloading(networkObject)) { - NetworkLog.LogWarning("Destroying in-scene network objects can lead to unexpected behavior. It is recommended to use NetworkObject.Despawn(false) instead."); + if (NetworkManager.LogLevel <= LogLevel.Normal) + { + NetworkLog.LogWarning("Destroying in-scene network objects can lead to unexpected behavior. It is recommended to use NetworkObject.Despawn(false) instead."); + } } // Get all child NetworkObjects @@ -1771,9 +1846,9 @@ internal void HandleNetworkObjectShow(bool forceSend = false) return; } - // In distributed authority mode, we send a single message that is broadcasted to all clients - // that will be shown the object (i.e. 1 message to service that then broadcasts that to the - // targeted clients). When using a DAHost, we skip this and send like we do in client-server + // In distributed authority mode, we send a single message to the service, + // which then broadcasts it to all clients that should see the object. + // When using a DAHost, we skip this and send using the client-server approach. if (isDistributedAuthorityClient) { var behaviourUpdater = NetworkManager.BehaviourUpdater; @@ -1781,6 +1856,8 @@ internal void HandleNetworkObjectShow(bool forceSend = false) { if (entry.Key != null && entry.Key.IsSpawned) { + // Try catch due to ensure that if anything throws we keep processing the list. + // This can throw if the GameObject was destroyed on this frame. try { // Always push the most recent deltas when showing a NetworkObject @@ -1810,6 +1887,8 @@ internal void HandleNetworkObjectShow(bool forceSend = false) { if (networkObject != null && networkObject.IsSpawned) { + // Try catch due to ensure that if anything throws we keep processing the list. + // This can throw if the GameObject was destroyed on this frame. try { if (forceSend) @@ -2030,7 +2109,10 @@ internal void DistributeNetworkObjects(ulong clientId) } if (!child.IsOwnershipDistributable || !child.IsOwnershipTransferable) { - NetworkLog.LogWarning($"Sibling {child.name} of root parent {ownerList.Value[i].name} is neither transferable or distributable! Object distribution skipped and could lead to a potentially un-owned or owner-mismatched {nameof(NetworkObject)}!"); + if (NetworkManager.LogLevel <= LogLevel.Normal) + { + NetworkLog.LogWarning($"Sibling {child.name} of root parent {ownerList.Value[i].name} is neither transferable or distributable! Object distribution skipped and could lead to a potentially un-owned or owner-mismatched {nameof(NetworkObject)}!"); + } continue; } // Transfer ownership of all distributable =or= transferable children with the same owner to the same client to preserve the sibling ownership tree. @@ -2194,12 +2276,6 @@ internal void NotifyNetworkObjectsSynchronized() /// internal void ShowHiddenObjectsToNewlyJoinedClient(ulong newClientId) { - if (NetworkManager == null || NetworkManager.ShutdownInProgress && NetworkManager.LogLevel <= LogLevel.Developer) - { - Debug.LogWarning($"[Internal Error] {nameof(ShowHiddenObjectsToNewlyJoinedClient)} invoked while shutdown is in progress!"); - return; - } - if (!NetworkManager.DistributedAuthorityMode) { Debug.LogError($"[Internal Error] {nameof(ShowHiddenObjectsToNewlyJoinedClient)} should only be invoked when using a distributed authority network topology!"); @@ -2222,7 +2298,7 @@ internal void ShowHiddenObjectsToNewlyJoinedClient(ulong newClientId) if (NetworkManager.LogLevel <= LogLevel.Developer) { // Track if there is some other location where the client is being added to the observers list when the object is hidden from the session owner - Debug.LogWarning($"[{networkObject.name}] Has new client as an observer but it is hidden from the session owner!"); + NetworkLog.LogWarning($"[{networkObject.name}] Has new client as an observer but it is hidden from the session owner!"); } // For now, remove the client (impossible for the new client to have an instance since the session owner doesn't) to make sure newly added // code to handle this edge case works. @@ -2235,12 +2311,6 @@ internal void ShowHiddenObjectsToNewlyJoinedClient(ulong newClientId) internal void SynchronizeObjectsToNewlyJoinedClient(ulong newClientId) { - if (NetworkManager == null || NetworkManager.ShutdownInProgress && NetworkManager.LogLevel <= LogLevel.Developer) - { - Debug.LogWarning($"[Internal Error] {nameof(SynchronizeObjectsToNewlyJoinedClient)} invoked while shutdown is in progress!"); - return; - } - if (!NetworkManager.DistributedAuthorityMode) { Debug.LogError($"[Internal Error] {nameof(SynchronizeObjectsToNewlyJoinedClient)} should only be invoked when using a distributed authority network topology!"); diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkSpawnManagerTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkSpawnManagerTests.cs index 3039ea298c..a0d51696bb 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkSpawnManagerTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkSpawnManagerTests.cs @@ -61,10 +61,9 @@ public void TestClientCantAccessServerPlayer() return; } // client can't access server player - Assert.Throws(() => - { - m_ClientNetworkManagers[0].SpawnManager.GetPlayerNetworkObject(serverSideClientId); - }); + string expectedLog = $"[Netcode-Server Sender=0] {serverSideClientId} Only the server can find player objects from other clients."; + LogAssert.Expect(UnityEngine.LogType.Error, expectedLog); + m_ClientNetworkManagers[0].SpawnManager.GetPlayerNetworkObject(serverSideClientId); } [Test] @@ -100,10 +99,9 @@ public void TestClientCantAccessOtherPlayer() } // client can't access other player - Assert.Throws(() => - { - m_ClientNetworkManagers[0].SpawnManager.GetPlayerNetworkObject(otherClientSideClientId); - }); + string expectedLog = $"[Netcode-Server Sender=0] {otherClientSideClientId} Only the server can find player objects from other clients."; + LogAssert.Expect(UnityEngine.LogType.Error, expectedLog); + m_ClientNetworkManagers[0].SpawnManager.GetPlayerNetworkObject(otherClientSideClientId); } [Test] diff --git a/testproject/Assets/Tests/Runtime/PrefabExtendedTests.cs b/testproject/Assets/Tests/Runtime/PrefabExtendedTests.cs index 665590d342..d0f7e294cb 100644 --- a/testproject/Assets/Tests/Runtime/PrefabExtendedTests.cs +++ b/testproject/Assets/Tests/Runtime/PrefabExtendedTests.cs @@ -284,7 +284,7 @@ public IEnumerator TestPrefabsSpawning([Values] InstantiateAndSpawnMethods insta if (instantiateAndSpawnType != InstantiateAndSpawnMethods.Manual) { - LogAssert.Expect(LogType.Error, NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NotAuthority]); + LogAssert.Expect(LogType.Error, $"[Netcode] {NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NotAuthority]}"); InstantiateAndSpawn(m_ObjectsToSpawn[0], instantiateAndSpawnType, true); } } @@ -314,29 +314,29 @@ public IEnumerator TestsInstantiateAndSpawnErrors([Values] InstantiateAndSpawnMe yield return WaitForConditionOrTimeOut(ValidateAllClientsSpawnedObjects); AssertOnTimeout($"[First Stage] Validating spawned objects faild with the following error: {m_ErrorLog}"); - LogAssert.Expect(LogType.Error, NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NotRegisteredNetworkPrefab]); + LogAssert.Expect(LogType.Error, $"[Netcode] {NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NotRegisteredNetworkPrefab]}"); InstantiateAndSpawn(m_ServerSpawnedObjects[0], instantiateAndSpawnType); // The Network Prefab is null error can only happen when invoking from NetworkSpawnManager if (instantiateAndSpawnType == InstantiateAndSpawnMethods.SpawnManager) { - LogAssert.Expect(LogType.Error, NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NetworkPrefabNull]); + LogAssert.Expect(LogType.Error, $"[Netcode] {NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NetworkPrefabNull]}"); InstantiateAndSpawn(null, instantiateAndSpawnType); } else { // The NetworkManager is null error can only happen when invoking from Network Prefab - LogAssert.Expect(LogType.Error, NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NetworkManagerNull]); + LogAssert.Expect(LogType.Error, $"[Netcode] {NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NetworkManagerNull]}"); InstantiateAndSpawn(m_ObjectsToSpawn[0], instantiateAndSpawnType, false, true); } m_ServerNetworkManager.Shutdown(); - LogAssert.Expect(LogType.Warning, NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.InvokedWhenShuttingDown]); + LogAssert.Expect(LogType.Warning, $"[Netcode] {NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.InvokedWhenShuttingDown]}"); InstantiateAndSpawn(m_ObjectsToSpawn[0], instantiateAndSpawnType); // The not listening error can only happen when trying to instantiate and spawn on a Network Prefab if (instantiateAndSpawnType == InstantiateAndSpawnMethods.NetworkObject) { - LogAssert.Expect(LogType.Error, NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NoActiveSession]); + LogAssert.Expect(LogType.Error, $"[Netcode] {NetworkSpawnManager.InstantiateAndSpawnErrors[NetworkSpawnManager.InstantiateAndSpawnErrorTypes.NoActiveSession]}"); yield return WaitForConditionOrTimeOut(() => !m_ServerNetworkManager.IsListening); InstantiateAndSpawn(m_ObjectsToSpawn[0], instantiateAndSpawnType); }