diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index ef22703a5d..12accde27a 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -12,6 +12,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed +- Fixed exception being thrown when a `GameObject` with an associated `NetworkTransform` is disabled. (#3243) - Fixed `NetworkObject.DeferDespawn` to respect the `DestroyGameObject` parameter. (#3219) - Changed the `NetworkTimeSystem.Sync` method to use half RTT to calculate the desired local time offset as opposed to the full RTT. (#3212) - Fixed issue where a spawned `NetworkObject` that was registered with a prefab handler and owned by a client would invoke destroy more than once on the host-server side if the client disconnected while the `NetworkObject` was still spawned. (#3200) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkTransformMessage.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkTransformMessage.cs index cf4013f469..cb01fcf7b4 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkTransformMessage.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkTransformMessage.cs @@ -71,8 +71,21 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int if (isSpawnedLocally) { networkObject = networkManager.SpawnManager.SpawnedObjects[networkObjectId]; + if (networkObject.ChildNetworkBehaviours.Count <= networkBehaviourId || networkObject.ChildNetworkBehaviours[networkBehaviourId] == null) + { + Debug.LogError($"[{nameof(NetworkTransformMessage)}][Invalid] Targeted {nameof(NetworkTransform)}, {nameof(NetworkBehaviour.NetworkBehaviourId)} ({networkBehaviourId}), does not exist! Make sure you are not spawning {nameof(NetworkObject)}s with disabled {nameof(GameObject)}s that have {nameof(NetworkBehaviour)} components on them."); + return false; + } + // Get the target NetworkTransform - NetworkTransform = networkObject.ChildNetworkBehaviours[networkBehaviourId] as NetworkTransform; + var transform = networkObject.ChildNetworkBehaviours[networkBehaviourId] as NetworkTransform; + if (transform == null) + { + Debug.LogError($"[{nameof(NetworkTransformMessage)}][Invalid] Targeted {nameof(NetworkTransform)}, {nameof(NetworkBehaviour.NetworkBehaviourId)} ({networkBehaviourId}), does not exist! Make sure you are not spawning {nameof(NetworkObject)}s with disabled {nameof(GameObject)}s that have {nameof(NetworkBehaviour)} components on them."); + return false; + } + + NetworkTransform = transform; isServerAuthoritative = NetworkTransform.IsServerAuthoritative(); ownerAuthoritativeServerSide = !isServerAuthoritative && networkManager.IsServer; @@ -81,8 +94,20 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int } else { - // Deserialize the state - reader.ReadNetworkSerializableInPlace(ref State); + ownerAuthoritativeServerSide = networkManager.DAHost; + // If we are the DAHost and the NetworkObject is hidden from the host we still need to forward this message. + if (ownerAuthoritativeServerSide) + { + // We need to deserialize the state to our local State property so we can extract the reliability used. + reader.ReadNetworkSerializableInPlace(ref State); + // Fall through to act like a proxy for this message. + } + else + { + // Otherwise we can error out because we either shouldn't be receiving this message. + Debug.LogError($"[{nameof(NetworkTransformMessage)}][Invalid] Target NetworkObject ({networkObjectId}) does not exist!"); + return false; + } } unsafe @@ -106,12 +131,6 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int ByteUnpacker.ReadValueBitPacked(reader, out targetId); targetIds[i] = targetId; } - - if (!isSpawnedLocally) - { - // If we are the DAHost and the NetworkObject is hidden from the host we still need to forward this message - ownerAuthoritativeServerSide = networkManager.DAHost && !isSpawnedLocally; - } } var ownerClientId = (ulong)0; @@ -132,7 +151,10 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int ownerClientId = context.SenderId; } - var networkDelivery = State.IsReliableStateUpdate() ? NetworkDelivery.ReliableSequenced : NetworkDelivery.UnreliableSequenced; + // Depending upon whether it is spawned locally or not, get the deserialized state + var stateToUse = NetworkTransform != null ? NetworkTransform.InboundState : State; + // Determine the reliability used to send the message + var networkDelivery = stateToUse.IsReliableStateUpdate() ? NetworkDelivery.ReliableSequenced : NetworkDelivery.UnreliableSequenced; // Forward the state update if there are any remote clients to foward it to if (networkManager.ConnectionManager.ConnectedClientsList.Count > (networkManager.IsHost ? 2 : 1)) @@ -160,7 +182,6 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int { continue; } - networkManager.MessageManager.SendMessage(ref currentMessage, networkDelivery, clientId); } // Dispose of the reader used for forwarding diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformErrorTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformErrorTests.cs new file mode 100644 index 0000000000..d70984062e --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformErrorTests.cs @@ -0,0 +1,140 @@ +using System.Collections; +using Unity.Netcode.Components; +using Unity.Netcode.TestHelpers.Runtime; +using UnityEngine; +using UnityEngine.TestTools; + + +namespace Unity.Netcode.RuntimeTests +{ + internal class NetworkTransformErrorTests : NetcodeIntegrationTest + { + protected override int NumberOfClients => 1; + + private GameObject m_AuthorityPrefab; + private GameObject m_NonAuthorityPrefab; + + private HostAndClientPrefabHandler m_HostAndClientPrefabHandler; + + public class EmptyNetworkBehaviour : NetworkBehaviour { } + + /// + /// PrefabHandler that tracks and separates the client GameObject from the host GameObject. + /// Allows independent management of client and host game world while still instantiating NetworkObjects as expected. + /// + private class HostAndClientPrefabHandler : INetworkPrefabInstanceHandler + { + /// + /// The registered prefab is the prefab the networking stack is instantiated with. + /// Registering the prefab simulates the prefab that exists on the authority. + /// + private readonly GameObject m_RegisteredPrefab; + + /// + /// Mocks the registered prefab changing on the non-authority after registration. + /// Allows testing situations mismatched GameObject state between the authority and non-authority. + /// + private readonly GameObject m_InstantiatedPrefab; + + public HostAndClientPrefabHandler(GameObject authorityPrefab, GameObject nonAuthorityPrefab) + { + m_RegisteredPrefab = authorityPrefab; + m_InstantiatedPrefab = nonAuthorityPrefab; + } + + /// + /// Returns the prefab that will mock the instantiated prefab not matching the registered prefab + /// + public NetworkObject Instantiate(ulong ownerClientId, Vector3 position, Quaternion rotation) + { + return Object.Instantiate(m_InstantiatedPrefab).GetComponent(); + } + + public void Destroy(NetworkObject networkObject) + { + Object.Destroy(networkObject.gameObject); + } + + public void Register(NetworkManager networkManager) + { + // Register the version that will be spawned by the authority (i.e. Host) + networkManager.PrefabHandler.AddHandler(m_RegisteredPrefab, this); + } + } + + /// + /// Creates a GameObject and sets the transform parent to the given transform + /// Adds a component of the given type to the GameObject + /// + private static void AddChildToNetworkObject(Transform transform) where T : Component + { + var gameObj = new GameObject(); + gameObj.transform.parent = transform; + gameObj.AddComponent(); + } + + protected override void OnServerAndClientsCreated() + { + // Create a prefab that has many child NetworkBehaviours + m_AuthorityPrefab = CreateNetworkObjectPrefab("AuthorityPrefab"); + AddChildToNetworkObject(m_AuthorityPrefab.transform); + AddChildToNetworkObject(m_AuthorityPrefab.transform); + AddChildToNetworkObject(m_AuthorityPrefab.transform); + + // Create a second prefab with only one NetworkBehaviour + // This simulates the GameObjects on the other NetworkBehaviours being disabled + m_NonAuthorityPrefab = CreateNetworkObjectPrefab("NonAuthorityPrefab"); + AddChildToNetworkObject(m_NonAuthorityPrefab.transform); + + // Create and register a prefab handler + // The prefab handler will behave as if the GameObjects have been disabled on the non-authority client + m_HostAndClientPrefabHandler = new HostAndClientPrefabHandler(m_AuthorityPrefab, m_NonAuthorityPrefab); + m_HostAndClientPrefabHandler.Register(m_ServerNetworkManager); + foreach (var client in m_ClientNetworkManagers) + { + m_HostAndClientPrefabHandler.Register(client); + } + + base.OnServerAndClientsCreated(); + } + + /// + /// Validates the fix where would throw an exception + /// if a user sets a with one or more components + /// to inactive. + /// + [UnityTest] + public IEnumerator DisabledGameObjectErrorTest() + { + var instance = SpawnObject(m_AuthorityPrefab, m_ServerNetworkManager); + var networkObjectInstance = instance.GetComponent(); + var networkTransformInstance = instance.GetComponentInChildren(); + + yield return WaitForConditionOrTimeOut(() => ObjectSpawnedOnAllClients(networkObjectInstance.NetworkObjectId)); + AssertOnTimeout("Timed out waiting for object to spawn!"); + + var errorMessage = $"[Netcode] {nameof(NetworkBehaviour)} index {networkTransformInstance.NetworkBehaviourId} was out of bounds for {m_NonAuthorityPrefab.name}(Clone). " + + $"{nameof(NetworkBehaviour)}s must be the same, and in the same order, between server and client."; + LogAssert.Expect(LogType.Error, errorMessage); + errorMessage = $"[{nameof(NetworkTransformMessage)}][Invalid] Targeted {nameof(NetworkTransform)}, {nameof(NetworkBehaviour.NetworkBehaviourId)} " + + $"({networkTransformInstance.NetworkBehaviourId}), does not exist! Make sure you are not spawning {nameof(NetworkObject)}s with disabled {nameof(GameObject)}s that have " + + $"{nameof(NetworkBehaviour)} components on them."; + LogAssert.Expect(LogType.Error, errorMessage); + + yield return new WaitForSeconds(0.3f); + } + + private bool ObjectSpawnedOnAllClients(ulong networkObjectId) + { + foreach (var client in m_ClientNetworkManagers) + { + if (!client.SpawnManager.SpawnedObjects.ContainsKey(networkObjectId)) + { + return false; + } + } + return true; + } + } + +} diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformErrorTests.cs.meta b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformErrorTests.cs.meta new file mode 100644 index 0000000000..b99b07b5a5 --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformErrorTests.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: 9f0f7bfcdffd47139aae1788ee8d2063 +timeCreated: 1738881698 \ No newline at end of file