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

ShapeCast2D doesn't differentiate get_closest_collision_*_fraction safe and unsafe versions #8552

Closed
romlok opened this issue Dec 1, 2023 · 5 comments
Labels
area:class reference Issues and PRs about the class reference, which should be addressed on the Godot engine repository enhancement topic:physics

Comments

@romlok
Copy link
Contributor

romlok commented Dec 1, 2023

Your Godot version:
v4.2.stable.official [46dc27791]

Issue description:
ShapeCast2D.get_closest_collision_safe_fraction is described as:

... how far the shape can move without triggering a collision.

ShapeCast2D.get_closest_collision_unsafe_fraction is described as:

... how far the shape must move to trigger a collision.

(The text I elided is the same in both descriptions)

But as far as I can tell these are different ways of describing essentially the same value - the distance between the start position and the first collision. So I think both myself and the documentation are missing something? 😕

shapecast

URL to the documentation page:
https://docs.godotengine.org/en/latest/classes/class_shapecast2d.html#class-shapecast2d-method-get-closest-collision-safe-fraction

@romlok romlok added the bug label Dec 1, 2023
@AThousandShips
Copy link
Member

AThousandShips commented Dec 1, 2023

In a perfect world they would be the same but the physics here works in fixed steps, so it iterates and tests the shape at intervals, the safe value is the largest fraction that was safe, and unsafe the smallest value that was unsafe, as the point where the collision would occur can be somewhere between two steps

This also applies to the 3D case

@AThousandShips AThousandShips added enhancement topic:physics area:class reference Issues and PRs about the class reference, which should be addressed on the Godot engine repository and removed bug labels Dec 1, 2023
@romlok
Copy link
Contributor Author

romlok commented Dec 2, 2023

Ah, I see!

So what do you think about expanding the description to something like:


float get_closest_collision_unsafe_fraction ( ) const

The fraction from the ShapeCast2D's origin to its target_position (between 0 and 1) of how far the shape must move to trigger a collision.

In the real world this would return the same as get_closest_collision_safe_fraction, however shape casting is calculated in discrete steps, so the precise point of collision can occur between two calculated positions. This means that the unsafe fraction will usually be one calculated step beyond the corresponding safe fraction.


Do you think that's clear and accurate enough?

@AThousandShips
Copy link
Member

I'd limit it to just describing that it does steps and that it picks the fraction from the steps that fit, keep it consise and simple, not making any statements on what's likely or not

@romlok
Copy link
Contributor Author

romlok commented Dec 6, 2023

Thanks! I've open a PR without the speculative part: godotengine/godot#85839

@akien-mga
Copy link
Member

Fixed by godotengine/godot#85839.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:class reference Issues and PRs about the class reference, which should be addressed on the Godot engine repository enhancement topic:physics
Projects
None yet
Development

No branches or pull requests

3 participants