fix: server doesn't apply triggers to itself; add host and server Animation tests (#1746) (#1751)

* fix: remove auto-send animation parameters (#1746)
* comment updates

Author: mattwalsh-unity

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

@@ -300,6 +300,12 @@ private unsafe void ReadParameters(FastBufferReader reader)
             }
         }
 
+        /// <summary>
+        /// Internally-called RPC client receiving function to update some animation parameters on a client when
+        ///   the server wants to update them
+        /// </summary>
+        /// <param name="animSnapshot">the payload containing the parameters to apply</param>
+        /// <param name="clientRpcParams">unused</param>
         [ClientRpc]
         private unsafe void SendAnimStateClientRpc(AnimationMessage animSnapshot, ClientRpcParams clientRpcParams = default)
         {
@@ -320,6 +326,12 @@ private unsafe void SendAnimStateClientRpc(AnimationMessage animSnapshot, Client
             }
         }
 
+        /// <summary>
+        /// Internally-called RPC client receiving function to update a trigger when the server wants to forward
+        ///   a trigger for a client to play / reset
+        /// </summary>
+        /// <param name="animSnapshot">the payload containing the trigger data to apply</param>
+        /// <param name="clientRpcParams">unused</param>
         [ClientRpc]
         private void SendAnimTriggerClientRpc(AnimationTriggerMessage animSnapshot, ClientRpcParams clientRpcParams = default)
         {
@@ -359,8 +371,23 @@ public void SetTrigger(int hash, bool reset = false)
 
             if (IsServer)
             {
+                //  trigger the animation locally on the server...
+                if (reset)
+                {
+                    m_Animator.ResetTrigger(hash);
+                }
+                else
+                {
+                    m_Animator.SetTrigger(hash);
+                }
+
+                // ...then tell all the clients to do the same
                 SendAnimTriggerClientRpc(animMsg);
             }
+            else
+            {
+                Debug.LogWarning("Trying to call NetworkAnimator.SetTrigger on a client...ignoring");
+            }
         }
 
         /// <summary>

com.unity.netcode.gameobjects/Tests/Runtime/NetworkAnimator/NetworkAnimatorTests.cs

@@ -18,10 +18,21 @@ public class NetworkAnimatorTests : BaseMultiInstanceTest
         private Animator m_PlayerOnServerAnimator;
         private Animator m_PlayerOnClientAnimator;
 
+
         [UnitySetUp]
         public override IEnumerator Setup()
         {
-            yield return StartSomeClientsAndServerWithPlayers(useHost: true, nbClients: NbClients,
+            // this is treacherous...normally BaseMultiInstance calls StartSomeClientsAndServerWithPlayers for you
+            //  in its version of Setup.  In this case, I want to alternatively test Server and Host mode via parameters
+            //  in each test.  However I cannot inject the host / server mode into the corresponding Setup calls, hence
+            //  I wrote and manually call an Init function.  However, if I don't write this null version of setup
+            //  I will get 2 calls to StartSomeClientsAndServerWithPlayers, which wrecks everything.
+            yield return null;
+        }
+
+        public IEnumerator Init(bool hostMode)
+        {
+            yield return StartSomeClientsAndServerWithPlayers(useHost: hostMode, nbClients: NbClients,
                 updatePlayerPrefab: playerPrefab =>
                 {
                     // ideally, we would build up the AnimatorController entirely in code and not need an asset,
@@ -63,8 +74,10 @@ private bool HasClip(Animator animator, string clipName)
         }
 
         [UnityTest]
-        public IEnumerator AnimationTriggerReset([Values(true, false)] bool asHash)
+        public IEnumerator AnimationTriggerReset([Values(true, false)] bool asHash, [Values(true, false)] bool hostMode)
         {
+            yield return Init(hostMode);
+
             // We have "UnboundTrigger" purposely not bound to any animations so we can test resetting.
             //  If we used a trigger that was bound to a transition, then the trigger would reset as soon as the
             //  transition happens.  This way it will stay stuck on
@@ -114,9 +127,12 @@ public IEnumerator AnimationTriggerReset([Values(true, false)] bool asHash)
             Assert.False(s_GloabalTimeOutHelper.TimedOut, "Timed out on client reset check");
         }
 
+
         [UnityTest]
-        public IEnumerator AnimationStateSyncTest()
+        public IEnumerator AnimationStateSyncTest([Values(true, false)] bool hostMode)
         {
+            yield return Init(hostMode);
+
             // check that we have started in the default state
             Assert.True(m_PlayerOnServerAnimator.GetCurrentAnimatorStateInfo(0).IsName("DefaultState"));
             Assert.True(m_PlayerOnClientAnimator.GetCurrentAnimatorStateInfo(0).IsName("DefaultState"));
@@ -137,8 +153,10 @@ public IEnumerator AnimationStateSyncTest()
         }
 
         [UnityTest]
-        public IEnumerator AnimationStateSyncTriggerTest([Values(true, false)] bool asHash)
+        public IEnumerator AnimationStateSyncTriggerTest([Values(true, false)] bool asHash, [Values(true, false)] bool hostMode)
         {
+            yield return Init(hostMode);
+
             string triggerString = "TestTrigger";
             int triggerHash = Animator.StringToHash(triggerString);
 
@@ -172,8 +190,9 @@ public IEnumerator AnimationStateSyncTriggerTest([Values(true, false)] bool asHa
         }
 
         [UnityTest]
-        public IEnumerator AnimationStateSyncTestWithOverride()
+        public IEnumerator AnimationStateSyncTestWithOverride([Values(true, false)] bool hostMode)
         {
+            yield return Init(hostMode);
             // set up the animation override controller
             var overrideController = Resources.Load("TestAnimatorOverrideController") as AnimatorOverrideController;
             m_PlayerOnServer.GetComponent<Animator>().runtimeAnimatorController = overrideController;

com.unity.netcode.gameobjects/Tests/Runtime/NetworkAnimator/Resources/TestAnimatorController.controller

@@ -147,19 +147,19 @@ AnimatorController:
     m_DefaultFloat: 0
     m_DefaultInt: 0
     m_DefaultBool: 0
-    m_Controller: {fileID: 0}
+    m_Controller: {fileID: 9100000}
   - m_Name: TestTrigger
     m_Type: 9
     m_DefaultFloat: 0
     m_DefaultInt: 0
     m_DefaultBool: 0
-    m_Controller: {fileID: 0}
+    m_Controller: {fileID: 9100000}
   - m_Name: UnboundTrigger
     m_Type: 9
     m_DefaultFloat: 0
     m_DefaultInt: 0
     m_DefaultBool: 0
-    m_Controller: {fileID: 0}
+    m_Controller: {fileID: 9100000}
   m_AnimatorLayers:
   - serializedVersion: 5
     m_Name: Base Layer