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

Kinematic Rigidbody Interpolation bug only when using Netcode? #2765

Closed
zachstronaut opened this issue Nov 16, 2023 · 12 comments
Closed

Kinematic Rigidbody Interpolation bug only when using Netcode? #2765

zachstronaut opened this issue Nov 16, 2023 · 12 comments
Assignees
Labels
stat:awaiting response Status - Awaiting response from author. type:support Questions or other support

Comments

@zachstronaut
Copy link

zachstronaut commented Nov 16, 2023

I'm seeing unexpected behavior from a kinematic Rigidbody when using Netcode that's sort of related to this Unity bug:

https://forum.unity.com/threads/interpolation-still-occurs-when-directly-setting-position-rotation-of-a-kinematic-rigidbody.1332504/

I'm using Unity 2022.3.10f1 and Netcode for Gameobject 1.6.0

If I destroy a kinematic Rigidbody (using Object.Destroy) while Interpolation is set to Interpolate, and then I use NetworkTransform.Teleport to move the position of that Rigidbody, the object will snap back to its old Rigidbody position if the origin of the Teleport call was a Server RPC called by a remote client.

If the code is run by the host/server, the position doesn't snap back. If Interpolation is set to None on the Rigidbody before the Teleport, the position doesn't snap back.

Repro Use Case

I have a stationary cube:

image

And I have a moving sphere with a NetworkBehaviour (and no, it doesn't have a NetworkRigidbody):

image

image

CarriestTest.cs

using UnityEngine;
using Unity.Netcode;
using Unity.Netcode.Components;

public class CarriedTest : NetworkBehaviour
{
    public Transform holder;
    bool m_once = true;

    void Update()
    {
        if (transform.parent != null) return;

        transform.position += Vector3.right * Time.deltaTime;

        // Bug only occurs if you press Z on a remote client. If you press Z on host/server the teleport behaves as expected.
        if (Input.GetKeyDown(KeyCode.Z) && m_once)
        {
            m_once = false;
            Drop_ServerRpc();
        }
    }

    [ServerRpc(RequireOwnership = false)]
    void Drop_ServerRpc()
    {
        var rb = GetComponent<Rigidbody>();
        // rb.interpolation = RigidbodyInterpolation.None; // This fixes the bug
        Object.Destroy(rb);
        transform.SetParent(holder);
        GetComponent<NetworkTransform>().Teleport( new Vector3(0, 1f, 0), Quaternion.identity, Vector3.one );
    }
}

Rarely, the bug won't happen (framerate dependency? I dunno?). But it seems like an almost 100% repro rate for me.

Perhaps this is Unity Rigidbody interpolation behaving as normal, and the issue that Netcode is introducing here is a race condition between whether the Teleport happens before or after the final interpolation? I dunno?

Personally, I think that setting the position on the Transform of a kinematic Rigidbody should teleport it and bypass Interpolation all the time.

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

Rigidbody should not have interpolation enabled if you are interpolating with NetworkTransform.

@zachstronaut
Copy link
Author

@NoelStephensUnity But the problem occurs on the authoritative instance. NetworkTransform shouldn't be interpolating on the authoritative side?

"Interpolation is enabled by default and is recommended if you desire smooth transitions between transform updates on non-authoritative instances."

@NoelStephensUnity
Copy link
Collaborator

NoelStephensUnity commented Nov 17, 2023

But the problem occurs on the authoritative instance. NetworkTransform shouldn't be interpolating on the authoritative side?

Well, I can definitely understand the confusion and I should have provided a bit more information to help clarify what is happening.

With RPCs there are some timing/order of operations things to consider. The first place to look would be the NetworkManager.NetworkUpdate method. The thing to take note of is that incoming messages are processed during the EarlyUpdate portion of a frame.

The Order of Execution for Event Functions page doesn't provide the whole picture in regards to all updates that occur in one frame, but is always a good thing to better understand when physics stuff is applied and that particular order of operations.

Fortunately, if we look at the NetworkUpdateStage we can see how all of the pertinent stages are invoked during a single frame:

  • Initialization
  • EarlyUpdate (Messages are processed and applied at this time)
  • FixedUpdate (Physics is then applied next)
  • PreUpdate
  • Update
  • PreLateUpdate
  • PostLateUpdate

What I believe is happening is:

  • The client invokes Drop_ServerRpc
    • The ServerRpc message is sent
  • The server receives the message and processes it during the NetworkUpdateStage.EarlyUpdate.
    • The server applies the parent
      • Since WorldPositionStays defaults to true, its relative world position is maintained and the local space position to the player carrying the object is calculated (i.e. the local space position/offset could be much greater than just 0,1,0)
    • The server then teleports to the (0,1,0) offset position
  • The server then runs the FixedUpdate loop
    • With Rigibody interpolation enabled, it will attempt to interpolate from the "world position applied local space position" to the (0, 1, 0) position which will result in a position "close to but not exactly" its last world space position relative to the player(?) carrying the object.

I believe this is why, when Rigidbody interpolation on the server side is enabled, you are seeing this "snap back" when a client invokes the Rpc. The reason it doesn't happen at all on the server side is that you are handling the dropping of the object during the component's Update (Update portion of a frame) which FixedUpdate has already occurred and any physics relative updates to the transform applied.

If you want to keep the Rigidbody interpolate property enabled, then you will need to defer when the "dropping of the object" is handled when triggered by an incoming Rpc message on the server side. Here is a quick revised version of the class you provided as an example:

public class CarriedTest : NetworkBehaviour
{
    public Transform Holder;
    private bool m_Once = true;
    private bool m_ShouldDropObject;
    private NetworkTransform m_NetworkTransform;

    private void Awake()
    {
        m_NetworkTransform = GetComponent<NetworkTransform>();
    }

    private void Update()
    {
        if (transform.parent != null)
        {
            return;
        }

        // Assuming you only want the server moving and/or handling client requests drop objects
        if (IsServer)
        {
            // Remote drop object requests are deferred to happen after FixedUpdate
            if (m_ShouldDropObject)
            {
                m_ShouldDropObject = false;
                DropObject();
            }
            else
            {
                transform.position += Vector3.right * Time.deltaTime;
            }
        }

        if (Input.GetKeyDown(KeyCode.Z) && m_Once)
        {
            m_Once = false;
            if (IsServer)
            {
                // Server invokes directly
                DropObject();
            }
            else
            {
                // Client notifies it wants to drop
                DropObjectServerRpc();
            }
        }
    }

    private void DropObject()
    {
        // Do you want to do this?
        var rb = GetComponent<Rigidbody>();
        Destroy(rb);

        // I am assuming this is the transform of the thing that "held the object before it was picked up"?
        // You could need to revert the false, but if you are in local space then you might want parenting
        // to only be "local space" relative (i.e. not WorldPositionStays)
        transform.SetParent(Holder, false);
        m_NetworkTransform.Teleport(new Vector3(0, 1f, 0), Quaternion.identity, Vector3.one);
    }

    [ServerRpc(RequireOwnership = false)]
    private void DropObjectServerRpc()
    {
        // This is used to handle clients dropping the object after FixedUpdate
        m_ShouldDropObject = true;
    }
}

The gist of this is that if you want to apply adjustments to a server authoritative NetworkTransform via an Rpc and have other Physics forces/related actions being applied (and look right) then you don't want to apply it while processing the Rpc (during the EarlyUpdate)... but apply it after FixedUpdate has run for the frame that the Rpc was received.

Really, this needs to be outlined in the documentation and I just haven't had the time to add these kinds of details... but I am now putting this on my list of things to update 👍

Let me know if this makes sense and resolves your issue?

@NoelStephensUnity
Copy link
Collaborator

NoelStephensUnity commented Nov 17, 2023

As a side note, if you handle everything in local space...then you might not even need to teleport as long as you don't parent with WorldPositionStays set to true... whatever the offset is on the player holding the item will be the same local space offset if you parent with WorldPositionStays set to false... so you could have a pre-defined "desired target offset" on the CarriedTest and upon the parent being set you could interpolate (server side) towards the desired offset until it reaches a "close to approximation" of the desired offset. This "should" make it look like the object is being "set down".

@zachstronaut
Copy link
Author

@NoelStephensUnity Excited to dive into this on Monday! Have a great weekend.

@NoelStephensUnity NoelStephensUnity added the stat:awaiting response Status - Awaiting response from author. label Nov 20, 2023
@zachstronaut
Copy link
Author

@NoelStephensUnity Ok, I read your much appreciated response and yes this is pretty much what I expected as the source of the problem. Your proposed fix does work. It'll be interesting to see how much we have to use that pattern in our game... I'm genuinely curious how often the order of operations will be a problem.

I've still got one place where it seems like this same flavor of problem is happening, but it is much less consistent. Any clever ideas to boost the chances of this timing issue occurring so I can convince myself I've actually fixed it? A big change to the fixed timestep for testing purposes?

@NoelStephensUnity
Copy link
Collaborator

NoelStephensUnity commented Nov 20, 2023

Hmmm... yeah it depends upon what that code looks like and the things that could be impacted...
For testing purposes, you could always implement INetworkUpdateSystem and register that with the NetworkUpdateLoop. Then have the related code to replicate receiving, assigning, and applying values get invoked during the EarlyUpdate on a regular basis, allow FixedUpdate to run, and then during the LateUpdate do a check on the pertinent areas (again not sure what all is involved) to see if you can get it to happen after running for a specific period of time...
You might be able to adjust the fixed timestep and/or add "artificial" strain using the same class you implemented INetworkUpdateSystem during the Update to force FixedUpdate to iterate more than once the next frame to see if you can replicate the issue.

Once you replicate the issue... you can then just add a "toggle" that you can set during runtime to have the same code that is being invoked during EarlyUpdate (i.e. simulating processing messages and applying values) to simply update some values and optionally apply the values during Update.
So like:

  • EarlyUpdate
    • Code to simulate receiving deltas or data from a message gets applied to
    • If (for lack of a better name) m_ForceEarlyUpdateIssue is true then you apply the values from the properties to <insert whatever physics related values/components here>
  • FixedUpdate (do nothing)
  • PreUpdate
    • (optional) Add some form of delay (a while loop that iterates 500-1000k times or the like) to simulate an impacted frame rate which the delta time between frames could be long enough to force FixedUpdate to iterate several times the next frame.
  • Update
    • If m_ForceEarlyUpdateIssue is false then the property/properties set during the Early update are applied to <insert whatever physics related values/components here>
  • LateUpdate
    • Check final result to see if the issue has occurred

Something along those lines might help you artificially replicate the issue and verify by applying the values to the <insert whatever physics related values/components here> during Update vs EarlyUpate the issue does not occur.

Again...sort of "winging it" since I don't have all of the details on that one place that could be hard to determine if it is fixed or not...but if I wanted to try and replicate an issue like this I would most likely start with implementing INetworkUpdateSystem in a class, registering it with NetworkUpdateLoop and then trying to artificially replicate the issue that way.

@zachstronaut
Copy link
Author

Thanks!

@NoelStephensUnity
Copy link
Collaborator

Let me know if this resolved your issue (when you can for certain come to that conclusion).

@NoelStephensUnity NoelStephensUnity added type:support Questions or other support and removed type:bug Bug Report stat:awaiting triage Status - Awaiting triage from the Netcode team. labels Nov 21, 2023
@NoelStephensUnity NoelStephensUnity self-assigned this Nov 21, 2023
@zachstronaut
Copy link
Author

@NoelStephensUnity yes, I believe this is resolved. I didn't actually end up needing to try using INetworkUpdateSystem because our interface to our rigidbodies was wrapped in such a way that I should have fixed the issue globally now in our code base.

@zachstronaut
Copy link
Author

So the biggest source of my problems with objects snapping back to old positions and driving me crazy is actually that the built-in networked re-parenting in NetworkObject is completely broken when you have any client authoritative positioning in your game: #2842

@NoelStephensUnity
Copy link
Collaborator

Side note: The distributed authority update includes some updates to Rigidbody stuff that allows for better NT <--> Rigidbody synchronization by allowing you to select whether you want the Rigidbody to control transform updates or not. This allows you to use Rigidbody interpolation on both the authoritative and non-authoritative instances and keeps better synchronization between the PhysX and Unity transforms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stat:awaiting response Status - Awaiting response from author. type:support Questions or other support
Projects
None yet
Development

No branches or pull requests

2 participants