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

NetworkTransform sends wrong initial position to remote clients, and this error can persist #2913

Open
zachstronaut opened this issue May 2, 2024 · 9 comments
Assignees
Labels
priority:high stat:awaiting response Status - Awaiting response from author. stat:imported Issue is tracked internally at Unity type:bug Bug Report

Comments

@zachstronaut
Copy link

Bug report for v1.9.1, and I believe this problem did not exist in 1.7.1.

A host and a client join a game, and a new scene is additively loaded.

I have a NetworkObject with a NetworkTransform, let's call it Andy. It is placed in the additively loaded scene as a child of a child of another NetworkObject. So that means Andy's direct parent is not a NetworkObject. Andy and its parents are not at 0,0,0 world positions.

In NetworkTransform, the first time ApplyTransformToNetworkStateWithInfo runs on the host, WorldPositionStays() is false because m_CachedWorldPositionStays was set to false in NetworkObject.ApplyNetworkParenting -- because var parentNetworkObject = transform.parent.GetComponent<NetworkObject>(); is null. This means the host always sends a LOCAL position over the wire!

When ApplyTeleportingState runs for the first time on a remote client (via OnSynchronize), it receives that local position however it will apply the local Vec3 data as a world position because InLocalSpace is not true for the NetworkTransform.

This problem tends to be HIDDEN by the fact that much of the time, but not always, ApplyTeleportingState runs a second time on the remote client (via ApplyUpdatedState) and this second time the position data over the wire is in world space.

I don't know why ApplyTeleportingState doesn't always run for the remote client when the scene loads. When it doesn't run, the NetworkObject with the NetworkTransform -- Andy -- can remain in the wrong world position!

@zachstronaut zachstronaut added stat:awaiting triage Status - Awaiting triage from the Netcode team. type:bug Bug Report labels May 2, 2024
@NoelStephensUnity
Copy link
Collaborator

To make sure I understand:

  • In-Scene NetworkObject (root parent)
    • GameObject (1st child) (position is not 0,0,0 but never changes from its preset offset)
      • "Andy" NetworkObject (2nd child)

Is this the hierarchy you are describing?

@NoelStephensUnity NoelStephensUnity added the stat:awaiting response Status - Awaiting response from author. label May 3, 2024
@zachstronaut
Copy link
Author

yeah, that's basically correct although Andy is also not at 0,0,0 relative to its immediate parent.

The exact scenario is:

  • In-Scene NetworkObject (root parent)
    • Child 1
      • Child 2
        • Child 3 (local 0, -3, 0)
          • Child 4 (local 36, -2, 39)
            • Child 5
              • Andy NetworkObject (local -14, 0.15, -10)

Although I think the extra nesting of children doesn't matter and a single non-network object immediate parent that's not at world 0,0,0 would be enough to reproduce.

@NoelStephensUnity
Copy link
Collaborator

@zachstronaut
I think I can solve this by doing a recursive parent crawl:

        private NetworkObject GetNetworkObjectParent(Transform transform)
        {
            if (transform.parent != null)
            {
                var networkObject = transform.parent.GetComponent<NetworkObject>();
                if (networkObject == null)
                {
                    return GetNetworkObjectParent(transform.parent);
                }
                return networkObject;
            }
            return null;
        }

Then replace this:
var parentNetworkObject = transform.parent.GetComponent<NetworkObject>();

with this:
var parentNetworkObject = GetNetworkObjectParent(transform);

Which should assure world position stays will not be false and the position sent will be sent world space relative.

@NoelStephensUnity NoelStephensUnity added priority:high stat:import and removed stat:awaiting response Status - Awaiting response from author. stat:awaiting triage Status - Awaiting triage from the Netcode team. labels May 8, 2024
@fluong6 fluong6 added stat:imported Issue is tracked internally at Unity and removed stat:import labels May 8, 2024
@NoelStephensUnity NoelStephensUnity self-assigned this May 8, 2024
@NoelStephensUnity
Copy link
Collaborator

NoelStephensUnity commented May 9, 2024

@zachstronaut
So, after writing some more tests and looking further into this... I am wondering if there isn't something else you might be doing that could be impacting things?

Server-side synchronization:

When the server first spawns an in-scene placed NetworkObject after it additively loads a scene, it will invoke NetworkManager.SpawnManager.SpawnNetworkObjectLocally which invokes NetworkSpawnManager.SpawnNetworkObjectLocallyCommon. Within NetworkSpawnManager.SpawnNetworkObjectLocallyCommon, it will invoke NetworkObject.ApplyNetworkParenting which, as you pointed out, views a NetworkObject nested under a GameObject as being parenting with WorldPositionStays set to false (i.e. use local space values).

However, the more important thing to note here is that it sets m_CachedWorldPositionStays to false ==and== it sets the HasParent flag to notify the client it has a parent.

So far (based on the hierarchy presented) the server will serialize the following values:

  • WorldPositionStays = false
  • HasParent = true

Then, for normal NetworkObject transform synchronization, the server determines whether it should use world or local space here :

var syncRotationPositionLocalSpaceRelative = obj.HasParent && !m_CachedWorldPositionStays;

Which based on the above values (HasParent & !WorldPositionStays), it should set syncRotationPositionLocalSpaceRelative to true.

Next it determines local vs worldspace values to use within the NetworkObject.GetMessageSceneObject method here:

Position = syncRotationPositionLocalSpaceRelative ? transform.localPosition : transform.position,

So, the final values of interest for Andy's NetworkObject should be:

  • WorldPositionStays = false
  • HasParent = true
  • Position = (the local space position values)

Now, during the server-side NetworkTransform.OnSynchronize method it eventually makes its way down to determine which worldspace value to use here:

                if (isSynchronization)
                {
                    if (NetworkObject.WorldPositionStays())
                    {
                        position = transformToUse.position;
                    }
                    else
                    {
                        position = transformToUse.localPosition;
                    }
                }

Which the position variable is used to set all axis enabled for synchronization on the state to send for the initial NetworkTransform synchronization. Since WorldPositionStays == false, it will use the local space position values.

Client-side synchronization:

During the initial synchronization of an in-scene placed NetworkObject, the client will invoke NetworkSpawnManager.CreateLocalNetworkObject which this eventually applies the NetworkObject's m_CachecWorldPositionStays value via NetworkObject.SetNetworkParenting.

So, we know the In-Scene placed NetworkObject's m_CachecWorldPositionStays value will be the same as the servers (i.e. false). Then the client will apply the NetworkObject's transform values here:

                    if (worldPositionStays || !networkObject.AutoObjectParentSync)
                    {
                        networkObject.transform.position = position;
                        networkObject.transform.rotation = rotation;
                    }
                    else
                    {
                        networkObject.transform.localPosition = position;
                        networkObject.transform.localRotation = rotation;
                    }

Since WorldPositionStays is false and I am assuming you have not disabled AutoObjectParentSync, it should apply the serialized NetworkObject's transform values (if transform sync is enabled) as local space relative. Then during NetworkTransform.OnSynchronize it applies position based on its world position stays setting:

                if (isSynchronization)
                {
                    if (NetworkObject.WorldPositionStays())
                    {
                        position = transformToUse.position;
                    }
                    else
                    {
                        position = transformToUse.localPosition;
                    }
                }

Final States of Interest Serialized:

  • Server-Side:
    • NetworkObject
      • WorldPositionStays = false
      • HasParent = true
      • Position = (the local space position values)
    • NetworkTransform
      • Position = (the local space position values)
  • Client-Side:
    • NetworkObject
      • Applies WorldPositionStays value when "creating" the NetworkObject.
        • For in-scene placed, it gets the instance created when the scene is loaded.
      • Applies the Transform's local space position
    • NetworkTransform.OnSynchronize:
      • It applies the position of the NetworkTransformState to the transform's local space value because WorldPositionStays is false.

So, under the scenario you have outlined... unless you are changing any of the children transform values (other than the one with the NetworkTransform), parenting things on fly (i.e. dynamically creating a scene to be loaded or adding things to the in-scene placed hierarchy prior to invoking OnSynchronize), or have a derived version of NetworkTransform that overrides the OnSynchronize or OnNetworkSpawn methods and makes changes to values after OnSynchronize is invoked or prior to OnNetworkSpawn being invoked....

I am having a hard time replicating the issue with v1.9.1... as at first pass glance it seemed like what you were saying made sense....but when I wrote tests for the fix...my test scene that gets loaded (arranged exactly as you outlined) passes without that fix... which made me do a double take on the process...and I came to the realization that what you are seeing shouldn't be happening unless there is something else you might have left out about your configuration (i.e. any additional scripts or the like that could be changing things during serialization)?

@NoelStephensUnity NoelStephensUnity added the stat:awaiting response Status - Awaiting response from author. label May 9, 2024
@zachstronaut
Copy link
Author

zachstronaut commented May 9, 2024 via email

@NoelStephensUnity
Copy link
Collaborator

@zachstronaut

When ApplyTeleportingState runs for the first time on a remote client (via OnSynchronize), it receives that local position however it will apply the local Vec3 data as a world position because InLocalSpace is not true for the NetworkTransform.

That shouldn't be happening during synchronization.

This problem tends to be HIDDEN by the fact that much of the time, but not always, ApplyTeleportingState runs a second time on the remote client (via ApplyUpdatedState) and this second time the position data over the wire is in world space.
I don't know why ApplyTeleportingState doesn't always run for the remote client when the scene loads. When it doesn't run, the NetworkObject with the NetworkTransform -- Andy -- can remain in the wrong world position!

It could be sending the state update from the server immediately after the synchronization message and a client could potentially receive and process both messages back to back... but even then it should always send and process those messages in the same order... but I will look over the SetState being sent when a NetworkTransform spawns to see if that could be causing the issue.

@NoelStephensUnity
Copy link
Collaborator

I’m starting a paternity leave now so unfortunately I’m not gonna be much help investigating this with you for about 4 weeks! 🥰On May 8, 2024, at 19:18, Noel Stephens @.***> wrote:

Congrats!!!!!!! 😻

(No worries... I will continue digging into this and if I can replicate it then will push out a fix for it...)

@AsmPrgmC3
Copy link

It sounds to me like you're running into #2888 (with an additional network object ancestor).

@NoelStephensUnity
As for the synchronization overview: The code snippet for Client-Side Network Transform synchronization is from ApplyTransformToNetworkStateWithInfo, which is executed server-side. The client-side executes ApplyTeleportingState which doesn't query WorldPositionStays.

@NoelStephensUnity
Copy link
Collaborator

@AsmPrgmC3
(It was late and I see the disconnect... for some reason I was still looking at the authority side. Thank you for catching that!)

As for the synchronization overview: The code snippet for Client-Side Network Transform synchronization is from ApplyTransformToNetworkStateWithInfo, which is executed server-side. The client-side executes ApplyTeleportingState which doesn't query WorldPositionStays.

💯

What needs to happen is that it needs to perform the same parenting and world position stays check during OnSynchronize on the non-authority side when ApplyTeleportingState is invoked.
👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:high stat:awaiting response Status - Awaiting response from author. stat:imported Issue is tracked internally at Unity type:bug Bug Report
Projects
None yet
Development

No branches or pull requests

4 participants