diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 26a8188f99..e980ac1b5b 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/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) diff --git a/com.unity.netcode.gameobjects/Components/NetworkAnimator.cs b/com.unity.netcode.gameobjects/Components/NetworkAnimator.cs index 4eee6a0e11..2cae1d61a2 100644 --- a/com.unity.netcode.gameobjects/Components/NetworkAnimator.cs +++ b/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()) { diff --git a/testproject/Assets/Tests/Runtime/Animation/NetworkAnimatorTests.cs b/testproject/Assets/Tests/Runtime/Animation/NetworkAnimatorTests.cs index 6ab1c6cbe6..c9d47c8237 100644 --- a/testproject/Assets/Tests/Runtime/Animation/NetworkAnimatorTests.cs +++ b/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(); + // 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!"); + } /// /// Verifies that triggers are synchronized with currently connected clients