Skip to content

Commit c34a2c2

Browse files
authored
Query::get_many should not check for duplicates (#17724)
# Objective Restore the behavior of `Query::get_many` prior to #15858. When passed duplicate `Entity`s, `get_many` is supposed to return results for all of them, since read-only queries don't alias. However, #15858 merged the implementation with `get_many_mut` and caused it to return `QueryEntityError::AliasedMutability`. ## Solution Introduce a new `Query::get_many_readonly` method that consumes the `Query` like `get_many_inner`, but that is constrained to `D: ReadOnlyQueryData` so that it can skip the aliasing check. Implement `Query::get_many` in terms of that new method. Add a test, and a comment explaining why it doesn't match the pattern of the other `&self` methods.
1 parent 7f9588d commit c34a2c2

File tree

2 files changed

+45
-3
lines changed

2 files changed

+45
-3
lines changed

crates/bevy_ecs/src/query/mod.rs

+12
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,18 @@ mod tests {
437437
);
438438
}
439439

440+
#[test]
441+
fn get_many_only_mut_checks_duplicates() {
442+
let mut world = World::new();
443+
let id = world.spawn(A(10)).id();
444+
let mut query_state = world.query::<&mut A>();
445+
let mut query = query_state.query_mut(&mut world);
446+
let result = query.get_many([id, id]);
447+
assert_eq!(result, Ok([&A(10), &A(10)]));
448+
let mut_result = query.get_many_mut([id, id]);
449+
assert!(mut_result.is_err());
450+
}
451+
440452
#[test]
441453
fn multi_storage_query() {
442454
let mut world = World::new();

crates/bevy_ecs/src/system/query.rs

+33-3
Original file line numberDiff line numberDiff line change
@@ -1135,7 +1135,9 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> {
11351135
&self,
11361136
entities: [Entity; N],
11371137
) -> Result<[ROQueryItem<'_, D>; N], QueryEntityError> {
1138-
self.as_readonly().get_many_inner(entities)
1138+
// Note that this calls `get_many_readonly` instead of `get_many_inner`
1139+
// since we don't need to check for duplicates.
1140+
self.as_readonly().get_many_readonly(entities)
11391141
}
11401142

11411143
/// Returns the read-only query items for the given array of [`Entity`].
@@ -1249,7 +1251,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> {
12491251
///
12501252
/// # See also
12511253
///
1252-
/// - [`get_many`](Self::get_many) to get read-only query items.
1254+
/// - [`get_many`](Self::get_many) to get read-only query items without checking for duplicate entities.
12531255
/// - [`many_mut`](Self::many_mut) for the panicking version.
12541256
#[inline]
12551257
pub fn get_many_mut<const N: usize>(
@@ -1267,8 +1269,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> {
12671269
///
12681270
/// # See also
12691271
///
1270-
/// - [`get_many`](Self::get_many) to get read-only query items.
1272+
/// - [`get_many`](Self::get_many) to get read-only query items without checking for duplicate entities.
12711273
/// - [`get_many_mut`](Self::get_many_mut) to get items using a mutable reference.
1274+
/// - [`get_many_readonly`](Self::get_many_readonly) to get read-only query items without checking for duplicate entities
1275+
/// with the actual "inner" world lifetime.
12721276
#[inline]
12731277
pub fn get_many_inner<const N: usize>(
12741278
self,
@@ -1281,6 +1285,32 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> {
12811285
}
12821286
}
12831287

1288+
/// Returns the query items for the given array of [`Entity`].
1289+
/// This consumes the [`Query`] to return results with the actual "inner" world lifetime.
1290+
///
1291+
/// The returned query items are in the same order as the input.
1292+
/// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is returned instead.
1293+
///
1294+
/// # See also
1295+
///
1296+
/// - [`get_many`](Self::get_many) to get read-only query items without checking for duplicate entities.
1297+
/// - [`get_many_mut`](Self::get_many_mut) to get items using a mutable reference.
1298+
/// - [`get_many_inner`](Self::get_many_readonly) to get mutable query items with the actual "inner" world lifetime.
1299+
#[inline]
1300+
pub fn get_many_readonly<const N: usize>(
1301+
self,
1302+
entities: [Entity; N],
1303+
) -> Result<[D::Item<'w>; N], QueryEntityError<'w>>
1304+
where
1305+
D: ReadOnlyQueryData,
1306+
{
1307+
// SAFETY: scheduler ensures safe Query world access
1308+
unsafe {
1309+
self.state
1310+
.get_many_read_only_manual(self.world, entities, self.last_run, self.this_run)
1311+
}
1312+
}
1313+
12841314
/// Returns the query items for the given array of [`Entity`].
12851315
///
12861316
/// # Panics

0 commit comments

Comments
 (0)