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

Implement cast_ray_predicate to allow filtering the colliders with a function #297

Merged
merged 8 commits into from
Jan 30, 2024

Conversation

Affinator
Copy link
Contributor

@Affinator Affinator commented Jan 12, 2024

As I was writing a xpbd backend for bevy_mod_picking, I found that a ray cast with a predicate was missing to implement the functionality efficiently.

I was inspired by the predicate from bevy_rapier: https://github.com/dimforge/bevy_rapier/blob/c6bcce4695d596a7a9c8e91748d4dbb3d31f6d13/src/pipeline/query_filter.rs#L12

Objective

  • Implement a ray cast function with the support for a predicate function to filter the colliders

Solution

  • Added support for a predicate starting at SpatialQuery and down the pipeline
  • I added a small (and fun to play with) example to illustrate one possible use case
  • I chose not to change existing functionality. Therefore there is some code duplication in QueryPipelineAsCompositeShapeWithPredicate. This could be unified by introducing an Option in QueryPipelineAsCompositeShape, but that would create breaking changes at many places.

@Jondolf Jondolf added C-Enhancement New feature or request A-Spatial-Query Relates to spatial queries, such as ray casting, shape casting, and intersection tests labels Jan 20, 2024
@Affinator
Copy link
Contributor Author

My branch should now pass all workflows (and I updated it to the current state of main).

@Affinator
Copy link
Contributor Author

Should i squash the commits or is this fine the way it is?
(Or are there other concerns?)

@Jondolf
Copy link
Owner

Jondolf commented Jan 26, 2024

Sorry for responding so late.

I was originally considering a predicate in SpatialQueryFilter like what Rapier does, but I think that would have annoying lifetime issues when used with the RayCaster/ShapeCaster components, so your approach is definitely fine too.

Note that predicates should already work when using ray_hits_callback, but having one for cast_ray is definitely useful and more efficient for the "first hit only" case. I'd like to have cast_shape_predicate and project_point_predicate too if you have time, but leaving them to a follow-up is fine as well.

The example is really fun, but it's a bit confusing in that it only colors one box even though the laser/indicator goes through multiple:

example

I think it should either color all of the non-glass boxes it hits (using e.g. ray_hits_callback to get many hits) or stop the laser at the first hit (draw it from left to the hit point with gizmos).

No need to squash the commits, I tend to squash merge like Bevy :)

@Affinator
Copy link
Contributor Author

Hi,
I updated the example to only show the red line until the first hit.

PS: My first approach was also to try to add the predicate to SpatialQueryFilter, but that had the exact problem you described. I therefore chose to do it in a separate function. I understand that this is not ideal, but without a major refactoring this seems to be an acceptable solution.

@Affinator
Copy link
Contributor Author

OK, so the failed lints are from deprecated methods outside my pull request. Should I rebase on a different branch or can this be merged?

@Jondolf
Copy link
Owner

Jondolf commented Jan 30, 2024

I just pushed a quick fix for it here, it was simply a method that was renamed and deprecated in a dependency. I think everything looks good, so I'll merge this once CI passes :)

@Jondolf Jondolf merged commit adb3a19 into Jondolf:main Jan 30, 2024
4 checks passed
@Affinator Affinator deleted the predicates_v2 branch January 30, 2024 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Spatial-Query Relates to spatial queries, such as ray casting, shape casting, and intersection tests C-Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants