Multiple re-parents and Teleport() calls on NetworkTransform corrupts In Local Space position

[zachstronaut]

Description

If you use NetworkObject.TrySetParent() and NetworkTransform.Teleport() multiple times in the same frame and the NetworkTransform is InLocalSpace, the position data will get corrupted and out of sync between clients.

Repro is very straightforward!

ParrelSync with simulated 750ms lag so you can more easily see the problem.

using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using Unity.Netcode;
using Unity.Netcode.Components;

public class Bug0000 : NetworkBehaviour
{
    public Transform position1; // None of these Transforms should be at world (0,0,0)
    public Transform position2;
    public Transform position3;
    public Transform parent1;
    public Transform parent2;
    public GameObject prefab;

    void SpawnBug()
    {
        var temp = GameObject.Instantiate(prefab, position1.position, Quaternion.identity) as GameObject;

        NetworkObject networkObj = temp.GetComponent<NetworkObject>();
        networkObj.SpawnWithOwnership( NetworkManager.ServerClientId );

        networkObj.TrySetParent( parent1 );

        Bug0000Move(networkObj, position2.position);

        networkObj.TrySetParent( parent2 );

        Bug0000Move(networkObj, position3.position);
    }

    void Bug0000Move(NetworkObject networkObj, Vector3 pos)
    {
        NetworkTransform netTransform = networkObj.GetComponent<NetworkTransform>();
        Debug.Assert(netTransform.InLocalSpace);
        var newPos = netTransform.transform.parent.InverseTransformPoint(pos);
        netTransform.Teleport(newPos, networkObj.transform.rotation, networkObj.transform.localScale);
    }
}

The spawned object will end up in the incorrect position on the screen on the non-host client, and the incorrect position will persist indefinitely until the object is moved.

It appears to me that NetworkTransform:AdjustForChangeInTransformSpace() -- called via NetworkTransform:UpdateInterpolation () via NetworkTransform:OnUpdate() -- calculates an incorrect m_InternalCurrentPosition and therefore ApplyAuthoritativeState() moves the object to the wrong position.

If you run this repro case with the spawned object owner instead being the non-host, the same bug occurs but it resolves itself with an interpolation rather than persisting indefinitely.

Environment

OS: Windows 11
Unity Version: 6000.1.3f1
Netcode Version: 2.4.2
Netcode Topology: Client-Server

[zachstronaut]

I suspect this is somehow a cousin of one and/or both of these:

#3645
#3646

[NoelStephensUnity]

@zachstronaut

Ok... my initial gut reaction was "why would you be parenting and then re-parenting within the same callstack"... so I had to circle back to this after having thought about some of your projects...

Just to make sure I am understanding the "real-world" scenario you have captured in this replication script... 😄

Is the real-world issue pertaining to some kind of lowest generation child invocation conundrum where a derived class is changing the parenting but somewhere in the upper-generation child derivations it is also defaulting to changing the parenting (but to a different NetworkObject)....which the lowest generation child would need to invoke the base.OnNetworkSpawn method first and then apply its own parenting?

Or something like that?

(Trying to understand the actual real-world replication of this issue)

Regarding the script:

    public Transform position1; // None of these Transforms should be at world (0,0,0)
    public Transform position2;
    public Transform position3;
    public Transform parent1;
    public Transform parent2;
    public GameObject prefab;

My assumption is that parent1 & parent2 are transforms of already spawned NetworkObjects?

As a side note (which may be pertinent): Have you had a chance to play around with AttachableBehaviour and/or ComponentController yet?

[NoelStephensUnity]

Regarding AttachableBehaviour documentation:
Just realized the documentation needs some adjusted language. It provides a scenario where a player is picking something up... but it is not limited to just players and the combination of AttachableBehaviours, AttachableNodes, and ComponentControllers allows you to create "lego-like" prefabs.

The example in the documentation provided assumes "motion" comes with animation that is synchronized when in reality you could have something like:

Thing to attach to:

• GameObject-A Root (NetworkObject, NetworkTransform, etc)
  • AttachPoint (AttachableNode, NetworkTransform in local space, etc)

Thing to attach:

• GameObject-B Root (NetworkObject, maybe NetworkTransform if it will move around when not attached)
  • ThingToAttach (AttachableBehaviour, might have other components)
    • (possibly has children with components)

So, when attached:

• Scene Root
  • GameObject-A Root (NetworkObject, NetworkTransform, etc)
    • AttachPoint (AttachableNode, NetworkTransform in local space, etc)
      • ThingToAttach (AttachableBehaviour, might have other components)
      • (possibly has children with components)
  • GameObject-B Root (NetworkObject, maybe NetworkTransform if it will move around when not attached)

Then the authority of the GameObject-A could move around the AttachPoint (local space relative) and the ThingToAttach would move with it and synchronize that way on all clients.

Alternatively, if your project supported user mods you could provide an API to derived classes from AttachableBehaviour and ComponentController where the users of your product could create "custom things" that can be attached to a preset list of "base things" (your project) where the user defined "custom things" could have unique NetworkBehaviour scripts and the like. The minimal cost of an attachable "thing" is that when it is spawned and attached, its GameObject root instance remains instantiated and the associated NetworkObject still handles the routing of messages to the attachable "thing" (i.e. a single GameObject instance with a NetworkObject component remains in the scene hierarchy for each spawned attachable "thing" that has been attached to some other base spawned object).

[NoelStephensUnity]

After looking at the script provided I am wondering why you would want to send two, back-to-back, parenting messages after a NetworkObject has been spawned within the same frame/call-stack?

From an NGO messaging perspective, this is what is happening (if spawned on the server side):

Server Side:

• Spawn --> a CreateObjectMessage is generated that includes the initial NetworkTransform state when spawned.
  • Before any parenting has been done, this message is already in the queue based on your script.
    • The serialized NetworkTransformState included in the message sets the client instances to be at position1.position.
• Parent under Parent1 ---> generates a ParentSyncMessage (1st message of this script to be received by a client)
  • Move using NetworkTransform.Teleport which does nothing but updates the local authority's NetworkTransformState with the teleport information but does not send any message since the NetworkObject was just spawned and only updates each network tick.
• Parent under Parent2 ---> generates a ParentSyncMessage (2nd to be received by a client)
  • Move using NetworkTransform.Teleport which does nothing but updates the local authority's NetworkTransformState with the teleport information but does not send any message since the NetworkObject was just spawned and only updates each network tick.
    ...
• On the next network tick, it sends the last Bug0000Move teleport.
  • The 1st teleport is basically being overwritten by the second one since they are being called within the same tick update period of time.

Client Side

• CreateObjectMessage
  • Spawns NetworkObject and applies the initial transform state that has a position value equal to that of position1.position.
• ParentSyncMessage (parent 1 with world position stays)
  • Offsets the client-side instance by parent 1's world space position relative to its spawned position1.position.
• ParentSyncMessage (parent 2 with world position stays)
  • Offsets the client-side instance, now offset by previous parenting, by parent2's world space position.
    ... Client latency + ~1 network tick later (relative to when the server spawned the object).
• NetworkTransformMessage is received that teleports the object to the position3.position that is a local space relative to parent2.

Which if the NetworkTransform is using LocalSpace, then the over-all net expected position of the spawned object should be the local space position3.position relative to parent2...and the previous parent1 parenting and the move prior to it are irrelevant.

The first thing I would recommend (if possible) is to make an adjustment to the object being spawned NetworkTransform and make that a custom NetworkTransform. The second thing I would do is calculate the final position offset you want just using transforms and no parenting (since the 1st parenting has no real impact on the 2nd with the scenario you provided).

It would look something like this:

public class CustomNetworkTransform : NetworkTransform
{
    public Transform position1; // None of these Transforms should be at world (0,0,0)
    public Transform position2;
    public Transform position3;
    public Transform parent1;
    public Transform parent2;

    public bool PreserveOffset;

    private Vector3 m_FinalPosition;

    public override void OnNetworkSpawn()
    {
        transform.position = position1.position;
        //Apply transform prior to spawning so the transform's modified state will be included in the serialized spawn data
        var firstPosition = parent1.InverseTransformPoint(position2.position);

        if (PreserveOffset)
        {
            firstPosition = parent1.InverseTransformPoint(transform.position) + firstPosition;
        }

        var secondPosition = parent2.InverseTransformPoint(position3.position);
        // If not preserving the offset and using local space, then the above local space calculation is all needed.
        if (PreserveOffset)
        {
            secondPosition = parent2.InverseTransformPoint(parent1.TransformPoint(firstPosition)) + secondPosition;
        }

        // Apply the adjusted transform position based on the other transforms, but transform it from local to world
        // so the world space position sychronized upon spawn is the precise local space position you want
        transform.position = parent2.TransformPoint(secondPosition);

        base.OnNetworkSpawn();
    }

    protected override void OnNetworkPostSpawn()
    {
        base.OnNetworkPostSpawn();

        // Now just parent under the 2nd parent.
        NetworkObject.TrySetParent(parent2);
    }
}

Where your script you provided would look more like this:

public class Bug0000 : NetworkBehaviour
{
    public Transform position1; // None of these Transforms should be at world (0,0,0)
    public Transform position2;
    public Transform position3;
    public Transform parent1;
    public Transform parent2;
    public GameObject prefab;

    void SpawnBug()
    {
        var temp = GameObject.Instantiate(prefab, position1.position, Quaternion.identity) as GameObject;
        var customNetworkTransform = temp.GetComponent<CustomNetworkTransform>();
        customNetworkTransform.position1 = position1;
        customNetworkTransform.position2 = position2;
        customNetworkTransform.position3 = position3;
        customNetworkTransform.parent1 = parent1;
        customNetworkTransform.parent2 = parent2;
        NetworkObject networkObj = temp.GetComponent<NetworkObject>();
        networkObj.SpawnWithOwnership(NetworkManager.ServerClientId);
    }
}

Otherwise, you will definitely run into issues trying to immediately apply transform state multiple times within the same call stack and method.

[zachstronaut]

Thanks @NoelStephensUnity for looking into this and providing some suggestions. I can't analyze them at the moment, but I will look at them tomorrow (Tues) to see if I can apply them.

As for your question:

I am wondering why you would want to send two, back-to-back, parenting messages after a NetworkObject has been spawned within the same frame/call-stack?

I'm working on a large, complex game. It spawns player avatars at different door positions which can be under different parents. But also there is save state, so potentially instead of a spawn at a door you spawn in a deeper gameplay state further along in the level. Additionally, players can be on moving objects like elevators and such. This means when players spawn they could need to be parented to the default spawn position, or save state could indicate they need to be parented elsewhere and that elsewhere could happen to be on top of a moving platform, in which case re-parenting needs to happen yet again. We don't serialize that parentage onto those moving things into game save data because it can be derived from what we do save: the spawn world position. And because it is a big complex game, there's a spawn system, and a door system, and a save state restore system, and a dynamic-what-the-hell-are-you-standing-on-and-does-it-move system. So that first frame upon spawn is conditionally flowing through all/some/none of those to arrive at the final parent and final world position.

And yeah I can make an arbitrator that watches all this happen and then applies one parent operation and one position operation at the end of all that flow. Or I could arbitrarily introduce a frame delay between these systems (although, maybe it would have to be a Tick delay?). In some cases I have already even made my own NetworkTransform replacement that handles parenting internally and avoids NetworkObject's parenting methods. (I am currently on the fence about whether I just go all in on that custom replacement and stop trying to use Netcode's provided position sync and parent sync at all.)

But, I really think that re-parenting and re-positioning (interpolated and teleport) are so fundamental to a net library that they ought to be idiot proof and battle hardened. I'd be a bit bummed to end up shipping this game not using the library provided methods for parenting and positioning. I'd love to get upstream improvements for such core functionality as they come along, and as I work on other games, and not find myself forking too much.

I keep running into problems with re-parenting and re-positioning where the operations either fail or there is an interpolation when there should not be. This class of bug with Netcode has been an ongoing theme for me over the past year and a half or so. I suspect the root of the problem is the separation of NetworkObject and NetworkTransform because obviously local coordinate position sync and parent sync go hand-in-hand. A great many (nearly all) of these bugs have been fixed (probably by you, thanks!) -- and I really think this one and #3645 and #3646 ought to be fixed too and not left to the end user to try to mitigate.

Which is a long way of saying that I don't really think it matters why I would do something so silly as re-teleport and re-parent multiple times in a row on the same frame as spawn. It is something that can happen, so it should either be explicitly not supported and throw an error, or it should work, imho.

[zachstronaut]

My assumption is that parent1 & parent2 are transforms of already spawned NetworkObjects?

Sorry, yep! The parents are NetworkObjects that exist in the scene file.