Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: NetworkTransform dropped packets can cause interpolation stutter [MTT-7551] #2713

Conversation

NoelStephensUnity
Copy link
Collaborator

@NoelStephensUnity NoelStephensUnity commented Sep 25, 2023

Fixes

NetworkTransform can handle latency issues but not dropping packets when interpolation is enabled due to the original default delivery method being reliable fragmented sequenced. This update sends common delta state updates as unreliable fragmented while also assuring that the following more critical state updates are still sent as reliable fragmented sequenced:

  • The initial synchronization state update is still sent as reliable fragmented sequenced
  • The teleporting state update is still sent as as reliable fragmented sequenced
  • When using half float precision and the NetworkDeltaPosition delta exceeds the maximum delta forcing the axis in question to be collapsed into the core base position, the state update will be sent as reliable fragmented sequenced.

In order to preserve a continual consistency of axial values when unreliable delta messaging is enabled ( due to the possibility of dropping packets), NetworkTransform instances will send 1 axial frame synchronization update per second (only for the axis marked to synchronize) as long as at least one delta state update had been previously sent. When a NetworkObject is at rest, axial frame synchronization updates are not sent.

In order to prevent "stacking" of axial frame synchronization updates, NetworkTransform instances are assigned to a tick position from the total total number of ticks that can occur within a second (i.e. a NetworkConfig.TickRate of 30 would mean there are 30 possible tick positions available). All instances' tick positions are assigned in a way that distributes their axial frame synchronization updates across the allotted 1 second time interval as best possible.

The additional bandwidth cost per instance is one full axis synchronization per second per instance:

  • Full precision:
    • Position, Scale, and Euler Rotation: ranges from 4 to 36 bytes per update depending on the axis are marked for synch.
      • [Rotation Caveat] If quaternion sync is enabled:
        • With no compression rotation is always 16 bytes per update
        • With quaternion compression it is always 4 bytes per update
  • Half Precision:
    • Position, Scale, and Euler Rotation: ranges from 2 to 18 bytes per update depending upon which axis are marked for sync
      • [Rotation Caveat] If quaternion sync is enabled:
        • With no compression it is 8 bytes per update
        • With compression it is 4 bytes per update

Example:
90 instances with half precision and quaternion synchronization enabled and the X, Y, and Z axis for position marked for synchronization would send full axis synchronizations for 3 instances per network tick over a 1 second period of time. Each full axis synchronization update would have a payload size of 14 bytes per instance at a total cost of an additional 42 bytes per network tick if all 90 instances were continually moving/rotating.

Additional Updates & Fixes

  • Authoritative NetworkTransform instances now use a tick system registration process in order to improve performance. Invoking all authoritative tick updates directly from within a single tick event can shave up to 1-2ms off of total processing time (with ~500 or more instances updating).
  • NetworkDeltaPosition now has a unique state update when UseUnreliableDeltas is enabled to assure collapsing of an axis into the NetworkDeltaPosition.CurrentBasePosition is sent using reliable fragmented sequenced network delivery.
  • Fixed an issue with initial synchronization an parenting depending upon whether world position stays (or not) and whether the NetworkTransform is using world or local space.

MTT-7551

Changelog

  • Fixed: Issue where a NetworkTransform instance with interpolation enabled would result in wide visual motion gaps (stuttering) under above normal latency conditions and a 1-5% or higher packet are drop rate.
  • Changed: NetworkTransform authoritative instance tick registration so a single NetworkTransform specific tick event update will update all authoritative instances to improve perofmance.

Testing and Documentation

  • Includes integration tests with a continual 5% packet drop rate and 100ms latency:
    • NetworkTransformPacketLossTests.ParentedNetworkTransformTest
    • NetworkTransformPacketLossTests.NetworkTransformMultipleChangesOverTime
    • NetworkTransformPacketLossTests.TestAuthoritativeTransformChangeOneAtATime
  • Includes integration test updates to NetworkTransformTests:
    • Disabling UseUnreliableDeltas to validate the original reliable fragmented sequenced mode still works as expected
  • Includes edits to existing public API documentation. (PR-1127)

UX/UI Update:

Added additional "Use Unreliable Deltas" property to NetworkTransformEditor:
image

Adding OnOwnershipChanged(ulong previous, ulong current)
This resolves the issue with client authoritative network transforms and the random "noise" that would occur when transitioning ownership from a remote client back to the host-server.
- Latent messages from the client would still be received and processed after ownership changed.
- Ownership changed messages would proceed the NetworkTransform initialization state update message. Now ownership changed messages precede the NetworkTransform initialization state update message.
- Clients could sometimes have the same network tick value even when the tick event had triggered, which for NetworkDeltaPosition would cause dropped state updates.
minor adjustment to a test that would fail from time to time due to precision.
This is another bug in the validation test suite where removing an override of a virtual method that can still be overridden will throw an API validation error.
Send deltas unreliable but synchronization and teleporting reliably.
white space fixes
Fixing issue with changes that require a NetworkManager to be present.
Fixing new issues (due to changes) with position not synchronizing to the target position when half float precision was enabled.
minor adjustments to account for minor precision delta when quaternion compression is enabled in the NetworkTransformTests.ParentedNetworkTransformTest
Resolves the api validation issue.
Updating rotation and scale delta checks when sending unreliable messages.
Was doing some profiling and found a few low hanging fruit areas that helps improve performance.
@NoelStephensUnity NoelStephensUnity changed the title fix: NetworkTransform dropped packets can cause interpolation stutter fix: NetworkTransform dropped packets can cause interpolation stutter [MTT-7551] Oct 10, 2023
removing whitespace
More adjustments to handle sending unreliable deltas. These changes will (when sending unreliable deltas):
- Stagger NetworkTransform axial frame synchronization
  - Axial frame synchronization is sent reliably
  - Axial frame synchronization is sent only if deltas have been sent (once to assure when an object stops it no longer sends syncs)

This also adjusts the "one tick event updates all NetworkTransform authority instances" addition.
Fixing an issue with precision adjustments for rotation.
Adding integration test to validate the handling of dropped packets.
Removing unused namespace.
Removing reference to static variable that no longer exists (was for debugging)
removed whitespaces
Removing UseUnreliableDeltas when running the TestRotationThresholdDeltaCheck as the synchronization frame could cause this test to fail.
Noticed a potential timing related parenting issue where a parented object should be fully synchronized by  NetworkTransform (even though it sends transform information when parenting).
Some additional tweaks to the parenting portion of the NetworkTransform tests.
Reverting the parenting adjustment, that was a bad idea.
Only reset and update interpolators if interpolation is enabled seems to resolve the earlier issue.
removing unused methods.
adding 100ms latency to the NetworkTransformUTPTests.
Renaming NetworkTransformUTPTests to NetworkTransformPacketLossTests
Exposing the UseUnreliableDeltas property and adding UI/UX to provide users a way to disable this via the inspector view.
Making NetworkTransformTests run with UseUnreliableDeltas disabled to validate this operation mode of NetworkTransform.
Adding additional wait for added latency.
adding change log entries.
@NoelStephensUnity NoelStephensUnity marked this pull request as ready for review October 18, 2023 01:33
@NoelStephensUnity NoelStephensUnity requested a review from a team as a code owner October 18, 2023 01:33
Removing some todo statements and finalizing a bit flag to assure non-authoritative instances mirror the authoritative instance's UseUnreliableDeltas settings (i.e. if owner authoritative and ownership changes the same settings should apply).
Updating change log entry
Fixing bad changelog merge
adding CR/LF
Discovered a long time standing parenting issue with the initial synchronization (while working on something else). The issue involves WorldPositionStays, InLocalSpace,  and the initial synchronization. Added comments to describe the adjustments.
Basically, when synchronizing, ignore the InLocalSpace setting and only use the WorldPositionStays value to determine which position space to use on the authoritative side only when building the initial synchronization state update.
@@ -269,6 +274,7 @@ public void TestSyncAxes([Values] SynchronizationType synchronizationType, [Valu

if (syncPosX || syncPosY || syncPosZ || syncRotX || syncRotY || syncRotZ || syncScaX || syncScaY || syncScaZ)
{
Assert.IsTrue(networkTransform.NetworkManager != null, "NetworkManager is NULL!");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use Assert.NotNull(networkTransform.NetworkManager)
or Assert.That(networkTransform.NetworkManager, Is.Not.Null)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh... true that! I would think that would help clarify the context faster for reviewers.
(nice tip)
👍

Just adjusting the way a test asserts on null based on suggested review feedback.
minor adjustments to when we need to send a reliable packet when sending unreliable deltas.
@NoelStephensUnity NoelStephensUnity merged commit d099d64 into develop Nov 14, 2023
@NoelStephensUnity NoelStephensUnity deleted the fix/networktransform-dropped-packets-interpolation-stutter branch November 14, 2023 00:14
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants