diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 7b75f45d30..10d3ecafb2 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -18,6 +18,8 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed +- Issue with mixed authority nested `NetworkTransform` instances can stop child/nested `NetworkTransform` instances from updating due to a parent (root or otherwise) `NetworkTransform` that is the authority instance will remove the `NetworkObject` completely from the non-authority update group causing non-authority instances to never update their state (interpolating or not) on the authority side. (#4169) + ### Security ### Obsolete diff --git a/com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs b/com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs index 338d1f991f..4114651b71 100644 --- a/com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs +++ b/com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs @@ -3736,6 +3736,38 @@ private void ResetInterpolatedStateToCurrentAuthoritativeState() m_ScaleInterpolator.ResetTo(transform.parent, transform.localScale, serverTime); } + /// + /// Determines if this has any instances that are non-authority and are updated during the same update stage. + /// + /// + /// See to better understand how the parameter is used to determine which update stage to check for non-authority instances. + /// + /// true to check the instances updated during the standard update and false to check the instances updated during the fixed update. + /// true if a non-authority NetworkTransform exists on this NetworkObject and false if there are none. + private bool HasNonAuthorityNetworkTransform(bool forUpdate) + { + var networkTransforms = NetworkObject.NetworkTransforms; + for (int i = 0; i < networkTransforms.Count; i++) + { + var networkTransform = networkTransforms[i]; +#if COM_UNITY_MODULES_PHYSICS || COM_UNITY_MODULES_PHYSICS2D + // If the update stages don't match, then skip this instance. + // Reference: + // forUpdate is true for the standard update and false for the fixed update. + // m_UseRigidbodyForMotion is false for the standard update and true for the fixed update. + if (forUpdate == networkTransform.m_UseRigidbodyForMotion) + { + continue; + } +#endif + if (!(networkTransform.IsServerAuthoritative() ? networkTransform.IsServer : networkTransform.IsOwner)) + { + return true; + } + } + return false; + } + /// /// The internal initialization method to allow for internal API adjustments /// @@ -3807,8 +3839,12 @@ internal virtual void InternalInitialization(bool isOwnershipChange = false) if (CanCommitToTransform) { - // Make sure authority doesn't get added to updates (no need to do this on the authority side) - m_CachedNetworkManager.NetworkTransformRegistration(NetworkObject, forUpdate, false); + // If there are no non-authority NetworkTransform instances on this NetworkObject using this update, then remove this instance from the NetworkManager's update list. + // Otherwise, we need to keep it registered for updates so the non-authority instances will process their received state updates and apply them to the transform. + if (!HasNonAuthorityNetworkTransform(forUpdate)) + { + m_CachedNetworkManager.NetworkTransformRegistration(NetworkObject, forUpdate, false); + } if (UseHalfFloatPrecision) { m_HalfPositionState = new NetworkDeltaPosition(currentPosition, m_CachedNetworkManager.ServerTime.Tick, math.bool3(SyncPositionX, SyncPositionY, SyncPositionZ)); diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformMixedAuthorityTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformMixedAuthorityTests.cs index 760e916bda..ab6ef475e5 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformMixedAuthorityTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformMixedAuthorityTests.cs @@ -1,5 +1,6 @@ using System.Collections; using System.Text; +using NUnit.Framework; using Unity.Netcode.Components; using Unity.Netcode.TestHelpers.Runtime; using UnityEngine; @@ -7,6 +8,10 @@ namespace Unity.Netcode.RuntimeTests { + [TestFixture(HostOrServer.Host, NetworkTransform.AuthorityModes.Server)] + [TestFixture(HostOrServer.Host, NetworkTransform.AuthorityModes.Owner)] + [TestFixture(HostOrServer.Server, NetworkTransform.AuthorityModes.Server)] + [TestFixture(HostOrServer.Server, NetworkTransform.AuthorityModes.Owner)] internal class NetworkTransformMixedAuthorityTests : IntegrationTestWithApproximation { private const float k_MotionMagnitude = 5.5f; @@ -14,64 +19,75 @@ internal class NetworkTransformMixedAuthorityTests : IntegrationTestWithApproxim protected override int NumberOfClients => 2; + private readonly NetworkTransform.AuthorityModes m_RootAuthorityMode; + private readonly NetworkTransform.AuthorityModes m_ChildAuthorityMode; + private StringBuilder m_ErrorMsg = new StringBuilder(); + public NetworkTransformMixedAuthorityTests(HostOrServer hostOrServer, NetworkTransform.AuthorityModes rootAuthorityMode) : base(hostOrServer) + { + m_RootAuthorityMode = rootAuthorityMode; + m_ChildAuthorityMode = rootAuthorityMode == NetworkTransform.AuthorityModes.Server ? NetworkTransform.AuthorityModes.Owner : NetworkTransform.AuthorityModes.Server; + } + protected override void OnCreatePlayerPrefab() { - m_PlayerPrefab.AddComponent(); + m_PlayerPrefab.AddComponent().AuthorityMode = m_RootAuthorityMode; var childGameObject = new GameObject(); childGameObject.transform.parent = m_PlayerPrefab.transform; var childNetworkTransform = childGameObject.AddComponent(); - childNetworkTransform.AuthorityMode = NetworkTransform.AuthorityModes.Owner; + childNetworkTransform.AuthorityMode = m_ChildAuthorityMode; childNetworkTransform.InLocalSpace = true; base.OnCreatePlayerPrefab(); } + /// + /// Returns the instance of 's player object that has authority over a + /// using the authority mode. + /// + private NetworkObject GetAuthorityInstance(NetworkManager player, NetworkTransform.AuthorityModes authorityMode) + { + var authority = authorityMode == NetworkTransform.AuthorityModes.Server ? m_ServerNetworkManager : player; + return authority.SpawnManager.SpawnedObjects[player.LocalClient.PlayerObject.NetworkObjectId]; + } + private void MovePlayers() { - foreach (var networkManager in m_NetworkManagers) + foreach (var networkManager in m_ClientNetworkManagers) { var direction = GetRandomVector3(-1.0f, 1.0f); - var playerObject = networkManager.LocalClient.PlayerObject; - var playerObjectId = networkManager.LocalClient.PlayerObject.NetworkObjectId; - // Server authoritative - var serverPlayerClone = m_ServerNetworkManager.SpawnManager.SpawnedObjects[playerObjectId]; - serverPlayerClone.transform.position += direction * k_MotionMagnitude; - // Owner authoritative - var childTransform = networkManager.LocalClient.PlayerObject.transform.GetChild(0); - childTransform.localPosition += direction * k_MotionMagnitude; + GetAuthorityInstance(networkManager, m_RootAuthorityMode).transform.position += direction * k_MotionMagnitude; + GetAuthorityInstance(networkManager, m_ChildAuthorityMode).transform.GetChild(0).localPosition += direction * k_MotionMagnitude; } } private bool AllInstancePositionsMatch() { m_ErrorMsg.Clear(); - foreach (var networkManager in m_NetworkManagers) + foreach (var networkManager in m_ClientNetworkManagers) { - var playerObject = networkManager.LocalClient.PlayerObject; var playerObjectId = networkManager.LocalClient.PlayerObject.NetworkObjectId; - var serverRootPosition = m_ServerNetworkManager.SpawnManager.SpawnedObjects[playerObjectId].transform.position; - var ownerChildPosition = networkManager.LocalClient.PlayerObject.transform.GetChild(0).localPosition; + var authorityRootPosition = GetAuthorityInstance(networkManager, m_RootAuthorityMode).transform.position; + var authorityChildPosition = GetAuthorityInstance(networkManager, m_ChildAuthorityMode).transform.GetChild(0).localPosition; + + // The authority instances are compared too, as an instance with authority over one nested + // NetworkTransform is still non-authority for the other. foreach (var client in m_NetworkManagers) { - if (client == networkManager) - { - continue; - } var playerClone = client.SpawnManager.SpawnedObjects[playerObjectId]; var cloneRootPosition = playerClone.transform.position; var cloneChildPosition = playerClone.transform.GetChild(0).localPosition; - if (!Approximately(serverRootPosition, cloneRootPosition)) + if (!Approximately(authorityRootPosition, cloneRootPosition)) { - m_ErrorMsg.AppendLine($"[{playerObject.name}][{playerClone.name}] Root mismatch ({GetVector3Values(serverRootPosition)})({GetVector3Values(cloneRootPosition)})!"); + m_ErrorMsg.AppendLine($"[Client-{client.LocalClientId}][{playerClone.name}] Root mismatch ({GetVector3Values(authorityRootPosition)})({GetVector3Values(cloneRootPosition)})!"); } - if (!Approximately(ownerChildPosition, cloneChildPosition)) + if (!Approximately(authorityChildPosition, cloneChildPosition)) { - m_ErrorMsg.AppendLine($"[{playerObject.name}][{playerClone.name}] Child mismatch ({GetVector3Values(ownerChildPosition)})({GetVector3Values(cloneChildPosition)})!"); + m_ErrorMsg.AppendLine($"[Client-{client.LocalClientId}][{playerClone.name}] Child mismatch ({GetVector3Values(authorityChildPosition)})({GetVector3Values(cloneChildPosition)})!"); } } } @@ -81,8 +97,8 @@ private bool AllInstancePositionsMatch() /// /// Client-Server Only /// Validates that mixed authority is working properly - /// Root -- Server Authoritative - /// |--Child -- Owner Authoritative + /// Root -- Server or Owner authoritative + /// |--Child -- The inverse of the root's authority mode /// [UnityTest] public IEnumerator MixedAuthorityTest() @@ -91,7 +107,31 @@ public IEnumerator MixedAuthorityTest() { MovePlayers(); yield return WaitForConditionOrTimeOut(AllInstancePositionsMatch); - AssertOnTimeout($"Transforms failed to synchronize!"); + AssertOnTimeout($"Transforms failed to synchronize!\n{m_ErrorMsg}"); + } + } + + /// + /// The update registration is per-NetworkObject while the authority motion model is per-NetworkTransform, + /// so an instance stays registered for as long as any one of its nested NetworkTransform components is + /// non-authority. + /// + [Test] + public void MixedAuthorityUpdateRegistration() + { + foreach (var networkManager in m_ClientNetworkManagers) + { + var playerObjectId = networkManager.LocalClient.PlayerObject.NetworkObjectId; + foreach (var client in m_NetworkManagers) + { + var playerClone = client.SpawnManager.SpawnedObjects[playerObjectId]; + var hasNonAuthority = false; + foreach (var networkTransform in playerClone.NetworkTransforms) + { + hasNonAuthority |= !networkTransform.CanCommitToTransform; + } + Assert.AreEqual(hasNonAuthority, client.NetworkTransformUpdate.ContainsKey(playerObjectId), $"[Client-{client.LocalClientId}][{playerClone.name}] Unexpected update registration!"); + } } } } diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformMixedMotionModelTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformMixedMotionModelTests.cs new file mode 100644 index 0000000000..25ec9894eb --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformMixedMotionModelTests.cs @@ -0,0 +1,67 @@ +#if COM_UNITY_MODULES_PHYSICS +using System.Collections; +using NUnit.Framework; +using Unity.Netcode.Components; +using Unity.Netcode.TestHelpers.Runtime; +using UnityEngine; +using UnityEngine.TestTools; + +namespace Unity.Netcode.RuntimeTests +{ + internal class NetworkTransformMixedMotionModelTests : NetcodeIntegrationTest + { + protected override int NumberOfClients => 2; + + private GameObject m_MixedMotionModelPrefab; + + protected override void OnServerAndClientsCreated() + { + m_MixedMotionModelPrefab = CreateNetworkObjectPrefab("MixedMotionModel"); + + // The root is owner authoritative and driven by the rigidbody, which places it in the fixed update registration + var rootNetworkTransform = m_MixedMotionModelPrefab.AddComponent(); + rootNetworkTransform.AuthorityMode = NetworkTransform.AuthorityModes.Owner; + var rigidbody = m_MixedMotionModelPrefab.AddComponent(); + rigidbody.useGravity = false; + rigidbody.detectCollisions = false; + m_MixedMotionModelPrefab.AddComponent().UseRigidBodyForMotion = true; + + // The nested child is server authoritative and driven by the transform, which places it in the update registration + var childGameObject = new GameObject(); + childGameObject.transform.parent = m_MixedMotionModelPrefab.transform; + var childNetworkTransform = childGameObject.AddComponent(); + childNetworkTransform.AuthorityMode = NetworkTransform.AuthorityModes.Server; + childNetworkTransform.InLocalSpace = true; + + base.OnServerAndClientsCreated(); + } + + /// + /// A NetworkObject that mixes both the authority motion model and the rigidbody motion model has each nested + /// NetworkTransform registered under a different update. Gaining authority over the instance in one update + /// should not leave it registered for the other. + /// + [UnityTest] + public IEnumerator UpdateRegistrationFollowsMotionModel() + { + var instance = SpawnObject(m_MixedMotionModelPrefab, m_ServerNetworkManager).GetComponent(); + yield return WaitForSpawnedOnAllOrTimeOut(instance); + AssertOnTimeout($"Failed to spawn {instance.name} on all clients!"); + + var newOwner = m_ClientNetworkManagers[0]; + + // Establish the baseline before ownership is transferred, otherwise the check below would still pass if this instance was never registered for the fixed update to begin with. + Assert.True(newOwner.NetworkTransformFixedUpdate.ContainsKey(instance.NetworkObjectId), $"Client-{newOwner.LocalClientId} should initially be registered for the fixed update!"); + + instance.ChangeOwnership(newOwner.LocalClientId); + yield return WaitForConditionOrTimeOut(() => newOwner.SpawnManager.SpawnedObjects[instance.NetworkObjectId].OwnerClientId == newOwner.LocalClientId); + AssertOnTimeout($"Client-{newOwner.LocalClientId} never gained ownership of {instance.name}!"); + + // The new owner is the authority for the rigidbody driven root, so nothing on this instance needs the fixed + // update any longer. The server authoritative child still needs the standard update. + Assert.False(newOwner.NetworkTransformFixedUpdate.ContainsKey(instance.NetworkObjectId), $"Client-{newOwner.LocalClientId} is still registered for the fixed update!"); + Assert.True(newOwner.NetworkTransformUpdate.ContainsKey(instance.NetworkObjectId), $"Client-{newOwner.LocalClientId} is not registered for the update!"); + } + } +} +#endif diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformMixedMotionModelTests.cs.meta b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformMixedMotionModelTests.cs.meta new file mode 100644 index 0000000000..8b1683109d --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformMixedMotionModelTests.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: b12b5cdba06a3814fbe6cf7c4df188f7 \ No newline at end of file