Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix: ClientNetworkTransform ownership change with half precision synchronization issues [MTT-7271] #2696

Closed

Conversation

NoelStephensUnity
Copy link
Collaborator

@NoelStephensUnity NoelStephensUnity commented Sep 9, 2023

This PR resolves an issue with owner to host-server ownership transfer where the synchronization could get corrupted by noise due to:

  • The ownership message would proceed the NetworkTransform initialization message when transferring ownership from a client to a host when the ownership message should precede the NetworkTransform initialization message.
  • In transit NetworkTransform states from the client owner could get forwarded back to the client owner after ownership had already been transferred (on the host-server side) back to the host which would introduce echo "noise" of previous state updates by the original client owner.
  • Some client owner generated state updates could be skipped/ignored due to an edge case scenario where the client side network tick was the same even though the next tick event had been triggered.

fix: #2628

Changelog

  • Fixed: WIP

Testing and Documentation

  • Includes integration test (WIP)
  • No documentation changes or additions were necessary.

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.
@@ -2510,24 +2542,25 @@ public override void OnDestroy()
}

/// <inheritdoc/>
public override void OnGainedOwnership()
public override void OnLostOwnership()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Leaving these here otherwise the validation test will fail even though a user can still override both methods (see OnGainedOwnership too).

@@ -588,6 +588,7 @@ private void AllChildrenLocalTransformValuesMatch(bool useSubChild)
success = WaitForConditionOrTimeOutWithTimeTravel(AllChildObjectInstancesHaveChild);
Assert.True(success, "Timed out waiting for all instances to have parented a child!");

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Both minor tweaks to this test will help prevent random edge case precision validation errors.

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
Copy link
Collaborator Author

Split this PR into two PRs:
PR-2712
PR-2713

@NoelStephensUnity NoelStephensUnity deleted the fix/networktransform-halffloat-ownership-sync branch June 4, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Short desync of ClientNetworkTransform after ownership change with half precision enabled
1 participant