Unity-Technologies / com.unity.netcode.gameobjects

Netcode for GameObjects is a high-level netcode SDK that provides networking capabilities to GameObject/MonoBehaviour workflows within Unity and sits on top of underlying transport layer.
MIT License
2.11k stars 429 forks source link

Jitters when reparenting a rigidbody #2512

Closed Maclay74 closed 1 year ago

Maclay74 commented 1 year ago

Description

Following this part of the documentation I'm trying to implement the carrying behavior. It works, but with noticeable jitter when character drops the object.

This is my pick up code (it actually matches the documentation example):

GetComponent<NetworkObject>().TrySetParent(newParent);
GetComponent<Rigidbody>().isKinematic = true;
GetComponent<NetworkTransform>().InLocalSpace = true;
transform.localPosition = newParent.InverseTransformPoint(handlePosition);

And drop:

transform.transform.parent = null;
GetComponent<Rigidbody>().isKinematic = false;
GetComponent<NetworkTransform>().InLocalSpace = false;

Reproduce Steps

  1. Setup a scene with two network objects, one is rigidbody
  2. Parent rigidbody to another object
  3. Move them by moving the parent one
  4. Set rigidbody's parent to zero

Actual Outcome

Object teleports for a moment to 0,0,0 (apparently, not sure) and then teleports back to its correct position. Also, it also participates in physics simulation, so it can move other objects located in 0,0,0 (you can see on the video that it pushes another character)

Expected Outcome

Object should persist it's current position without teleports or jitters

Screenshots

Host is on the left. Jitters appear only on the client.

https://user-images.githubusercontent.com/5504685/232324945-be4b15d3-17f1-4824-be27-a76def1f850e.mp4

Environment

NoelStephensUnity commented 1 year ago

@Maclay74 Would it be possible to share this project and/or provide access to a repository? It would help me to better troubleshoot and expedite a solution to the issue you are experiencing.

Maclay74 commented 1 year ago

@Maclay74 Would it be possible to share this project and/or provide access to a repository? It would help me to better troubleshoot and expedite a solution to the issue you are experiencing.

Hey, sure, is there a way I could share it only with the team? Can't publish it and we don't have a repository yet.

NoelStephensUnity commented 1 year ago

Ahh... yeah...you could do that via the Unity editor by submitting a bug and sending me the email response you get with the bug submission ticket number (I can then get QA to forward that to our bug DB and access the project that way).

Another thing I noticed is that you might be referencing older/legacy code (or the documentation is). The code snippet you are pulling from comes from this bit size sample (or an earlier version of it): https://github.com/Unity-Technologies/com.unity.multiplayer.samples.bitesize/tree/main/Basic/ClientDriven Which looks to have been updated and is a bit different than the one you referenced...so that might also be related to the issue you are experiencing.

Maclay74 commented 1 year ago

Alright, I don't want to bother more people with that, to be honest.

I see that in the updated example there is additional line

pickUpObjectRigidbody.interpolation = RigidbodyInterpolation.None;

that might be the key.

I'll give it a test and have a closer look at the code. Thanks for that!

Maclay74 commented 1 year ago

I gave it a try, unfortunately, it didn't help. Rigidbody-jitter-parenting.zip

So, there is small repro I have, the code I shared is from DropTarget.cs and PickUpTarget.cs files.

There is a issue with client transform as well, but please just ignore it.

Controls - WASD, E is for the action.

Thanks!

NoelStephensUnity commented 1 year ago

Will look over this and report back to you any findings and/or an update version that resolves the issue.

NoelStephensUnity commented 1 year ago

@Maclay74 I think the biggest issue was the CharacterController was still enabled on the non-owner instances. With the below updates, it seemed to work fine with some relatively basic settings:

I just modified the ClientNetworkTransform to disable the CharacterController for non-owner instances:

    public class ClientNetworkTransform: NetworkTransform {
        protected override bool OnIsServerAuthoritative() => false;

        public override void OnNetworkSpawn()
        {
            if (!IsOwner)
            {
                GetComponent<CharacterController>().enabled = false;
            }
            base.OnNetworkSpawn();
        }
    }

I then culled down the complexity of the parenting action to just setting the server side to kinematic, setting the position to the same world space position as the pickup socket, left the local space settings alone for the NetworkTransform, and used the NetworkObject.TrySetParent:

    public class PickUpTarget : ActionTarget {

        public override ActionTagType TagType => ActionTagType.PickUp;

        public void SetParent(Transform newParent, Vector3 position, Quaternion rotation) {
            var pickUpObjectRigidbody = GetComponent<Rigidbody>();
            pickUpObjectRigidbody.isKinematic = true;
            pickUpObjectRigidbody.transform.position = position;
            GetComponent<NetworkObject>().TrySetParent(newParent);

            if (TryGetComponent<DropTarget>(out var dropTarget)) {
                dropTarget.IsActive = true;
            }

            IsActive = false;
        }
    }

For the drop action, I just used the NetworkObject.TryRemoveParent, and set the server-side to not kinematic.

    public class DropTarget : ActionTarget {

        public override ActionTagType TagType => ActionTagType.Drop;

        public void SetParent(Transform newParent) {

            GetComponent<NetworkObject>().TryRemoveParent();
            var pickUpObjectRigidbody = GetComponent<Rigidbody>();
            pickUpObjectRigidbody.isKinematic = false;

            if (TryGetComponent<PickUpTarget>(out var dropTarget)) {
                dropTarget.IsActive = true;
            }

            IsActive = false;
        }
    }

The one issue you might run into is the mixing of owner authoritative NetworkTransform (the player) and server authoritative NetworkTransform (the cubes) as they will be relatively out of sync and can cause a delay in the update (especially on a client where the server has to be updated of the client player's position which, in turn, will update the cube's position that the server then has to update the player's cube instance). You might think about changing the cube to use a ClientNetworkTransform (without the above modifications) and change the ownership of the cube when it is picked up, but there is a bit of a trick to this as you have to make a custom (derived) NetworkRigidBody to handle the changing of the owner's kinematic setting after it has defaulted it to non-kinematic when ownership is changed.

Oh, and I disabled the SceneManager and just assigned the NetworkManager's player prefab property with the Player prefab (still haven't verified if this made a difference or not...sort of got carried away a bit trying to narrow down the issue...so if I find some additional time I will re-apply the above changes without make any other modifications to double verify that just disabling the non-owner's CharacterController was all that was really needed).

NoelStephensUnity commented 1 year ago

Here is a video of the applied updates: https://user-images.githubusercontent.com/73188597/232952087-23c7ef13-719e-4f5f-abfa-b93535c1bec4.mp4

Here is the Code folder with the applied updates: CodeUpdates.zip

Let me know if this resolves your immediate issue, and if you need any assistance with making the cube work with an owner authoritative NetworkTransform and customized NetworkRigidBody.

๐Ÿ‘

Maclay74 commented 1 year ago

I... can't find suitable words to express how I appreciate such a detailed research! Thanks a lot for your time and dedication!

I tried your changes, and indeed the cubes don't move to the origin on drop, although there is a jitters when character rotates with a cube "in hands". It feels like the cube goes in the opposite direction at the beginning and swings back then.

It's very hard to capture because it happens very fast, I'm not sure you have the same problem on the video you posted, but here is mine

https://user-images.githubusercontent.com/5504685/233150535-bac3f5d0-024d-4e98-b2cf-a85dbf8c92e1.mp4

About the issue you mentioned with mixing transforms - I think I could use client authoritative cubes with changing ownership, but I don't understand one thing.

I have a problem with the host (server authority) carrying a cube (server authority) and then results of it are streamed to a client. So client's player (client authority) isn't the part of the "equation", is it?

With that in mind I changed ClientNetworkTransform so it's not actually client authoritative anymore:

protected override bool OnIsServerAuthoritative() => true;

and the jitters persist.

I probably miss something still, it's little hard to wrap my head around these dependencies.

NoelStephensUnity commented 1 year ago

Ok. Let me fiddle with it a bit more to see if I can provide some additional approaches. However, if you make ClientNetworkTransform server authoritative then that will change which instance should have the CharacterController enabled. In the video is that the host or client you are moving around?

Maclay74 commented 1 year ago

I always move the host, never the client (in fact, client can't carry cubes for some reason, but it's a problem of tomorrow's me)

Anyway, you disabled CharacterController for non-owners, so in case of the host it shouldn't change anything, right?

NoelStephensUnity commented 1 year ago

Ok, I had to make a number of changes that might seem a bit confusing but for picking up objects with an owner authoritative player NetworkTransform and a server authoritative object NetworkTransform this approach will yield the best results as you can see below:

https://user-images.githubusercontent.com/73188597/233686900-f29d6a72-692a-4303-a89b-0920d1ed1a9b.mp4

Server Authoritative NetworkTransform Objects

image To avoid the issues with synchronization for objects you are picking up and moving around, the best approach is to divide your object (cube) up into two parts:

One neat trick that isn't discussed a bunch is that once a NetworkObject is spawned it keeps references to all of its associated NetworkBehaviour components...even on nested children...which there is no rule saying you can parent a GameObject with NetworkBehaviour components under another GameObject that does or does not have NetworkObject component! ๐Ÿ˜ธ So, as long as you mirror the parenting in code that is executed on all instances (i.e. OnNetworkObjectParentChanged is perfect for this!) and the to-be-parented GameObject (currently parented under a NetworkObject parent GameObject) has been spawned...you can mix and match as much as you need...just always be cautious as this is where "gotchas" can crop up if you mix things up a bit too much and start having order of operations related issues... but for something like this...pretty simple to track and only occurs when parented and only occurs on one thing. ๐Ÿ‘

The DropTarget class now looks like this:

    public class DropTarget : ActionTarget {

        public override ActionTagType TagType => ActionTagType.Drop;

        public GameObject CubeBodyAndSync;
        public void SetParent(Transform newParent) {

            if (TryGetComponent<PickUpTarget>(out var dropTarget)) {
                dropTarget.IsActive = true;
            }

            if (IsServer)
            {
                transform.position = CubeBodyAndSync.transform.parent.position;

                var tossCube = transform.parent.forward * 2.0f;
                var tossAngular = transform.parent.right * 3.0f;
                GetComponent<NetworkObject>().TryRemoveParent();
                var pickUpObjectRigidbody = GetComponentInChildren<Rigidbody>();
                pickUpObjectRigidbody.isKinematic = false;
                pickUpObjectRigidbody.AddForce(tossCube, ForceMode.Impulse);
                pickUpObjectRigidbody.angularVelocity = tossAngular;
            }
            IsActive = false;
        }

        public override void OnNetworkObjectParentChanged(NetworkObject parentNetworkObject)
        {
            if (parentNetworkObject == null)
            {
                gameObject.layer = LayerMask.NameToLayer("Default");
                CubeBodyAndSync.transform.SetParent(transform);
            }
            base.OnNetworkObjectParentChanged(parentNetworkObject);
        }
    }

Where once again, we leverage from the OnNetworkObjectParentChanged virtual method to set the Cube's original parent back to the CubeObject when the CubeObject no longer has a parent.

Action System

One thing you should consider is possibly simplifying your action system (i.e. remove it/set it to the side at this stage) while you are working on the core mechanics as there are some problematic areas in there that could give you false positives that could slow down your core mechanics progress.
I might suggest: Just make a "PlayerController" class that maps directly to some of the fundamental key actions, is a NetworkBehaviour, and is disabled on non-owner instances.

Detecting What To Pickup

To detect whether a player can pick something up or not, I would exclude using collision boxes as triggers to populate objects as there are other elements to the changes made which could impact that approach (i.e. having the CharacterController exclude anything in the "Ignore" layer and include anything in the "Default" layer to help with the transition when picking things up) I might suggest a simple ServerRpc that clients would invoke and the server-host could do something like a Physics.BoxCastAll to find things directly in front of the player.

        [ServerRpc]
        private void TryPickSomethingUpServerRpc(ServerRpcParams serverRpcParams)
        {
            var requestingPlayer = NetworkManager.ConnectedClients[serverRpcParams.Receive.SenderClientId].PlayerObject;
            var charController = requestingPlayer.GetComponent<CharacterController>();
            var castResults = Physics.BoxCastAll(requestingPlayer.transform.position, charController.bounds.extents, requestingPlayer.transform.forward, requestingPlayer.transform.rotation, 5.0f);
            var potentialTargets = new System.Collections.Generic.List<ActionTarget>();
            foreach(var result in castResults)
            {
                var actionTargets = result.collider.transform.parent.GetComponents<ActionTarget>();
                foreach(var actionTarget in actionTargets)
                {
                    potentialTargets.Add(actionTarget);
                }
                if (potentialTargets.Count > 0)
                {
                    // Found a thing to pickup
                    break;
                }    
            }

            // Process potentialTargets
        }

And you could have an "inventory" for the player where once it is picked up it could simply just add that to a list of Objects the player is holding (or just one while prototyping). Not trying to downplay your approach or anything like that, they were just some observations made while looking for the right solution to meet your project's needs.

Either case, here is the project with the above modifications... I changed some of the packages to better suite my current setup here so sorry about that... you might have to extract what you want to keep into your version (once you get a repo setup it will make this kind of update a bit simpler). UpdatedProject.zip

This should get you past the issue you were having with picking up an object and the associated jitter you were experiencing (with a small bonus of the player sort of "tossing" the cube forward when it is dropped ๐Ÿ˜ธ )

Cheers,

-Noel

NoelStephensUnity commented 1 year ago

Oh yeah... on the non-authoritative instances it is possible to pick something up when the non-authority side is still a bit further away from the target object. There are several ways to handle this synchronization... one of the simpler approaches would be to add one more "intermediate" GameObject between CubeObject and Cube that you could adjust when parented based on the distance from the socket and the Cube itself. The general idea here is that you can always adjust normal GameObject's transforms that are not tied to a NetworkTransform to work around some of these latency related issues. Sometimes a little "smoke and mirrors" are more effective than trying to gain the precise synchronization...

Anyway, let me know if this resolves your issues and meets your project's needs?

Maclay74 commented 1 year ago

Wow... This is just insane, such level of support must be illegal! You should write a book, it would be a bestseller! I wish there was one with all these tricks to learn!..

You even added music to the video!

I need some time to process all this information, but what I see is that it definitely solves my problem! Also, special thanks for your suggestions beyond the problem, I'll definitely try to apply them!

NoelStephensUnity commented 1 year ago

@Maclay74 Glad I could help and we are making efforts to improve documentation, samples, and things of this nature as NGO evolves over time. For now, I am going to consider your issue resolved and close this out. Remember: Just because it is closed out doesn't mean you can't continue to ask questions as you move forward in this same thread or if you feel it is beyond the scope of this issue always feel free to submit a new issue.

Cheers and Happy Netcoding! ๐Ÿ˜ธ

p.s. We always enjoy seeing projects progress... so feel free to post any advances in your project (you feel comfortable sharing) through video or screenshots. It is always fulfilling to see projects evolve. ๐Ÿ‘

Maclay74 commented 1 year ago

Running your example I found an interesting detail.

According to your screenshot (and the explanation, if I got it correct) we should have both GameObjects attached to the player - "core" one is to socket, and the "wrapper" just to the player.

But actually, it doesn't work this way on client, since it doesn't have m_TargetSocket variable set. Eventually,

CubeBodyAndSync.transform.SetParent(m_TargetSocket);

just places Cube in the root of the tree, like this:

image But.. it still works! Because server still updates position of the cube as well as players.

But it isn't the most funny about that! If I fix this and set it on client the same way it's on server, it will jitter again!

CubeBodyAndSync.transform.SetParent(parentNetworkObject.transform.Find("Model").Find("PickUpSocket").transform);

Also, I changed the approach for adding potential actions, as you suggested, it's raycast based now. (And it fixed client, backend had CharacterController disabled and didn't participate in collisions)

But now when I pick up cube on client (with the cube in the root or in socket) it still interpolates like it's a separated object .

https://user-images.githubusercontent.com/5504685/233500425-651e3ff1-8a17-4dcc-bbc9-f304b86cf0c1.mp4

Sorry if I missed something from your explanation :c

I added raycast to your code and put "core" cube in "Ignore" layer too.

UpdateProject_with_client.zip

NoelStephensUnity commented 1 year ago

Will take a gander at it and see where my disconnected thoughts were at the time. ๐Ÿ˜น

NoelStephensUnity commented 1 year ago

Ok, I think I have a solution but it might require a slight minor update to NetworkTransform. Same concept, just need to handle resetting the interpolators when a NetworkTransform is enabled when already spawned. Both sides still place the Cube under the socket... there is a timing issue with switching between Kinematic and non-Kinematic when dropping (it can collide with the player and cause slight bouncing off of the player), but that type of issue is easily handled. Will post the solution and have a PR up for the minor change (since you are pointing to the develop branch anyway).

Maclay74 commented 1 year ago

Gotcha! Look forward to checking it out!

NoelStephensUnity commented 1 year ago

@Maclay74 Sorry for the delay... I decided to take a different approach that I think worked out better in the end (I forgot you are using the develop branch with the v1.4.0 version of NetworkTransform in it). Basically no more funky migrating of a child or anything like that... Just a single layer Cube with a customized NetworkTransform and NetworkRigidBody. The Cube's NetworkObject has AutoParentSync disabled, which you can then parent a NetworkObject under anything you want but have to handle synchronizing parenting yourself (see custom NetworkRigidBody). The custom NetworkTransform is owner authoritative and I left some notes in there for you as to what it does. The custom NetworkRigidbody handles parenting and uses a NetworkVariable to handle this.

The end result: https://user-images.githubusercontent.com/73188597/233752044-0ef2dd05-ab69-45f1-82d7-a5c3221cfc7d.mp4

I just zipped up the Assets folder since I sort of made some more editor related updates while I was working on it. WorkingVersion.zip

This should give you a good start and is actually a better approach than the first one. Don't enable half float precision until you see release/v1.4.0 get merged into the develop branch as there was a fix in that branch for properly resetting the position interpolator when a non-authoritative side gets a teleport state (otherwise you will see weird anomalies). Once you see that merged and/or v1.4.0 is public (in the package manager) then you can enabled half float precision on the cubes.

Also, I am going to pass along this approach to the MTT samples team so they can come up with some better name for the "CustomNetworkRigidBody" component... it will just be that and the ClientNetworkTransformCube that I think other users might find useful to handle a client authoritative way to pick up "things" that have rigid bodies (using v1.4.0 that is).

Cheers,

-Noel

Maclay74 commented 1 year ago

@NoelStephensUnity thanks again!

I managed to implement your changes to my initial project and everything now is butter smooth! The only thing left for me is to understand the solution, thanks to so many comments in the code it shouldn't take long.

I'm sure that the example would be appreciated by the community, since the one that I've followed doesn't provide accurate result.

So, the issue is 110% resolved! I am so inspired now!

NoelStephensUnity commented 1 year ago

Excellent! ๐Ÿš€ Feel free to reach out to us any time, and if later in your development cycle you feel like you have achieved (or are getting close to achieving) your project's goals we always love to see the progress made!

Happy Netcoding! ๐Ÿ˜ธ