bevy_xpbd
bevy_xpbd copied to clipboard
Improve `SpatialQuery` APIs and docs, and add more configuration
Objective
Fixes #458, among many issues that have come up in discussions.
The API for ray casts and shape casts through SpatialQuery is quite hard to read. If we supported all of Parry's configuration options for shape casts, shape_hits would end up looking like this:
// Note: target_distance and compute_impact_on_penetration don't exist prior to this PR
let hits = spatial.shape_hits(
&shape,
origin,
shape_rotation,
direction,
max_time_of_impact,
max_hits,
target_distance,
compute_impact_on_penetration,
ignore_origin_penetration,
&query_filter,
);
Without variables for everything, it would be almost entirely unreadable:
let hits = spatial.shape_hits(
&Collider::sphere(0.5),
Vec3::ZERO,
Quat::default(),
Dir3::ZERO,
100.0,
10,
0.0,
true,
false,
&default(),
);
Aside from bad ergonomics and poor readability, this is also bad for documentation, as the docs for each configuration option must be duplicated for each method that uses them. There are also no sane defaults, as every setting must be configured explicitly, even if the user is unlikely to need to care about some of them. This is especially bad for new users for whom the long list of arguments may be confusing and daunting.
Another slightly unrelated issue is that many properties and docs use the term "time of impact", which has reportedly been a bit confusing for many users. In Avian's case, for ray casts and shape casts, it is actually just the distance travelled along the ray, because the cast direction is normalized. Most occurrences of "time of impact" should likely be renamed to just "distance", except for time-of-impact queries where both shapes are moving or there is angular velocity involved.
Solution
This PR fixes several API and documentation issues, along with doing general cleanup. It's a bit large with some slightly unrelated changes, so apologies in advance for potential reviewers!
Ray casting and shape casting methods now take a RayCastConfig/ShapeCastConfig. It holds general configuration options for the cast, and could be extended in the future to even have things like debug rendering options.
The earlier example would now look like this:
let hits = spatial.shape_hits(
&shape,
origin,
shape_rotation,
direction,
max_hits,
&ShapeCastConfig {
max_distance,
target_distance,
compute_impact_on_penetration,
ignore_origin_penetration,
filter,
},
);
In practice, you can often use default values for most properties, use constructor methods, and might move the configuration outside the method call (and even reuse it across shape casts), so it could actually look like this:
let config = ShapeCastConfig::from_max_distance(100.0);
let hits = spatial.shape_hits(
&Collider::sphere(0.5),
Vec3::ZERO,
Quat::default(),
Dir3::ZERO,
10,
&config,
);
Ray casts have received a similar treatment. If no extra configuration is needed, a simple cast_ray call can often collapse into a very nice form:
// Before
let hit = spatial.cast_ray(Vec2::ZERO, Dir2::X, f32::MAX, true, &SpatialQueryFilter::default());
// After
let hit = spatial.cast_ray(Vec2::ZERO, Dir2::X, &RayCastConfig::default());
// Or even just this:
let hit = spatial.cast_ray(Vec2::ZERO, Dir2::X, &default());
(this could potentially be even nicer if it accepted a Ray2d/Ray3d as an argument, but that's another issue)
As you may have noticed, I also changed max_time_of_impact to max_distance. Most occurrences of "time of impact" have indeed been renamed to "distance" as per the reasons mentioned earlier.
Additionally, I fixed several documentation issues and added missing methods and configuration options. See the changelog below for a slightly more thorough overview.
Changelog
- Added
RayCastConfig. Most ray casting methods now take the config instead of many separate arguments - Added
ShapeCastConfig. Most shape casting methods now take the config instead of many separate arguments - Renamed
RayHitData::time_of_impact,ShapeHitData::time_of_impact, and many other occurrences of "time of impact" todistance - Added missing predicate versions of spatial query methods which existed on
SpatialQueryPipeline, but notSpatialQuery - Added
target_distanceandcompute_impact_on_penetrationconfiguration options for shape casts, to match Parry's capabilities - Fixed several documentation issues, such as:
- Shape hit data is in world space, not local space
- Many intra-doc for "See also" sections link to self, not other methods
- Mentions of "ray" in shape casting docs
- Confusing wording for
predicateargument docs
- Improved documentation examples
- Improved clarity and readability in a lot of docs
- Changed
excluded_entitiesto use anEntityHashSet
Migration Guide
Ray Casting and Shape Casting Configuration
SpatialQuery methods like cast_ray and ray_hits now take a RayCastConfig, which contains a lot of the existing configuration options.
let origin = Vec2::ZERO;
let direction = Dir2::X;
// Before
let hit = spatial.cast_ray(origin, direction, 100.0, true, &SpatialQueryFilter::default());
// After
let config = RayCastConfig::from_max_distance(100.0);
let hit = spatial.cast_ray(origin, direction, &config);
Shape casts have received a similar treatment. For example, shape_hits:
// Before
let hits = spatial.shape_hits(
&Collider::sphere(0.5),
Vec3::ZERO,
Quat::default(),
Dir3::ZERO,
100.0,
10,
false,
&SpatialQueryFilter::from_mask(LayerMask::ALL),
);
// After
let config = ShapeCastConfig {
max_distance: 100.0,
filter: SpatialQueryFilter::from_mask(LayerMask::ALL),
..default()
};
let hits = spatial.shape_hits(
&Collider::sphere(0.5),
Vec3::ZERO,
Quat::default(),
Dir3::ZERO,
10,
&config,
);
Why was the API changed? Separating the configuration options improves readability and documentation, allows the config to be reused, provides sane default values for many options, and makes it possible for Avian to add more configuration options in the future without bloating the method calls further. In many cases, it can also just be more ergonomic and shorter, as several options can typically use default values.
Time of Impact → Distance
Spatial query APIs that mention the "time of impact" have been changed to refer to "distance". This affects names of properties and methods, such as:
RayCaster::max_time_of_impact→RayCaster::max_distanceRayCaster::with_max_time_of_impact→RayCaster::with_max_distanceShapeCaster::max_time_of_impact→ShapeCaster::max_distanceShapeCaster::with_max_time_of_impact→ShapeCaster::with_max_distanceRayHitData::time_of_impact→RayHitData::distanceShapeHitData::time_of_impact→ShapeHitData::distancemax_time_of_impactonSpatialQuerymethods →RayCastConfig::max_distanceorShapeCastConfig::max_distance
This was changed because "distance" is clearer than "time of impact" for many users, and it is still an accurate term, as the cast directions in Avian are always normalized, so the "velocity" is of unit length.