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 for Issue 511: ShapeCastHit has no details on ShapeCastStatus::Converged #521

Merged
merged 4 commits into from
Jun 14, 2024

Conversation

kirilllysenko
Copy link
Contributor

This is a PR with suggested bug fix in issue #511. The fix seems also logical when looking at documentation for field details of ShapeCastHit class:

/// The result of a shape cast.
#[derive(Copy, Clone, Debug, PartialEq)]
pub struct ShapeCastHit {
    /// The time at which the objects touch.
    pub time_of_impact: Real,
    /// Detail about the impact points.
    ///
    /// `None` if `status` is `PenetratingOrWithinTargetDist` and
    /// [`ShapeCastOptions::compute_impact_geometry_on_penetration`] was `false`.
    pub details: Option<ShapeCastHitDetails>,
    /// The way the time-of-impact computation algorithm terminated.
    pub status: ShapeCastStatus,
}

…ng from ShapeCastHit in parry to ShapeCastHit in bevy
@Vrixyz Vrixyz added C-Bug Something isn't working D-Easy P-High arbitrary important item A-Geometry A-Integration very bevy specific labels Jun 13, 2024
@kirilllysenko
Copy link
Contributor Author

@Vrixyz Thank you for approving! I see tests failing and they are definitely not failing because of my changes. Does it require actions from my side?

@Vrixyz
Copy link
Contributor

Vrixyz commented Jun 14, 2024

@kirilllysenko CI fails are indeed not related, as you merged master in I think they'll pass now, thanks!

src/geometry/mod.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@Vrixyz Vrixyz left a comment

Choose a reason for hiding this comment

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

After consideration, the correct condition is a bit more involved, see https://github.com/dimforge/bevy_rapier/pull/521/files#r1639994370

It's especially useful to read through ShapeCastStatus doc to understand different cases.

@kirilllysenko
Copy link
Contributor Author

I did all of the requested changes

@Vrixyz Vrixyz merged commit b1ddde7 into dimforge:master Jun 14, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Geometry A-Integration very bevy specific C-Bug Something isn't working D-Easy P-High arbitrary important item
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants