Skip to content

Client/Owner authoritative NetworkTransform and Re-Parenting issue #2842

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

Closed
zachstronaut opened this issue Mar 12, 2024 · 14 comments · Fixed by #3453
Closed

Client/Owner authoritative NetworkTransform and Re-Parenting issue #2842

zachstronaut opened this issue Mar 12, 2024 · 14 comments · Fixed by #3453
Labels
priority:high This issue has high priority and we are focusing to resolve it stat:imported Status - Issue is tracked internally at Unity type:bug Bug Report

Comments

@zachstronaut
Copy link

zachstronaut commented Mar 12, 2024

When you have a moving client/owner authoritative NetworkTransform and the server changes its parent, the position where that object was on the server at the moment of the re-parenting gets serialized back to the client. The client is pushed back to a previous position (essentially back in time).

This problem isn't happening inside of NetworkTransform, though. You can observe the same issue where the client jumps to the server's position upon re-parenting if you have no NT at all.

The source of this problem is actually in ParentSyncMessage.Handle() messing with the position of objects and not respecting that somebody other than the server may own the position of the object.

@zachstronaut zachstronaut added stat:awaiting-triage Status - Awaiting triage from the Netcode team. type:support Questions or other support labels Mar 12, 2024
@NoelStephensUnity
Copy link
Collaborator

I see where the issue could arise with an owner authoritative instance.
I could add a NetworkObject.AutoParentingTransformSync property that would default to true (to not break the existing behavior), but if that was disabled then it would not apply any transform values when parenting occurred... but your issue is really specific to the owner/client authoritative motion model.

Of course, we are getting much closer to releasing the alpha package for distributed authority that provides full client-side authority over owned NetworkObjects... so at that point you can just parent the NetworkObject on the client-side where transform values will synchronize to the client owner's values (which would completely resolve your issue).

I am restricted from providing details on when this will be available, but I can say to keep watching the repository as it will get a relatively large update soon (i.e. in a few weeks). While the update won't be specific to the develop branch it will have some new branches that will have some rather large changes.

Let me weigh the benefits of adding that property vs holding back until the distributed authority is available (i.e. landed in the repo not the official package).

(Have several ideas of how you could work around this issue...but again... distributed authority update will resolve these kinds of issues "by design"...so I am hesitant sending you in a direction that you will most likely replace once DA is available).

@zachstronaut
Copy link
Author

@NoelStephensUnity Thanks! I'm pretty sure I can make customizations to Netcode myself locally to fix this one, but I wanted to make sure it was on the radar as a problem with client authority.

However, I haven't been able to figure out my own solution to this other kinda related problem yet:

#2843

I've been able to reduce the problem down to what I assume is a single tick of bad positioning, but that's still a huge visual glitch.

@zachstronaut
Copy link
Author

@NoelStephensUnity It seems like a small change to ParentSyncMessage.cs fixes this issue with owner/client authoritative position snapping back in time:

public void Handle(ref NetworkContext context)
{
    var networkManager = (NetworkManager)context.SystemOwner;
    var networkObject = networkManager.SpawnManager.SpawnedObjects[NetworkObjectId];
    networkObject.SetNetworkParenting(LatestParent, WorldPositionStays);
    networkObject.ApplyNetworkParenting(RemoveParent);

    // We set all of the transform values after parenting as they are
    // the values of the server-side post-parenting transform values
    if (!WorldPositionStays)
    {
        networkObject.transform.localPosition = Position;
        networkObject.transform.localRotation = Rotation;
    }
    // THE FIX
    // else
    // {
    //     networkObject.transform.position = Position;
    //     networkObject.transform.rotation = Rotation;
    // }
    networkObject.transform.localScale = Scale;
}

This makes me wonder in what circumstances you would even need to be writing position/rotation information when re-parenting is WorldPositionStays since by definition the position hasn't changed from the re-parenting?

@NoelStephensUnity
Copy link
Collaborator

NoelStephensUnity commented Apr 4, 2024

If changes are made on the authority side to the transform upon a NetworkObject being parented (i.e. within OnNetworkObjectParentChanged) and there is no NetworkTransform then this scenario applies. There have been on-going issues with handling parenting under various scenarios (especially if a NetworkTransform is attached to the GameObject in question).

Side Note:
With NGO v2.0.0 we will be migrating all of the components assembly into the runtime assembly which will make the above conundrum much easier to determine if it should apply changes or let NetworkTransform handle the changes when parenting.

@NoelStephensUnity
Copy link
Collaborator

Closing this issue as it was fixed in #3013.

@michalChrobot michalChrobot removed the stat:awaiting-triage Status - Awaiting triage from the Netcode team. label Mar 19, 2025
@zachstronaut
Copy link
Author

@NoelStephensUnity this was closed when it was fixed in 2.x but was never backported to 1.x. This issue and #2843 actually create some problems bad enough in a Netcode 1.x game that make it unshippable. (Not being able to smoothly re-parent without a major visual hiccup is way too janky to ship.)

Upgrading to Unity 6 is not possible, so maybe I will have to backport all of 2.x for our own uses.

I think the two tickets I mentioned are just as disruptive as #3080 which was mostly fixed by the backport of #3388.

@NoelStephensUnity
Copy link
Collaborator

NoelStephensUnity commented Apr 29, 2025

@zachstronaut
You are wanting some of the ParentSyncMessage fixes in v2.x to be back ported to v1.x is that correct?

As a side note, it definitely helps to provide the Unity editor and NGO version you are using when you experienced the issue so in the event we are backlogged and circle back to an issue we can make sure any fixes are targeted for the appropriate versions.

@NoelStephensUnity NoelStephensUnity added the type:bug Bug Report label Apr 29, 2025
@github-actions github-actions bot added the stat:awaiting-response Awaiting response from author. This label should be added manually. label Apr 29, 2025
@NoelStephensUnity NoelStephensUnity removed the type:support Questions or other support label Apr 29, 2025
@zachstronaut
Copy link
Author

@NoelStephensUnity sorry about that... Unity 2022.3.60f1 and Netcode 1.13

Yes, I think I am looking for the ParentSyncMessage fixes to be backported for this purposes of this ticket.

I think for #2843 we're talking different fixes.

@github-actions github-actions bot added stat:reply-needed Awaiting reply from Unity account and removed stat:awaiting-response Awaiting response from author. This label should be added manually. labels Apr 29, 2025
@NoelStephensUnity
Copy link
Collaborator

NoelStephensUnity commented Apr 29, 2025

I think for #2843 we're talking different fixes.

Ok, I will look and see if I can include an "over-all" back-port that includes the v2.x NetworkTransform related parenting fixes along with the ParentSyncMessage... let's see if we can get this all squared away.

@github-actions github-actions bot added stat:awaiting-response Awaiting response from author. This label should be added manually. and removed stat:reply-needed Awaiting reply from Unity account labels Apr 29, 2025
@zachstronaut
Copy link
Author

@NoelStephensUnity ooooOOOooo wishing you luck!

@github-actions github-actions bot added stat:reply-needed Awaiting reply from Unity account and removed stat:awaiting-response Awaiting response from author. This label should be added manually. labels Apr 29, 2025
@NoelStephensUnity NoelStephensUnity added priority:high This issue has high priority and we are focusing to resolve it stat:import Status - Issue is going to be saved internally labels Apr 29, 2025
@NoelStephensUnity
Copy link
Collaborator

I marked this one particular issue to be imported and high and will make sure to include the fact that there are several other "orbiting" issues where some backports did not make it due to the PRs contained changes which require a major version increment of the package.

Now that a bunch of the v2.x stuff is starting to settle a bit it is definitely time to circle back and bring over any fixes (that can be brought over without requiring a major version increment) that pertain to synchronization and parenting.

@github-actions github-actions bot added stat:awaiting-response Awaiting response from author. This label should be added manually. and removed stat:reply-needed Awaiting reply from Unity account labels Apr 29, 2025
@michalChrobot michalChrobot added stat:imported Status - Issue is tracked internally at Unity and removed stat:import Status - Issue is going to be saved internally stat:awaiting-response Awaiting response from author. This label should be added manually. labels Apr 30, 2025
@NoelStephensUnity
Copy link
Collaborator

NoelStephensUnity commented May 19, 2025

@zachstronaut
I started the initial back port in #3543 and looking at the various things that I "can" bring backwards.
The lowest hanging fruit was to provide:

  • NetworkObject.SyncOwnerTransformWhenParented: Will not apply the server/host's transform values when parenting.
  • NetworkObject.AllowOwnerToParent: Allows the client owner to handle parenting.

If you get a chance, could you see if using either of the two helps to resolve your issue? The AllowOwnerToParent might workout the best as you can just handle the parenting on the client side which should keep everything in-synch.
The alternate approach (if you want the server to handle parenting still) is to disable SyncOwnerTransformWhenParented which prevents the client from applying the transform values.

Let me know if this helps to resolve your issue?

@github-actions github-actions bot added the stat:awaiting-response Awaiting response from author. This label should be added manually. label May 19, 2025
@zachstronaut
Copy link
Author

@NoelStephensUnity these sound promising, I will take a look as soon as I can. thanks!

@github-actions github-actions bot added stat:reply-needed Awaiting reply from Unity account and removed stat:awaiting-response Awaiting response from author. This label should be added manually. labels May 19, 2025
@NoelStephensUnity
Copy link
Collaborator

Awesome... if you run into other issues while testing this branch with your project let me know so I can try to see what parts from v2.x I can bring over to resolve those as well.

@github-actions github-actions bot added stat:awaiting-response Awaiting response from author. This label should be added manually. and removed stat:reply-needed Awaiting reply from Unity account labels May 19, 2025
NoelStephensUnity added a commit that referenced this issue May 27, 2025
This PR backports the the `NetworkObject.SyncOwnerTransformWhenParented`
and `NetworkObject.AllowOwnerToParent` functionality.

[MTTB-1284](https://jira.unity3d.com/browse/MTTB-1284)

fix: #2842

## Changelog

- Added: `NetworkObject.SyncOwnerTransformWhenParented` which will not
synchronize transform values sent by the server when parented (primarily
for owner authoritative motion models).
- Added: `NetworkObject.AllowOwnerToParent` which allows a client owner
to parent locally and have the changes synchronize with the server and
other clients (primarily for owner authoritative motion models).


## Testing and Documentation

- Includes backported integration test modified to the constraints of
v1.x.
- Includes additions to public API documentation.

<!-- Uncomment and mark items off with a * if this PR deprecates any
API:
### Deprecated API
- [ ] An `[Obsolete]` attribute was added along with a `(RemovedAfter
yyyy-mm-dd)` entry.
- [ ] An [api updater] was added.
- [ ] Deprecation of the API is explained in the CHANGELOG.
- [ ] The users can understand why this API was removed and what they
should use instead.
-->

## Backport

No back port needed as this is a targeted backport of certain v2.x
features that could be useful to projects still using v1.x.

---------

Co-authored-by: Emma <[email protected]>
@github-actions github-actions bot removed the stat:awaiting-response Awaiting response from author. This label should be added manually. label May 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:high This issue has high priority and we are focusing to resolve it stat:imported Status - Issue is tracked internally at Unity type:bug Bug Report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants