Clarify and clean-up how kinematic bodies uses physics state

e344fde6bf commented 3 years ago

Describe the project you are working on

A game that uses KinematicBody nodes

Describe the problem or limitation you are having in your project

During each physics frame the state of the physics engine transitions from state A -> B. When working with KinematicBody the game world should go through several transition states for each KinematicBody that calls its _physics_process() handler: A -> A1 -> A2 -> ... -> B. However, the physics engine moves them all independently based off the snapshot it took of the world at the start of the frame (A).

Here's an example project that demonstrates the kinds of issues this causes: test-collision.zip.

In this project two cubes use move_and_collide() on the same frame to move towards a location where they should detect a collision, however since they don't update the world state until the end of the frame, they end up overlapping without seeing any collision.

physics_collision

Even more confusing, the blue cube which gets processed second evaluates RedCube.global_transform.origin as the updated version, however it still does not collide because the physics server still uses the old one. This extends to objects velocities as well, if a KinematicBody moves during a physics frame functions like move_and_slide() will always get the velocity objects had at the start of the frame, even if they've moved already, which should have updated their velocity.

Old explanation, slightly wrong/confusing:

During each physics frame the state of the physics engine transitions from state A -> B. However, when working with KinematicBody the game world goes through several transition states for each KinematicBody that calls its _physics_process() handler: A -> A1 -> A2 -> ... -> B.

As far as I can tell, the API mostly assumes that these transition states do not exist. For example, KinematicBody objects keep track of their velocity which gets updated in between physics frames (state A and B). However, if I move a KinematicBody X it will be in a transition state An updating it's position, velocity, etc. Now if a second KinematicBody Y calls move_and_slide() to collide with X and queries the velocity of X, it will get the value X had in state A not state An yet the collision detection happens using the positions X and Y have in state An. To further complicate things, if Y calls move_and_slide() in the next frame it will end up using physics state from at least 4 different states: A, An, B, Bn.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Instead of kinematic bodies moving independently, they should move based on the order they are processed, updating their positions and velocities accordingly.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Under this proposal the above example would behave like this:

physics_collision_resolved

Note the processing order effects the outcome, so in some cases it will be necessary to control it with Node.process_priority. While this may seem confusing, consider that the nodes which execute first have no way to predict what any of the nodes that follow will do. Similarly, the last nodes have no way to decide what to do until all the preceding nodes have decided.

Under this proposal, it should also be possible to implement moving platforms without the Sync to Physics feature by setting the Node.process_priority to achieve a similar result.

If this enhancement will not be used often, can it be worked around with a few lines of script?

This can't be worked around.

Is there a reason why this should be core and not an add-on in the asset library?

This is an issue with KinematicBody API in the engine.

pouleyKetchoupp commented 3 years ago

Yeah, it does seem inconsistent. After calling any move function, get_linear_velocity still returns the value from the previous frame, while get_position is the updated one.

This could be improved by making get_linear_velocity and get_angular_velocity return updated values for KinematicBody. I'm not sure if there's a need for accessing the previous position and velocity, but in case there was an identified use case for this, new functions could be added.

Functions like move_and_slide and move_and_collide already provide detailed collision information (using get_slide_collision in the first case) so I don't think they are missing much on this aspect.

e344fde6bf commented 3 years ago

Yeah that seems like the most straightforward approach. Here's some questions about the implementation:

If a KinematicBody moves multiple times in a frame (eg A -> A3 -> A6), is its velocity calculate from start of frame (eg A -> A6)? Or is it calculated from the last time it was moved (eg A3 -> A6)? It probably makes the most sense if it's calculate from the start of frame.
Wouldn't there be a discrepancy between how rigid and kinematic bodies report there position/velocity. Since rigid bodies only move at the end of the frame and kinematic bodies move intra-frame.
There should probably be some way to reset the velocity estimate of a kinematic body. If I teleport the KinematicBody, I don't want it to see a crazy spike in its velocity.

These are mostly edge cases, but the documentation should explain how it handles them.

get_linear_velocity and get_angular_velocity return updated values for KinematicBody ... Functions like move_and_slide and move_and_collide already provide detailed collision information

Unless I'm missing something obvious these methods don't exist for KinematicBodies and KinematicCollision.collider_velocity = linear_velocity + angular_velocity.cross(distance_from_origin). How should I get the linear/angular velocity of a KinematicBody that I have collided with?

I'm not sure if there's a need for accessing the previous position and velocity, but in case there was an identified use case for this

I guess what I'm more interested in is knowing their acceleration. For example, if the player is on a platform that decelerates quickly I might want to launch them off the platform. While I could calculate this from the point of view of the player, there'd be a 1 frame window when I first land on the platform where I don't know it's acceleration. Alternatively I could calculate it in all the platforms, but looking at this code it seems like kinematic bodies already have enough information to calculate their acceleration, so it seems a waste to duplicate these calculations when then engine already has the information.

pouleyKetchoupp commented 3 years ago

If a KinematicBody moves multiple times in a frame (eg A -> A3 -> A6), is its velocity calculate from start of frame (eg A -> A6)? Or is it calculated from the last time it was moved (eg A3 -> A6)? It probably makes the most sense if it's calculate from the start of frame.

Yeah I agree velocity should be from A to A6 in this case. From what I see in the internal code of the physics server, it seems that's what we could expose to scripts: https://github.com/godotengine/godot/blob/c5a88e6e91ca9c500dc920a4c0e3e27ae1f90cd6/servers/physics_3d/body_3d_sw.cpp#L495-L505

Wouldn't there be a discrepancy between how rigid and kinematic bodies report there position/velocity. Since rigid bodies only move at the end of the frame and kinematic bodies move intra-frame.

It's a good point, but it's not easy to make everything consistent between RigidBody and KinematicBody. The easiest way I see would be to properly document this difference, but I'm open to suggestions. It looks like it would be hard to work around the fact that KinematicBody's global transform is set on the last set position, while it's only updated after the physics step when it comes to RigidBody.

Unless I'm missing something obvious these methods don't exist for KinematicBodies

Ah yes you're right, I thought get_linear_velocity and get_angular_velocity were already exposed. On master, these methods are actually inherited from PhysicsBody3D, but it's only exposed to script for RigidBody3D. That could be fixed easily though.

I guess what I'm more interested in is knowing their acceleration.

There's still the possibility to add previous velocities to the API for KinematicBody, but I don't know if it's a very common need. In general, it seems easy enough to track the previous velocity and calculate acceleration in scripts. It's a bit of extra calculations but probably not critical enough to make it part of the engine core.

e344fde6bf commented 3 years ago

There's still the possibility to add previous velocities to the API for KinematicBody, but I don't know if it's a very common need. In general, it seems easy enough to track the previous velocity and calculate acceleration in scripts. It's a bit of extra calculations but probably not critical enough to make it part of the engine core.

Yeah I agree we don't really need previous velocities. What I'd be more interested in is a get_linear_accelertion() method. This way I could query the acceleration of both Kinematic and Rigid bodies with the same call. However, it'll still a little bit messy since Rigid/Kinematitc bodies get their position/velocity/acceleration updated at different times in the physics frame

pouleyKetchoupp commented 3 years ago

The use cases described in https://github.com/godotengine/godot-proposals/issues/2315#issuecomment-784114418 make a lot of sense: A. KinematicBody using move_and_slide on top of a moving KinematicBody using move_and_slide as well B. Case A on a moving platform that uses Sync to Physics

It would be interesting to check how these cases go in 2D, and in 3D once Sync to Physics is supported. They might require a rework of the way floor velocity is used in move_and_slide (it currently uses previous frame velocity when the floor is another KinematicBody using move_and_slide).

e344fde6bf commented 3 years ago

Physics and Reparenting KinematicBodies

Another approach to tackle the moving platform problem is to change the KinematicBody's parent to the platform it is riding on. This is very similar to the node-ordering method since it shuffles the node tree such that platforms always execute before the bodies riding on them. I made a demo that illustrates this approach.

This demo exposes another big assumption which the documentation does not mention, all physics calculations are performed using the global reference frame and many will be wrong for objects in non-stationary reference frames. Looking through the issue tracker, this seems intentional and there's no plans to change this: https://github.com/godotengine/godot/issues/22904, https://github.com/godotengine/godot/issues/26003. If that's the case, that leaves the node-ordering approach as the only way to write a non-trivial platformer without major changes to the current KinematicBody API (I could work around most of these issues in my demo projects, but there's still some I can't fix without writing my own move_and_slide function).

While the reparenting approach seems more intuitive at first, there's some issues that I think make it harder to use in practice:

You normally don't want all the rotations of the parent body applied to the child (eg don't want the player camera getting rotated side ways).
Your code needs to work no matter where the KinematicBody is parented. (eg if the parent gets delete, have to make sure the child doesn't get destroyed too)
Less likely to encounter weird bugs when you modify the scene tree. (For example, while writing the demo I was getting 50ms stutters whenever I reparented a KinematicBody. I took ages to figure out that this was because I used a CSGCapsule for the players model which was getting regenerated each time it was removed from the tree.)

So I don't think it's necessary to support it (wouldn't hurt if it was), but it'd be good to mention in the documentation that if a KinematicBody's position is dependant on another KinematicBody, it should be placed lower in the scene tree and you can't use node parenting to solve this.

Potential Solution to Reordering the Scene Tree

It feels hacky using the scene tree to control execution order, so maybe it would be better if we could assign processing priorities to nodes. This seems like an extremely useful feature and would even have many uses outside the physics engine. This would also remove the need for the Sync to physics feature, since for a moving platform you could just set its priority higher. (Edit: It turns out you can do this already using Node.process_priority)

In hindsight it feels like an obvious feature, so maybe there's some reason it doesn't already exist. If not, it's probably worth opening a separate issue to discuss it (I couldn't find one if it already exists).

e344fde6bf commented 3 years ago

Another potential solution is to handle process priority manually with scripts:

func _physics_process(delta):

    # ...

    # all the nodes that depend on the current one get processed here
    for node in dependants:
        node._physics_process(delta)

This seems way uglier than if nodes had explicit execution priorities.

e344fde6bf commented 3 years ago

In hindsight it feels like an obvious feature

It turns out it already exists, I just didn't look properly. You can use Node.process_priority to control the priority that nodes get executed in.

So with these changes it should be possibility to make moving platforms without needing Sync to Physics, since you can just use an expression likeprocess_priority = -100 as a replacement. Maybe you still need it for interacting with rigid bodies though?

e344fde6bf commented 3 years ago

I updated the proposal with what was discussed so far. I also added a concrete example and cleared up the explanations to clearly demonstrate the issue.

pouleyKetchoupp commented 3 years ago

Thanks for the update! Could you actually open an issue in the main repo that links to this proposal with the test project you've added? This example clearly shows a bug with collision detection.

e344fde6bf commented 3 years ago

I also managed to reproduce this kind of issue between rigid and kinematic bodies: https://github.com/godotengine/godot/issues/46653.

Although kinematic-kinematic collisions need the most up to date positions, I think to then resolve kinematic-rigid body collisions the kinematic bodies still need to be treated as moving from the start of frame A to the end of frame B. So they have a start position A and an effective velocity of (B-A)/h which is used to try and find the exact collision point between the kinematic and rigid bodies. It seems like the current system is already designed to work this way, so I'm not sure why the bug is happening for kinematic-rigid bodies.

djrain commented 2 years ago

Any status update on this issue? I can confirm that it applies to 2D also. In 3.4 I'm having the problem of two KinematicBody2D's penetrating when they're both moving toward each other, causing my "immovable" object to be pushed during the resolution. Is there any workaround?

pouleyKetchoupp commented 2 years ago

@djrain There's no update at the moment, although in 4.0 you can make one of them ignore the other one completely using collision layers since they are not checked both ways anymore, which can help with the second part of your issue (immovable object being pushed).

Something you can try in 3.4 is to make sure the immovable one is updated last (using node order in the tree or node process priority). It might alleviate the issue, but I'm not sure it would solve it entirely (and the kinematic bodies would still overlap for one frame).

Feel free to open an issue for your case if it's easy enough to make a minimal project, since there's none for 2D at the moment.

MJacred commented 2 years ago

Yeah, it does seem inconsistent. After calling any move function, get_linear_velocity still returns the value from the previous frame, while get_position is the updated one.

If I'm not mistaken, these values will be accurate with these PRs for Godot 3.5 and 4. At least it sounds related

jitspoe commented 2 years ago

I entered a bug about this issue a while back and have a kind of crappy workaround here: https://github.com/godotengine/godot/issues/30481#issuecomment-545741077

godotengine / godot-proposals