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

[Merged by Bors] - Add get_multiple and get_multiple_mut APIs for Query and QueryState #4298

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
a78a231
Basic function signatures
alice-i-cecile Mar 22, 2022
2a22d98
Add AliasedMutability variant to QueryEntityError
alice-i-cecile Mar 22, 2022
f822094
QueryState methods
alice-i-cecile Mar 22, 2022
f47b7ea
Query methods
alice-i-cecile Mar 22, 2022
854bb1e
Infallible variants
alice-i-cecile Mar 22, 2022
712abda
Fix safety comment
alice-i-cecile Mar 22, 2022
34e92c7
Make QueryEntityError more useful
alice-i-cecile Mar 22, 2022
063e616
Basic docs
alice-i-cecile Mar 22, 2022
2423515
Basic doc tests
alice-i-cecile Mar 22, 2022
f04c390
Doc tests for unhappy paths
alice-i-cecile Mar 22, 2022
3bc9432
Fix lifetimes on self
alice-i-cecile Mar 22, 2022
fce42c7
Use unwrap_err to fix broken doc test
alice-i-cecile Mar 22, 2022
1932ee7
Deduplicate logic
alice-i-cecile Mar 22, 2022
1544dbb
Split logic into mutable and non-mutable variants to reduce unsafety
alice-i-cecile Mar 23, 2022
0f0d8ae
Use read-write Fetch type for get_multiple_unchecked_manual
alice-i-cecile Mar 23, 2022
f5bb379
Add compile-fail tests to verify lifetimes
alice-i-cecile Mar 23, 2022
913178b
Fail to compile correctly
alice-i-cecile Mar 23, 2022
b2e3786
Blessed be the compiler error message
alice-i-cecile Mar 23, 2022
7d1b592
Leave TODO
alice-i-cecile Mar 23, 2022
a266f5a
Compile fail test for get_multiple
alice-i-cecile Mar 23, 2022
f41213d
Dedicated aliased mutability test
alice-i-cecile Mar 23, 2022
606c36e
Verify that we're working on the right World
alice-i-cecile Mar 24, 2022
2edcb87
Remove from_raw from doc tests
alice-i-cecile Mar 24, 2022
d2ad4fb
More robust error handling
alice-i-cecile Mar 24, 2022
5871b10
Explicitly test for invalid entities
alice-i-cecile Mar 24, 2022
1cd2963
Also return the Entity for `QueryEntityError::QueryDoesNotMatch`
alice-i-cecile Mar 24, 2022
22337a8
Fix tests
alice-i-cecile Mar 24, 2022
86f1521
Move world validation into `QueryState::get_mulitple_mut`
alice-i-cecile Mar 30, 2022
be22235
World is already validated by update_archetypes call
alice-i-cecile Mar 30, 2022
f7aa741
Avoid repeated calls to validate_world in Query::get_multiple
alice-i-cecile Mar 30, 2022
141c01d
Yeet validation
alice-i-cecile Mar 30, 2022
c0e5903
Cargo fmt
alice-i-cecile Mar 30, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
311 changes: 305 additions & 6 deletions crates/bevy_ecs/src/query/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,57 @@ where
}
}

/// Returns the read-only query results for the given array of [`Entity`].
///
/// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is
/// returned instead.
///
/// Note that the unlike [`QueryState::get_multiple_mut`], the entities passed in do not need to be unique.
///
/// # Examples
///
/// ```rust
/// use bevy_ecs::prelude::*;
/// use bevy_ecs::query::QueryEntityError;
///
/// #[derive(Component, PartialEq, Debug)]
/// struct A(usize);
///
/// let mut world = World::new();
/// let entity_vec: Vec<Entity> = (0..3).map(|i|world.spawn().insert(A(i)).id()).collect();
/// let entities: [Entity; 3] = entity_vec.try_into().unwrap();
///
/// world.spawn().insert(A(73));
///
/// let mut query_state = world.query::<&A>();
///
/// let component_values = query_state.get_multiple(&world, entities).unwrap();
///
/// assert_eq!(component_values, [&A(0), &A(1), &A(2)]);
///
/// let wrong_entity = Entity::from_raw(365);
///
/// assert_eq!(query_state.get_multiple(&world, [wrong_entity]), Err(QueryEntityError::NoSuchEntity(wrong_entity)));
/// ```
#[inline]
pub fn get_multiple<'w, 's, const N: usize>(
&'s mut self,
world: &'w World,
entities: [Entity; N],
) -> Result<[<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item; N], QueryEntityError> {
self.update_archetypes(world);

// SAFE: update_archetypes validates the `World` matches
unsafe {
self.get_multiple_read_only_manual(
world,
entities,
world.last_change_tick(),
world.read_change_tick(),
)
}
}

/// Gets the query result for the given [`World`] and [`Entity`].
#[inline]
pub fn get_mut<'w, 's>(
Expand All @@ -172,6 +223,64 @@ where
}
}

/// Returns the query results for the given array of [`Entity`].
///
/// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is
/// returned instead.
///
/// ```rust
/// use bevy_ecs::prelude::*;
/// use bevy_ecs::query::QueryEntityError;
///
/// #[derive(Component, PartialEq, Debug)]
/// struct A(usize);
///
/// let mut world = World::new();
///
/// let entities: Vec<Entity> = (0..3).map(|i|world.spawn().insert(A(i)).id()).collect();
/// let entities: [Entity; 3] = entities.try_into().unwrap();
///
/// world.spawn().insert(A(73));
///
/// let mut query_state = world.query::<&mut A>();
///
/// let mut mutable_component_values = query_state.get_multiple_mut(&mut world, entities).unwrap();
///
/// for mut a in mutable_component_values.iter_mut(){
/// a.0 += 5;
/// }
///
/// let component_values = query_state.get_multiple(&world, entities).unwrap();
///
/// assert_eq!(component_values, [&A(5), &A(6), &A(7)]);
///
/// let wrong_entity = Entity::from_raw(57);
/// let invalid_entity = world.spawn().id();
///
/// assert_eq!(query_state.get_multiple_mut(&mut world, [wrong_entity]).unwrap_err(), QueryEntityError::NoSuchEntity(wrong_entity));
/// assert_eq!(query_state.get_multiple_mut(&mut world, [invalid_entity]).unwrap_err(), QueryEntityError::QueryDoesNotMatch(invalid_entity));
/// assert_eq!(query_state.get_multiple_mut(&mut world, [entities[0], entities[0]]).unwrap_err(), QueryEntityError::AliasedMutability(entities[0]));
/// ```
#[inline]
pub fn get_multiple_mut<'w, 's, const N: usize>(
&'s mut self,
world: &'w mut World,
entities: [Entity; N],
) -> Result<[<Q::Fetch as Fetch<'w, 's>>::Item; N], QueryEntityError> {
self.update_archetypes(world);

// SAFE: method requires exclusive world access
// and world has been validated via update_archetypes
unsafe {
self.get_multiple_unchecked_manual(
world,
entities,
world.last_change_tick(),
world.read_change_tick(),
)
}
}

#[inline]
pub fn get_manual<'w, 's>(
&'s self,
Expand Down Expand Up @@ -218,6 +327,9 @@ where
///
/// This does not check for mutable query correctness. To be safe, make sure mutable queries
/// have unique access to the components they query.
///
/// This must be called on the same `World` that the `Query` was generated from:
/// use `QueryState::validate_world` to verify this.
pub(crate) unsafe fn get_unchecked_manual<'w, 's, QF: Fetch<'w, 's, State = Q::State>>(
&'s self,
world: &'w World,
Expand All @@ -228,12 +340,12 @@ where
let location = world
.entities
.get(entity)
.ok_or(QueryEntityError::NoSuchEntity)?;
.ok_or(QueryEntityError::NoSuchEntity(entity))?;
if !self
.matched_archetypes
.contains(location.archetype_id.index())
{
return Err(QueryEntityError::QueryDoesNotMatch);
return Err(QueryEntityError::QueryDoesNotMatch(entity));
}
let archetype = &world.archetypes[location.archetype_id];
let mut fetch = QF::init(world, &self.fetch_state, last_change_tick, change_tick);
Expand All @@ -245,10 +357,90 @@ where
if filter.archetype_filter_fetch(location.index) {
Ok(fetch.archetype_fetch(location.index))
} else {
Err(QueryEntityError::QueryDoesNotMatch)
Err(QueryEntityError::QueryDoesNotMatch(entity))
}
}

/// Gets the read-only query results for the given [`World`] and array of [`Entity`], where the last change and
/// the current change tick are given.
///
/// # Safety
///
/// This must be called on the same `World` that the `Query` was generated from:
/// use `QueryState::validate_world` to verify this.
pub(crate) unsafe fn get_multiple_read_only_manual<'s, 'w, const N: usize>(
&'s self,
world: &'w World,
entities: [Entity; N],
last_change_tick: u32,
change_tick: u32,
) -> Result<[<Q::ReadOnlyFetch as Fetch<'w, 's>>::Item; N], QueryEntityError> {
// SAFE: fetch is read-only
alice-i-cecile marked this conversation as resolved.
Show resolved Hide resolved
// and world must be validated
let array_of_results = entities.map(|entity| {
self.get_unchecked_manual::<Q::ReadOnlyFetch>(
world,
entity,
last_change_tick,
change_tick,
)
});

// TODO: Replace with TryMap once https://github.com/rust-lang/rust/issues/79711 is stabilized
// If any of the get calls failed, bubble up the error
for result in &array_of_results {
match result {
Ok(_) => (),
Err(error) => return Err(*error),
}
}

// Since we have verified that all entities are present, we can safely unwrap
Ok(array_of_results.map(|result| result.unwrap()))
}

/// Gets the query results for the given [`World`] and array of [`Entity`], where the last change and
/// the current change tick are given.
///
/// # Safety
///
/// This does not check for unique access to subsets of the entity-component data.
/// To be safe, make sure mutable queries have unique access to the components they query.
///
/// This must be called on the same `World` that the `Query` was generated from:
/// use `QueryState::validate_world` to verify this.
pub(crate) unsafe fn get_multiple_unchecked_manual<'s, 'w, const N: usize>(
&'s self,
world: &'w World,
entities: [Entity; N],
last_change_tick: u32,
change_tick: u32,
) -> Result<[<Q::Fetch as Fetch<'w, 's>>::Item; N], QueryEntityError> {
// Verify that all entities are unique
alice-i-cecile marked this conversation as resolved.
Show resolved Hide resolved
for i in 0..N {
for j in 0..i {
if entities[i] == entities[j] {
return Err(QueryEntityError::AliasedMutability(entities[i]));
}
}
}

let array_of_results = entities.map(|entity| {
self.get_unchecked_manual::<Q::Fetch>(world, entity, last_change_tick, change_tick)
});

// If any of the get calls failed, bubble up the error
for result in &array_of_results {
match result {
Ok(_) => (),
Err(error) => return Err(*error),
}
}

// Since we have verified that all entities are present, we can safely unwrap
Ok(array_of_results.map(|result| result.unwrap()))
}

/// Returns an [`Iterator`] over the query results for the given [`World`].
///
/// This can only be called for read-only queries, see [`Self::iter_mut`] for write-queries.
Expand Down Expand Up @@ -733,10 +925,117 @@ where
}

/// An error that occurs when retrieving a specific [`Entity`]'s query result.
#[derive(Error, Debug)]
// TODO: return the type_name as part of this error
#[derive(Error, Debug, PartialEq, Clone, Copy)]
pub enum QueryEntityError {
#[error("The given entity does not have the requested component.")]
QueryDoesNotMatch,
QueryDoesNotMatch(Entity),
#[error("The requested entity does not exist.")]
NoSuchEntity,
NoSuchEntity(Entity),
#[error("The entity was requested mutably more than once.")]
AliasedMutability(Entity),
}

#[cfg(test)]
mod tests {
use crate::{prelude::*, query::QueryEntityError};

#[test]
fn get_multiple_unchecked_manual_uniqueness() {
let mut world = World::new();

let entities: Vec<Entity> = (0..10).map(|_| world.spawn().id()).collect();

let query_state = world.query::<Entity>();

// These don't matter for the test
let last_change_tick = world.last_change_tick();
let change_tick = world.read_change_tick();

// It's best to test get_multiple_unchecked_manual directly,
// as it is shared and unsafe
// We don't care about aliased mutabilty for the read-only equivalent
assert!(unsafe {
query_state
.get_multiple_unchecked_manual::<10>(
&world,
entities.clone().try_into().unwrap(),
last_change_tick,
change_tick,
)
.is_ok()
});

assert_eq!(
unsafe {
query_state
.get_multiple_unchecked_manual(
&world,
[entities[0], entities[0]],
last_change_tick,
change_tick,
)
.unwrap_err()
},
QueryEntityError::AliasedMutability(entities[0])
);

assert_eq!(
unsafe {
query_state
.get_multiple_unchecked_manual(
&world,
[entities[0], entities[1], entities[0]],
last_change_tick,
change_tick,
)
.unwrap_err()
},
QueryEntityError::AliasedMutability(entities[0])
);

assert_eq!(
unsafe {
query_state
.get_multiple_unchecked_manual(
&world,
[entities[9], entities[9]],
last_change_tick,
change_tick,
)
.unwrap_err()
},
QueryEntityError::AliasedMutability(entities[9])
);
}

#[test]
#[should_panic]
fn right_world_get() {
let mut world_1 = World::new();
let world_2 = World::new();

let mut query_state = world_1.query::<Entity>();
let _panics = query_state.get(&world_2, Entity::from_raw(0));
}

#[test]
#[should_panic]
fn right_world_get_multiple() {
let mut world_1 = World::new();
let world_2 = World::new();

let mut query_state = world_1.query::<Entity>();
let _panics = query_state.get_multiple(&world_2, []);
}

#[test]
#[should_panic]
fn right_world_get_multiple_mut() {
let mut world_1 = World::new();
let mut world_2 = World::new();

let mut query_state = world_1.query::<Entity>();
let _panics = query_state.get_multiple_mut(&mut world_2, []);
}
}
Loading