feat: ClientPlayerAvatarNetworkAnimator added to handle client synchronizations [MTT-3152] (#886)

* ClientPlayerAvatarNetworkAnimator introduced to handle client syncs

* reformat class

* format

* changelog addition

Author: fernando-cortez

Assets/Prefabs/Character/PlayerAvatar.prefab

@@ -140,9 +140,9 @@ public override void OnNetworkSpawn()
             {
                 name = "AvatarGraphics" + m_ServerCharacter.OwnerClientId;
 
-                if (m_ServerCharacter.TryGetComponent(out ClientAvatarGuidHandler clientAvatarGuidHandler))
+                if (m_ServerCharacter.TryGetComponent(out ClientPlayerAvatarNetworkAnimator characterNetworkAnimator))
                 {
-                    m_ClientVisualsAnimator = clientAvatarGuidHandler.graphicsAnimator;
+                    m_ClientVisualsAnimator = characterNetworkAnimator.Animator;
                 }
 
                 m_CharacterSwapper = GetComponentInChildren<CharacterSwap>();

Assets/Scripts/Gameplay/GameplayObjects/Character/ClientAvatarGuidHandler.cs

@@ -0,0 +1,70 @@
+using Unity.Netcode;
+using Unity.Netcode.Components;
+using UnityEngine;
+
+namespace Unity.BossRoom.Gameplay.GameplayObjects.Character
+{
+    /// <summary>
+    /// Component that spawns a PlayerAvatar's Avatar. It does this in two places:
+    /// 1) either inside OnNetworkSpawn() or
+    /// 2) inside NetworkAnimator's OnSynchronize method.
+    /// The latter is necessary for clients receiving initial synchronizing data, where the Animator needs to be present
+    /// and bound (Animator.Bind()) *before* the incoming animation data is applied.
+    /// </summary>
+    public class ClientPlayerAvatarNetworkAnimator : NetworkAnimator
+    {
+        [SerializeField]
+        NetworkAvatarGuidState m_NetworkAvatarGuidState;
+
+        bool m_AvatarInstantiated;
+
+        public override void OnNetworkSpawn()
+        {
+            base.OnNetworkSpawn();
+            if (!IsClient || m_AvatarInstantiated)
+            {
+                return;
+            }
+
+            InstantiateAvatar();
+        }
+
+        public override void OnNetworkDespawn()
+        {
+            base.OnNetworkDespawn();
+            m_AvatarInstantiated = false;
+            var avatarGraphics = Animator.transform.GetChild(0);
+            if (avatarGraphics != null)
+            {
+                Destroy(avatarGraphics.gameObject);
+            }
+        }
+
+        protected override void OnSynchronize<T>(ref BufferSerializer<T> serializer)
+        {
+            if (NetworkManager.Singleton.IsClient && !m_AvatarInstantiated)
+            {
+                InstantiateAvatar();
+            }
+
+            base.OnSynchronize(ref serializer);
+        }
+
+        void InstantiateAvatar()
+        {
+            if (Animator.transform.childCount > 0)
+            {
+                // we may receive a NetworkVariable's OnValueChanged callback more than once as a client
+                // this makes sure we don't spawn a duplicate graphics GameObject
+                return;
+            }
+
+            // spawn avatar graphics GameObject
+            Instantiate(m_NetworkAvatarGuidState.RegisteredAvatar.Graphics, Animator.transform);
+
+            Animator.Rebind();
+
+            m_AvatarInstantiated = true;
+        }
+    }
+}

Assets/Scripts/Gameplay/GameplayObjects/Character/ClientCharacter.cs

@@ -1,5 +1,4 @@
 using System;
-using System.Collections;
 using Unity.BossRoom.Gameplay.Configuration;
 using Unity.Netcode;
 using Unity.Netcode.Components;
@@ -24,18 +23,7 @@ public override void OnNetworkSpawn()
         {
             if (IsServer)
             {
-                // Wait until next frame before registering on OnValueChanged to make sure NetworkAnimator has spawned before.
-                StartCoroutine(WaitToRegisterOnLifeStateChanged());
-            }
-        }
-
-        IEnumerator WaitToRegisterOnLifeStateChanged()
-        {
-            yield return new WaitForEndOfFrame();
-            m_NetworkLifeState.LifeState.OnValueChanged += OnLifeStateChanged;
-            if (m_NetworkLifeState.LifeState.Value != LifeState.Alive)
-            {
-                OnLifeStateChanged(LifeState.Alive, m_NetworkLifeState.LifeState.Value);
+                m_NetworkLifeState.LifeState.OnValueChanged += OnLifeStateChanged;
             }
         }
 
@@ -59,7 +47,7 @@ void OnLifeStateChanged(LifeState previousValue, LifeState newValue)
 
         public override void OnNetworkDespawn()
         {
-            if (IsServer)
+            if (IsServer && m_NetworkLifeState != null)
             {
                 m_NetworkLifeState.LifeState.OnValueChanged -= OnLifeStateChanged;
             }

Assets/Scripts/Gameplay/GameplayObjects/Character/ClientPlayerAvatarNetworkAnimator.cs

@@ -1,4 +1,3 @@
-using System;
 using System.Collections;
 using Unity.BossRoom.Gameplay.GameplayObjects;
 using Unity.BossRoom.Gameplay.GameplayObjects.Character;
@@ -44,7 +43,7 @@ public class UIStateDisplayHandler : NetworkBehaviour
 
         ServerCharacter m_ServerCharacter;
 
-        ClientAvatarGuidHandler m_ClientAvatarGuidHandler;
+        ClientPlayerAvatarNetworkAnimator m_ClientPlayerAvatarNetworkAnimator;
 
         NetworkAvatarGuidState m_NetworkAvatarGuidState;
 
@@ -111,18 +110,11 @@ public override void OnNetworkSpawn()
             m_VerticalOffset = new Vector3(0f, m_VerticalScreenOffset, 0f);
 
             // if PC, find our graphics transform and update health through callbacks, if displayed
-            if (TryGetComponent(out m_ClientAvatarGuidHandler) && TryGetComponent(out m_NetworkAvatarGuidState))
+            if (TryGetComponent(out m_ClientPlayerAvatarNetworkAnimator) && TryGetComponent(out m_NetworkAvatarGuidState))
             {
                 m_BaseHP = m_NetworkAvatarGuidState.RegisteredAvatar.CharacterClass.BaseHP;
 
-                if (m_ServerCharacter.clientCharacter)
-                {
-                    TrackGraphicsTransform(m_ServerCharacter.clientCharacter.gameObject);
-                }
-                else
-                {
-                    m_ClientAvatarGuidHandler.AvatarGraphicsSpawned += TrackGraphicsTransform;
-                }
+                m_TransformToTrack = m_ClientPlayerAvatarNetworkAnimator.Animator.transform;
 
                 if (m_DisplayHealth)
                 {
@@ -154,11 +146,6 @@ void OnDisable()
                 m_NetworkHealthState.HitPointsReplenished -= DisplayUIHealth;
                 m_NetworkHealthState.HitPointsDepleted -= RemoveUIHealth;
             }
-
-            if (m_ClientAvatarGuidHandler)
-            {
-                m_ClientAvatarGuidHandler.AvatarGraphicsSpawned -= TrackGraphicsTransform;
-            }
         }
 
         void DisplayUIName()
@@ -213,11 +200,6 @@ IEnumerator WaitToHideHealthBar()
             m_UIState.HideHealth();
         }
 
-        void TrackGraphicsTransform(GameObject graphicsGameObject)
-        {
-            m_TransformToTrack = graphicsGameObject.transform;
-        }
-
         /// <remarks>
         /// Moving UI objects on LateUpdate ensures that the game camera is at its final position pre-render.
         /// </remarks>

Assets/Scripts/Gameplay/GameplayObjects/Character/ClientAvatarGuidHandler.cs.meta renamed to Assets/Scripts/Gameplay/GameplayObjects/Character/ClientPlayerAvatarNetworkAnimator.cs.meta

@@ -14,6 +14,9 @@ Additional documentation and release notes are available at [Multiplayer Documen
   * ClientConnectedState has been modified to account for server/host now populating DisconnectReason before disconnecting a client before shutting down
 * Upgraded editor version to 2022.3.22f1 (#884)
   * com.unity.render-pipelines.universal upgraded to v14.0.10
+* ClientPlayerAvatarNetworkAnimator has been created to: instantiate the player model based on a networked GUID, rebind this rig to the player's Animator, and apply synchronize data to said Animator (#886)
+  * This change allows for NetworkAnimator's synchronize step to properly apply its sync data to clients instead of applying an animation state change on OnNetworkSpawn()
+  * A side-effect of this change has been that a coroutine that had been awaiting the assignment of NetworkAnimator has since been removed as it is no longer an issue on Netcode for GameObjects (since v1.3.1)
 
 ### Cleanup
 * Removed NetworkObject from MainMenuState (#881)