-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Allow query transmutation with immutable query references #14429
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
Comments
So I added immutable variations of /// ```rust
/// # use bevy_ecs::prelude::*;
/// # use bevy_ecs::system::QueryLens;
/// #
/// # #[derive(Component)]
/// # struct A(usize);
/// #
/// # #[derive(Component)]
/// # struct B(usize);
/// #
/// # let mut world = World::new();
/// #
/// # world.spawn((A(10), B(5)));
/// #
/// fn reusable_function(q: &Query<&A>) {
/// assert_eq!(q.single().0, 10);
/// }
///
/// fn system1(query: Query<(&A, &B)>) {
/// let mut lens = query.transmute_lens_i::<&A>();
/// reusable_function(&lens.query());
/// }
///
/// # let mut schedule = Schedule::default();
/// # schedule.add_systems((system1));
/// # schedule.run(&mut world);
/// ```
pub fn transmute_lens_i<NewD: QueryData>(&self) -> QueryLens<'_, NewD> {
self.transmute_lens_filtered_i::<NewD, ()>()
} Therefore it seems to me that it is just a matter of adding immutable and mutable variations of the |
It's unsound to use immutable references. If you do that you can get 2 query lens out of one query and get two mutable references to the same value. If you run the ecs compile fail tests there should be a test checking for this. It might be valid to allow using &self for read only queries, but it's late here and I haven't thought it through all the way. |
So the problem essentially is that the user can get access to two mutable queries that contain mutable references to same component data? I created a test case here. It compiles and does not crash, but I don't know if this is something that could cause issues otherwise. If this is unwanted behavior, could it be possible to create a compile time mechanism to prevent this kind of access and yet still allow the usage of transmuting read only queries? I created a fork and started using this feature on my project for now regardless and it works, no issues so far. The changes are here in the last two commits. |
Your immutable helper in that code doesn't actually alias the data. It'd be more like: fn immutable_helper(subquery: &mut Query<(&mut A)>, subquery2: &mut Query<(&mut A)>) {
let a1 = subquery.iter_mut().next();
let a2 = subquery.iter_mut().next();
// oops we violated rusts safety rules since we have 2 &mut references to the same data. This is UB.
assert_eq!(a1 as *const A, a2 as *const B);
} Note that just because something doesn't crash that it doesn't mean that it doesn't have undefined behavior. |
This is an advanced case of #3774. |
Consider the following example:
Why does using
transmute_lens()
andquery()
require me to have mutable reference to the query if the query itself is not mutating anything? This prevents me from using transmute lens if the destination of the transmute result accepts only immutable query references. Is there a way around this without forcing the user to perform painful refactorings for all helper functions to accept mutable references of the query, assuming that is even always possible?Going down the path of definitions from
transmute_lens
, it would also seem that final transmute function call takes immutable reference anyway, so maybe this feature could be implemented without much hassle?The text was updated successfully, but these errors were encountered: