fix: NetworkTransform synchronization fixes and owner authoritative performance improvement on server-host [MTT-6971] (#2636)

* fix

This resolves an issue with half float precision not synchronizing properly when running in owner authoritative mode, the owner client sends an update to the host-server, the host-server then forwards the state update to the remaining clients at the end of the frame but during the initial processing of the state update the delta position value is collapsed into the full float position due to exceeding the maximum delta. The primary issue was the order of operations that occurred using NetworkVariables to update the transform state when in owner authority mode.

This also reduces the amount of serialization and processing a host has to perform when receiving owner authoritative updates and there are 2 or more remote clients connected (i.e. it has to forward the result to the other clients).  Now, it just forwards the message payload to the non-owner clients upon receiving the state update.

This also fixes:
- an on-going issue with scale where really the answer was to send both the lossy and local scale values when synchronizing and/or teleporting and letting the client-side determine which to use depending upon whether the NetworkObject was parented, was the parenting applied when applying the states, and whether the parenting used WorldPositionStays.

- some minor issues with the parenting test not checking if the NetworkObject was parented before testing the final values.

- updates named message handling slightly so it doesn't spam the log console during shutdown if there are incoming messages while shutting down.

- resolves an issue where you have an owner authoritative NetworkTransform and a host-server with two remote clients connected and the following sequence occurs:
  - Client A connects to the host first and takes ownership of a NetworkObject but does not change the transform state.
  - Client B connects and attempts to move the NetworkObject that Client A owns
  - Client B was able to move the NetworkObject

* test

Adding NetworkTransformStateFlags test to validate the NetworkTransformState flags.
Adding internal BitSet setter and getter for testing and a BitSet flag test.

This test adds a "sub-child" (NetworkObject parented under a child) to the parenting test to add the additional check and make sure that all nested children preserve their values when changed for connected clients and those changes are applied and preserved for late joining clients.

Adding a test to verify that when a late joining client is connected after ownership has been transferred from the host-server to an already connected client and the owner authoritative transform has not changed yet that the non-owner client cannot change the transform values.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -12,6 +12,19 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed issue with client synchronization of position when using half precision and the delta position reaches the maximum value and is collapsed on the host prior to being forwarded to the non-owner clients. (#2636)
+- Fixed issue with scale not synchronizing properly depending upon the spawn order of NetworkObjects. (#2636)
+- Fixed issue position was not properly transitioning between ownership changes with an owner authoritative NetworkTransform. (#2636)
+- Fixed issue where a late joining non-owner client could update an owner authoritative NetworkTransform if ownership changed without any updates to position prior to the non-owner client joining. (#2636)
+
+### Changed
+
+## [1.5.2] - 2023-07-24
+
+### Added
+
+### Fixed
+
 - Fixed issue where `NetworkClient.OwnedObjects` was not returning any owned objects due to the `NetworkClient.IsConnected` not being properly set. (#2631)
 - Fixed a crash when calling TrySetParent with a null Transform (#2625)
 - Fixed issue where a `NetworkTransform` using full precision state updates was losing transform state updates when interpolation was enabled. (#2624)

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

@@ -27,7 +27,7 @@ public struct HalfVector3 : INetworkSerializable
         /// <summary>
         /// The half float precision value of the z-axis as a <see cref="half"/>.
         /// </summary>
-        public half Z => Axis.x;
+        public half Z => Axis.z;
 
         /// <summary>
         /// Used to store the half float precision values as a <see cref="half3"/>
@@ -39,6 +39,17 @@ public struct HalfVector3 : INetworkSerializable
         /// </summary>
         public bool3 AxisToSynchronize;
 
+        /// <summary>
+        /// Directly sets each axial value to the passed in full precision values
+        /// that are converted to half precision
+        /// </summary>
+        internal void Set(float x, float y, float z)
+        {
+            Axis.x = math.half(x);
+            Axis.y = math.half(y);
+            Axis.z = math.half(z);
+        }
+
         private void SerializeWrite(FastBufferWriter writer)
         {
             for (int i = 0; i < Length; i++)

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

@@ -24,7 +24,11 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
 
         public void Handle(ref NetworkContext context)
         {
-            ((NetworkManager)context.SystemOwner).CustomMessagingManager.InvokeNamedMessage(Hash, context.SenderId, m_ReceiveData, context.SerializedHeaderSize);
+            var networkManager = (NetworkManager)context.SystemOwner;
+            if (!networkManager.ShutdownInProgress)
+            {
+                ((NetworkManager)context.SystemOwner).CustomMessagingManager.InvokeNamedMessage(Hash, context.SenderId, m_ReceiveData, context.SerializedHeaderSize);
+            }
         }
     }
 }

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NamedMessage.cs

@@ -518,15 +518,18 @@ internal int GetMessageVersion(Type type, ulong clientId, bool forReceive = fals
         {
             if (!m_PerClientMessageVersions.TryGetValue(clientId, out var versionMap))
             {
-                if (forReceive)
+                var networkManager = NetworkManager.Singleton;
+                if (networkManager != null && networkManager.LogLevel == LogLevel.Developer)
                 {
-                    Debug.LogWarning($"Trying to receive {type.Name} from client {clientId} which is not in a connected state.");
-                }
-                else
-                {
-                    Debug.LogWarning($"Trying to send {type.Name} to client {clientId} which is not in a connected state.");
+                    if (forReceive)
+                    {
+                        NetworkLog.LogWarning($"Trying to receive {type.Name} from client {clientId} which is not in a connected state.");
+                    }
+                    else
+                    {
+                        NetworkLog.LogWarning($"Trying to send {type.Name} to client {clientId} which is not in a connected state.");
+                    }
                 }
-
                 return -1;
             }
 

com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs

@@ -46,6 +46,93 @@ protected override void OnServerAndClientsCreated()
             base.OnServerAndClientsCreated();
         }
 
+        /// <summary>
+        /// Clients created during a test need to have their prefabs list updated to
+        /// match the server's prefab list.
+        /// </summary>
+        protected override void OnNewClientCreated(NetworkManager networkManager)
+        {
+            foreach (var networkPrefab in m_ServerNetworkManager.NetworkConfig.Prefabs.Prefabs)
+            {
+                networkManager.NetworkConfig.Prefabs.Add(networkPrefab);
+            }
+
+            base.OnNewClientCreated(networkManager);
+        }
+
+        private bool ClientIsOwner()
+        {
+            var clientId = m_ClientNetworkManagers[0].LocalClientId;
+            if (!VerifyObjectIsSpawnedOnClient.GetClientsThatSpawnedThisPrefab().Contains(clientId))
+            {
+                return false;
+            }
+            if (VerifyObjectIsSpawnedOnClient.GetClientInstance(clientId).OwnerClientId != clientId)
+            {
+                return false;
+            }
+            return true;
+        }
+
+        /// <summary>
+        /// This test verifies a late joining client cannot change the transform when:
+        /// - A NetworkObject is spawned with a host and one or more connected clients
+        /// - The NetworkTransform is owner authoritative and spawned with the host as the owner
+        /// - The host does not change the transform values
+        /// - One of the already connected clients gains ownership of the spawned NetworkObject
+        /// - The new client owner does not change the transform values
+        /// - A new late joining client connects and is synchronized
+        /// - The newly connected late joining client tries to change the transform of the NetworkObject
+        /// it does not own
+        /// </summary>
+        [UnityTest]
+        public IEnumerator LateJoinedNonOwnerClientCannotChangeTransform()
+        {
+            // Spawn the m_ClientNetworkTransformPrefab with the host starting as the owner
+            var hostInstance = SpawnObject(m_ClientNetworkTransformPrefab, m_ServerNetworkManager);
+
+            // Wait for the client to spawn it
+            yield return WaitForConditionOrTimeOut(() => VerifyObjectIsSpawnedOnClient.GetClientsThatSpawnedThisPrefab().Contains(m_ClientNetworkManagers[0].LocalClientId));
+
+            // Change the ownership to the connectd client
+            hostInstance.GetComponent<NetworkObject>().ChangeOwnership(m_ClientNetworkManagers[0].LocalClientId);
+
+            // Wait until the client gains ownership
+            yield return WaitForConditionOrTimeOut(ClientIsOwner);
+
+            // Spawn a new client
+            yield return CreateAndStartNewClient();
+
+            // Get the instance of the object relative to the newly joined client
+            var newClientObjectInstance = VerifyObjectIsSpawnedOnClient.GetClientInstance(m_ClientNetworkManagers[1].LocalClientId);
+
+            // Attempt to change the transform values
+            var currentPosition = newClientObjectInstance.transform.position;
+            newClientObjectInstance.transform.position = GetRandomVector3(0.5f, 10.0f);
+            var rotation = newClientObjectInstance.transform.rotation;
+            var currentRotation = rotation.eulerAngles;
+            rotation.eulerAngles = GetRandomVector3(1.0f, 180.0f);
+            var currentScale = newClientObjectInstance.transform.localScale;
+            newClientObjectInstance.transform.localScale = GetRandomVector3(0.25f, 4.0f);
+
+            // Wait one frame so the NetworkTransform can apply the owner's last state received on the late joining client side
+            // (i.e. prevent the non-owner from changing the transform)
+            yield return null;
+
+            // Get the owner instance
+            var ownerInstance = VerifyObjectIsSpawnedOnClient.GetClientInstance(m_ClientNetworkManagers[0].LocalClientId);
+
+            // Verify that the non-owner instance transform values are the same before they were changed last frame
+            Assert.True(Approximately(currentPosition, newClientObjectInstance.transform.position), $"Non-owner instance was able to change the position!");
+            Assert.True(Approximately(currentRotation, newClientObjectInstance.transform.rotation.eulerAngles), $"Non-owner instance was able to change the rotation!");
+            Assert.True(Approximately(currentScale, newClientObjectInstance.transform.localScale), $"Non-owner instance was able to change the scale!");
+
+            // Verify that the non-owner instance transform is still the same as the owner instance transform
+            Assert.True(Approximately(ownerInstance.transform.position, newClientObjectInstance.transform.position), "Non-owner and owner instance position values are not the same!");
+            Assert.True(Approximately(ownerInstance.transform.rotation.eulerAngles, newClientObjectInstance.transform.rotation.eulerAngles), "Non-owner and owner instance rotation values are not the same!");
+            Assert.True(Approximately(ownerInstance.transform.localScale, newClientObjectInstance.transform.localScale), "Non-owner and owner instance scale values are not the same!");
+        }
+
         public enum StartingOwnership
         {
             HostStartsAsOwner,

com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformOwnershipTests.cs

@@ -2,6 +2,7 @@
 using Unity.Netcode.Components;
 using UnityEngine;
 
+
 namespace Unity.Netcode.RuntimeTests
 {
 
@@ -89,6 +90,125 @@ private bool WillAnAxisBeSynchronized(ref NetworkTransform networkTransform)
                 networkTransform.SyncPositionX || networkTransform.SyncPositionY || networkTransform.SyncPositionZ;
         }
 
+        [Test]
+        public void NetworkTransformStateFlags()
+        {
+            var indexValues = new System.Collections.Generic.List<uint>();
+            var currentFlag = (uint)0x00000001;
+            for (int j = 0; j < 18; j++)
+            {
+                indexValues.Add(currentFlag);
+                currentFlag = currentFlag << 1;
+            }
+
+            // TrackByStateId is unique
+            indexValues.Add(0x10000000);
+
+            var boolSet = new System.Collections.Generic.List<bool>();
+            var transformState = new NetworkTransform.NetworkTransformState();
+            // Test setting one at a time.
+            for (int j = 0; j < 19; j++)
+            {
+                boolSet = new System.Collections.Generic.List<bool>();
+                for (int i = 0; i < 19; i++)
+                {
+                    if (i == j)
+                    {
+                        boolSet.Add(true);
+                    }
+                    else
+                    {
+                        boolSet.Add(false);
+                    }
+                }
+                transformState = new NetworkTransform.NetworkTransformState()
+                {
+                    InLocalSpace = boolSet[0],
+                    HasPositionX = boolSet[1],
+                    HasPositionY = boolSet[2],
+                    HasPositionZ = boolSet[3],
+                    HasRotAngleX = boolSet[4],
+                    HasRotAngleY = boolSet[5],
+                    HasRotAngleZ = boolSet[6],
+                    HasScaleX = boolSet[7],
+                    HasScaleY = boolSet[8],
+                    HasScaleZ = boolSet[9],
+                    IsTeleportingNextFrame = boolSet[10],
+                    UseInterpolation = boolSet[11],
+                    QuaternionSync = boolSet[12],
+                    QuaternionCompression = boolSet[13],
+                    UseHalfFloatPrecision = boolSet[14],
+                    IsSynchronizing = boolSet[15],
+                    UsePositionSlerp = boolSet[16],
+                    IsParented = boolSet[17],
+                    TrackByStateId = boolSet[18],
+                };
+                Assert.True((transformState.BitSet & indexValues[j]) == indexValues[j], $"[FlagTest][Individual] Set flag value {indexValues[j]} at index {j}, but BitSet value did not match!");
+            }
+
+            // Test setting all flag values
+            boolSet = new System.Collections.Generic.List<bool>();
+            for (int i = 0; i < 19; i++)
+            {
+                boolSet.Add(true);
+            }
+
+            transformState = new NetworkTransform.NetworkTransformState()
+            {
+                InLocalSpace = boolSet[0],
+                HasPositionX = boolSet[1],
+                HasPositionY = boolSet[2],
+                HasPositionZ = boolSet[3],
+                HasRotAngleX = boolSet[4],
+                HasRotAngleY = boolSet[5],
+                HasRotAngleZ = boolSet[6],
+                HasScaleX = boolSet[7],
+                HasScaleY = boolSet[8],
+                HasScaleZ = boolSet[9],
+                IsTeleportingNextFrame = boolSet[10],
+                UseInterpolation = boolSet[11],
+                QuaternionSync = boolSet[12],
+                QuaternionCompression = boolSet[13],
+                UseHalfFloatPrecision = boolSet[14],
+                IsSynchronizing = boolSet[15],
+                UsePositionSlerp = boolSet[16],
+                IsParented = boolSet[17],
+                TrackByStateId = boolSet[18],
+            };
+
+            for (int j = 0; j < 19; j++)
+            {
+                Assert.True((transformState.BitSet & indexValues[j]) == indexValues[j], $"[FlagTest][All] All flag values are set but failed to detect flag value {indexValues[j]}!");
+            }
+
+            // Test getting all flag values
+            transformState = new NetworkTransform.NetworkTransformState();
+            for (int i = 0; i < 19; i++)
+            {
+                transformState.BitSet |= indexValues[i];
+            }
+
+            Assert.True(transformState.InLocalSpace, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.InLocalSpace)}!");
+            Assert.True(transformState.HasPositionX, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.HasPositionX)}!");
+            Assert.True(transformState.HasPositionY, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.HasPositionY)}!");
+            Assert.True(transformState.HasPositionZ, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.HasPositionZ)}!");
+            Assert.True(transformState.HasRotAngleX, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.HasRotAngleX)}!");
+            Assert.True(transformState.HasRotAngleY, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.HasRotAngleY)}!");
+            Assert.True(transformState.HasRotAngleZ, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.HasRotAngleZ)}!");
+            Assert.True(transformState.HasScaleX, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.HasScaleX)}!");
+            Assert.True(transformState.HasScaleY, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.HasScaleY)}!");
+            Assert.True(transformState.HasScaleZ, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.HasScaleZ)}!");
+            Assert.True(transformState.IsTeleportingNextFrame, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.IsTeleportingNextFrame)}!");
+            Assert.True(transformState.UseInterpolation, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.UseInterpolation)}!");
+            Assert.True(transformState.QuaternionSync, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.QuaternionSync)}!");
+            Assert.True(transformState.QuaternionCompression, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.QuaternionCompression)}!");
+            Assert.True(transformState.UseHalfFloatPrecision, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.UseHalfFloatPrecision)}!");
+            Assert.True(transformState.IsSynchronizing, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.IsSynchronizing)}!");
+            Assert.True(transformState.UsePositionSlerp, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.UsePositionSlerp)}!");
+            Assert.True(transformState.IsParented, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.IsParented)}!");
+            Assert.True(transformState.TrackByStateId, $"[FlagTest][Get] Failed to detect {nameof(NetworkTransform.NetworkTransformState.TrackByStateId)}!");
+        }
+
         [Test]
         public void TestSyncAxes([Values] SynchronizationType synchronizationType, [Values] SyncAxis syncAxis)