Skip to content

Split RapierContext#585

Merged
ThierryBerger merged 31 commits intodimforge:masterfrom
ThierryBerger:split_rapiercontext
Dec 10, 2024
Merged

Split RapierContext#585
ThierryBerger merged 31 commits intodimforge:masterfrom
ThierryBerger:split_rapiercontext

Conversation

@ThierryBerger
Copy link
Contributor

@ThierryBerger ThierryBerger commented Sep 11, 2024

Opportunities for parallelized execution seem limited, but exist, this also enables better separation of systems, with more typed information leading to a clearer intent.

I'm not 100% convinced it's a net positive in terms of maintainability, bevy_mod_debugdump is currently harder to read due to this bug: jakobhellermann/bevy_mod_debugdump#50.

ThierryBerger added a commit to ThierryBerger/rapier.rs that referenced this pull request Oct 15, 2024
@ThierryBerger ThierryBerger marked this pull request as ready for review October 15, 2024 08:35
ThierryBerger added a commit to ThierryBerger/rapier.rs that referenced this pull request Oct 15, 2024
@ThierryBerger ThierryBerger mentioned this pull request Oct 31, 2024
3 tasks
Copy link
Member

@sebcrozet sebcrozet left a comment

Choose a reason for hiding this comment

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

🎉

Comment on lines +93 to +95
let context = rapier_context.single();
// Then cast the ray.
let hit = rapier_context.cast_ray(
let hit = context.cast_ray(
Copy link
Contributor Author

@ThierryBerger ThierryBerger Dec 9, 2024

Choose a reason for hiding this comment

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

We could avoid this indirection to single() if we implemented all methods of RapierContext on its systemParam ; but:

  • it would require making the query each time we call a method
    • Probably not so much of an issue if queries are cached
  • it would require to add another 400 lines of code, I already fear for the maintenance of current layer of indirection, I'd like to keep it at the minimum.
    • a declarative macro to generate those ended up a bit too complex, and not much lines saved
    • maybe a procedural or a template step could be interesting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of added lines come from this file, which holds a shortcut to functions users are accustomed to. Rather than looking up which component they have to rely on, they have a RapierContext-like API, and can look into it to see what queries it resolves to if they want.

@ThierryBerger ThierryBerger merged commit ac4fb03 into dimforge:master Dec 10, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants