Skip to content

Return MoveAndSlideHitReponse enum from on_hit callback#933

Merged
Jondolf merged 5 commits into
mainfrom
fix-mns-callbacks
Feb 1, 2026
Merged

Return MoveAndSlideHitReponse enum from on_hit callback#933
Jondolf merged 5 commits into
mainfrom
fix-mns-callbacks

Conversation

@Jondolf

@Jondolf Jondolf commented Jan 31, 2026

Copy link
Copy Markdown
Member

Objective

The callbacks for move_and_slide and intersections don't currently work like their docs describe. The former should abort when the callback returns false (currently it just ignores a single hit), and the latter doesn't actually do anything with the returned boolean.

For move_and_slide, the callback should ideally return an enum with three variants: accept the hit, ignore the hit but continue, and ignore the hit and abort. For intersections, the callback should still return a boolean, but it should actually abort when false is returned.

Solution

Add a MoveAndSlideHitResponse enum:

/// Indicates how to handle a hit detected during [`MoveAndSlide::move_and_slide`].
///
/// This is returned by the `on_hit` callback provided to [`MoveAndSlide::move_and_slide`].
#[derive(Debug, PartialEq)]
pub enum MoveAndSlideHitResponse {
    /// Accept the hit and continue the move and slide algorithm.
    Accept,

    /// Ignore the hit and continue the move and slide algorithm.
    ///
    /// Note that the shape will still be moved up to the point of collision,
    /// but the velocity will not be modified to slide along the surface.
    Ignore,

    /// Ignore the hit and abort the move and slide algorithm.
    ///
    /// Note that the shape will still be moved up to the point of collision,
    /// but no further movement or velocity modification will be performed.
    Abort,
}

This is handled as the docs describe.

Also make intersections abort further hit processing when the callback returns false.

@Jondolf Jondolf added this to the 0.6 milestone Jan 31, 2026
@Jondolf Jondolf added C-Bug Something isn't working A-Character-Controller Relates to character controllers D-Straightforward Simple bug fixes and API improvements, docs, test and examples C-Refinement Improves quality or results in some way, without fixing a clear bug or adding new functionality S-Needs-Review Needs reviewer attention to move forward labels Jan 31, 2026
@Jondolf Jondolf requested a review from janhohenheim January 31, 2026 21:43
planes.push(Dir::new_unchecked(sweep_hit.normal1.f32()));
} else if hit_response == MoveAndSlideHitResponse::Abort {
break;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

how come an ignore doesn't continue here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ignore skips this specific surface (i.e. doesn't push the plane like Accept does) but continues processing other hits. The rest of the hits here are processed below in intersections.

The semantics here are a bit confusing since Ignore doesn't ignore the actual shape cast hit. The shape is still moved up to the surface, but the return value of the callback just determines what to do with each plane found at that point, which affects the velocity projection. Having it ignore the shape cast hit would require a filter on the shape cast itself (tbf we should have this too), not on the collection of collision planes.

Tangentially related: I wonder if using Abort on the first hit is actually equivalent to move_and_collide in Godot, or slide: false in Rapier 🤔 I think it should be; it stops at the first hit and then aborts move-and-slide without any sliding

@Jondolf Jondolf merged commit 1d7e7c2 into main Feb 1, 2026
6 checks passed
@Jondolf Jondolf deleted the fix-mns-callbacks branch February 1, 2026 12:04
@Jondolf Jondolf added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention to move forward labels Feb 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Character-Controller Relates to character controllers C-Bug Something isn't working C-Refinement Improves quality or results in some way, without fixing a clear bug or adding new functionality D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants