fix: NetworkAnimator does not update animator's parameters unless the animator's layer changes [MTT-3492] (#1946)

* fix

Fix for animators that have one layer and handle most animation state changes via properties.

* test and style

Adding an integration test to validate the changes.
Updated comments in NetworkAnimator.

* style

MTT-3492
Remove namespace references I was not using.

* style

only applying changes required for asmdef file.

* update and test

Removed the duplicate tracking for parameters.
Added the ability to make it owner or server authoritative.
Included triggers in this update.
Added test for triggers.

* style

* update

removing unused parameter state class

* update and fix

This updates the manual animation test so we can visually see NetworkAnimator working properly.
This fixes the issue where late joining clients would not be updated to the NetworkAnimator's state.
This fixes some test project asset related issues including to make sure we are always set to run in the background and have a target fps of 120.

* update and fix

This fixes the issue where it would sometimes not clean up/dispose the "cache" native array.
This improves the client synchronization (as best for the current way that we write information).

Manual Test Updates:
This fixes an issue with scale as well as getting other players to join and distribute evenly.
This fixes the issue with the prefab asset itself (scale issue impacted pulse).
Increased send/receive buffer sizes (we send a lot of data for animations currently)

* update

Making CleanUp private.

* fix

This lost its parameters for some reason...

* fix

Missed adding the trigger state back.

* test

This verifies that late joining clients get synchronized to the current NetworkAnimator state.
This also fixes the issue where stopping a client during an integration test always meant that you wanted to delete it.  Now, you have the option to not destroy in the event you want to stop a client, do some stuff, and then reconnect the client.

* test and style

Minor tweak and additional check that the AnimatorTestHelper instances equals the TotalClient count (more of a sanity check).
Added some comments and removed some LFs.

* Test and Style

I hooked up the TriggerTest to the wrong state.  Fixed issue by creating a new state that is activated by the test trigger.
Changing the warning to an actual error in TriggerTest (i.e. if that happens the test should fail).

* Update

This includes several adjustments to improve performance.
-Parameters are now sent separately
-New internal NetworkAnimatorStateChangeHandler that changes when:
  -It detects changes to the Animator
  -The server-side client forwarded RPC messages are processed here
-Removed the OwnerAuthoritative public bool

This also include integration test adjustments in order to be able to create and join a new client on the fly during a test.

* fix

This got test running locally in a stand alone testrunner build.
Going to give this a shot in Yamato land.

* wip

Sorting out the issue with server-side owner late joining client synchronization.  This version works with all tests ==but== the server-side owner late joining client portion... still trying to find this bug.

* fix

This reverts Triggers back to being set by an RPC (better to separate for now).
This also fixes minor issues with a player joining right in the middle of a transition.
This also fixes issue where trigger paths were not being set when transitioning (still could use more work).
This fixes the issue with the NetworkAnimator failing on the late joining player test where the late joining player could join while an Animator is in the middle of a transition.

* style

* style

* update

This improves upon the synchronization of a late joining client when connected while an animation is in the middle of a transition (it doesn't fix the issue completely, but is closer to being synchronized with the source animation)

* update

Removing VerboseDebug information

* Test

Added a small profiling test to profile where we spend time.
(sending RPCs)

* update - optimize

Optimizing how we send parameter updates.  Now we only send the parameters that changed.
This also avoids some smaller allocations by reusing a ClientRpcParams instance and a List<ulong> to hold the client ids to send to.

* style

updating a missed whitespace

* update

MTT-3492 MTT-2644
Updating what was fixed in the change log.

* update

Minor tweaks suggested by Fatih.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -25,6 +25,12 @@ Additional documentation and release notes are available at [Multiplayer Documen
 - Fixed issues when multiple `ConnectionApprovalCallback`s were registered (#1972)
 - `FixedString` types can now be used in NetworkVariables and RPCs again without requiring a `ForceNetworkSerializeByMemcpy<>` wrapper (#1961)
 - Fixed endless dialog boxes when adding a `NetworkBehaviour` to a `NetworkManager` or vice-versa. (#1947)
+- Fixed `NetworkAnimator` issue where it was only synchronizing parameters if the layer or state changed or was transitioning between states. (#1946)
+- Fixed `NetworkAnimator` issue where when it did detect a parameter had changed it would send all parameters as opposed to only the parameters that changed. (#1946)
+- Fixed `NetworkAnimator` issue where it was not always disposing the `NativeArray` that is allocated when spawned. (#1946)
+- Fixed `NetworkAnimator` issue where it was not taking the animation speed or state speed multiplier into consideration. (#1946)
+- Fixed `NetworkAnimator` issue where it was not properly synchronizing late joining clients if they joined while `Animator` was transitioning between states. (#1946)
+- Fixed `NetworkAnimator` issue where the server was not relaying changes to non-owner clients when a client was the owner. (#1946)
 
 ## [1.0.0-pre.9] - 2022-05-10
 

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

@@ -1,3 +1,4 @@
+using System;
 namespace Unity.Netcode.TestHelpers.Runtime
 {
     public class ObjectNameIdentifier : NetworkBehaviour
@@ -13,6 +14,7 @@ public class ObjectNameIdentifier : NetworkBehaviour
         /// Keep a reference to the assigned NetworkObject
         /// <see cref="OnDestroy"/>
         /// </summary>
+        [NonSerialized]
         private NetworkObject m_NetworkObject;
         private string m_OriginalName;
 

com.unity.netcode.gameobjects/TestHelpers/Runtime/Components/ObjectNameIdentifier.cs

@@ -22,7 +22,7 @@ public abstract class NetcodeIntegrationTest
         /// determine how clients will load scenes
         /// </summary>
         internal static bool IsRunning { get; private set; }
-        protected static TimeoutHelper s_GlobalTimeoutHelper = new TimeoutHelper(4.0f);
+        protected static TimeoutHelper s_GlobalTimeoutHelper = new TimeoutHelper(8.0f);
         protected static WaitForSeconds s_DefaultWaitForTick = new WaitForSeconds(1.0f / k_DefaultTickRate);
 
         /// <summary>
@@ -80,9 +80,11 @@ public static void DeregisterNetworkObject(ulong localClientId, ulong networkObj
             }
         }
 
-        protected int TotalClients => m_UseHost ? NumberOfClients + 1 : NumberOfClients;
+        protected int TotalClients => m_UseHost ? m_NumberOfClients + 1 : m_NumberOfClients;
 
         protected const uint k_DefaultTickRate = 30;
+
+        private int m_NumberOfClients;
         protected abstract int NumberOfClients { get; }
 
         /// <summary>
@@ -125,7 +127,7 @@ public enum HostOrServer
 
         private NetworkManagerInstatiationMode m_NetworkManagerInstatiationMode;
 
-        protected bool m_EnableVerboseDebug { get; private set; }
+        protected bool m_EnableVerboseDebug { get; set; }
 
         /// <summary>
         /// Used to display the various integration test
@@ -171,6 +173,8 @@ protected virtual void OnOneTimeSetup()
         [OneTimeSetUp]
         public void OneTimeSetup()
         {
+            Application.runInBackground = true;
+            m_NumberOfClients = NumberOfClients;
             IsRunning = true;
             m_EnableVerboseDebug = OnSetVerboseDebug();
             IntegrationTestSceneHandler.VerboseDebugMode = m_EnableVerboseDebug;
@@ -255,6 +259,59 @@ protected void CreateServerAndClients()
             CreateServerAndClients(NumberOfClients);
         }
 
+        protected virtual void OnNewClientCreated(NetworkManager networkManager)
+        {
+
+        }
+
+        protected virtual void OnNewClientStartedAndConnected(NetworkManager networkManager)
+        {
+
+        }
+
+        private void AddRemoveNetworkManager(NetworkManager networkManager, bool addNetworkManager)
+        {
+            var clientNetworkManagersList = new List<NetworkManager>(m_ClientNetworkManagers);
+            if (addNetworkManager)
+            {
+                clientNetworkManagersList.Add(networkManager);
+            }
+            else
+            {
+                clientNetworkManagersList.Remove(networkManager);
+            }
+            m_ClientNetworkManagers = clientNetworkManagersList.ToArray();
+            m_NumberOfClients = clientNetworkManagersList.Count;
+        }
+
+        protected IEnumerator CreateAndStartNewClient()
+        {
+            var networkManager = NetcodeIntegrationTestHelpers.CreateNewClient(m_ClientNetworkManagers.Length);
+            networkManager.NetworkConfig.PlayerPrefab = m_PlayerPrefab;
+            OnNewClientCreated(networkManager);
+
+            NetcodeIntegrationTestHelpers.StartOneClient(networkManager);
+            AddRemoveNetworkManager(networkManager, true);
+            // Wait for the new client to connect
+            yield return WaitForClientsConnectedOrTimeOut();
+            if (s_GlobalTimeoutHelper.TimedOut)
+            {
+                AddRemoveNetworkManager(networkManager, false);
+                Object.Destroy(networkManager.gameObject);
+            }
+            AssertOnTimeout($"{nameof(CreateAndStartNewClient)} timed out waiting for the new client to be connected!");
+            ClientNetworkManagerPostStart(networkManager);
+            VerboseDebug($"[{networkManager.name}] Created and connected!");
+            OnNewClientCreated(networkManager);
+        }
+
+        protected IEnumerator StopOneClient(NetworkManager networkManager, bool destroy = false)
+        {
+            NetcodeIntegrationTestHelpers.StopOneClient(networkManager, destroy);
+            AddRemoveNetworkManager(networkManager, false);
+            yield return WaitForConditionOrTimeOut(() => !networkManager.IsConnectedClient);
+        }
+
         /// <summary>
         /// Creates the server and clients
         /// </summary>
@@ -322,30 +379,35 @@ protected virtual IEnumerator OnServerAndClientsConnected()
             yield return null;
         }
 
+        private void ClientNetworkManagerPostStart(NetworkManager networkManager)
+        {
+            networkManager.name = $"NetworkManager - Client - {networkManager.LocalClientId}";
+            Assert.NotNull(networkManager.LocalClient.PlayerObject, $"{nameof(StartServerAndClients)} detected that client {networkManager.LocalClientId} does not have an assigned player NetworkObject!");
+
+            // Get all player instances for the current client NetworkManager instance
+            var clientPlayerClones = Object.FindObjectsOfType<NetworkObject>().Where((c) => c.IsPlayerObject && c.OwnerClientId == networkManager.LocalClientId);
+            // Add this player instance to each client player entry
+            foreach (var playerNetworkObject in clientPlayerClones)
+            {
+                // When the server is not the host this needs to be done
+                if (!m_PlayerNetworkObjects.ContainsKey(playerNetworkObject.NetworkManager.LocalClientId))
+                {
+                    m_PlayerNetworkObjects.Add(playerNetworkObject.NetworkManager.LocalClientId, new Dictionary<ulong, NetworkObject>());
+                }
+                if (!m_PlayerNetworkObjects[playerNetworkObject.NetworkManager.LocalClientId].ContainsKey(networkManager.LocalClientId))
+                {
+                    m_PlayerNetworkObjects[playerNetworkObject.NetworkManager.LocalClientId].Add(networkManager.LocalClientId, playerNetworkObject);
+                }
+            }
+        }
+
         protected void ClientNetworkManagerPostStartInit()
         {
             // Creates a dictionary for all player instances client and server relative
             // This provides a simpler way to get a specific player instance relative to a client instance
             foreach (var networkManager in m_ClientNetworkManagers)
             {
-                networkManager.name = $"NetworkManager - Client - {networkManager.LocalClientId}";
-                Assert.NotNull(networkManager.LocalClient.PlayerObject, $"{nameof(StartServerAndClients)} detected that client {networkManager.LocalClientId} does not have an assigned player NetworkObject!");
-
-                // Get all player instances for the current client NetworkManager instance
-                var clientPlayerClones = Object.FindObjectsOfType<NetworkObject>().Where((c) => c.IsPlayerObject && c.OwnerClientId == networkManager.LocalClientId);
-                // Add this player instance to each client player entry
-                foreach (var playerNetworkObject in clientPlayerClones)
-                {
-                    // When the server is not the host this needs to be done
-                    if (!m_PlayerNetworkObjects.ContainsKey(playerNetworkObject.NetworkManager.LocalClientId))
-                    {
-                        m_PlayerNetworkObjects.Add(playerNetworkObject.NetworkManager.LocalClientId, new Dictionary<ulong, NetworkObject>());
-                    }
-                    if (!m_PlayerNetworkObjects[playerNetworkObject.NetworkManager.LocalClientId].ContainsKey(networkManager.LocalClientId))
-                    {
-                        m_PlayerNetworkObjects[playerNetworkObject.NetworkManager.LocalClientId].Add(networkManager.LocalClientId, playerNetworkObject);
-                    }
-                }
+                ClientNetworkManagerPostStart(networkManager);
             }
         }
 

com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs

@@ -169,17 +169,10 @@ public static void RegisterHandlers(NetworkManager networkManager, bool serverSi
             }
         }
 
-        public static NetworkManager CreateServer()
+        private static void AddUnityTransport(NetworkManager networkManager)
         {
-            // Create gameObject
-            var go = new GameObject("NetworkManager - Server");
-
-            // Create networkManager component
-            var server = go.AddComponent<NetworkManager>();
-            NetworkManagerInstances.Insert(0, server);
-
             // Create transport
-            var unityTransport = go.AddComponent<UnityTransport>();
+            var unityTransport = networkManager.gameObject.AddComponent<UnityTransport>();
             // We need to increase this buffer size for tests that spawn a bunch of things
             unityTransport.MaxPayloadSize = 256000;
             unityTransport.MaxSendQueueSize = 1024 * 1024;
@@ -189,11 +182,22 @@ public static NetworkManager CreateServer()
             unityTransport.ConnectTimeoutMS = 500;
 
             // Set the NetworkConfig
-            server.NetworkConfig = new NetworkConfig()
+            networkManager.NetworkConfig = new NetworkConfig()
             {
                 // Set transport
                 NetworkTransport = unityTransport
             };
+        }
+
+        public static NetworkManager CreateServer()
+        {
+            // Create gameObject
+            var go = new GameObject("NetworkManager - Server");
+
+            // Create networkManager component
+            var server = go.AddComponent<NetworkManager>();
+            NetworkManagerInstances.Insert(0, server);
+            AddUnityTransport(server);
             return server;
         }
 
@@ -227,6 +231,17 @@ public static bool Create(int clientCount, out NetworkManager server, out Networ
             return true;
         }
 
+        internal static NetworkManager CreateNewClient(int identifier)
+        {
+            // Create gameObject
+            var go = new GameObject("NetworkManager - Client - " + identifier);
+            // Create networkManager component
+            var networkManager = go.AddComponent<NetworkManager>();
+            AddUnityTransport(networkManager);
+            return networkManager;
+        }
+
+
         /// <summary>
         /// Used to add a client to the already existing list of clients
         /// </summary>
@@ -235,23 +250,10 @@ public static bool Create(int clientCount, out NetworkManager server, out Networ
         public static bool CreateNewClients(int clientCount, out NetworkManager[] clients)
         {
             clients = new NetworkManager[clientCount];
-            var activeSceneName = SceneManager.GetActiveScene().name;
             for (int i = 0; i < clientCount; i++)
             {
-                // Create gameObject
-                var go = new GameObject("NetworkManager - Client - " + i);
                 // Create networkManager component
-                clients[i] = go.AddComponent<NetworkManager>();
-
-                // Create transport
-                var unityTransport = go.AddComponent<UnityTransport>();
-
-                // Set the NetworkConfig
-                clients[i].NetworkConfig = new NetworkConfig()
-                {
-                    // Set transport
-                    NetworkTransport = unityTransport
-                };
+                clients[i] = CreateNewClient(i);
             }
 
             NetworkManagerInstances.AddRange(clients);
@@ -262,12 +264,15 @@ public static bool CreateNewClients(int clientCount, out NetworkManager[] client
         /// Stops one single client and makes sure to cleanup any static variables in this helper
         /// </summary>
         /// <param name="clientToStop"></param>
-        public static void StopOneClient(NetworkManager clientToStop)
+        public static void StopOneClient(NetworkManager clientToStop, bool destroy = true)
         {
             clientToStop.Shutdown();
             s_Hooks.Remove(clientToStop);
-            Object.Destroy(clientToStop.gameObject);
-            NetworkManagerInstances.Remove(clientToStop);
+            if (destroy)
+            {
+                Object.Destroy(clientToStop.gameObject);
+                NetworkManagerInstances.Remove(clientToStop);
+            }
         }
 
         /// <summary>
@@ -279,6 +284,10 @@ public static void StartOneClient(NetworkManager clientToStart)
             clientToStart.StartClient();
             s_Hooks[clientToStart] = new MultiInstanceHooks();
             clientToStart.MessagingSystem.Hook(s_Hooks[clientToStart]);
+            if (!NetworkManagerInstances.Contains(clientToStart))
+            {
+                NetworkManagerInstances.Add(clientToStart);
+            }
             // if set, then invoke this for the client
             RegisterHandlers(clientToStart);
         }
@@ -469,8 +478,11 @@ public static void MakeNetworkObjectTestPrefab(NetworkObject networkObject, uint
             // this feature only works with NetcodeIntegrationTest derived classes
             if (IsNetcodeIntegrationTestRunning)
             {
-                // Add the object identifier component
-                networkObject.gameObject.AddComponent<ObjectNameIdentifier>();
+                if (networkObject.GetComponent<ObjectNameIdentifier>() == null && networkObject.GetComponentInChildren<ObjectNameIdentifier>() == null)
+                {
+                    // Add the object identifier component
+                    networkObject.gameObject.AddComponent<ObjectNameIdentifier>();
+                }
             }
         }
 

com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTestHelpers.cs

@@ -13,18 +13,32 @@ MonoBehaviour:
   m_Name: AddressableAssetSettings
   m_EditorClassIdentifier: 
   m_DefaultGroup: 93aa504d1b753cb41a8a779ae63f5795
+  m_CachedHash:
+    serializedVersion: 2
+    Hash: 00000000000000000000000000000000
   m_OptimizeCatalogSize: 0
   m_BuildRemoteCatalog: 0
   m_BundleLocalCatalog: 0
+  m_CatalogRequestsTimeout: 0
   m_DisableCatalogUpdateOnStart: 0
   m_IgnoreUnsupportedFilesInBuild: 0
   m_UniqueBundleIds: 0
+  m_NonRecursiveBuilding: 1
+  m_CCDEnabled: 0
   m_maxConcurrentWebRequests: 500
   m_ContiguousBundles: 0
+  m_StripUnityVersionFromBundleBuild: 0
+  m_DisableVisibleSubAssetRepresentations: 0
+  m_ShaderBundleNaming: 0
+  m_ShaderBundleCustomNaming: 
+  m_MonoScriptBundleNaming: 0
+  m_MonoScriptBundleCustomNaming: 
   m_RemoteCatalogBuildPath:
     m_Id: eaae5cc67c56cfe4299363a742a284b3
   m_RemoteCatalogLoadPath:
     m_Id: 2a3d80e942fdbfe49a979d4a22bbe893
+  m_ContentStateBuildPath: 
+  m_BuildAddressablesWithPlayerBuild: 2
   m_overridePlayerVersion: 
   m_GroupAssets:
   - {fileID: 11400000, guid: 783a7ba7cfd87084388760e6c08547b4, type: 2}

testproject/Assets/AddressableAssetsData/AddressableAssetSettings.asset

@@ -14,6 +14,6 @@ MonoBehaviour:
   m_EditorClassIdentifier: 
   SceneToReference: {fileID: 102900000, guid: f88da8bb8d07e11418eaad6524d5cc12, type: 3}
   m_IncludedScenes: []
-  m_DisplayName: NetworkAnimator
+  m_DisplayName: NetworkAnimator Test
   m_ReferencedScenes:
-  - NetworkTransformEnhancementsScene
+  - NetworkAnimatorEnhancement

testproject/Assets/References/Scene/NetworkAnimatorEnhancements.asset

@@ -2,13 +2,22 @@
 using UnityEngine;
 using UnityEngine.SceneManagement;
 using UnityEngine.UI;
+#if UNITY_EDITOR
+using UnityEditor;
+#endif
 
 /// <summary>
 /// Main Menu Manager only accepts types of MenuReference
 /// </summary>
 public class MainMenuManager : MenuManager<MenuReference>
 {
-
+#if UNITY_EDITOR
+    [InitializeOnEnterPlayMode]
+    public static void OnEnterPlaymodeInEditor(EnterPlayModeOptions options)
+    {
+        Initialize();
+    }
+#endif
 }
 
 
@@ -37,8 +46,16 @@ public class MenuManager<T> : MonoBehaviour where T : ISceneReference
     protected Dictionary<string, T> m_SceneMenuReferencesByDisplayName = new Dictionary<string, T>();
 
 
+    protected static void Initialize()
+    {
+        Application.runInBackground = true;
+        Application.targetFrameRate = 60;
+    }
+
+
     private void Awake()
     {
+        Initialize();
         Screen.SetResolution(HorizontalResolution, VerticalResolution, false);
     }