fix: nested NetworkTransforms with different authority settings (#3209)

* fix

First pass fixes for mixed authority mode nested networktransforms.

* style

adding additional comments

* style

Remove commented code.
Add additional comments for clarity.

* update

adding changelog entry.

* test

Adding integration test to validate that mixed authority NetworkTransform hierarchies are working properly.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -14,6 +14,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 - Fixed exception being thrown when a `GameObject` with an associated `NetworkTransform` is disabled. (#3243)
 - Fixed `NetworkObject.DeferDespawn` to respect the `DestroyGameObject` parameter. (#3219)
+- Fixed issue where a `NetworkObject` with nested `NetworkTransform` components of varying authority modes was not being taken into consideration and would break both the initial `NetworkTransform` synchronization and fail to properly handle synchronized state updates of the nested `NetworkTransform` components. (#3209)
 - Fixed issue with distributing parented children that have the distributable and/or transferrable permissions set and have the same owner as the root parent, that has the distributable permission set, were not being distributed to the same client upon the owning client disconnecting when using a distributed authority network topology. (#3203)
 - 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)
 - Fixed issue where `NetworkVariableBase` derived classes were not being re-initialized if the associated `NetworkObject` instance was not destroyed and re-spawned. (#3181)

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

@@ -1679,8 +1679,8 @@ private void TryCommitTransform(ref Transform transformToCommit, bool synchroniz
                     // Synchronize any nested NetworkTransforms with the parent's
                     foreach (var childNetworkTransform in NetworkObject.NetworkTransforms)
                     {
-                        // Don't update the same instance
-                        if (childNetworkTransform == this)
+                        // Don't update the same instance or any nested NetworkTransform with a different authority mode
+                        if (childNetworkTransform == this || childNetworkTransform.AuthorityMode != AuthorityMode)
                         {
                             continue;
                         }
@@ -2908,8 +2908,8 @@ private void OnNetworkStateChanged(NetworkTransformState oldState, NetworkTransf
                 // Synchronize any nested NetworkTransforms with the parent's
                 foreach (var childNetworkTransform in NetworkObject.NetworkTransforms)
                 {
-                    // Don't update the same instance
-                    if (childNetworkTransform == this)
+                    // Don't update the same instance or any nested NetworkTransform with a different authority mode
+                    if (childNetworkTransform == this || childNetworkTransform.AuthorityMode != AuthorityMode)
                     {
                         continue;
                     }
@@ -3032,15 +3032,29 @@ private void NonAuthorityFinalizeSynchronization()
             // For all child NetworkTransforms nested under the same NetworkObject,
             // we apply the initial synchronization based on their parented/ordered
             // heirarchy.
-            if (SynchronizeState.IsSynchronizing && m_IsFirstNetworkTransform)
+            if (SynchronizeState.IsSynchronizing)
             {
-                foreach (var child in NetworkObject.NetworkTransforms)
+                if (m_IsFirstNetworkTransform)
                 {
-                    child.ApplySynchronization();
+                    foreach (var child in NetworkObject.NetworkTransforms)
+                    {
+                        // Don't initialize any nested NetworkTransforms that this instance has authority over
+                        if (child.CanCommitToTransform)
+                        {
+                            continue;
+                        }
+                        child.ApplySynchronization();
 
-                    // For all nested (under the root/same NetworkObject) child NetworkTransforms, we need to run through
-                    // initialization once more to assure any values applied or stored are relative to the Root's transform.
-                    child.InternalInitialization();
+                        // For all like-authority nested (under the root/same NetworkObject) child NetworkTransforms, we need to run through
+                        // initialization once more to assure any values applied or stored are relative to the Root's transform.
+                        child.InternalInitialization();
+                    }
+                }
+                else // Otherwise, just run through standard synchronization of this instance
+                if (!CanCommitToTransform)
+                {
+                    ApplySynchronization();
+                    InternalInitialization();
                 }
             }
         }
@@ -3075,25 +3089,40 @@ protected internal override void InternalOnNetworkPostSpawn()
             // This is a special case for client-server where a server is spawning an owner authoritative NetworkObject but has yet to serialize anything.
             // When the server detects that:
             // - We are not in a distributed authority session (DAHost check).
-            // - This is the first/root NetworkTransform.
             // - We are in owner authoritative mode.
             // - The NetworkObject is not owned by the server.
             // - The SynchronizeState.IsSynchronizing is set to false.
             // Then we want to:
             // - Force the "IsSynchronizing" flag so the NetworkTransform has its state updated properly and runs through the initialization again.
             // - Make sure the SynchronizingState is updated to the instantiated prefab's default flags/settings.
-            if (NetworkManager.IsServer && !NetworkManager.DistributedAuthorityMode && m_IsFirstNetworkTransform && !OnIsServerAuthoritative() && !IsOwner && !SynchronizeState.IsSynchronizing)
+            if (NetworkManager.IsServer && !NetworkManager.DistributedAuthorityMode && !IsOwner && !OnIsServerAuthoritative() && !SynchronizeState.IsSynchronizing)
             {
-                // Assure the first/root NetworkTransform has the synchronizing flag set so the server runs through the final post initialization steps
-                SynchronizeState.IsSynchronizing = true;
-                // Assure the SynchronizeState matches the initial prefab's values for each associated NetworkTransfrom (this includes root + all children)
-                foreach (var child in NetworkObject.NetworkTransforms)
+                // Handle the first/root NetworkTransform slightly differently to have a sequenced synchronization of like authority nested NetworkTransform components
+                if (m_IsFirstNetworkTransform)
                 {
-                    child.ApplyPlayerTransformState();
+                    // Assure the NetworkTransform has the synchronizing flag set so the server runs through the final post initialization steps
+                    SynchronizeState.IsSynchronizing = true;
+
+                    // Assure the SynchronizeState matches the initial prefab's values for each associated NetworkTransfrom (this includes root + all children)
+                    foreach (var child in NetworkObject.NetworkTransforms)
+                    {
+                        // Don't ApplyPlayerTransformState to any nested NetworkTransform with a different authority mode
+                        if (child != this && child.AuthorityMode != AuthorityMode)
+                        {
+                            continue;
+                        }
+                        child.ApplyPlayerTransformState();
+                    }
                 }
+                else
+                {
+                    ApplyPlayerTransformState();
+                }
+
                 // Now fall through to the final synchronization portion of the spawning for NetworkTransform
             }
 
+            // Standard non-authority synchronization is handled here
             if (!CanCommitToTransform && NetworkManager.IsConnectedClient && SynchronizeState.IsSynchronizing)
             {
                 NonAuthorityFinalizeSynchronization();

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

@@ -0,0 +1,103 @@
+using System.Collections;
+using System.Collections.Generic;
+using System.Text;
+using Unity.Netcode.Components;
+using Unity.Netcode.TestHelpers.Runtime;
+using UnityEngine;
+using UnityEngine.TestTools;
+
+namespace Unity.Netcode.RuntimeTests
+{
+    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();
+        private List<NetworkManager> m_NetworkManagers = new List<NetworkManager>();
+
+        protected override void OnCreatePlayerPrefab()
+        {
+            m_PlayerPrefab.AddComponent<NetworkTransform>();
+
+            var childGameObject = new GameObject();
+            childGameObject.transform.parent = m_PlayerPrefab.transform;
+            var childNetworkTransform = childGameObject.AddComponent<NetworkTransform>();
+            childNetworkTransform.AuthorityMode = NetworkTransform.AuthorityModes.Owner;
+            childNetworkTransform.InLocalSpace = true;
+
+            base.OnCreatePlayerPrefab();
+        }
+
+        private void MovePlayers()
+        {
+            foreach (var networkManager in m_NetworkManagers)
+            {
+                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;
+            }
+        }
+
+        private bool AllInstancePositionsMatch()
+        {
+            m_ErrorMsg.Clear();
+            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;
+
+                    if (!Approximately(serverRootPosition, cloneRootPosition))
+                    {
+                        m_ErrorMsg.AppendLine($"[{playerObject.name}][{playerClone.name}] Root mismatch ({GetVector3Values(serverRootPosition)})({GetVector3Values(cloneRootPosition)})!");
+                    }
+
+                    if (!Approximately(ownerChildPosition, cloneChildPosition))
+                    {
+                        m_ErrorMsg.AppendLine($"[{playerObject.name}][{playerClone.name}] Child mismatch ({GetVector3Values(ownerChildPosition)})({GetVector3Values(cloneChildPosition)})!");
+                    }
+                }
+            }
+            return m_ErrorMsg.Length == 0;
+        }
+
+        /// <summary>
+        /// Client-Server Only
+        /// Validates that mixed authority is working properly
+        /// Root -- Server Authoritative
+        /// |--Child -- Owner Authoritative
+        /// </summary>
+        [UnityTest]
+        public IEnumerator MixedAuthorityTest()
+        {
+            m_NetworkManagers.Add(m_ServerNetworkManager);
+            m_NetworkManagers.AddRange(m_ClientNetworkManagers);
+
+            for (int i = 0; i < k_Iterations; i++)
+            {
+                MovePlayers();
+                yield return WaitForConditionOrTimeOut(AllInstancePositionsMatch);
+                AssertOnTimeout($"Transforms failed to synchronize!");
+            }
+        }
+    }
+}