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

Split up RapierContext #502

Open
Jondolf opened this issue May 9, 2024 · 1 comment · May be fixed by #585
Open

Split up RapierContext #502

Jondolf opened this issue May 9, 2024 · 1 comment · May be fixed by #585
Labels
A-Integration very bevy specific D-Difficult Needs strong technical background, domain knowledge, or impacts are high, needs testing... P-Medium S-not-started Work has not started

Comments

@Jondolf
Copy link

Jondolf commented May 9, 2024

The RapierContext resource is massive. It has the island manager, broad phase, narrow phase, CCD solver, physics pipeline, query pipeline, integration parameters, rigid bodies, colliders, and joints. Quoting the docs: "The Rapier context, containing all the state of the physics engine." It is also what is used to step the simulation, perform scene queries, query for collision pairs, and control character controllers.

This kind of "god struct" feels almost antithetical to an ECS. Almost everything is handled by one struct with dozens of unrelated responsibilities. This is unidiomatic, bad for composability and modularity, bad for documentation, and bad from a UX standpoint.

What solution would you like?

RapierContext should be split up into several resources. For methods that require access to multiple different resources, system parameters can be used.

For example, the query pipeline could be turned into its own resource. It could be renamed to SceneQuery, or there could be a new system parameter for it, like bevy_xpbd's SpatialQuery. If it was its own system parameter, ray casting could look like this:

fn cast_ray(scene_query: SceneQuery) {
    let ray_pos = Vec3::new(1.0, 2.0, 3.0);
    let ray_dir = Vec3::new(0.0, 1.0, 0.0);
    let max_toi = 4.0;
    let solid = true;
    let filter = QueryFilter::default();

    if let Some((entity, toi)) = scene_query.cast_ray(
        ray_pos, ray_dir, max_toi, solid, filter
    ) {
        let hit_point = ray_pos + ray_dir * toi;
        println!("Entity {:?} hit at point {}", entity, hit_point);
    }
}

I find that IntegrationParameters also doesn't really make sense in RapierContext. It could either be in RapierConfiguration where other global configuration resides, or be its own resource.

Some more internal structures like the island manager, rigid body sets, collider sets, and so on might be fine in the same resource, or split into separate resources.

Plugin architecture?

Personally, I would also like to see Rapier try a more plugin-oriented architecture. It would allow for some nice flexibility and composability, making it more straightforward for third-party alternatives to be used if a specific first-party solution is insufficient, while also providing a logical place for docs.rs documentation. For example, collision detection could be handled by one plugin, and the solver by another. bevy_xpbd has a very plugin-heavy architecture, but even just a few plugins would help break up the monolithic structure.

Of course, this would probably require changes to how the simulation loop in bevy_rapier works, but I don't see why it wouldn't inherently be doable.

@Vrixyz
Copy link
Contributor

Vrixyz commented May 21, 2024

#328 suggests taking the route of physics World as entities, that's the best option in my opinion too, which plays well with this PR: add more or split components to customize a physics world.

@Vrixyz Vrixyz added D-Difficult Needs strong technical background, domain knowledge, or impacts are high, needs testing... P-Medium S-not-started Work has not started A-Integration very bevy specific labels May 21, 2024
@Vrixyz Vrixyz linked a pull request Sep 11, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Integration very bevy specific D-Difficult Needs strong technical background, domain knowledge, or impacts are high, needs testing... P-Medium S-not-started Work has not started
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants