fix: networkanimator only updates observers backport (#3058)

* fix

only send animation updates to observers.

* test

validates that NetworkAnimator only sends animation updates to observers.

* update

Added changelog entry

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -15,6 +15,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed issue where `NetworkAnimator` would send updates to non-observer clients. (#3058)
 - Fixed issue where an exception could occur when receiving a universal RPC for a `NetworkObject` that has been despawned. (#3055)
 - Fixed issue where setting a prefab hash value during connection approval but not having a player prefab assigned could cause an exception when spawning a player. (#3046)
 - Fixed issue where collections v2.2.x was not supported when using UTP v2.2.x within Unity v2022.3. (#3033)

com.unity.netcode.gameobjects/Components/NetworkAnimator.cs

@@ -924,8 +924,14 @@ internal void CheckForAnimatorChanges()
                 {
                     // Just notify all remote clients and not the local server
                     m_ClientSendList.Clear();
-                    m_ClientSendList.AddRange(NetworkManager.ConnectedClientsIds);
-                    m_ClientSendList.Remove(NetworkManager.LocalClientId);
+                    foreach (var clientId in NetworkManager.ConnectedClientsIds)
+                    {
+                        if (clientId == NetworkManager.LocalClientId || !NetworkObject.Observers.Contains(clientId))
+                        {
+                            continue;
+                        }
+                        m_ClientSendList.Add(clientId);
+                    }
                     m_ClientRpcParams.Send.TargetClientIds = m_ClientSendList;
                     SendAnimStateClientRpc(m_AnimationMessage, m_ClientRpcParams);
                 }
@@ -1223,9 +1229,14 @@ private unsafe void SendParametersUpdateServerRpc(ParametersUpdateMessage parame
                 if (NetworkManager.ConnectedClientsIds.Count > (IsHost ? 2 : 1))
                 {
                     m_ClientSendList.Clear();
-                    m_ClientSendList.AddRange(NetworkManager.ConnectedClientsIds);
-                    m_ClientSendList.Remove(serverRpcParams.Receive.SenderClientId);
-                    m_ClientSendList.Remove(NetworkManager.ServerClientId);
+                    foreach (var clientId in NetworkManager.ConnectedClientsIds)
+                    {
+                        if (clientId == serverRpcParams.Receive.SenderClientId || clientId == NetworkManager.ServerClientId || !NetworkObject.Observers.Contains(clientId))
+                        {
+                            continue;
+                        }
+                        m_ClientSendList.Add(clientId);
+                    }
                     m_ClientRpcParams.Send.TargetClientIds = m_ClientSendList;
                     m_NetworkAnimatorStateChangeHandler.SendParameterUpdate(parametersUpdate, m_ClientRpcParams);
                 }
@@ -1271,9 +1282,14 @@ private unsafe void SendAnimStateServerRpc(AnimationMessage animationMessage, Se
                 if (NetworkManager.ConnectedClientsIds.Count > (IsHost ? 2 : 1))
                 {
                     m_ClientSendList.Clear();
-                    m_ClientSendList.AddRange(NetworkManager.ConnectedClientsIds);
-                    m_ClientSendList.Remove(serverRpcParams.Receive.SenderClientId);
-                    m_ClientSendList.Remove(NetworkManager.ServerClientId);
+                    foreach (var clientId in NetworkManager.ConnectedClientsIds)
+                    {
+                        if (clientId == serverRpcParams.Receive.SenderClientId || clientId == NetworkManager.ServerClientId || !NetworkObject.Observers.Contains(clientId))
+                        {
+                            continue;
+                        }
+                        m_ClientSendList.Add(clientId);
+                    }
                     m_ClientRpcParams.Send.TargetClientIds = m_ClientSendList;
                     m_NetworkAnimatorStateChangeHandler.SendAnimationUpdate(animationMessage, m_ClientRpcParams);
                 }
@@ -1322,8 +1338,14 @@ internal void SendAnimTriggerServerRpc(AnimationTriggerMessage animationTriggerM
             InternalSetTrigger(animationTriggerMessage.Hash, animationTriggerMessage.IsTriggerSet);
 
             m_ClientSendList.Clear();
-            m_ClientSendList.AddRange(NetworkManager.ConnectedClientsIds);
-            m_ClientSendList.Remove(NetworkManager.ServerClientId);
+            foreach (var clientId in NetworkManager.ConnectedClientsIds)
+            {
+                if (clientId == NetworkManager.ServerClientId || !NetworkObject.Observers.Contains(clientId))
+                {
+                    continue;
+                }
+                m_ClientSendList.Add(clientId);
+            }
 
             if (IsServerAuthoritative())
             {

testproject/Assets/Tests/Runtime/Animation/NetworkAnimatorTests.cs

@@ -400,6 +400,103 @@ public IEnumerator CrossFadeTransitionTests([Values] OwnerShipMode ownerShipMode
             AssertOnTimeout($"Timed out waiting for all clients to transition from synchronized cross fade!");
         }
 
+        private bool AllTriggersDetectedOnObserversOnly(OwnerShipMode ownerShipMode, ulong nonObserverId)
+        {
+            if (ownerShipMode == OwnerShipMode.ClientOwner)
+            {
+                if (!TriggerTest.ClientsThatTriggered.Contains(m_ServerNetworkManager.LocalClientId))
+                {
+                    return false;
+                }
+            }
+
+            foreach (var animatorTestHelper in AnimatorTestHelper.ClientSideInstances)
+            {
+                var currentClientId = animatorTestHelper.Value.NetworkManager.LocalClientId;
+                if (currentClientId == nonObserverId || (ownerShipMode == OwnerShipMode.ClientOwner && currentClientId == animatorTestHelper.Value.OwnerClientId))
+                {
+                    continue;
+                }
+
+                if (!TriggerTest.ClientsThatTriggered.Contains(currentClientId))
+                {
+                    return false;
+                }
+            }
+
+            // Should return false always
+            return !TriggerTest.ClientsThatTriggered.Contains(nonObserverId);
+        }
+
+        private bool AllObserversSameLayerWeight(OwnerShipMode ownerShipMode, int layer, float targetWeight, ulong nonObserverId)
+        {
+
+            if (ownerShipMode == OwnerShipMode.ClientOwner)
+            {
+                if (AnimatorTestHelper.ServerSideInstance.GetLayerWeight(layer) != targetWeight)
+                {
+                    return false;
+                }
+            }
+
+            foreach (var animatorTestHelper in AnimatorTestHelper.ClientSideInstances)
+            {
+                var currentClientId = animatorTestHelper.Value.NetworkManager.LocalClientId;
+                if (ownerShipMode == OwnerShipMode.ClientOwner && animatorTestHelper.Value.OwnerClientId == currentClientId)
+                {
+                    continue;
+                }
+                if (currentClientId == nonObserverId)
+                {
+                    if (animatorTestHelper.Value.GetLayerWeight(layer) == targetWeight)
+                    {
+                        return false;
+                    }
+                }
+                else
+                if (animatorTestHelper.Value.GetLayerWeight(layer) != targetWeight)
+                {
+                    return false;
+                }
+            }
+            return true;
+        }
+
+        [UnityTest]
+        public IEnumerator OnlyObserversAnimateTest([Values] OwnerShipMode ownerShipMode, [Values(AuthoritativeMode.ServerAuth, AuthoritativeMode.OwnerAuth)] AuthoritativeMode authoritativeMode)
+        {
+            // Spawn our test animator object
+            var objectInstance = SpawnPrefab(ownerShipMode == OwnerShipMode.ClientOwner, authoritativeMode);
+            var networkObject = objectInstance.GetComponent<NetworkObject>();
+            // Wait for it to spawn server-side
+            var success = WaitForConditionOrTimeOutWithTimeTravel(() => AnimatorTestHelper.ServerSideInstance != null);
+            Assert.True(success, $"Timed out waiting for the server-side instance of {GetNetworkAnimatorName(authoritativeMode)} to be spawned!");
+
+            // Wait for it to spawn client-side
+            success = WaitForConditionOrTimeOutWithTimeTravel(WaitForClientsToInitialize);
+            Assert.True(success, $"Timed out waiting for the server-side instance of {GetNetworkAnimatorName(authoritativeMode)} to be spawned!");
+
+            var animatorTestHelper = ownerShipMode == OwnerShipMode.ClientOwner ? AnimatorTestHelper.ClientSideInstances[m_ClientNetworkManagers[0].LocalClientId] : AnimatorTestHelper.ServerSideInstance;
+
+            networkObject.NetworkHide(m_ClientNetworkManagers[1].LocalClientId);
+
+            yield return WaitForConditionOrTimeOut(() => !m_ClientNetworkManagers[1].SpawnManager.SpawnedObjects.ContainsKey(networkObject.NetworkObjectId));
+            AssertOnTimeout($"Client-{m_ClientNetworkManagers[1].LocalClientId} timed out waiting to hide {networkObject.name}!");
+
+            if (authoritativeMode == AuthoritativeMode.ServerAuth)
+            {
+                animatorTestHelper = AnimatorTestHelper.ServerSideInstance;
+            }
+            animatorTestHelper.SetTrigger();
+            // Wait for all triggers to fire
+            yield return WaitForConditionOrTimeOut(() => AllTriggersDetectedOnObserversOnly(ownerShipMode, m_ClientNetworkManagers[1].LocalClientId));
+            AssertOnTimeout($"Timed out waiting for all triggers to match!");
+
+            animatorTestHelper.SetLayerWeight(1, 0.75f);
+            // Wait for all instances to update their weight value for layer 1
+            success = WaitForConditionOrTimeOutWithTimeTravel(() => AllObserversSameLayerWeight(ownerShipMode, 1, 0.75f, m_ClientNetworkManagers[1].LocalClientId));
+            Assert.True(success, $"Timed out waiting for all instances to match weight 0.75 on layer 1!");
+        }
 
         /// <summary>
         /// Verifies that triggers are synchronized with currently connected clients