Lack of ability to inspect mutable queries via &self receiver in 0.6 breaks Rapier integration #2744
Comments
pcwalton
added
C-Bug
alice-i-cecile
added
A-ECS
|
@cart I'd prefer not to break a major plugin with the launch. Should we add this to the 0.6 milestone? |
|
#2305 tackles a related issue; perhaps there's techniques we can share? |
Oh, interesting. Would it be helpful to see if I can successfully port Rapier on top of that PR? |
Yes! I would start by resolving the merge conflicts and submitting them as a PR to @Guvante's branch. In general, Bevy needs to be able to convert mutable accesses into immutable ones. The reverse is obviously illegal, but the weakening of access restrictions should always be allowed. There's a number of places where this comes to mind: queries, query iteration, resource access and entity access to start. It's probably not correct to handle them all in one PR, but the broader context is helpful to bear in mind as you investigate to see if you can identify common techniques or a unified API. |
alice-i-cecile
added
S-Needs-Design
The biggest change here is that doing the same query twice, one mutably and one immutably, is no longer needed, once bevyengine#2305 lands. Before that lands, however, this upgrade is actually impossible to do safely, since the `q0`/`q0_mut` distinction is gone and therefore it's impossible to implement the Rapier component traits on Bevy queries. (This is filed upstream as bevyengine#2744.) Therefore, this upgrade has to wait until that Bevy PR lands. Other minor changes: * `Light` has been renamed to `PointLight`. * `App::build()` is now `App::new()`, and `AppBuilder` is now `App`. * `glam` has been upgraded upstream.
Done: Guvante#1 Though it's probably easier for @Guvante to just do this via a rebase, since it was trivial. I'm happy to report that |
The biggest change here is that doing the same query twice, one mutably and one immutably, is no longer needed, once bevyengine/bevy#2305 lands. Before that lands, however, this upgrade is actually impossible to do safely, since the `q0`/`q0_mut` distinction is gone and therefore it's impossible to implement the Rapier component traits on Bevy queries. (This is filed upstream as bevyengine/bevy#2744.) Therefore, this upgrade has to wait until that Bevy PR lands. Other minor changes: * `Light` has been renamed to `PointLight`. * `App::build()` is now `App::new()`, and `AppBuilder` is now `App`. * `glam` has been upgraded upstream.
The biggest change here is that doing the same query twice, one mutably and one immutably, is no longer needed, once bevyengine/bevy#2305 lands. Before that lands, however, this upgrade is actually impossible to do safely, since the `q0`/`q0_mut` distinction is gone and therefore it's impossible to implement the Rapier component traits on Bevy queries. (This is filed upstream as bevyengine/bevy#2744.) Therefore, this upgrade has to wait until that Bevy PR lands. Other minor changes: * `Light` has been renamed to `PointLight`. * `App::build()` is now `App::new()`, and `AppBuilder` is now `App`. * `glam` has been upgraded upstream.
|
Brilliant. Glad to hear that the new apis compose so well. I agree that this is something that should be included in the 0.6 release, so I just added #2305 to the 0.6 milestone. |
alice-i-cecile
removed
the
S-Needs-Design
The biggest change here is that doing the same query twice, one mutably and one immutably, is no longer needed, once bevyengine/bevy#2305 lands. Before that lands, however, this upgrade is actually impossible to do safely, since the `q0`/`q0_mut` distinction is gone and therefore it's impossible to implement the Rapier component traits on Bevy queries. (This is filed upstream as bevyengine/bevy#2744.) Therefore, this upgrade has to wait until that Bevy PR lands. Other minor changes: * `Light` has been renamed to `PointLight`. * `App::build()` is now `App::new()`, and `AppBuilder` is now `App`. * `glam` has been upgraded upstream.
|
Can this issue be closed now that #2305 is merged? |
The biggest change here is that doing the same query twice, one mutably and one immutably, is no longer needed, once bevyengine/bevy#2305 lands. Before that lands, however, this upgrade is actually impossible to do safely, since the `q0`/`q0_mut` distinction is gone and therefore it's impossible to implement the Rapier component traits on Bevy queries. (This is filed upstream as bevyengine/bevy#2744.) Therefore, this upgrade has to wait until that Bevy PR lands. Other minor changes: * `Light` has been renamed to `PointLight`. * `App::build()` is now `App::new()`, and `AppBuilder` is now `App`. * `glam` has been upgraded upstream.
Bevy version
0.6 prerelease, f6b42b8
Details
The Rapier physics library abstracts over scene graphs with traits called ComponentSet and ComponentSetMut, where ComponentSetMut requires that ComponentSet be implemented. ComponentSet allows immutable lookup of physics-related properties with a
fn get(&self, ...) -> Option<&T>, while ComponentSetMut allows mutation of those properties withfn set(&mut self, ...). Thebevy_rapiercrate targeting Bevy 0.5 implements this trait on a QuerySet that performs immutable and mutable versions of the query, and takes advantage of the fact thatfn q0(&self)andfn q0_mut(&mut self)are separate methods. But in Bevy 0.6 as of #2605, thefn q0(&self)method has been removed in favor of the mutable version. This means that it's not possible to safely simultaneously implement ComponentSet and ComponentSetMut anymore on a query.The best safe workaround I can think of is to copy all the physics-related data for every entity out to a temporary collection every frame and then copy it back in after the physics code has run. This is obviously not shippable :) Even the Bevy 0.5 workaround isn't great, because it performs the query twice.
It'd be very nice if temporary immutable access to mutable queries were possible, by borrowing the mutable query. I'm not familiar enough with all the lifetimes involved to be able to propose a concrete API here, though.
The text was updated successfully, but these errors were encountered: