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

[Bullet] Inaccurate behaviour of 3D KinematicBody move_and_slide_with_snap on vertical moving platform. #36334

Open
Tracked by #45022 ...
igorr0driguez opened this issue Feb 18, 2020 · 16 comments · Fixed by #49328

Comments

@igorr0driguez
Copy link

Godot version:
V3.2.stable.official

OS/device including version:
Pop!_OS 19.10

Issue description:
When a 3D KinematicBody is being moved by a move_and_slide_with_snap function and sits on top of a vertical moving platform the behaviour is inaccurate: when moving upwards the mesh enters the moving platform and when moving downwards the mesh floats slightly above it.

Steps to reproduce:
3D Space:
Create a KinematicBody for the player, add as a child a MeshRenderer (CubeMesh) and a CollisionShape (BoxShape); set all margins to 0.001. Write a move_and_slide_with_snap script and attach to it.

For the moving platform, create another kinematic body with a MeshRenderer (CubeMesh) and a CollisionShape (BoxShape); again, set all margins to 0.001. As a child add an AnimationPlayer, create a new animation and animate it's translation property up and down (Y axis) with the desired speed, duration and distances. Set the ProcessMode to Physics.

Also add a Camera and adjust it so you can see the scene.

Player script:

extends KinematicBody

export var move_speed = 750

export var gravity = 3000
export var jump_force = 1300
export var max_fall_speed = 3000

var y_velo = 0

var snap_normal = Vector3.DOWN

func movement(delta):
	var move_vec = Vector3()
	if Input.is_action_pressed("move_left"):
		move_vec.x -= 1
	if Input.is_action_pressed("move_right"):
		move_vec.x += 1
	move_vec *= move_speed
	move_vec.y = y_velo
	move_and_slide_with_snap(move_vec * delta, snap_normal, Vector3.UP)
	
	var grounded = is_on_floor()
	y_velo -= gravity * delta

	if grounded:
		if Input.is_action_pressed("jump"):
			set_snap_normal(Vector3(0,0,0))
			y_velo = jump_force
		if y_velo <= 0:
			set_snap_normal(Vector3.DOWN)

func set_snap_normal(new_snap_normal):
	snap_normal = new_snap_normal

func _physics_process(delta):
	movement(delta)

Minimal reproduction project:
moving_platform.zip

@igorr0driguez igorr0driguez changed the title Inaccurte behaviour of 3D KinematicBody move_and_slide_with_snap on vertical moving platform. Inaccurate behaviour of 3D KinematicBody move_and_slide_with_snap on vertical moving platform. Feb 19, 2020
@igorr0driguez
Copy link
Author

After some tests I noticed that the inaccuracy increases if the Animation speed is higher and decreases if the speed is slower.

Y Distance travelled 14 ( -4 to 10 ) in a 2 seconds animation.
Speed 0.1:
slow

Speed 2.0:
fast

In these gifs there was no gravity affecting the moved box.

@e344fde6bf
Copy link
Contributor

e344fde6bf commented Mar 20, 2021

The root cause is this bug #30481 (see also godotengine/godot-proposals#2332). Functions like move_and_slide will collide with the platforms start of frame position, then at the end of the physics tick the platform's position is updated moving it away from where the cube performed its collision detection. So when the platform is moving down it floats above it, and when it moves up it intersects with it.

@pouleyKetchoupp
Copy link
Contributor

This issue will be fixed by enabling sync to physics on the moving platform, once it's supported in 3D (for now it's a 2D only feature). It will be available in 3.4 through #49446.

@pouleyKetchoupp
Copy link
Contributor

I'm re-opening because issues are only closed when the PR that fixes them is merged, which is not the case yet.

@akien-mga akien-mga added this to the 4.0 milestone Jul 15, 2021
@TokageItLab
Copy link
Member

TokageItLab commented Jul 22, 2021

I don't think this problem has been solved.
I enabled sync_to_physics in the project #50732, but the feet will penetrate moving platform.

@akien-mga
Copy link
Member

I tested the project in #36334 (comment) and enabling Sync to Physics on the moving platform does seem to help (no jitter), but it doesn't fully solve the problem indeed (still penetrating the platform).

@akien-mga akien-mga reopened this Jul 22, 2021
@TokageItLab
Copy link
Member

TokageItLab commented Jul 22, 2021

When moving platform's kinematic_motion is disabled, it is working fine. But when the kinematic_motion is enabled, this problem occurs again.

@TokageItLab
Copy link
Member

I see, it is happening when the player is moved in _process and the lift is moved in _phyisics_process. If the player movement is done in _phyisics_process, the jitter is more likely to occur, but this may be reasonable considering the influence from other physics. If this is the cause, this issue may can be closed.

@lyuma
Copy link
Contributor

lyuma commented Jul 28, 2021

Moving the player in _physics_process is not the correct workaround: not only because it causes jitter and creates a bad experience, but because the player is kinematic, and kinematic objects are supposed to be moved outside of the physics system by design.

At least, that's the word I know from other engines... I don't think Godot is any different in this regard. See here for an example from how people do this in Unity: https://answers.unity.com/questions/866484/move-a-rigidbody-when-kinematic.html

When the rigidbody is marked as Kinematic, it means it's supposed to be moved outside of the physics system.

Move it like any other non-physics Transform, in the Update method using Transform.Translate or Transform.position.

(unity's Update method is like godot's _process)

@pouleyKetchoupp
Copy link
Contributor

@TokageItLab It does make sense, using kinematic motion in _process is handled in simple cases, but it's definitely not recommended to interact with moving platforms set with sync to physics. Thanks for finding the cause! I was struggling to figure it out. I'll see if I can at least add a warning when a character is moved in _process and interacts with a platform set with sync to physics, otherwise this kind of issue can be hard to track down.

@lyuma You can't assume Unity's documentation applies to Godot. You need to check Godot's documentation, which states kinematic character movements should be done in _physics_process:
https://docs.godotengine.org/en/stable/classes/class_kinematicbody.html#class-kinematicbody-method-move-and-slide
https://docs.godotengine.org/en/stable/tutorials/physics/using_kinematic_body_2d.html#movement-and-collision

The general rule is physics objects should be moved in _physics_process. The only exception I can think of is animated platforms, and sync to physics is used to help interactions with characters in this case.

If that causes other undesirable effects, they need to be addressed as separate issues. For jittering issues due to physics and render using different time steps, there's a new project setting you can try in 3.4 beta that can help in some cases:
#48390

About the current issue:
Switching to _physics_process solves the issue of sync to physics not working correctly in Godot Physics.

When using the MRP from #36334 (comment), it works well in Godot Physics, but there's still some lag in Bullet, so I'm going to switch this issue to Bullet specific.

@pouleyKetchoupp pouleyKetchoupp changed the title Inaccurate behaviour of 3D KinematicBody move_and_slide_with_snap on vertical moving platform. [Bullet] Inaccurate behaviour of 3D KinematicBody move_and_slide_with_snap on vertical moving platform. Jul 28, 2021
@TokageItLab
Copy link
Member

@pouleyKetchoupp There is no delta_smoothing implemented in 4.0 yet, but is there a possibility that it will be implemented in the future? In Windows, jittering doesn't occur very often, but in Mac, jittering occurs often.

@Calinou
Copy link
Member

Calinou commented Jul 28, 2021

@pouleyKetchoupp There is no delta_smoothing implemented in 4.0 yet, but is there a possibility that it will be implemented in the future? In Windows, jittering doesn't occur very often, but in Mac, jittering occurs often.

According to #48390:

  • Once this PR is settled we can do the same in master.

I would wait for 3.4 to be released first, so that we can get some feedback on delta smoothing and apply it immediately to the port in master. We can then port #48555 as well, and likely enable it by default in master if it proves successful in 3.x.

@fabriceci fabriceci modified the milestones: 4.0, 3.x Mar 26, 2022
@igorr0driguez
Copy link
Author

Thanks a lot (all of you). Giving some feedback for myself, can confirm that when the platform is animated setting the script above to _process(delta) solved the issue.

@igorr0driguez
Copy link
Author

Thanks a lot (all of you). Giving some feedback for myself, can confirm that when the platform is animated setting the script above to _process(delta) solved the issue.

Of course, if you're using manual physics movement for the platform it would be best if the player movement is also calculated within physics process.

@akien-mga
Copy link
Member

I tested this in 3.5 RC 1 and it actually looks fine for me with Sync to Physics enabled on the moving platform. Can anyone confirm, or do I miss how the bug manifests itself?

On the other hand, there seems to be a regression from #56801 where is_on_floor() changes value constantly when the platform moves up (which is also fixed when switching from _physics_process to _process as pointed out in #36334 (comment) - despite the AnimationPlayer being configured to process in Physics.

@igorr0driguez
Copy link
Author

Just tested in 3.5 RC 1 with Sync to Physics enabled on the moving platform and movement() running on _physics_process.

There's indeed an issue when the platform is moving upwards where jitter occurs and you can't actually move it (probably related to the constant changing of is_on_floor() ?)

And using _process instead of _physics_process really fixes it regardless of Sync to Physics being on or off on the moving platform.

Although as @pouleyKetchoupp mentioned, it should be running on _physics_process, so the bug still exists?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants