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

Fix regression 70154 caused by my prior CCD fix. #70160

Merged
merged 1 commit into from
Dec 17, 2022

Conversation

Geekotron
Copy link
Contributor

@Geekotron Geekotron commented Dec 16, 2022

My prior commit (foolishly) trusted the comment that said from - mnormal * mlen * 0.1 would be "a little inside the bounding box". It's not necessarily; at high velocities this calculation puts the point fully outside the body.

So my attempt to use that same point later to determine the length of motion we needed did not work.

I've partially reverted, it's back to only using the -10% motion for calculating the cast start point and NOT using it to determine length of velocity needed. So to fix collision fails, I realized at the end where it was previously subtracting 1% of body length it actually just needed to add it to the distance between from (support point) and to (collision point) instead.

I've confirmed this fix gets both my original issue repro working and the official physics tests: (https://github.com/fabriceci/Godot-Physics-Tests)

(with the exception of 2d cast shape which seems to have already been broken before my change)

fix #70154

At high velocities `from - motion *.1` is *behind the RB* - not within its collider as the comment suggested - so it could not be used for determining movement length
@Geekotron Geekotron requested a review from a team as a code owner December 16, 2022 18:47
@clayjohn clayjohn requested a review from rburing December 16, 2022 20:24
@clayjohn clayjohn added this to the 4.0 milestone Dec 16, 2022
Copy link
Member

@rburing rburing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense and passes the tests. Thanks! Do you plan to look into the 2D shape cast CCD? (Separately from this PR.)

@akien-mga akien-mga added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Dec 17, 2022
@akien-mga akien-mga merged commit c903312 into godotengine:master Dec 17, 2022
@akien-mga
Copy link
Member

Thanks!

@LakshayaG73
Copy link

@Geekotron can you elaborate a bit on this 2D shape cast CCD issue?

@Geekotron
Copy link
Contributor Author

@Geekotron can you elaborate a bit on this 2D shape cast CCD issue?

@LakshayaG73 - fabriceci has a test project which is how he located regression 70154. The test project is (https://github.com/fabriceci/Godot-Physics-Tests

The 2d test shows this as of Godot4beta7 (which was before my regression was introduced):
image

It appears that cast shape CCD does not currently work, at least at very high speeds. @fabriceci could maybe say how long this has been the case since he set up the test.

Makes sense and passes the tests. Thanks! Do you plan to look into the 2D shape cast CCD? (Separately from this PR.)

@rburing - Sorry, I've taken a quick look at the cast_shape code and it would take a while to dig through. Given I'm mainly a 3D guy (didn't even know we had a cast_shape option in 2D), I'd probably not be the best to fix it right now. At the moment I'm actually looking into trying to eliminate the shaking reported in issue #61827 and a few others.

@fabriceci
Copy link
Contributor

fabriceci commented Dec 18, 2022

@Geekotron The shape cast is bugged since a long time.

I added in the tests the rotation and collision cases, I came across another bug, @Geekotron could you take a look at it too?

It only happens at very high speed, but it happens in 2D as well, and in 3D, only when there are rotations:

Here is a video frame by frame:

failed.mp4

Here is in 3D:
Screenshot 2022-12-18 at 14 00 47

Tips 1: in the test project if you press P you enter in Frame by Frame/Pause mode, and then press O to go to the next frame
Tips 2: If you edit base/pause.gd and change the first variable "paused" to true, you can start a scene in Pause mode

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:3.x Considered for cherry-picking into a future 3.x release regression topic:physics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Continuous Collision Detection no longer works with high speed body
6 participants