fix: NetworkTransform teleport interpolating from last position on non-authoritative side [NCCBUG-166] (#2110)

* fix

NetworkTransform was setting the value of m_Bitset but not resetting the individual bits if there was no deltas.  This would result in a gradually increasing message size over time when updating the NetworkTransform.

* update

migrating the assignment back down below the CanCommitToTransform

* fix

This resolves the issue where clients were applying every value marked to be synchronized even if the update didn't include specific axis values.

* test

This includes some fixes to prefabs that hadn't been updated to the more recent changes in NetworkTransform that limited the minimum threshold (i.e. they were set to 0 which causes issues)

* fix

Since we are now no longer sending the entire transform values, this addition prevents the non-authoritative side from making changes to the transform.

* style

removing whitespace

* update

Adding some test adjustments and including rotation to interpolation measurements.

* style

removing an unused namespace.

* style

fixing issue with reference to UnityEngine's Object after I removed the system namespace.

* fix

This includes the adjustments for client authoritative mode to work without sending RPCs.

* fix

This update fixes the issue where the authoritative side was interpolating when interpolate is enabled.
Now, the authoritative side no longer interpolates.

* manual test updates

These changes make the player client authoritative within the Samples Menu->Additive Scene Loading manual test/sample.

* update

reverting the change of m_Bitset to an internal.

* test update

updating test to account for reverting back to the private m_Bitset.

* update

Migrated the code that updates interpolators with the current value if the axis is not set into the AddInterpolatedState method as opposed to it being in the ApplyInterpolatedNetworkStateToTransform

* fix

First pass fix for teleporting.

* test manual

Manual test adjustments (still working on player motion controller from client side)

* manual test

Adjustments for the teleporting sample/manual test.
Still working on the new player cube motion model.

* test manual

Improved motion model (still could use some more work for client to server side).
Now packing data sent from client to host/server.
Added UI text that displays whether the instance is a server, host, or client.

* update

removing debug code

* fix

This fixes the last issues with teleporting and the updated player motion model.

* fix

Cleaning up how we handle transitioning to and from global and local space.
Minor adjustment to the validation test to assure the range of error being tested on the client side is the same as the delta position threshold range.

* update

Removing the NetworkTransformState Position, Rotation, and Scale since we are only sending the values that changed (i.e. if the x-axis of position was the only thing that was set and NetworkTransformState.Position was used, then the Y and Z axial values would be invalid).

* fix

removing code block no longer needed.

* style

removing a comment that was no longer needed.

* update

applying suggested changes

* update

replacing the GetReplicatedNetworkState() method with a getter ReplicatedNetworkState property.

* update

This is a new approach:
- Make in-between state updates additive
- Clear our state's Bitset value and prepare to capture any deltas for the next network tick period.

* update

Realized since we are back to additive (per network tick), we can now remove the non-authoritative interpolation work around for not having all values that changed within the associated NetworkTransformState's network tick period.

* update

Disposing the NetworkVariables when the NetworkTransform is destroyed.

* update

Suggestion updates and comment fixes.

* style

adding Sam's comment.

* style

updated comment to include what we are preserving.

* update

Removing the ApplyTransformValues and SetTransformValues as they are no longer needed.
Also, removing the associated properties.

* Test Manual

Updates to manual test related assets and scripts.
Removed the "make a pooled object invisible to hide the teleport interpolation issue" hack that would disable the render mesh when despawned and enable it shortly after it is spawned.
Increased tick rate to 32 in the additive scene loading test.
Fixing white space issues.

* update

Removing the unused serverTime addition as well.

* update

Renamed ApplyInterpolatedNetworkStateToTransform to ApplyAuthoritativeState.
(It didn't make sense if interpolation is disabled)

* tests update

This includes an updated NetworkTransformTests that tests both authority types while running in Server and Host modes.
This includes some updates to naming for easier identification of what each test does (in test runner and in code).

* Update

Minor adjustment where we don't need to send a local properties over the stack via parameters.

* test

Added interpolate enable/disable values to all tests.
Now, all NetworkTranaformTests are done with interpolation enabled and disabled.

* fix and update

Fixed another issue that was exposed while cleaning up the code base.
Was able to remove even more of the code added previously.
Some clean up as well as making one additional method private as opposed to protected to prevent us from having to release a minor version vs patch.
Also,  ApplyTransformToNetworkState, and ApplyTransformToNetworkStateWithInfo only return if the state is dirty (the rest can be determined from within the state itself).

ApplyLocalNetworkState, used for integration tests, now just returns the NetworkTransformState.

* fix

This fixes the issue with the memory leak message in the editor NetworkTimeTests  (very odd this fixed the issue).

* test manual

Switched over to an owner authoritative NetworkTransform that also includes the playermovement.
Adjusted the PlayerCube NetworkPrefab and minor adjustment to the CollideTeleporter.

* update

updating the changelog

* fix

Found a minor logic issue in ApplyTransformToNetworkStateWithInfo where the state dirty flag could be reset before end of the current network tick.  This could result in state being carried forward into the next network tick.  This resolves that issue.

* update

Minor adjustment to how we handle applying a rotation measurement on the non-authoritative side.
Removed a property not being used outside of a single method and added additional comments to the PlayerMovement component.

* update

This update includes the final changes required in order to support the previous NetworkTransform model where it was always server authoritative and client owners could direct/apply state via internal RPCs.  This also includes the ability for a server to apply/dictate state when running in client owner authority mode.

This includes updates to the NetworkTransform tests that validate all of these updates and increase the amount of coverage for NetworkTransform.

* style

moving new private m_ClientRpcParams and m_ClientIds up to the rest of the property declarations.
Adjusting comment

* test manual

Minor adjustments for the random player movers.
Applying the server authoritative player prefab back to be the default for the additive scene sample (this makes it easier to compare to main branch).

* style

removing a no longer needed comment.
adding note about future clean up for NetworkTransform.

* style

fixed spelling issue

* style

removing left over comment and spelling

* style

removing legacy/unused namespaces

* update

realized our CommitLocallyAndReplicate method was serving no purpose at this point.
added the assignment of NetworkTransformState to the ReplicatedNetworkState within TryCommit and removed CommitLocallyAndReplicate.

* update

removing legacy check for TryCommitTransformToServer that is no longer needed.

* update

removing m_Transform (no purpose for it)
Fixing minor grammar issue with a comment

* test manual update

removing additional IsOwner check.

* revert

reverting the changes I made to the editor test NetworkTimeTests as I just had Yamato crash again in a different area in NetworkTimeTests with the same memory leak error.

* style

replacing LF/CR

* Style

Improving comments

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -18,6 +18,9 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Fixed
 
 - Fixed an issue where reading/writing more than 8 bits at a time with BitReader/BitWriter would write/read from the wrong place, returning and incorrect result. (#2130)
+- Fixed issue with the internal `NetworkTransformState.m_Bitset` flag not getting cleared upon the next tick advancement. (#2110)
+- Fixed interpolation issue with `NetworkTransform.Teleport`. (#2110)
+- Fixed issue where the authoritative side was interpolating its transform. (#2110)
 - Fixed Owner-written NetworkVariable infinitely write themselves (#2109)
 - Fixed NetworkList issue that showed when inserting at the very end of a NetworkList (#2099)
 - Fixed issue where a client owner of a `NetworkVariable` with both owner read and write permissions would not update the server side when changed. (#2097)

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

@@ -10,21 +10,19 @@ namespace Unity.Netcode.RuntimeTests
 {
     public class TransformInterpolationObject : NetworkBehaviour
     {
+        // Set the minimum threshold which we will use as our margin of error
+        public const float MinThreshold = 0.001f;
+
         public bool CheckPosition;
         public bool IsMoving;
         public bool IsFixed;
 
         private void Update()
         {
-            // Since the local position is transformed from local to global and vice-versa on the server and client
-            // it may accumulate some error. We allow an error of 0.01 over the range of 1000 used in this test.
-            // This requires precision to 5 digits, so it doesn't weaken the test, while preventing spurious failures
-            const float maxRoundingError = 0.01f;
-
             // Check the position of the nested object on the client
             if (CheckPosition)
             {
-                if (transform.position.y < -maxRoundingError || transform.position.y > 100.0f + maxRoundingError)
+                if (transform.position.y < -MinThreshold || transform.position.y > 100.0f + MinThreshold)
                 {
                     Debug.LogError($"Interpolation failure. transform.position.y is {transform.position.y}. Should be between 0.0 and 100.0");
                 }
@@ -65,7 +63,8 @@ public class TransformInterpolationTests : NetcodeIntegrationTest
         protected override void OnServerAndClientsCreated()
         {
             m_PrefabToSpawn = CreateNetworkObjectPrefab("InterpTestObject");
-            m_PrefabToSpawn.AddComponent<NetworkTransform>();
+            var networkTransform = m_PrefabToSpawn.AddComponent<NetworkTransform>();
+            networkTransform.PositionThreshold = TransformInterpolationObject.MinThreshold;
             m_PrefabToSpawn.AddComponent<TransformInterpolationObject>();
         }
 
@@ -85,8 +84,6 @@ private IEnumerator RefreshNetworkObjects()
             m_SpawnedObjectOnClient = s_GlobalNetworkObjects[clientId][m_SpawnedAsNetworkObject.NetworkObjectId];
             // make sure the objects are set with the right network manager
             m_SpawnedObjectOnClient.NetworkManagerOwner = m_ClientNetworkManagers[0];
-
-
         }
 
         [UnityTest]

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

@@ -82,5 +82,10 @@ PrefabInstance:
       propertyPath: m_LocalEulerAnglesHint.z
       value: 0
       objectReference: {fileID: 0}
+    - target: {fileID: 4600632750638426092, guid: 29cabf623d47bb345a9bb4140e4397d7,
+        type: 3}
+      propertyPath: PositionThreshold
+      value: 0.01
+      objectReference: {fileID: 0}
     m_RemovedComponents: []
   m_SourcePrefab: {fileID: 100100000, guid: 29cabf623d47bb345a9bb4140e4397d7, type: 3}

com.unity.netcode.gameobjects/Tests/Runtime/TransformInterpolationTests.cs

@@ -34,8 +34,9 @@ Transform:
   m_PrefabAsset: {fileID: 0}
   m_GameObject: {fileID: 4079352819444256614}
   m_LocalRotation: {x: 0, y: 0, z: 0, w: 1}
-  m_LocalPosition: {x: 5, y: 0.625, z: 5}
+  m_LocalPosition: {x: 0, y: 0.55, z: 0}
   m_LocalScale: {x: 1.25, y: 1.25, z: 1.25}
+  m_ConstrainProportionsScale: 0
   m_Children:
   - {fileID: 3519470446676406143}
   m_Father: {fileID: 0}
@@ -62,12 +63,11 @@ MonoBehaviour:
   SyncScaleX: 1
   SyncScaleY: 1
   SyncScaleZ: 1
-  PositionThreshold: 0
-  RotAngleThreshold: 0
-  ScaleThreshold: 0
+  PositionThreshold: 0.01
+  RotAngleThreshold: 0.01
+  ScaleThreshold: 0.01
   InLocalSpace: 0
   Interpolate: 1
-  FixedSendsPerSecond: 15
 --- !u!114 &-3775814466963834669
 MonoBehaviour:
   m_ObjectHideFlags: 0
@@ -103,6 +103,7 @@ MeshRenderer:
   m_CastShadows: 1
   m_ReceiveShadows: 1
   m_DynamicOccludee: 1
+  m_StaticShadowCaster: 0
   m_MotionVectors: 1
   m_LightProbeUsage: 1
   m_ReflectionProbeUsage: 1
@@ -239,6 +240,7 @@ Transform:
   m_LocalRotation: {x: 0, y: 0, z: 0, w: 1}
   m_LocalPosition: {x: 0, y: 1.045, z: 0}
   m_LocalScale: {x: 1, y: 1, z: 1}
+  m_ConstrainProportionsScale: 0
   m_Children: []
   m_Father: {fileID: 4079352819444256611}
   m_RootOrder: 0
@@ -262,6 +264,7 @@ MeshRenderer:
   m_CastShadows: 1
   m_ReceiveShadows: 1
   m_DynamicOccludee: 1
+  m_StaticShadowCaster: 0
   m_MotionVectors: 1
   m_LightProbeUsage: 1
   m_ReflectionProbeUsage: 1

testproject/Assets/Prefabs/ObjectToOverride-VariantBlueCylinder.prefab

@@ -18,7 +18,6 @@ GameObject:
   - component: {fileID: 8685790303553767876}
   - component: {fileID: 3809075828520557319}
   - component: {fileID: 7138389085065872747}
-  - component: {fileID: 8585540885791567915}
   - component: {fileID: 2744080254494315543}
   m_Layer: 0
   m_Name: PlayerCube
@@ -37,6 +36,7 @@ Transform:
   m_LocalRotation: {x: 0, y: 0, z: 0, w: 1}
   m_LocalPosition: {x: 0, y: 0.5, z: 0}
   m_LocalScale: {x: 0.5, y: 0.5, z: 0.5}
+  m_ConstrainProportionsScale: 0
   m_Children: []
   m_Father: {fileID: 0}
   m_RootOrder: 0
@@ -49,14 +49,14 @@ Rigidbody:
   m_PrefabAsset: {fileID: 0}
   m_GameObject: {fileID: 8685790303553767886}
   serializedVersion: 2
-  m_Mass: 1
-  m_Drag: 0
-  m_AngularDrag: 0.05
+  m_Mass: 2
+  m_Drag: 0.05
+  m_AngularDrag: 0.02
   m_UseGravity: 1
   m_IsKinematic: 1
-  m_Interpolate: 0
+  m_Interpolate: 1
   m_Constraints: 80
-  m_CollisionDetection: 0
+  m_CollisionDetection: 2
 --- !u!65 &8685790303553767873
 BoxCollider:
   m_ObjectHideFlags: 0
@@ -89,6 +89,7 @@ MeshRenderer:
   m_CastShadows: 1
   m_ReceiveShadows: 1
   m_DynamicOccludee: 1
+  m_StaticShadowCaster: 0
   m_MotionVectors: 1
   m_LightProbeUsage: 1
   m_ReflectionProbeUsage: 1
@@ -127,9 +128,10 @@ LineRenderer:
   m_PrefabAsset: {fileID: 0}
   m_GameObject: {fileID: 8685790303553767886}
   m_Enabled: 1
-  m_CastShadows: 1
-  m_ReceiveShadows: 1
+  m_CastShadows: 0
+  m_ReceiveShadows: 0
   m_DynamicOccludee: 1
+  m_StaticShadowCaster: 0
   m_MotionVectors: 0
   m_LightProbeUsage: 0
   m_ReflectionProbeUsage: 0
@@ -138,7 +140,7 @@ LineRenderer:
   m_RenderingLayerMask: 1
   m_RendererPriority: 0
   m_Materials:
-  - {fileID: 0}
+  - {fileID: 2100000, guid: 712559e4bd05f1942a8fd4bfa4e10c56, type: 2}
   m_StaticBatchInfo:
     firstSubMesh: 0
     subMeshCount: 0
@@ -182,16 +184,16 @@ LineRenderer:
       m_RotationOrder: 4
     colorGradient:
       serializedVersion: 2
-      key0: {r: 0.73333335, g: 0.7137255, b: 0.6666667, a: 1}
-      key1: {r: 1, g: 1, b: 1, a: 1}
-      key2: {r: 1, g: 1, b: 1, a: 0}
+      key0: {r: 0.30566037, g: 0.90742147, b: 1, a: 1}
+      key1: {r: 0.28166604, g: 0.8183648, b: 0.8679245, a: 1}
+      key2: {r: 0.0391598, g: 0.9433962, b: 0.89236206, a: 0}
       key3: {r: 0, g: 0, b: 0, a: 0}
       key4: {r: 0, g: 0, b: 0, a: 0}
       key5: {r: 0, g: 0, b: 0, a: 0}
       key6: {r: 0, g: 0, b: 0, a: 0}
       key7: {r: 0, g: 0, b: 0, a: 0}
       ctime0: 0
-      ctime1: 25443
+      ctime1: 26021
       ctime2: 65535
       ctime3: 0
       ctime4: 0
@@ -206,15 +208,15 @@ LineRenderer:
       atime5: 0
       atime6: 0
       atime7: 0
-      m_Mode: 0
+      m_Mode: 1
       m_NumColorKeys: 3
       m_NumAlphaKeys: 2
     numCornerVertices: 0
     numCapVertices: 0
     alignment: 0
     textureMode: 0
-    shadowBias: 0.5
-    generateLightingData: 0
+    shadowBias: 1
+    generateLightingData: 1
   m_UseWorldSpace: 1
   m_Loop: 0
 --- !u!114 &947981134
@@ -245,8 +247,22 @@ MonoBehaviour:
   m_Script: {fileID: 11500000, guid: 82b41b172a31546ffba450f1418f4e69, type: 3}
   m_Name: 
   m_EditorClassIdentifier: 
-  m_Speed: 10
-  m_RotSpeed: 2
+  SyncPositionX: 1
+  SyncPositionY: 1
+  SyncPositionZ: 1
+  SyncRotAngleX: 0
+  SyncRotAngleY: 1
+  SyncRotAngleZ: 0
+  SyncScaleX: 0
+  SyncScaleY: 0
+  SyncScaleZ: 0
+  PositionThreshold: 0.001
+  RotAngleThreshold: 0.01
+  ScaleThreshold: 0.01
+  InLocalSpace: 0
+  Interpolate: 1
+  Speed: 7
+  RotSpeed: 2
 --- !u!114 &3809075828520557319
 MonoBehaviour:
   m_ObjectHideFlags: 0
@@ -271,34 +287,7 @@ MonoBehaviour:
   m_Script: {fileID: 11500000, guid: 3e34656ebae784afca7d1f7f6dc18580, type: 3}
   m_Name: 
   m_EditorClassIdentifier: 
-  Range: 10
---- !u!114 &8585540885791567915
-MonoBehaviour:
-  m_ObjectHideFlags: 0
-  m_CorrespondingSourceObject: {fileID: 0}
-  m_PrefabInstance: {fileID: 0}
-  m_PrefabAsset: {fileID: 0}
-  m_GameObject: {fileID: 8685790303553767886}
-  m_Enabled: 1
-  m_EditorHideFlags: 0
-  m_Script: {fileID: 11500000, guid: 54c9647dc784a46bca664910f182491e, type: 3}
-  m_Name: 
-  m_EditorClassIdentifier: 
-  SyncPositionX: 1
-  SyncPositionY: 1
-  SyncPositionZ: 1
-  SyncRotAngleX: 1
-  SyncRotAngleY: 1
-  SyncRotAngleZ: 1
-  SyncScaleX: 1
-  SyncScaleY: 1
-  SyncScaleZ: 1
-  PositionThreshold: 0
-  RotAngleThreshold: 0
-  ScaleThreshold: 0
-  InLocalSpace: 0
-  Interpolate: 1
-  FixedSendsPerSecond: 30
+  Range: 3
 --- !u!114 &2744080254494315543
 MonoBehaviour:
   m_ObjectHideFlags: 0

testproject/Assets/Prefabs/Player.prefab

@@ -0,0 +1,19 @@
+%YAML 1.1
+%TAG !u! tag:unity3d.com,2011:
+--- !u!114 &11400000
+MonoBehaviour:
+  m_ObjectHideFlags: 0
+  m_CorrespondingSourceObject: {fileID: 0}
+  m_PrefabInstance: {fileID: 0}
+  m_PrefabAsset: {fileID: 0}
+  m_GameObject: {fileID: 0}
+  m_Enabled: 1
+  m_EditorHideFlags: 0
+  m_Script: {fileID: 11500000, guid: 39a16938ffb5cd846a9f6df7a686a9c4, type: 3}
+  m_Name: Teleporting
+  m_EditorClassIdentifier: 
+  SceneToReference: {fileID: 102900000, guid: efa247d1f78ca694f8d2dcb5672e8f8b, type: 3}
+  m_IncludedScenes: []
+  m_DisplayName: Teleporting
+  m_ReferencedScenes:
+  - TeleportSample

testproject/Assets/Prefabs/PlayerCube.prefab

@@ -2523,8 +2523,9 @@ MonoBehaviour:
   m_Name: 
   m_EditorClassIdentifier: 
   m_SceneMenus:
-  - {fileID: 11400000, guid: 7fdc32fee173cca45a4601ba234954d0, type: 2}
+  - {fileID: 11400000, guid: 0509eb053ce4ef749afd8495f13128f1, type: 2}
   - {fileID: 11400000, guid: 9a8d9296fb33f794f95514bf38de3cf9, type: 2}
+  - {fileID: 11400000, guid: 7fdc32fee173cca45a4601ba234954d0, type: 2}
   - {fileID: 11400000, guid: 21aae92071ad50448a45b013d8346639, type: 2}
   - {fileID: 11400000, guid: 660535b6e155b5b4bbede52313fcb32e, type: 2}
   - {fileID: 11400000, guid: d2e34ed37c087154dbd7f89fd463801b, type: 2}