-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
implement get_many_unique #18315
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
implement get_many_unique #18315
Changes from 1 commit
e1b1439
a2f9630
5e6e71c
70ba56b
f5c4c9c
b6f4c6a
6993690
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| use crate::{ | ||
| archetype::{Archetype, ArchetypeComponentId, ArchetypeGeneration, ArchetypeId}, | ||
| component::{ComponentId, Tick}, | ||
| entity::{Entity, EntityBorrow, EntitySet}, | ||
| entity::{unique_array::UniqueEntityArray, Entity, EntityBorrow, EntitySet}, | ||
| entity_disabling::DefaultQueryFilters, | ||
| prelude::FromWorld, | ||
| query::{Access, FilteredAccess, QueryCombinationIter, QueryIter, QueryParIter, WorldQuery}, | ||
|
|
@@ -997,6 +997,44 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> { | |
| self.query(world).get_many_inner(entities) | ||
| } | ||
|
|
||
| /// Returns the read-only query results for the given [`UniqueEntityArray`]. | ||
| /// | ||
| /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is | ||
| /// returned instead. | ||
| /// | ||
| /// # Examples | ||
| /// | ||
| /// ``` | ||
| /// use bevy_ecs::{prelude::*, query::QueryEntityError, entity::{EntitySetIterator, unique_array::UniqueEntityArray, unique_vec::UniqueEntityVec}}; | ||
| /// | ||
| /// #[derive(Component, PartialEq, Debug)] | ||
| /// struct A(usize); | ||
| /// | ||
| /// let mut world = World::new(); | ||
| /// let entity_vec: UniqueEntityVec<Entity> = world.spawn_batch((0..3).map(A)).collect_set(); | ||
| /// let entities: UniqueEntityArray<Entity, 3> = entity_vec.try_into().unwrap(); | ||
| /// | ||
| /// world.spawn(A(73)); | ||
| /// | ||
| /// let mut query_state = world.query::<&A>(); | ||
| /// | ||
| /// let component_values = query_state.get_many_unique(&world, entities).unwrap(); | ||
| /// | ||
| /// assert_eq!(component_values, [&A(0), &A(1), &A(2)]); | ||
| /// | ||
| /// let wrong_entity = Entity::from_raw(365); | ||
| /// | ||
| /// assert_eq!(match query_state.get_many_unique(&mut world, UniqueEntityArray::from([wrong_entity])).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); | ||
| /// ``` | ||
| #[inline] | ||
| pub fn get_many_unique<'w, const N: usize>( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm hoping to deprecate the methods like this on
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I was a little confused why this function needed to be implemented for both the state module and query module. It seems like it would only really be needed in the query module
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For context: Until recently, the actual implementation of all of these methods was on
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am aware that we're trying to deprecate them! Given that sometimes PRs can miss a cut-off, or contributors don't have time before an RC/release, leaving changes unfinished seems like an uneccessary risk, when both adding and removing is simple implementation-wise.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Well, in that case, you forgot
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
These already don't exist for |
||
| &mut self, | ||
| world: &'w World, | ||
| entities: UniqueEntityArray<Entity, N>, | ||
| ) -> Result<[ROQueryItem<'w, D>; N], QueryEntityError> { | ||
| self.query(world).get_many_unique_inner(entities) | ||
| } | ||
|
|
||
| /// Gets the query result for the given [`World`] and [`Entity`]. | ||
| /// | ||
| /// This is always guaranteed to run in `O(1)` time. | ||
|
|
@@ -1053,7 +1091,52 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> { | |
| world: &'w mut World, | ||
| entities: [Entity; N], | ||
| ) -> Result<[D::Item<'w>; N], QueryEntityError> { | ||
| self.query_mut(world).get_many_inner(entities) | ||
| self.query_mut(world).get_many_mut_inner(entities) | ||
| } | ||
|
|
||
| /// Returns the query results for the given [`UniqueEntityArray`]. | ||
| /// | ||
| /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is | ||
| /// returned instead. | ||
| /// | ||
| /// ``` | ||
| /// use bevy_ecs::{prelude::*, query::QueryEntityError, entity::{EntitySetIterator, unique_array::UniqueEntityArray, unique_vec::UniqueEntityVec}}; | ||
| /// | ||
| /// #[derive(Component, PartialEq, Debug)] | ||
| /// struct A(usize); | ||
| /// | ||
| /// let mut world = World::new(); | ||
| /// | ||
| /// let entities: UniqueEntityVec<Entity> = world.spawn_batch((0..3).map(A)).collect_set(); | ||
| /// let entities: UniqueEntityArray<Entity, 3> = entities.try_into().unwrap(); | ||
| /// | ||
| /// world.spawn(A(73)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are spawning an entity here with component
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have copied and adjusted the existing doc tests for
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool beans! I think that an example that showcases this functionality would be good then cause you could comment why Totally not blocking tho! |
||
| /// | ||
| /// let mut query_state = world.query::<&mut A>(); | ||
| /// | ||
| /// let mut mutable_component_values = query_state.get_many_unique_mut(&mut world, entities).unwrap(); | ||
| /// | ||
| /// for mut a in &mut mutable_component_values { | ||
| /// a.0 += 5; | ||
| /// } | ||
| /// | ||
| /// let component_values = query_state.get_many_unique(&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_empty().id(); | ||
| /// | ||
| /// assert_eq!(match query_state.get_many_unique(&mut world, UniqueEntityArray::from([wrong_entity])).unwrap_err() {QueryEntityError::EntityDoesNotExist(error) => error.entity, _ => panic!()}, wrong_entity); | ||
| /// assert_eq!(match query_state.get_many_unique_mut(&mut world, UniqueEntityArray::from([invalid_entity])).unwrap_err() {QueryEntityError::QueryDoesNotMatch(entity, _) => entity, _ => panic!()}, invalid_entity); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the reason this is returning an error because you expect every entity to have component 'A'? If so, why not just return an empty list of components?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These asserts test that the error cases are produced properly. |
||
| /// ``` | ||
| #[inline] | ||
| pub fn get_many_unique_mut<'w, const N: usize>( | ||
| &mut self, | ||
| world: &'w mut World, | ||
| entities: UniqueEntityArray<Entity, N>, | ||
| ) -> Result<[D::Item<'w>; N], QueryEntityError> { | ||
| self.query_mut(world).get_many_unique_inner(entities) | ||
| } | ||
|
|
||
| /// Gets the query result for the given [`World`] and [`Entity`]. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,9 @@ | ||
| use crate::{ | ||
| batching::BatchingStrategy, | ||
| component::Tick, | ||
| entity::{Entity, EntityBorrow, EntityDoesNotExistError, EntitySet}, | ||
| entity::{ | ||
| unique_array::UniqueEntityArray, Entity, EntityBorrow, EntityDoesNotExistError, EntitySet, | ||
| }, | ||
| query::{ | ||
| DebugCheckedUnwrap, NopWorldQuery, QueryCombinationIter, QueryData, QueryEntityError, | ||
| QueryFilter, QueryIter, QueryManyIter, QueryManyUniqueIter, QueryParIter, QueryParManyIter, | ||
|
|
@@ -1323,6 +1325,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { | |
| /// # See also | ||
| /// | ||
| /// - [`get_many_mut`](Self::get_many_mut) to get mutable query items. | ||
| /// - [`get_many_unique`](Self::get_many_unique) to only handle unique inputs. | ||
| /// - [`many`](Self::many) for the panicking version. | ||
| #[inline] | ||
| pub fn get_many<const N: usize>( | ||
|
|
@@ -1331,7 +1334,58 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { | |
| ) -> Result<[ROQueryItem<'_, D>; N], QueryEntityError> { | ||
| // Note that this calls `get_many_readonly` instead of `get_many_inner` | ||
| // since we don't need to check for duplicates. | ||
| self.as_readonly().get_many_readonly(entities) | ||
| self.as_readonly().get_many_inner(entities) | ||
| } | ||
|
|
||
| /// Returns the read-only query items for the given [`UniqueEntityArray`]. | ||
| /// | ||
| /// The returned query items are in the same order as the input. | ||
| /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is returned instead. | ||
| /// | ||
| /// # Examples | ||
| /// | ||
| /// ``` | ||
| /// use bevy_ecs::{prelude::*, query::QueryEntityError, entity::{EntitySetIterator, unique_array::UniqueEntityArray, unique_vec::UniqueEntityVec}}; | ||
| /// | ||
| /// #[derive(Component, PartialEq, Debug)] | ||
| /// struct A(usize); | ||
| /// | ||
| /// let mut world = World::new(); | ||
| /// let entity_vec: UniqueEntityVec<Entity> = world.spawn_batch((0..3).map(A)).collect_set(); | ||
| /// let entities: UniqueEntityArray<Entity, 3> = entity_vec.try_into().unwrap(); | ||
| /// | ||
| /// world.spawn(A(73)); | ||
| /// | ||
| /// let mut query_state = world.query::<&A>(); | ||
| /// let query = query_state.query(&world); | ||
| /// | ||
| /// let component_values = query.get_many_unique(entities).unwrap(); | ||
| /// | ||
| /// assert_eq!(component_values, [&A(0), &A(1), &A(2)]); | ||
| /// | ||
| /// let wrong_entity = Entity::from_raw(365); | ||
| /// | ||
| /// assert_eq!( | ||
| /// match query.get_many_unique(UniqueEntityArray::from([wrong_entity])).unwrap_err() { | ||
| /// QueryEntityError::EntityDoesNotExist(error) => error.entity, | ||
| /// _ => panic!(), | ||
| /// }, | ||
| /// wrong_entity | ||
| /// ); | ||
| /// ``` | ||
| /// | ||
| /// # See also | ||
| /// | ||
| /// - [`get_many_unique_mut`](Self::get_many_mut) to get mutable query items. | ||
| /// - [`get_many`](Self::get_many) to handle inputs with duplicates. | ||
| #[inline] | ||
| pub fn get_many_unique<const N: usize>( | ||
| &self, | ||
| entities: UniqueEntityArray<Entity, N>, | ||
| ) -> Result<[ROQueryItem<'_, D>; N], QueryEntityError> { | ||
| // Note that this calls `get_many_readonly` instead of `get_many_inner` | ||
| // since we don't need to check for duplicates. | ||
| self.as_readonly().get_many_unique_inner(entities) | ||
| } | ||
|
|
||
| /// Returns the read-only query items for the given array of [`Entity`]. | ||
|
|
@@ -1560,7 +1614,75 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { | |
| &mut self, | ||
| entities: [Entity; N], | ||
| ) -> Result<[D::Item<'_>; N], QueryEntityError> { | ||
| self.reborrow().get_many_inner(entities) | ||
| self.reborrow().get_many_mut_inner(entities) | ||
| } | ||
|
|
||
| /// Returns the query items for the given [`UniqueEntityArray`]. | ||
| /// | ||
| /// The returned query items are in the same order as the input. | ||
| /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is returned instead. | ||
| /// | ||
| /// # Examples | ||
| /// | ||
| /// ``` | ||
| /// use bevy_ecs::{prelude::*, query::QueryEntityError, entity::{EntitySetIterator, unique_array::UniqueEntityArray, unique_vec::UniqueEntityVec}}; | ||
| /// | ||
| /// #[derive(Component, PartialEq, Debug)] | ||
| /// struct A(usize); | ||
| /// | ||
| /// let mut world = World::new(); | ||
| /// | ||
| /// let entities: UniqueEntityVec<Entity> = world.spawn_batch((0..3).map(A)).collect_set(); | ||
| /// let entities: UniqueEntityArray<Entity, 3> = entities.try_into().unwrap(); | ||
| /// | ||
| /// world.spawn(A(73)); | ||
| /// let wrong_entity = Entity::from_raw(57); | ||
| /// let invalid_entity = world.spawn_empty().id(); | ||
| /// | ||
| /// | ||
| /// let mut query_state = world.query::<&mut A>(); | ||
| /// let mut query = query_state.query_mut(&mut world); | ||
| /// | ||
| /// let mut mutable_component_values = query.get_many_unique_mut(entities).unwrap(); | ||
| /// | ||
| /// for mut a in &mut mutable_component_values { | ||
| /// a.0 += 5; | ||
| /// } | ||
| /// | ||
| /// let component_values = query.get_many_unique(entities).unwrap(); | ||
| /// | ||
| /// assert_eq!(component_values, [&A(5), &A(6), &A(7)]); | ||
| /// | ||
| /// assert_eq!( | ||
| /// match query | ||
| /// .get_many_unique_mut(UniqueEntityArray::from([wrong_entity])) | ||
| /// .unwrap_err() | ||
| /// { | ||
| /// QueryEntityError::EntityDoesNotExist(error) => error.entity, | ||
| /// _ => panic!(), | ||
| /// }, | ||
| /// wrong_entity | ||
| /// ); | ||
| /// assert_eq!( | ||
| /// match query | ||
| /// .get_many_unique_mut(UniqueEntityArray::from([invalid_entity])) | ||
| /// .unwrap_err() | ||
| /// { | ||
| /// QueryEntityError::QueryDoesNotMatch(entity, _) => entity, | ||
| /// _ => panic!(), | ||
| /// }, | ||
| /// invalid_entity | ||
| /// ); | ||
| /// ``` | ||
| /// # See also | ||
| /// | ||
| /// - [`get_many_unique`](Self::get_many) to get read-only query items. | ||
| #[inline] | ||
| pub fn get_many_unique_mut<const N: usize>( | ||
| &mut self, | ||
| entities: UniqueEntityArray<Entity, N>, | ||
| ) -> Result<[D::Item<'_>; N], QueryEntityError> { | ||
| self.reborrow().get_many_unique_inner(entities) | ||
| } | ||
|
|
||
| /// Returns the query items for the given array of [`Entity`]. | ||
|
|
@@ -1573,10 +1695,9 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { | |
| /// | ||
| /// - [`get_many`](Self::get_many) to get read-only query items without checking for duplicate entities. | ||
| /// - [`get_many_mut`](Self::get_many_mut) to get items using a mutable reference. | ||
| /// - [`get_many_readonly`](Self::get_many_readonly) to get read-only query items without checking for duplicate entities | ||
| /// with the actual "inner" world lifetime. | ||
| /// - [`get_many_inner`](Self::get_many_mut_inner) to get read-only query items with the actual "inner" world lifetime. | ||
| #[inline] | ||
| pub fn get_many_inner<const N: usize>( | ||
| pub fn get_many_mut_inner<const N: usize>( | ||
| self, | ||
| entities: [Entity; N], | ||
| ) -> Result<[D::Item<'w>; N], QueryEntityError> { | ||
|
|
@@ -1588,7 +1709,6 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { | |
| } | ||
| } | ||
| } | ||
|
|
||
| // SAFETY: All entities are unique, so the results don't alias. | ||
| unsafe { self.get_many_impl(entities) } | ||
| } | ||
|
|
@@ -1603,9 +1723,9 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { | |
| /// | ||
| /// - [`get_many`](Self::get_many) to get read-only query items without checking for duplicate entities. | ||
| /// - [`get_many_mut`](Self::get_many_mut) to get items using a mutable reference. | ||
| /// - [`get_many_inner`](Self::get_many_readonly) to get mutable query items with the actual "inner" world lifetime. | ||
| /// - [`get_many_mut_inner`](Self::get_many_mut_inner) to get mutable query items with the actual "inner" world lifetime. | ||
| #[inline] | ||
| pub fn get_many_readonly<const N: usize>( | ||
| pub fn get_many_inner<const N: usize>( | ||
| self, | ||
| entities: [Entity; N], | ||
| ) -> Result<[D::Item<'w>; N], QueryEntityError> | ||
|
|
@@ -1616,6 +1736,25 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { | |
| unsafe { self.get_many_impl(entities) } | ||
| } | ||
|
|
||
| /// Returns the query items for the given [`UniqueEntityArray`]. | ||
| /// This consumes the [`Query`] to return results with the actual "inner" world lifetime. | ||
| /// | ||
| /// The returned query items are in the same order as the input. | ||
| /// In case of a nonexisting entity, duplicate entities or mismatched component, a [`QueryEntityError`] is returned instead. | ||
| /// | ||
| /// # See also | ||
| /// | ||
| /// - [`get_many_unique`](Self::get_many_unique) to get read-only query items without checking for duplicate entities. | ||
| /// - [`get_many_unique_mut`](Self::get_many_unique_mut) to get items using a mutable reference. | ||
| #[inline] | ||
| pub fn get_many_unique_inner<const N: usize>( | ||
| self, | ||
| entities: UniqueEntityArray<Entity, N>, | ||
|
||
| ) -> Result<[D::Item<'w>; N], QueryEntityError> { | ||
| // SAFETY: All entities are unique, so the results don't alias. | ||
| unsafe { self.get_many_impl(entities.into_inner()) } | ||
| } | ||
|
|
||
| /// Returns the query items for the given array of [`Entity`]. | ||
| /// This consumes the [`Query`] to return results with the actual "inner" world lifetime. | ||
| /// | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two quick questions if you have the time
UniqueEntityVec<Entity>kinda redundant? What else would go between the <>?UniqueEntityVecjust a Set? Why not just call it a set?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean the questions in general? Or just in this specific case?
If its just these lines:
entity_set, yeah!Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, seems like the
Entityparameter in 1. doesn't infer!While
Entitycould be the type default forUniqueEntityVec, same withUniqueEntitySlice, this isn't easily the case forUniqueEntityArray.Because type defaults have to be trailing, this would turn
UniqueEntityArray<Entity, N>intoUniqueEntityArray<N, Entity>, which is quite an ugly inconsistency with[Entity; N].Not sure what should be done here.
As for what else can go in this parameter, it can entity newtypes like
MainEntity/RenderEntityorEntityRef-like types.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for responding!
Yeah, this is just a naming nit really. Maybe when the 'unique' functionality is all done it could be polished up in the future.
I think the type
UniqueEntityVec<Entity>could maybe be turned intoSet<Entity>or even justEntitySetat some point. But I was mostly asking just because I was curious about if there was a reason to call it UniqueEntityVec rather than anything else.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, adding defaults there is a good idea! I expect
Entityto be the most common type by far. The inconsistency withUniqueEntityArray<N, T>and[T; N]is unfortunate, but if the normal case isUniqueEntityArray<N>vs[Entity; N]then it doesn't seem so bad!Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, one way to potentially remedy the inconsistency is by introducing a different inconsistency, which is also a type default for
N, f.e.0, I haven't thought a lot about that yet, but would you think it makes sense?Edit: Actually, I think Rust is not intelligent enough to allow for
UniqueEntityArray<3>when both generics are defaulted, we'd need to mention both, defeating the whole purpose :P