fix: NetworkTransform Mixed Unreliable & Reliable Order of Operations

[NoelStephensUnity]

When sending unreliable delta state updates, it is possible for the following sequence of events to happen:

• Authority:
  • Send delta state update on Tick 400
  • Send a teleport state update on same frame Tick 400
• Non-Authority
  • Receive and process teleport state update for Tick 400
  • Receive and process delta state update for Tick 400

Where the teleport state update that should proceed the deltas state update ends up preceding it which can cause a temporary out-of-synch issue if interpolation is disabled but can cause a relatively noticeable "visual anomaly" if interpolation is enabled the the original position is a reasonable distance from the teleported position.

In this update:

• NetworkTransform was adjusted to assure explicitly set states are not sent during a fractional tick period.
  • If multiple explicitly set states occur during a fractional tick period, the cumulative sum of changes will be sent on the next up-coming tick.
  • Upon being sent on the up-coming tick, the explicit states set will be the only deltas sent on that tick.
    • Any delta variances between the last explicitly set state fractional tick time and when the next tick is updated will not be included and will be updated the following tick.
• Several adjustments to NetworkTransform related tests:
  • Help better distribute the tests amongst test fixtures.
  • Fixes and adjustments to timing.
  • Unified the commonly used methods between NetworkTransformPacketLossTests and NetworkTransformTests within a new base class: NetworkTransformBase.
  • Extracted some more generalized tests from NetworkTransformTests that were running on all test fixture iterations (and did not need to be) and migrated them to a new NetworkTransformGeneral set of tests.
• Added some new methods to NetcodeIntegrationTest:
  • GetFrameRate: virtual method used in ConfigureFramesPerTick calculations.
  • GetTickRate: virtual method used in ConfigureFramesPerTick calculations.
  • ConfigureFramesPerTick: This calculates the number of frames within a single tick period and the tick frequency
  • TimeTravelAdvanceTick: time travels for the duration of the tick frequency or the number of frames within a tick period has passed.

MTT-7775

Changelog

• Fixed: Issue where a teleport state could potentially be overridden by a previous unreliable delta state.
• Fixed: Issue where NetworkTransform was using the NetworkManager.ServerTime.Tick as opposed to NetworkManager.NetworkTickSystem.ServerTime.Tick during the authoritative side's tick update where it performed a delta state check.
• Changed: NetworkTransform.SetState (and related methods) now are cumulative during a fractional tick period and sent on the next pending tick.

Testing and Documentation

• Includes integration test NetworkTransformPacketLossTests.TestSameFrameDeltaStateAndTeleport
• Includes integration test NetworkTransformGeneral.TestMultipleExplicitSetStates
• No documentation changes or additions were necessary.

[Maclay74]

Hey @NoelStephensUnity.

I've noticed this merge request because I have a similar problem.
I have a client-authoritative movement and at some point I teleport player to another position. And once in a while it gets teleported back to its previous location.

I tried this branch and unfortunately it didn't resolve my problem, although I'm not sure it 's order of operation problem.

My character has CharacterController, NetworkTransform, NetworkRigidBody and this is how I teleport it from server:

[ClientRpc]
void SomeClientRpc(Vector3 moveTo, ClientRpcParams rpcParams = default) {
    var character = NetworkManager.LocalClient.PlayerObject.transform;
    character.position = new Vector3(moveTo.x, character.transform.position.y, moveTo.z);
    character.LookAt(new Vector3(transform.position.x, character.position.y, transform.position.z));
    character.GetComponent<Rigidbody>().position = character.position; // just tried this way too
    character.GetComponent<Rigidbody>().rotation = character.rotation;
}

This code is from another NetworkBehaviour.

This is what I have in my package.json (and I also checked that I have changes from this branch)

"com.unity.netcode.gameobjects": "https://github.com/Unity-Technologies/com.unity.netcode.gameobjects.git?path=com.unity.netcode.gameobjects#500f2be18805a16fead8ba9bcb96d8c333ca739f",

I'm not sure it's related to this PR, but still...

teleport_bug.webm

[NoelStephensUnity]

Hey @NoelStephensUnity.

I've noticed this merge request because I have a similar problem. I have a client-authoritative movement and at some point I teleport player to another position. And once in a while it gets teleported back to its previous location.

I tried this branch and unfortunately it didn't resolve my problem, although I'm not sure it 's order of operation problem.

My character has CharacterController, NetworkTransform, NetworkRigidBody and this is how I teleport it from server:

[ClientRpc]
void SomeClientRpc(Vector3 moveTo, ClientRpcParams rpcParams = default) {
    var character = NetworkManager.LocalClient.PlayerObject.transform;
    character.position = new Vector3(moveTo.x, character.transform.position.y, moveTo.z);
    character.LookAt(new Vector3(transform.position.x, character.position.y, transform.position.z));
    character.GetComponent<Rigidbody>().position = character.position; // just tried this way too
    character.GetComponent<Rigidbody>().rotation = character.rotation;
}

This code is from another NetworkBehaviour.

This is what I have in my package.json (and I also checked that I have changes from this branch)

"com.unity.netcode.gameobjects": "https://github.com/Unity-Technologies/com.unity.netcode.gameobjects.git?path=com.unity.netcode.gameobjects#500f2be18805a16fead8ba9bcb96d8c333ca739f",

I'm not sure it's related to this PR, but still...

teleport_bug.webm

I would highly recommend taking a look at issue #2765 as there can be an order of operations issue involved when dealing with physics bodies and applying values immediately upon an inbound message being received and processed. I have also opened up a documentation PR to provide more details about inbound message processing, order of operation, and physics so this information is more readily available.

The common symptom (as described in my explanation in #2765) is that when you apply your changes on the authoritative side it "seems to work just fine" but then when you have a remote client signal to apply changes (i.e. via ServerRpc) it can sometimes be inconsistent (or not work at all).

With your scenario it would seem that it is similar to the #2765 scenario but just inverted where the host-server is driving the teleport/move-to event and your authoritative side (owner/client authoritative) is applying the results of the event during the EarlyUpdate (when inbound messages are processed) prior to the FixedUpdate.

The solution is to not apply the changes immediately when the ClientRpc is received, but to defer the changes to a later update stage (i.e. MonoBehaviour.Update is fine) that occurs after FixedUpdate.
To test this, I would recommend adding a private Vector3 m_TeleportLocation; and a private bool m_TeleportToLocation; that are the only things that get set when the ClientRpc is received and processed and migrating the rest of the code into a different method you invoke during MonoBehaviour.Update (we can call it ApplyTeleportToLocation). In the same NetworkBehaviour during the MonoBehaviour.Update, just check for the m_TeleportToLocation to be set and if so invoke the ApplyTeleportToLocation method.
The differences being:

• You are using m_TeleportToLocation as your destination position and applying it after FixedUpdate has been run for the current frame.
• Within your ApplyTeleportToLocation method and after applying the position and rotation update to the CharacterController, if you want to "truly teleport" then you should also invoke NetworkTransform.SetState(character.position, character.rotation, null, false).
  • The false means to "teleport" to the new location and rotation (really it means to reset the interpolators to the new position, rotation, and scale values).
  • If you do want to preserve the interpolation to make it a smooth transition, then you shouldn't need to invoke SetState on the authoritative (owner/client) side.
• Reset the m_TeleportToLocation flag when done.

This should avoid the issue you are seeing on the client side and if you do want it to "truly teleport" then using SetState will make sure that it looks visually correct on the non-authoritative instances.

This PR was originally put together because of how unreliable and reliable messages do not have any guarantee in the order they are received and processed (relative to each other and not to messages of the same delivery type). This led me to discover an explicit state update (i.e. SetState) could be invoked after an implicit delta state update (i.e. normal tick update) had already been sent on the same frame. In order to preserve the explicit state update, this PR defers the explicit state update the next frame.

[Maclay74]

Yes, it worked in my case, thanks!