fix: [Up-port] mixed authority nested NetworkTransform stops updating non-authority nested NetworkTransforms

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -18,6 +18,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed an issue where mixed authority nested `NetworkTransform` instances could stop child/nested instances from updating because an authoritative `NetworkTransform` (root or otherwise) would remove the `NetworkObject` from the update group, preventing non-authoritative instances from updating their state on the authority side. (#4169)
+
 ### Security
 
 ### Obsolete

com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs

@@ -3736,6 +3736,38 @@ private void ResetInterpolatedStateToCurrentAuthoritativeState()
             m_ScaleInterpolator.ResetTo(transform.parent, transform.localScale, serverTime);
         }
 
+        /// <summary>
+        /// Determines if this <see cref="NetworkObject"/> has any <see cref="NetworkTransform"/> instances that are non-authority and are updated during the same update stage.
+        /// </summary>
+        /// <remarks>
+        /// See <see cref="InternalInitialization"/> to better understand how the <paramref name="forUpdate"/> parameter is used to determine which update stage to check for non-authority <see cref="NetworkTransform"/> instances.
+        /// </remarks>
+        /// <param name="forUpdate">true to check the instances updated during the standard update and false to check the instances updated during the fixed update.</param>
+        /// <returns>true if a non-authority NetworkTransform exists on this NetworkObject and false if there are none.</returns>
+        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;
+        }
+
         /// <summary>
         /// The internal initialization method to allow for internal API adjustments
         /// </summary>
@@ -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));

com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformMixedAuthorityTests.cs

@@ -1,97 +1,141 @@
 using System.Collections;
 using System.Text;
+using NUnit.Framework;
 using Unity.Netcode.Components;
 using Unity.Netcode.TestHelpers.Runtime;
 using UnityEngine;
 using UnityEngine.TestTools;
 
 namespace Unity.Netcode.RuntimeTests
 {
+    [TestFixture(HostOrServer.Host)]
+    [TestFixture(HostOrServer.Server)]
     internal class NetworkTransformMixedAuthorityTests : IntegrationTestWithApproximation
     {
         private const float k_MotionMagnitude = 5.5f;
         private const int k_Iterations = 4;
 
         protected override int NumberOfClients => 2;
 
-        private StringBuilder m_ErrorMsg = new StringBuilder();
-
-        protected override void OnCreatePlayerPrefab()
+        /// <summary>
+        /// The root's authority mode for each case. The nested child uses the inverse.
+        /// </summary>
+        private static readonly NetworkTransform.AuthorityModes[] k_RootAuthorityModes =
         {
-            m_PlayerPrefab.AddComponent<NetworkTransform>();
+            NetworkTransform.AuthorityModes.Server,
+            NetworkTransform.AuthorityModes.Owner,
+        };
+
+        private GameObject[] m_MixedAuthorityPrefabs;
 
-            var childGameObject = new GameObject();
-            childGameObject.transform.parent = m_PlayerPrefab.transform;
-            var childNetworkTransform = childGameObject.AddComponent<NetworkTransform>();
-            childNetworkTransform.AuthorityMode = NetworkTransform.AuthorityModes.Owner;
-            childNetworkTransform.InLocalSpace = true;
+        private StringBuilder m_ErrorMsg = new StringBuilder();
 
-            base.OnCreatePlayerPrefab();
+        public NetworkTransformMixedAuthorityTests(HostOrServer hostOrServer) : base(hostOrServer)
+        {
         }
 
-        private void MovePlayers()
+        protected override void OnServerAndClientsCreated()
         {
-            foreach (var networkManager in m_NetworkManagers)
+            m_MixedAuthorityPrefabs = new GameObject[k_RootAuthorityModes.Length];
+            for (int i = 0; i < k_RootAuthorityModes.Length; i++)
             {
-                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;
+                var rootAuthorityMode = k_RootAuthorityModes[i];
+                var prefab = CreateNetworkObjectPrefab($"MixedAuthority-{rootAuthorityMode}Root");
+                prefab.AddComponent<NetworkTransform>().AuthorityMode = rootAuthorityMode;
+
+                var childGameObject = new GameObject();
+                childGameObject.transform.parent = prefab.transform;
+                var childNetworkTransform = childGameObject.AddComponent<NetworkTransform>();
+                childNetworkTransform.AuthorityMode = InverseOf(rootAuthorityMode);
+                childNetworkTransform.InLocalSpace = true;
+
+                m_MixedAuthorityPrefabs[i] = prefab;
             }
+
+            base.OnServerAndClientsCreated();
+        }
+
+        private static NetworkTransform.AuthorityModes InverseOf(NetworkTransform.AuthorityModes authorityMode)
+        {
+            return authorityMode == NetworkTransform.AuthorityModes.Server ? NetworkTransform.AuthorityModes.Owner : NetworkTransform.AuthorityModes.Server;
+        }
+
+        /// <summary>
+        /// Returns the instance with authority over a <see cref="NetworkTransform"/> set to the given authority mode.
+        /// </summary>
+        private NetworkObject GetAuthorityInstance(NetworkObject instance, NetworkManager owner, NetworkTransform.AuthorityModes authorityMode)
+        {
+            return GetManagersInstance(authorityMode == NetworkTransform.AuthorityModes.Server ? m_ServerNetworkManager : owner, instance);
         }
 
-        private bool AllInstancePositionsMatch()
+        private bool AllInstancePositionsMatch(NetworkObject instance, NetworkManager owner, NetworkTransform.AuthorityModes rootAuthorityMode)
         {
             m_ErrorMsg.Clear();
+            var authorityRootPosition = GetAuthorityInstance(instance, owner, rootAuthorityMode).transform.position;
+            var authorityChildPosition = GetAuthorityInstance(instance, owner, InverseOf(rootAuthorityMode)).transform.GetChild(0).localPosition;
+
+            // The authority instances are compared too. An instance with authority over one nested
+            // NetworkTransform is still non-authority for the other.
             foreach (var networkManager in m_NetworkManagers)
             {
-                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;
-                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;
+                var clone = GetManagersInstance(networkManager, instance);
+                var cloneRootPosition = clone.transform.position;
+                var cloneChildPosition = clone.transform.GetChild(0).localPosition;
 
-                    if (!Approximately(serverRootPosition, cloneRootPosition))
-                    {
-                        m_ErrorMsg.AppendLine($"[{playerObject.name}][{playerClone.name}] Root mismatch ({GetVector3Values(serverRootPosition)})({GetVector3Values(cloneRootPosition)})!");
-                    }
+                if (!Approximately(authorityRootPosition, cloneRootPosition))
+                {
+                    m_ErrorMsg.AppendLine($"[{rootAuthorityMode}Root][{GetDisplayName(networkManager)}] Root mismatch ({GetVector3Values(authorityRootPosition)})({GetVector3Values(cloneRootPosition)})!");
+                }
 
-                    if (!Approximately(ownerChildPosition, cloneChildPosition))
-                    {
-                        m_ErrorMsg.AppendLine($"[{playerObject.name}][{playerClone.name}] Child mismatch ({GetVector3Values(ownerChildPosition)})({GetVector3Values(cloneChildPosition)})!");
-                    }
+                if (!Approximately(authorityChildPosition, cloneChildPosition))
+                {
+                    m_ErrorMsg.AppendLine($"[{rootAuthorityMode}Root][{GetDisplayName(networkManager)}] Child mismatch ({GetVector3Values(authorityChildPosition)})({GetVector3Values(cloneChildPosition)})!");
                 }
             }
             return m_ErrorMsg.Length == 0;
         }
 
         /// <summary>
         /// Client-Server Only
-        /// Validates that mixed authority is working properly
-        /// Root -- Server Authoritative
-        /// |--Child -- Owner Authoritative
+        /// Validates that mixed authority is working properly for both arrangements:
+        /// Root -- Server or Owner authoritative
+        /// |--Child -- The inverse of the root's authority mode
         /// </summary>
         [UnityTest]
         public IEnumerator MixedAuthorityTest()
         {
-            for (int i = 0; i < k_Iterations; i++)
+            // A client owns the instance so the owner authoritative half is never also the server.
+            var owner = m_ClientNetworkManagers[0];
+            for (int i = 0; i < k_RootAuthorityModes.Length; i++)
             {
-                MovePlayers();
-                yield return WaitForConditionOrTimeOut(AllInstancePositionsMatch);
-                AssertOnTimeout($"Transforms failed to synchronize!");
+                var rootAuthorityMode = k_RootAuthorityModes[i];
+                var instance = SpawnObject(m_MixedAuthorityPrefabs[i], owner).GetComponent<NetworkObject>();
+                yield return WaitForSpawnedOnAllOrTimeOut(instance);
+                AssertOnTimeout($"[{rootAuthorityMode}Root] Failed to spawn {instance.name} on all clients!");
+
+                // An instance stays registered for updates while any of its nested NetworkTransform components is non-authority.
+                foreach (var networkManager in m_NetworkManagers)
+                {
+                    var clone = GetManagersInstance(networkManager, instance);
+                    var hasNonAuthority = false;
+                    foreach (var networkTransform in clone.NetworkTransforms)
+                    {
+                        hasNonAuthority |= !networkTransform.CanCommitToTransform;
+                    }
+                    Assert.AreEqual(hasNonAuthority, networkManager.NetworkTransformUpdate.ContainsKey(instance.NetworkObjectId), $"[{rootAuthorityMode}Root][{GetDisplayName(networkManager)}] Unexpected update registration!");
+                }
+
+                for (int iteration = 0; iteration < k_Iterations; iteration++)
+                {
+                    var direction = GetRandomVector3(-1.0f, 1.0f);
+                    GetAuthorityInstance(instance, owner, rootAuthorityMode).transform.position += direction * k_MotionMagnitude;
+                    GetAuthorityInstance(instance, owner, InverseOf(rootAuthorityMode)).transform.GetChild(0).localPosition += direction * k_MotionMagnitude;
+
+                    yield return WaitForConditionOrTimeOut(() => AllInstancePositionsMatch(instance, owner, rootAuthorityMode));
+                    AssertOnTimeout($"[{rootAuthorityMode}Root] Transforms failed to synchronize!\n{m_ErrorMsg}");
+                }
+
+                instance.Despawn();
             }
         }
     }

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<NetworkTransform>();
+            rootNetworkTransform.AuthorityMode = NetworkTransform.AuthorityModes.Owner;
+            var rigidbody = m_MixedMotionModelPrefab.AddComponent<Rigidbody>();
+            rigidbody.useGravity = false;
+            rigidbody.detectCollisions = false;
+            m_MixedMotionModelPrefab.AddComponent<NetworkRigidbody>().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<NetworkTransform>();
+            childNetworkTransform.AuthorityMode = NetworkTransform.AuthorityModes.Server;
+            childNetworkTransform.InLocalSpace = true;
+
+            base.OnServerAndClientsCreated();
+        }
+
+        /// <summary>
+        /// 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.
+        /// </summary>
+        [UnityTest]
+        public IEnumerator UpdateRegistrationFollowsMotionModel()
+        {
+            var instance = SpawnObject(m_MixedMotionModelPrefab, m_ServerNetworkManager).GetComponent<NetworkObject>();
+            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