From 3e8288a45287a6d831aabff66be5e8341abddf0b Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 13 Dec 2021 15:51:14 -0500 Subject: [PATCH 01/49] Method skeletons --- crates/bevy_ecs/src/system/query.rs | 29 +++++++++++++++++- crates/bevy_ecs/src/world/mod.rs | 46 ++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index df1098fd71513..a6983b29bc7e6 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -233,6 +233,9 @@ use thiserror::Error; /// If you have an [`Entity`] ID, you can use the [`get`](Self::get) or /// [`get_mut`](Self::get_mut) methods to access the query result for that particular entity. /// +/// If you require access to the data of multiple entities at once, +/// you can use the ['get_multiple'](Self::get_multiple) or ['get_multiple_mut'](Self::get_multiple_mut) methods. +/// /// ## Getting a single query result /// /// While it's possible to get a single result from a query by using `iter.next()`, a more @@ -551,7 +554,7 @@ where }; } - /// Returns the query result for the given [`Entity`]. + /// Returns the read-only query result for the given [`Entity`]. /// /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is /// returned instead. @@ -640,6 +643,30 @@ where } } + /// Returns the read-only query result for the HashSet of [`Entity`] provided. + /// + /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is + /// returned instead. + /// + /// If you need to reduce performance overhead, you can (carefully) call the unsafe `get_unchecked` method repeatedly instead. + pub fn get_multiple( + &'s self, + entities: HashSet, + ) -> HashMap::Item, QueryEntityError>> { + } + + /// Returns the query result for the HashSet of [`Entity`] provided. + /// + /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is + /// returned instead. + /// + /// If you need to reduce performance overhead, you can (carefully) call the unsafe `get_unchecked` method repeatedly instead. + pub fn get_multiple_mut( + &mut self, + entities: HashSet, + ) -> HashMap::Item, QueryEntityError>> { + } + /// Returns the query result for the given [`Entity`]. /// /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index ad037819e89f7..572797c285ccf 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -22,7 +22,9 @@ use std::{ fmt, mem::ManuallyDrop, sync::atomic::{AtomicU32, Ordering}, + iter::IntoIterator, }; +use bevy_utils::{HashMap, HashSet}; mod identifier; @@ -311,7 +313,49 @@ impl World { Some(unsafe { EntityMut::new(self, entity, location) }) } - /// Spawns a new [`Entity`] and returns a corresponding [`EntityMut`], which can be used + /// Returns the [EntityRef] of multiple entities, which expose read-only operations for those entities + /// + /// Entity data will be returned in the same order as the provided iterator. + /// The `entities` argument does not need to be unique, as the underlying data cannot be modified. + /// + /// This will panic if any of the entities do not exist. Use [World::get_multiple_entities] if you want + /// to check for entity existence instead of implicitly panic-ing. + pub fn multiple_entities(&mut self, entities: impl IntoIterator) -> impl IntoIterator { + Vec::default() + } + + /// Returns the [EntityMut] of multiple entities, which expose read and write operations for those entities + /// + /// Entity data will be returned in the same order as the provided iterator. + /// This will panic if any of the entities do not exist. Use [World::get_multiple_entities_mut] if you want + /// to check for entity existence instead of implicitly panic-ing. + /// + /// SAFETY: + /// The iterator of entities passed in may not contain any duplicates. + pub unsafe fn multiple_entities_mut(&mut self, entities: impl IntoIterator) -> impl IntoIterator { + Vec::default() + } + + /// Returns the [EntityRef] of multiple entities, which expose read-only operations for those entities + /// + /// As HashSet's are unordered, the entity data is returned in a HashMap, keyed by the corresponding ['Entity'] identifier. + /// + /// If an entity was not found, the HashMap entry for that identifier will be `None`. + pub fn get_multiple_entities(&mut self, entities: HashSet) -> HashMap> { + HashMap::default() + } + + + /// Returns the [EntityMut] of multiple entities, which expose read and write operations for those entities + /// + /// As HashSet's are unordered, the entity data is returned in a HashMap, keyed by the corresponding ['Entity'] identifier. + /// + /// If an entity was not found, the HashMap entry for that identifier will be `None`. + pub fn get_multiple_entities_mut(&mut self, entities: HashSet) -> HashMap> { + HashMap::default() + } + + /// Spawns a new [Entity] and returns a corresponding [EntityMut], which can be used /// to add components to the entity or retrieve its id. /// /// ``` From 6e7600ba1a5b14604d6aafec34cac81a756e9902 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 13 Dec 2021 17:45:15 -0500 Subject: [PATCH 02/49] Preliminary implementation --- crates/bevy_ecs/src/system/query.rs | 34 ++++++++++++---- crates/bevy_ecs/src/world/mod.rs | 61 ++++++++++++++++++++--------- 2 files changed, 69 insertions(+), 26 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index a6983b29bc7e6..dd6cb6725905e 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -8,6 +8,7 @@ use crate::{ world::{Mut, World}, }; use bevy_tasks::TaskPool; +use bevy_utils::{HashMap, HashSet}; use std::{any::TypeId, fmt::Debug}; use thiserror::Error; @@ -643,28 +644,45 @@ where } } - /// Returns the read-only query result for the HashSet of [`Entity`] provided. + /// Returns the read-only query results for the ['HashSet'](bevy_utils::HashSet) of [`Entity`]s provided. /// - /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is - /// returned instead. + /// In case of a nonexisting entity or mismatched component, + /// a [`QueryEntityError`] is returned instead. /// - /// If you need to reduce performance overhead, you can (carefully) call the unsafe `get_unchecked` method repeatedly instead. + /// If you need to reduce performance overhead, you can call the [`get`](Self::get) method repeatedly instead. pub fn get_multiple( &'s self, entities: HashSet, ) -> HashMap::Item, QueryEntityError>> { + let mut entity_map = HashMap::default(); + for entity in entities { + entity_map.insert(entity, self.get(entity)); + } + + entity_map } - /// Returns the query result for the HashSet of [`Entity`] provided. + /// Returns the query results for the ['HashSet'](bevy_utils::HashSet) of [`Entity`]s provided. /// - /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is - /// returned instead. + /// As HashSets are unordered, the entity data is returned in a HashMap, keyed by the corresponding ['Entity'] identifier. + /// + /// In case of a nonexisting entity or mismatched component, + /// a [`QueryEntityError`] is returned instead. /// - /// If you need to reduce performance overhead, you can (carefully) call the unsafe `get_unchecked` method repeatedly instead. + /// If you need to reduce performance overhead, you can (carefully) call the unsafe [`get_unchecked`](Self::get_unchecked) method repeatedly instead. pub fn get_multiple_mut( &mut self, entities: HashSet, ) -> HashMap::Item, QueryEntityError>> { + let mut entity_map = HashMap::default(); + for entity in entities { + // SAFE: the entities are guaranteed to be unique, as they are passed in as a HashSet + unsafe { + entity_map.insert(entity, self.get_unchecked(entity)); + } + } + + entity_map } /// Returns the query result for the given [`Entity`]. diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 572797c285ccf..34b38ef26d1ec 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -17,14 +17,14 @@ use crate::{ storage::{Column, SparseSet, Storages}, system::Resource, }; +use bevy_utils::{HashMap, HashSet}; use std::{ any::TypeId, fmt, mem::ManuallyDrop, - sync::atomic::{AtomicU32, Ordering}, iter::IntoIterator, + sync::atomic::{AtomicU32, Ordering}, }; -use bevy_utils::{HashMap, HashSet}; mod identifier; @@ -314,45 +314,70 @@ impl World { } /// Returns the [EntityRef] of multiple entities, which expose read-only operations for those entities - /// + /// /// Entity data will be returned in the same order as the provided iterator. /// The `entities` argument does not need to be unique, as the underlying data cannot be modified. /// /// This will panic if any of the entities do not exist. Use [World::get_multiple_entities] if you want /// to check for entity existence instead of implicitly panic-ing. - pub fn multiple_entities(&mut self, entities: impl IntoIterator) -> impl IntoIterator { - Vec::default() + pub fn multiple_entities( + &mut self, + entities: impl IntoIterator, + ) -> impl IntoIterator { + entities.into_iter().map(|entity| self.entity(entity)) } /// Returns the [EntityMut] of multiple entities, which expose read and write operations for those entities - /// + /// /// Entity data will be returned in the same order as the provided iterator. /// This will panic if any of the entities do not exist. Use [World::get_multiple_entities_mut] if you want /// to check for entity existence instead of implicitly panic-ing. - /// + /// /// SAFETY: /// The iterator of entities passed in may not contain any duplicates. - pub unsafe fn multiple_entities_mut(&mut self, entities: impl IntoIterator) -> impl IntoIterator { - Vec::default() + pub unsafe fn multiple_entities_mut<'w>( + &'w mut self, + entities: impl IntoIterator, + ) -> impl IntoIterator { + entities.into_iter().map(|entity| { + let location = self.entities.get(entity).expect("Entity not found."); + // SAFE: `entity` exists and `location` is that entity's location + unsafe { EntityMut::new(self, entity, location) } + }) } /// Returns the [EntityRef] of multiple entities, which expose read-only operations for those entities - /// - /// As HashSet's are unordered, the entity data is returned in a HashMap, keyed by the corresponding ['Entity'] identifier. + /// + /// As HashSets are unordered, the entity data is returned in a HashMap, keyed by the corresponding ['Entity'] identifier. /// /// If an entity was not found, the HashMap entry for that identifier will be `None`. - pub fn get_multiple_entities(&mut self, entities: HashSet) -> HashMap> { - HashMap::default() + pub fn get_multiple_entities( + &mut self, + entities: HashSet, + ) -> HashMap> { + // PERF: we can probably preallocate this in a smarter fashion + let mut results = HashMap::default(); + for entity in entities { + results.insert(entity, self.get_entity(entity)); + } + results } - /// Returns the [EntityMut] of multiple entities, which expose read and write operations for those entities - /// - /// As HashSet's are unordered, the entity data is returned in a HashMap, keyed by the corresponding ['Entity'] identifier. + /// + /// As HashSets are unordered, the entity data is returned in a HashMap, keyed by the corresponding ['Entity'] identifier. /// /// If an entity was not found, the HashMap entry for that identifier will be `None`. - pub fn get_multiple_entities_mut(&mut self, entities: HashSet) -> HashMap> { - HashMap::default() + pub fn get_multiple_entities_mut( + &mut self, + entities: HashSet, + ) -> HashMap> { + // PERF: we can probably preallocate this in a smarter fashion + let mut results = HashMap::default(); + for entity in entities { + results.insert(entity, self.get_entity_mut(entity)); + } + results } /// Spawns a new [Entity] and returns a corresponding [EntityMut], which can be used From 6b81770b6d597e3cd9783468b6e1d89e6ec0435c Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 14 Dec 2021 21:59:28 -0500 Subject: [PATCH 03/49] Removed World API Getting multiple EntityMut is fundamentally impossible due to the ability to add and remove components. Fixing this is out of scope for this PR. --- crates/bevy_ecs/src/world/mod.rs | 69 +------------------------------- 1 file changed, 1 insertion(+), 68 deletions(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 34b38ef26d1ec..e062ddf8359e1 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -17,7 +17,7 @@ use crate::{ storage::{Column, SparseSet, Storages}, system::Resource, }; -use bevy_utils::{HashMap, HashSet}; + use std::{ any::TypeId, fmt, @@ -313,73 +313,6 @@ impl World { Some(unsafe { EntityMut::new(self, entity, location) }) } - /// Returns the [EntityRef] of multiple entities, which expose read-only operations for those entities - /// - /// Entity data will be returned in the same order as the provided iterator. - /// The `entities` argument does not need to be unique, as the underlying data cannot be modified. - /// - /// This will panic if any of the entities do not exist. Use [World::get_multiple_entities] if you want - /// to check for entity existence instead of implicitly panic-ing. - pub fn multiple_entities( - &mut self, - entities: impl IntoIterator, - ) -> impl IntoIterator { - entities.into_iter().map(|entity| self.entity(entity)) - } - - /// Returns the [EntityMut] of multiple entities, which expose read and write operations for those entities - /// - /// Entity data will be returned in the same order as the provided iterator. - /// This will panic if any of the entities do not exist. Use [World::get_multiple_entities_mut] if you want - /// to check for entity existence instead of implicitly panic-ing. - /// - /// SAFETY: - /// The iterator of entities passed in may not contain any duplicates. - pub unsafe fn multiple_entities_mut<'w>( - &'w mut self, - entities: impl IntoIterator, - ) -> impl IntoIterator { - entities.into_iter().map(|entity| { - let location = self.entities.get(entity).expect("Entity not found."); - // SAFE: `entity` exists and `location` is that entity's location - unsafe { EntityMut::new(self, entity, location) } - }) - } - - /// Returns the [EntityRef] of multiple entities, which expose read-only operations for those entities - /// - /// As HashSets are unordered, the entity data is returned in a HashMap, keyed by the corresponding ['Entity'] identifier. - /// - /// If an entity was not found, the HashMap entry for that identifier will be `None`. - pub fn get_multiple_entities( - &mut self, - entities: HashSet, - ) -> HashMap> { - // PERF: we can probably preallocate this in a smarter fashion - let mut results = HashMap::default(); - for entity in entities { - results.insert(entity, self.get_entity(entity)); - } - results - } - - /// Returns the [EntityMut] of multiple entities, which expose read and write operations for those entities - /// - /// As HashSets are unordered, the entity data is returned in a HashMap, keyed by the corresponding ['Entity'] identifier. - /// - /// If an entity was not found, the HashMap entry for that identifier will be `None`. - pub fn get_multiple_entities_mut( - &mut self, - entities: HashSet, - ) -> HashMap> { - // PERF: we can probably preallocate this in a smarter fashion - let mut results = HashMap::default(); - for entity in entities { - results.insert(entity, self.get_entity_mut(entity)); - } - results - } - /// Spawns a new [Entity] and returns a corresponding [EntityMut], which can be used /// to add components to the entity or retrieve its id. /// From ad9200c9ceee35d24c8784b17bd7fbd5084ce78e Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 14 Dec 2021 22:18:26 -0500 Subject: [PATCH 04/49] Added doc tests --- crates/bevy_ecs/src/system/query.rs | 41 +++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index dd6cb6725905e..2d629e23a21bd 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -650,6 +650,23 @@ where /// a [`QueryEntityError`] is returned instead. /// /// If you need to reduce performance overhead, you can call the [`get`](Self::get) method repeatedly instead. + /// + /// # Example + /// ```rust + /// # use bevy_ecs::prelude::*; + /// #[derive(Component, PartialEq)] + /// struct Name(String); + /// + /// let world = World::new(); + /// let entity_1 = world.spawn().insert(Name("Alan")).id(); + /// let entity_2 = world.spawn().insert(Name("Bob")).id(); + /// let entities = [entity_1, entity_2]; + /// + /// let name_query = world.query::(); + /// let name_map = name_query.get_multiple(HashSet::from_iter(entities)); + /// assert_eq!(*name_map.get(entity_1).unwrap(), Name("Alan")); + /// assert_eq!(*name_map.get(entity_2).unwrap(), Name("Bob")); + /// ``` pub fn get_multiple( &'s self, entities: HashSet, @@ -670,6 +687,30 @@ where /// a [`QueryEntityError`] is returned instead. /// /// If you need to reduce performance overhead, you can (carefully) call the unsafe [`get_unchecked`](Self::get_unchecked) method repeatedly instead. + /// + /// # Example + /// ```rust + /// # use bevy_ecs::prelude::*; + /// #[derive(Component, PartialEq)] + /// struct Name(String); + /// + /// let world = World::new(); + /// let entity_1 = world.spawn().insert(Name("Alan")).id(); + /// let entity_2 = world.spawn().insert(Name("Bob")).id(); + /// let entities = [entity_1, entity_2]; + /// + /// let name_query = world.query::(); + /// let mut name_map = name_query.get_multiple_mut(HashSet::from_iter(entities)); + /// + /// let mut entity_1_name = name_map.get_mut(entity_1).unwrap(); + /// let mut entity_2_name = name_map.get_mut(entity_1).unwrap(); + /// + /// *entity_1_name = Name("Alice"); + /// *entity_2_name = Name("Brigitte"); + /// + /// assert_eq!(*name_map.get(entity_1).unwrap(), Name("Alice")); + /// assert_eq!(*name_map.get(entity_2).unwrap(), Name("Brigitte")); + /// ``` pub fn get_multiple_mut( &mut self, entities: HashSet, From 3707686a7ffc959aa444f6fbfdc3d49cd768cead Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 14 Dec 2021 23:26:16 -0500 Subject: [PATCH 05/49] Improved API design --- crates/bevy_ecs/src/system/query.rs | 85 ++++++++++++++--------------- 1 file changed, 40 insertions(+), 45 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 2d629e23a21bd..4d06f679f2dcf 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -8,8 +8,7 @@ use crate::{ world::{Mut, World}, }; use bevy_tasks::TaskPool; -use bevy_utils::{HashMap, HashSet}; -use std::{any::TypeId, fmt::Debug}; +use std::{any::TypeId, collections::BTreeSet, fmt::Debug}; use thiserror::Error; /// Provides scoped access to components in a [`World`]. @@ -644,8 +643,9 @@ where } } - /// Returns the read-only query results for the ['HashSet'](bevy_utils::HashSet) of [`Entity`]s provided. + /// Returns the read-only query results for the iterator of [`Entity`]s provided. /// + /// These values follow the order of your input iterator (if any). /// In case of a nonexisting entity or mismatched component, /// a [`QueryEntityError`] is returned instead. /// @@ -655,37 +655,33 @@ where /// ```rust /// # use bevy_ecs::prelude::*; /// #[derive(Component, PartialEq)] - /// struct Name(String); + /// struct A(u64); /// /// let world = World::new(); - /// let entity_1 = world.spawn().insert(Name("Alan")).id(); - /// let entity_2 = world.spawn().insert(Name("Bob")).id(); - /// let entities = [entity_1, entity_2]; - /// - /// let name_query = world.query::(); - /// let name_map = name_query.get_multiple(HashSet::from_iter(entities)); - /// assert_eq!(*name_map.get(entity_1).unwrap(), Name("Alan")); - /// assert_eq!(*name_map.get(entity_2).unwrap(), Name("Bob")); + /// let entity_1 = world.spawn().insert(A(1)).id(); + /// let entity_2 = world.spawn().insert(A(2)).id(); + /// + /// let a_query = world.query::(); + /// let a_iterator = name_query.get_multiple([entity_1, entity_2]); + /// assert_eq!(a_iterator.next().unwrap(), A(1)); + /// assert_eq!(a_iterator.next().unwrap(), A(2)); /// ``` + #[inline] pub fn get_multiple( &'s self, - entities: HashSet, - ) -> HashMap::Item, QueryEntityError>> { - let mut entity_map = HashMap::default(); - for entity in entities { - entity_map.insert(entity, self.get(entity)); - } - - entity_map + entities: impl IntoIterator, + ) -> impl Iterator::Item, QueryEntityError>> { + entities.into_iter().map(|entity| self.get(entity)) } - /// Returns the query results for the ['HashSet'](bevy_utils::HashSet) of [`Entity`]s provided. - /// - /// As HashSets are unordered, the entity data is returned in a HashMap, keyed by the corresponding ['Entity'] identifier. + /// Returns the query results for the ['BTreeSet'](std::collections::BTreeSet) of [`Entity`]s provided. /// + /// These values are returned as a (Enity, COMPONENTS) tuple, following the order of your input iterator. /// In case of a nonexisting entity or mismatched component, /// a [`QueryEntityError`] is returned instead. /// + /// BTreeSets are used as both ordered and unique, and an ordered stream of component values will be produced, in the same order as was provided in the BTreeSet + /// /// If you need to reduce performance overhead, you can (carefully) call the unsafe [`get_unchecked`](Self::get_unchecked) method repeatedly instead. /// /// # Example @@ -695,35 +691,34 @@ where /// struct Name(String); /// /// let world = World::new(); - /// let entity_1 = world.spawn().insert(Name("Alan")).id(); - /// let entity_2 = world.spawn().insert(Name("Bob")).id(); - /// let entities = [entity_1, entity_2]; - /// - /// let name_query = world.query::(); - /// let mut name_map = name_query.get_multiple_mut(HashSet::from_iter(entities)); + /// let entity_1 = world.spawn().insert(A(1)).id(); + /// let entity_2 = world.spawn().insert(A(2)).id(); + /// let entity_3 = world.spawn().insert(A(3)).id(); /// - /// let mut entity_1_name = name_map.get_mut(entity_1).unwrap(); - /// let mut entity_2_name = name_map.get_mut(entity_1).unwrap(); + /// let a_query = world.query::(); + /// let a_iterator = name_query.get_multiple_mut(BTreeSet::from_iter([entity_1, entity_3])); + /// let mut a_1 = a_iterator.next().unwrap(); + /// let mut a_3 = a_iterator.next().unwrap(); /// - /// *entity_1_name = Name("Alice"); - /// *entity_2_name = Name("Brigitte"); + /// *a_1 = A(11); + /// *a_3 = A(33); /// - /// assert_eq!(*name_map.get(entity_1).unwrap(), Name("Alice")); - /// assert_eq!(*name_map.get(entity_2).unwrap(), Name("Brigitte")); + /// assert_eq!(world.get::(entity_1).unwrap(), A(11)); + /// assert_eq!(world.get::(entity_2).unwrap(), A(2)); + /// assert_eq!(world.get::(entity_2).unwrap(), A(33)); /// ``` + #[inline] pub fn get_multiple_mut( &mut self, - entities: HashSet, - ) -> HashMap::Item, QueryEntityError>> { - let mut entity_map = HashMap::default(); - for entity in entities { - // SAFE: the entities are guaranteed to be unique, as they are passed in as a HashSet - unsafe { - entity_map.insert(entity, self.get_unchecked(entity)); - } + entities: BTreeSet, + ) -> impl Iterator::Item, QueryEntityError>> { + // SAFE: the entities supplied are guaranteed to be unique, + // as they are passed in as a BTreeSet, which enforces uniqueness + unsafe { + entities + .into_iter() + .map(|entity| self.get_unchecked(entity)) } - - entity_map } /// Returns the query result for the given [`Entity`]. From 2a10fc2c73aa7e14836eb12e089d8b77e1e5cc99 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 14 Dec 2021 23:36:33 -0500 Subject: [PATCH 06/49] Improved docs --- crates/bevy_ecs/src/system/query.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 4d06f679f2dcf..b80c2bba6acd8 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -649,7 +649,8 @@ where /// In case of a nonexisting entity or mismatched component, /// a [`QueryEntityError`] is returned instead. /// - /// If you need to reduce performance overhead, you can call the [`get`](Self::get) method repeatedly instead. + /// If you need to verify the identity of each item returned, + /// add [`Entity`] to your [`Query`]. /// /// # Example /// ```rust @@ -680,9 +681,14 @@ where /// In case of a nonexisting entity or mismatched component, /// a [`QueryEntityError`] is returned instead. /// - /// BTreeSets are used as both ordered and unique, and an ordered stream of component values will be produced, in the same order as was provided in the BTreeSet + /// BTreeSets are used as both ordered and unique, and an ordered stream of component values will be produced, + /// in the same order as was provided in the BTreeSet. /// - /// If you need to reduce performance overhead, you can (carefully) call the unsafe [`get_unchecked`](Self::get_unchecked) method repeatedly instead. + /// If you need to verify the identity of each item returned, + /// add [`Entity`] to your [`Query`]. + /// + /// If you absolutely cannot afford the overhead of verifying uniqueness in this way, + /// you can (carefully) call the unsafe [`get_unchecked`](Self::get_unchecked) method repeatedly instead. /// /// # Example /// ```rust From 36df4e5a644f9e2422e0da554d860a5b38ecc6cd Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Tue, 14 Dec 2021 23:36:41 -0500 Subject: [PATCH 07/49] Remove PR noise --- crates/bevy_ecs/src/world/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index e062ddf8359e1..68a0ea45ed4c0 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -17,7 +17,6 @@ use crate::{ storage::{Column, SparseSet, Storages}, system::Resource, }; - use std::{ any::TypeId, fmt, From b7a1cbef6059c3bd2dc27e967f0c6f3baa34744a Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 15 Dec 2021 13:23:45 -0500 Subject: [PATCH 08/49] Typo fix Co-authored-by: MrGVSV <49806985+MrGVSV@users.noreply.github.com> --- crates/bevy_ecs/src/system/query.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index b80c2bba6acd8..40bf63bb5f97a 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -663,7 +663,7 @@ where /// let entity_2 = world.spawn().insert(A(2)).id(); /// /// let a_query = world.query::(); - /// let a_iterator = name_query.get_multiple([entity_1, entity_2]); + /// let a_iterator = a_query.get_multiple([entity_1, entity_2]); /// assert_eq!(a_iterator.next().unwrap(), A(1)); /// assert_eq!(a_iterator.next().unwrap(), A(2)); /// ``` From 499f804bfc20ec102977650d70a8ccb7880bd63c Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 15 Dec 2021 13:23:54 -0500 Subject: [PATCH 09/49] Typo fix Co-authored-by: MrGVSV <49806985+MrGVSV@users.noreply.github.com> --- crates/bevy_ecs/src/system/query.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 40bf63bb5f97a..2c14f5e961791 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -702,7 +702,7 @@ where /// let entity_3 = world.spawn().insert(A(3)).id(); /// /// let a_query = world.query::(); - /// let a_iterator = name_query.get_multiple_mut(BTreeSet::from_iter([entity_1, entity_3])); + /// let a_iterator = a_query.get_multiple_mut(BTreeSet::from_iter([entity_1, entity_3])); /// let mut a_1 = a_iterator.next().unwrap(); /// let mut a_3 = a_iterator.next().unwrap(); /// From 1a5895c9ab67cd257cc0e889e355e1b431faa9c8 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 15 Dec 2021 13:30:42 -0500 Subject: [PATCH 10/49] Minor docs fix --- crates/bevy_ecs/src/system/query.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 2c14f5e961791..d4b6a033e09c3 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -677,12 +677,10 @@ where /// Returns the query results for the ['BTreeSet'](std::collections::BTreeSet) of [`Entity`]s provided. /// - /// These values are returned as a (Enity, COMPONENTS) tuple, following the order of your input iterator. - /// In case of a nonexisting entity or mismatched component, - /// a [`QueryEntityError`] is returned instead. - /// /// BTreeSets are used as both ordered and unique, and an ordered stream of component values will be produced, /// in the same order as was provided in the BTreeSet. + /// In case of a nonexisting entity or mismatched component, + /// a [`QueryEntityError`] is returned instead. /// /// If you need to verify the identity of each item returned, /// add [`Entity`] to your [`Query`]. From b13d970ef45c2f3cbe305fcf064f8b9aecd71f6d Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 15 Dec 2021 13:51:17 -0500 Subject: [PATCH 11/49] Add get_pair and get_pair_mut --- crates/bevy_ecs/src/system/query.rs | 85 +++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index d4b6a033e09c3..3c2c9ab77dc0c 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -233,6 +233,10 @@ use thiserror::Error; /// If you have an [`Entity`] ID, you can use the [`get`](Self::get) or /// [`get_mut`](Self::get_mut) methods to access the query result for that particular entity. /// +/// To access the data of exactly two specific entities at once, use [`get_pair`](Self::get_pair), +/// or ['get_pair_mut'](Self::get_pair_mut). +/// This is a surprisingly common pattern, and the API provided is both faster and more convenient than ['get_multiple_mut'](Self::get_multiple_mut). +/// /// If you require access to the data of multiple entities at once, /// you can use the ['get_multiple'](Self::get_multiple) or ['get_multiple_mut'](Self::get_multiple_mut) methods. /// @@ -643,6 +647,81 @@ where } } + /// Returns the read-only query result for the given pair of [`Entity`]s. + /// + /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is + /// returned instead. + /// + /// If you need to get the data from more than two specific entities at once, + /// use ['get_multiple'](Self::get_multiple) + /// + /// # Example + /// ```rust + /// # use bevy::ecs::prelude::*; + /// #[derive(Component, PartialEq)] + /// struct Name(String); + /// + /// let world = World::new(); + /// let entity_1 = world.spawn().insert(Name("Ferris")).id(); + /// let entity_2 = world.spawn().insert(Name("Cart")).id(); + /// + /// let name_query = world.query::(); + /// let (entity_1_name, entity_2_name) = name_query.get_pair(entity_1, entity_2); + /// asserteq!(entity_1_name, Name("Ferris")); + /// asserteq!(entity_2_name, Name("Cart")); + #[inline] + pub fn get_pair( + &'s self, + entity_0: Entity, + entity_1: Entity, + ) -> ( + Result<>::Item, QueryEntityError>, + Result<>::Item, QueryEntityError>, + ) { + (self.get(entity_0), self.get(entity_1)) + } + + /// Returns the query result for the given pair of [`Entity`]s. + /// + /// These entities must be unique, and this method will panic if they are not. + /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is + /// returned instead. + /// + /// If you need to get the data from more than two specific entities at once, + /// use ['get_multiple_mut'](Self::get_multiple) + /// + /// # Example + /// ```rust + /// # use bevy::ecs::prelude::*; + /// #[derive(Component, PartialEq)] + /// struct Name(String); + /// + /// let world = World::new(); + /// let entity_1 = world.spawn().insert(Name("Alice")).id(); + /// let entity_2 = world.spawn().insert(Name("Bob")).id(); + /// + /// let name_query = world.query::(); + /// let (mut entity_1_name, mut entity_2_name) = name_query.get_pair_mut(entity_1, entity_2); + /// + /// *entity_1_name = Name("Alan"); + /// *entity_2_name = Name("Brigitte"); + /// + /// asserteq!(entity_1_name, Name("Alan")); + /// asserteq!(entity_2_name, Name("Brigitte")); + #[inline] + pub fn get_pair_mut( + &'s self, + entity_0: Entity, + entity_1: Entity, + ) -> ( + Result<>::Item, QueryEntityError>, + Result<>::Item, QueryEntityError>, + ) { + assert!(entity_0 != entity_1); + // SAFE: the two entities are distinct + unsafe { (self.get_unchecked(entity_0), self.get_unchecked(entity_1)) } + } + /// Returns the read-only query results for the iterator of [`Entity`]s provided. /// /// These values follow the order of your input iterator (if any). @@ -652,6 +731,9 @@ where /// If you need to verify the identity of each item returned, /// add [`Entity`] to your [`Query`]. /// + /// If you need to get the data from exactly two specific entities at once, + /// use ['get_pair'](Self::get_pair). + /// /// # Example /// ```rust /// # use bevy_ecs::prelude::*; @@ -688,6 +770,9 @@ where /// If you absolutely cannot afford the overhead of verifying uniqueness in this way, /// you can (carefully) call the unsafe [`get_unchecked`](Self::get_unchecked) method repeatedly instead. /// + /// If you need to get the data from exactly two specific entities at once, + /// use ['get_pair_mut'](Self::get_pair_mut). + /// /// # Example /// ```rust /// # use bevy_ecs::prelude::*; From bfa1d84d444b00b8bb543b5c31d818f5fe87fcb4 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 15 Dec 2021 13:52:52 -0500 Subject: [PATCH 12/49] Fix bugs in doc tests --- crates/bevy_ecs/src/system/query.rs | 57 +++++++++++++++-------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 3c2c9ab77dc0c..0a56a395a20dd 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -657,18 +657,18 @@ where /// /// # Example /// ```rust - /// # use bevy::ecs::prelude::*; - /// #[derive(Component, PartialEq)] - /// struct Name(String); + /// # use bevy_ecs::prelude::*; + /// #[derive(Component, PartialEq, Debug)] + /// struct Life(u64); /// /// let world = World::new(); - /// let entity_1 = world.spawn().insert(Name("Ferris")).id(); - /// let entity_2 = world.spawn().insert(Name("Cart")).id(); + /// let entity_1 = world.spawn().insert(Life(1)).id(); + /// let entity_2 = world.spawn().insert(Life(2)).id(); /// - /// let name_query = world.query::(); - /// let (entity_1_name, entity_2_name) = name_query.get_pair(entity_1, entity_2); - /// asserteq!(entity_1_name, Name("Ferris")); - /// asserteq!(entity_2_name, Name("Cart")); + /// let life_query = world.query::<&Life>(); + /// let (entity_1_life, entity_2_life) = life_query.get_pair(entity_1, entity_2); + /// assert_eq!(entity_1_life, Life(1)); + /// assert_eq!(entity_2_life, Life(2)); #[inline] pub fn get_pair( &'s self, @@ -692,22 +692,22 @@ where /// /// # Example /// ```rust - /// # use bevy::ecs::prelude::*; - /// #[derive(Component, PartialEq)] - /// struct Name(String); + /// # use bevy_ecs::prelude::*; + /// #[derive(Component, PartialEq, Debug)] + /// struct Life(u64); /// /// let world = World::new(); - /// let entity_1 = world.spawn().insert(Name("Alice")).id(); - /// let entity_2 = world.spawn().insert(Name("Bob")).id(); + /// let entity_1 = world.spawn().insert(Life(1)).id(); + /// let entity_2 = world.spawn().insert(Life(2)).id(); /// - /// let name_query = world.query::(); - /// let (mut entity_1_name, mut entity_2_name) = name_query.get_pair_mut(entity_1, entity_2); + /// let life_query = world.query::<&mut Life>(); + /// let (mut entity_1_life, mut entity_2_life) = life_query.get_pair_mut(entity_1, entity_2); /// - /// *entity_1_name = Name("Alan"); - /// *entity_2_name = Name("Brigitte"); + /// *entity_1_life = Life(0); + /// *entity_2_life = Life(100); /// - /// asserteq!(entity_1_name, Name("Alan")); - /// asserteq!(entity_2_name, Name("Brigitte")); + /// assert_eq!(entity_1_life, Life(0)); + /// assert_eq!(entity_2_life, Life(100)); #[inline] pub fn get_pair_mut( &'s self, @@ -737,14 +737,14 @@ where /// # Example /// ```rust /// # use bevy_ecs::prelude::*; - /// #[derive(Component, PartialEq)] + /// #[derive(Component, PartialEq, Debug)] /// struct A(u64); /// /// let world = World::new(); /// let entity_1 = world.spawn().insert(A(1)).id(); /// let entity_2 = world.spawn().insert(A(2)).id(); /// - /// let a_query = world.query::(); + /// let a_query = world.query::<&A>(); /// let a_iterator = a_query.get_multiple([entity_1, entity_2]); /// assert_eq!(a_iterator.next().unwrap(), A(1)); /// assert_eq!(a_iterator.next().unwrap(), A(2)); @@ -776,15 +776,16 @@ where /// # Example /// ```rust /// # use bevy_ecs::prelude::*; - /// #[derive(Component, PartialEq)] - /// struct Name(String); + /// use std::collections::BTreeSet; + /// #[derive(Component, PartialEq, Debug)] + /// struct A(u64); /// /// let world = World::new(); /// let entity_1 = world.spawn().insert(A(1)).id(); /// let entity_2 = world.spawn().insert(A(2)).id(); /// let entity_3 = world.spawn().insert(A(3)).id(); /// - /// let a_query = world.query::(); + /// let a_query = world.query::<&mut A>(); /// let a_iterator = a_query.get_multiple_mut(BTreeSet::from_iter([entity_1, entity_3])); /// let mut a_1 = a_iterator.next().unwrap(); /// let mut a_3 = a_iterator.next().unwrap(); @@ -792,9 +793,9 @@ where /// *a_1 = A(11); /// *a_3 = A(33); /// - /// assert_eq!(world.get::(entity_1).unwrap(), A(11)); - /// assert_eq!(world.get::(entity_2).unwrap(), A(2)); - /// assert_eq!(world.get::(entity_2).unwrap(), A(33)); + /// assert_eq!(*world.get::(entity_1).unwrap(), A(11)); + /// assert_eq!(*world.get::(entity_2).unwrap(), A(2)); + /// assert_eq!(*world.get::(entity_2).unwrap(), A(33)); /// ``` #[inline] pub fn get_multiple_mut( From d6173f1f042b484a883e94b53357fcb5edb2cc4e Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 15 Dec 2021 16:01:13 -0500 Subject: [PATCH 13/49] Add `Query::from_state` helper method --- crates/bevy_ecs/src/system/query.rs | 47 ++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 0a56a395a20dd..a80d6c2b264ed 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -282,6 +282,40 @@ where } } + /// Creates a new [`Query`] from a [`QueryState`] + /// + /// [`Query`] is a simply a borrow of [`QueryState`], + /// but when working directly with the [`World`] + /// we need to store the query's internal data somewhere, and so cannot return a [`Query`] directly. + /// + /// As queries can modify the entity-component data in ways that could conflict dangerously, + /// this method requires a mutable reference to the [`World`], + /// ensuring only one query is active at once. + /// This can be quite restrictive: consider using ['SystemState::new'](bevy_ecs::system::SystemState::new) if this is a problem. + /// + /// # Example + /// ```rust + /// # use bevy_ecs::prelude; + /// #[derive(Component)] + /// struct PowerLevel(u64); + /// + /// let mut world = World::new(); + /// world.spawn().insert(PowerLevel(9001)); + /// + /// let query_state = world.query::<&PowerLevel>(); + /// let query = Query::from_state(&mut world, query_state); + /// let power_level = query.single(); + /// + /// assert!(power_level.0 > 9000); + /// ``` + pub fn from_state(world: &'w mut World, state: &'s QueryState) -> Self { + let last_change_tick = world.last_change_tick(); + let change_tick = world.change_tick(); + + // SAFE: the `World` is borrowed mutably, so no other queries can have simultaneous access + unsafe { Self::new(world, state, last_change_tick, change_tick) } + } + /// Returns an [`Iterator`] over the query results. /// /// This can only return immutable data (mutable data will be cast to an immutable form). @@ -665,8 +699,10 @@ where /// let entity_1 = world.spawn().insert(Life(1)).id(); /// let entity_2 = world.spawn().insert(Life(2)).id(); /// - /// let life_query = world.query::<&Life>(); + /// let query_state = world.query::<&Life>(); + /// let life_query = Query::from_state(&mut world, query_state); /// let (entity_1_life, entity_2_life) = life_query.get_pair(entity_1, entity_2); + /// /// assert_eq!(entity_1_life, Life(1)); /// assert_eq!(entity_2_life, Life(2)); #[inline] @@ -700,7 +736,8 @@ where /// let entity_1 = world.spawn().insert(Life(1)).id(); /// let entity_2 = world.spawn().insert(Life(2)).id(); /// - /// let life_query = world.query::<&mut Life>(); + /// let query_state = world.query::<&mut Life>(); + /// let life_query = Query::from_state(&mut world, query_state); /// let (mut entity_1_life, mut entity_2_life) = life_query.get_pair_mut(entity_1, entity_2); /// /// *entity_1_life = Life(0); @@ -744,7 +781,8 @@ where /// let entity_1 = world.spawn().insert(A(1)).id(); /// let entity_2 = world.spawn().insert(A(2)).id(); /// - /// let a_query = world.query::<&A>(); + /// let query_state = world.query::<&A>(); + /// let a_query = Query::from_state(&mut world, query_state); /// let a_iterator = a_query.get_multiple([entity_1, entity_2]); /// assert_eq!(a_iterator.next().unwrap(), A(1)); /// assert_eq!(a_iterator.next().unwrap(), A(2)); @@ -785,7 +823,8 @@ where /// let entity_2 = world.spawn().insert(A(2)).id(); /// let entity_3 = world.spawn().insert(A(3)).id(); /// - /// let a_query = world.query::<&mut A>(); + /// let query_state = world.query::<&mut A>(); + /// let a_query = Query::from_state(&mut world, query_state); /// let a_iterator = a_query.get_multiple_mut(BTreeSet::from_iter([entity_1, entity_3])); /// let mut a_1 = a_iterator.next().unwrap(); /// let mut a_3 = a_iterator.next().unwrap(); From 687b4fc5b5a80c6593a7157fa7222bdbece2c8af Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 15 Dec 2021 16:19:03 -0500 Subject: [PATCH 14/49] Better error-handling for get_pair --- crates/bevy_ecs/src/query/state.rs | 2 + crates/bevy_ecs/src/system/query.rs | 59 ++++++++++++++++++----------- 2 files changed, 39 insertions(+), 22 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index f675de4d89a1d..9abff9da9e635 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -739,4 +739,6 @@ pub enum QueryEntityError { QueryDoesNotMatch, #[error("The requested entity does not exist.")] NoSuchEntity, + #[error("The same entity was accessed mutably more than once.")] + AliasedMutability, } diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index a80d6c2b264ed..c110e8a7d91c3 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -303,7 +303,7 @@ where /// world.spawn().insert(PowerLevel(9001)); /// /// let query_state = world.query::<&PowerLevel>(); - /// let query = Query::from_state(&mut world, query_state); + /// let query = Query::from_state(&mut world, &query_state); /// let power_level = query.single(); /// /// assert!(power_level.0 > 9000); @@ -700,8 +700,8 @@ where /// let entity_2 = world.spawn().insert(Life(2)).id(); /// /// let query_state = world.query::<&Life>(); - /// let life_query = Query::from_state(&mut world, query_state); - /// let (entity_1_life, entity_2_life) = life_query.get_pair(entity_1, entity_2); + /// let life_query = Query::from_state(&mut world, &query_state); + /// let (entity_1_life, entity_2_life) = life_query.get_pair(entity_1, entity_2).unwrap(); /// /// assert_eq!(entity_1_life, Life(1)); /// assert_eq!(entity_2_life, Life(2)); @@ -710,18 +710,23 @@ where &'s self, entity_0: Entity, entity_1: Entity, - ) -> ( - Result<>::Item, QueryEntityError>, - Result<>::Item, QueryEntityError>, - ) { - (self.get(entity_0), self.get(entity_1)) + ) -> Result< + ( + >::Item, + >::Item, + ), + QueryEntityError, + > { + let data_0 = self.get(entity_0)?; + let data_1 = self.get(entity_1)?; + + Ok((data_0, data_1)) } /// Returns the query result for the given pair of [`Entity`]s. /// - /// These entities must be unique, and this method will panic if they are not. - /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is - /// returned instead. + /// In case of a nonunique entities, a nonexisting entity or mismatched component, + /// a [`QueryEntityError`] is returned instead. /// /// If you need to get the data from more than two specific entities at once, /// use ['get_multiple_mut'](Self::get_multiple) @@ -737,8 +742,8 @@ where /// let entity_2 = world.spawn().insert(Life(2)).id(); /// /// let query_state = world.query::<&mut Life>(); - /// let life_query = Query::from_state(&mut world, query_state); - /// let (mut entity_1_life, mut entity_2_life) = life_query.get_pair_mut(entity_1, entity_2); + /// let life_query = Query::from_state(&mut world, &query_state); + /// let (mut entity_1_life, mut entity_2_life) = life_query.get_pair_mut(entity_1, entity_2).unwrap(); /// /// *entity_1_life = Life(0); /// *entity_2_life = Life(100); @@ -750,13 +755,23 @@ where &'s self, entity_0: Entity, entity_1: Entity, - ) -> ( - Result<>::Item, QueryEntityError>, - Result<>::Item, QueryEntityError>, - ) { - assert!(entity_0 != entity_1); - // SAFE: the two entities are distinct - unsafe { (self.get_unchecked(entity_0), self.get_unchecked(entity_1)) } + ) -> Result< + ( + >::Item, + >::Item, + ), + QueryEntityError, + > { + if entity_0 == entity_1 { + return Err(QueryEntityError::AliasedMutability); + } + + // SAFE: entities do not match + unsafe { + let data_0 = self.get_unchecked(entity_0)?; + let data_1 = self.get_unchecked(entity_1)?; + Ok((data_0, data_1)) + } } /// Returns the read-only query results for the iterator of [`Entity`]s provided. @@ -782,7 +797,7 @@ where /// let entity_2 = world.spawn().insert(A(2)).id(); /// /// let query_state = world.query::<&A>(); - /// let a_query = Query::from_state(&mut world, query_state); + /// let a_query = Query::from_state(&mut world, &query_state); /// let a_iterator = a_query.get_multiple([entity_1, entity_2]); /// assert_eq!(a_iterator.next().unwrap(), A(1)); /// assert_eq!(a_iterator.next().unwrap(), A(2)); @@ -824,7 +839,7 @@ where /// let entity_3 = world.spawn().insert(A(3)).id(); /// /// let query_state = world.query::<&mut A>(); - /// let a_query = Query::from_state(&mut world, query_state); + /// let a_query = Query::from_state(&mut world, &query_state); /// let a_iterator = a_query.get_multiple_mut(BTreeSet::from_iter([entity_1, entity_3])); /// let mut a_1 = a_iterator.next().unwrap(); /// let mut a_3 = a_iterator.next().unwrap(); From 04288bacee2ed12b7eb33121e63cf309f21ab504 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 15 Dec 2021 16:47:26 -0500 Subject: [PATCH 15/49] Allow arbitrary iterators for get_multiple_mut The BTreeSet ordering was wrong (thanks @Ixentus), so we're stuck checking for uniqueness on our own. --- crates/bevy_ecs/src/system/query.rs | 51 +++++++++++++++++++---------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index c110e8a7d91c3..93a4bd82c4d66 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -8,7 +8,8 @@ use crate::{ world::{Mut, World}, }; use bevy_tasks::TaskPool; -use std::{any::TypeId, collections::BTreeSet, fmt::Debug}; +use bevy_utils::{AHashExt, HashSet}; +use std::{any::TypeId, fmt::Debug}; use thiserror::Error; /// Provides scoped access to components in a [`World`]. @@ -725,7 +726,7 @@ where /// Returns the query result for the given pair of [`Entity`]s. /// - /// In case of a nonunique entities, a nonexisting entity or mismatched component, + /// In case of nonunique entities, a nonexisting entity or mismatched component, /// a [`QueryEntityError`] is returned instead. /// /// If you need to get the data from more than two specific entities at once, @@ -812,9 +813,8 @@ where /// Returns the query results for the ['BTreeSet'](std::collections::BTreeSet) of [`Entity`]s provided. /// - /// BTreeSets are used as both ordered and unique, and an ordered stream of component values will be produced, - /// in the same order as was provided in the BTreeSet. - /// In case of a nonexisting entity or mismatched component, + /// These values follow the order of your input iterator (if any). + /// In case of nonunique entities, a nonexisting entity or a mismatched component, /// a [`QueryEntityError`] is returned instead. /// /// If you need to verify the identity of each item returned, @@ -829,7 +829,6 @@ where /// # Example /// ```rust /// # use bevy_ecs::prelude::*; - /// use std::collections::BTreeSet; /// #[derive(Component, PartialEq, Debug)] /// struct A(u64); /// @@ -840,9 +839,9 @@ where /// /// let query_state = world.query::<&mut A>(); /// let a_query = Query::from_state(&mut world, &query_state); - /// let a_iterator = a_query.get_multiple_mut(BTreeSet::from_iter([entity_1, entity_3])); - /// let mut a_1 = a_iterator.next().unwrap(); - /// let mut a_3 = a_iterator.next().unwrap(); + /// let a_iterator = a_query.get_multiple_mut([entity_1, entity_3]).unwrap(); + /// let mut a_1 = a_iterator.next(); + /// let mut a_3 = a_iterator.next(); /// /// *a_1 = A(11); /// *a_3 = A(33); @@ -854,15 +853,33 @@ where #[inline] pub fn get_multiple_mut( &mut self, - entities: BTreeSet, - ) -> impl Iterator::Item, QueryEntityError>> { - // SAFE: the entities supplied are guaranteed to be unique, - // as they are passed in as a BTreeSet, which enforces uniqueness - unsafe { - entities - .into_iter() - .map(|entity| self.get_unchecked(entity)) + entities: impl IntoIterator, + ) -> Result::Item>, QueryEntityError> { + let entities_iter = entities.into_iter(); + + // Preallocating the hash-set used to check uniqueness based on the expected maximum amount of space + let (lower_bound, maybe_upper_bound) = entities_iter.size_hint(); + let best_bound = if let Some(upper_bound) = maybe_upper_bound { + upper_bound + } else { + lower_bound + }; + + let mut entities_seen = HashSet::with_capacity(best_bound * std::mem::size_of::()); + // PERF: we could estimate the capacity and preallocate + // based on the size of the data in the query and the number of items in the iter + let mut data = Vec::new(); + for entity in entities_iter { + if entities_seen.contains(&entity) { + return Err(QueryEntityError::AliasedMutability); + } else { + entities_seen.insert(entity); + unsafe { + data.push(self.get_unchecked(entity)?); + } + } } + Ok(data.into_iter()) } /// Returns the query result for the given [`Entity`]. From c42bd7072ddcbf09b3e7977a5ccebafa61949c32 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 15 Dec 2021 16:56:53 -0500 Subject: [PATCH 16/49] Close code blocks so they compile... --- crates/bevy_ecs/src/system/query.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 93a4bd82c4d66..e3829e54603be 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -706,6 +706,7 @@ where /// /// assert_eq!(entity_1_life, Life(1)); /// assert_eq!(entity_2_life, Life(2)); + /// ``` #[inline] pub fn get_pair( &'s self, @@ -751,6 +752,7 @@ where /// /// assert_eq!(entity_1_life, Life(0)); /// assert_eq!(entity_2_life, Life(100)); + /// ``` #[inline] pub fn get_pair_mut( &'s self, From 3bc52524bfe7f2050ac8e4ec4a339476d5470780 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 15 Dec 2021 17:50:07 -0500 Subject: [PATCH 17/49] Fixed broken tests --- crates/bevy_ecs/src/system/query.rs | 51 ++++++++++++++++++----------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index e3829e54603be..05ba89520a352 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -296,7 +296,7 @@ where /// /// # Example /// ```rust - /// # use bevy_ecs::prelude; + /// # use bevy_ecs::prelude::*; /// #[derive(Component)] /// struct PowerLevel(u64); /// @@ -696,7 +696,7 @@ where /// #[derive(Component, PartialEq, Debug)] /// struct Life(u64); /// - /// let world = World::new(); + /// let mut world = World::new(); /// let entity_1 = world.spawn().insert(Life(1)).id(); /// let entity_2 = world.spawn().insert(Life(2)).id(); /// @@ -704,8 +704,8 @@ where /// let life_query = Query::from_state(&mut world, &query_state); /// let (entity_1_life, entity_2_life) = life_query.get_pair(entity_1, entity_2).unwrap(); /// - /// assert_eq!(entity_1_life, Life(1)); - /// assert_eq!(entity_2_life, Life(2)); + /// assert_eq!(*entity_1_life, Life(1)); + /// assert_eq!(*entity_2_life, Life(2)); /// ``` #[inline] pub fn get_pair( @@ -739,7 +739,7 @@ where /// #[derive(Component, PartialEq, Debug)] /// struct Life(u64); /// - /// let world = World::new(); + /// let mut world = World::new(); /// let entity_1 = world.spawn().insert(Life(1)).id(); /// let entity_2 = world.spawn().insert(Life(2)).id(); /// @@ -750,8 +750,8 @@ where /// *entity_1_life = Life(0); /// *entity_2_life = Life(100); /// - /// assert_eq!(entity_1_life, Life(0)); - /// assert_eq!(entity_2_life, Life(100)); + /// assert_eq!(*entity_1_life, Life(0)); + /// assert_eq!(*entity_2_life, Life(100)); /// ``` #[inline] pub fn get_pair_mut( @@ -795,22 +795,31 @@ where /// #[derive(Component, PartialEq, Debug)] /// struct A(u64); /// - /// let world = World::new(); + /// let mut world = World::new(); /// let entity_1 = world.spawn().insert(A(1)).id(); /// let entity_2 = world.spawn().insert(A(2)).id(); + /// let entity_3 = world.spawn().insert(A(3)).id(); /// /// let query_state = world.query::<&A>(); /// let a_query = Query::from_state(&mut world, &query_state); - /// let a_iterator = a_query.get_multiple([entity_1, entity_2]); - /// assert_eq!(a_iterator.next().unwrap(), A(1)); - /// assert_eq!(a_iterator.next().unwrap(), A(2)); + /// let mut a_iterator = a_query.get_multiple([entity_3, entity_2, entity_1]).unwrap(); + /// assert_eq!(*a_iterator.next().unwrap(), A(3)); + /// assert_eq!(*a_iterator.next().unwrap(), A(2)); + /// assert_eq!(*a_iterator.next().unwrap(), A(1)); /// ``` #[inline] pub fn get_multiple( &'s self, entities: impl IntoIterator, - ) -> impl Iterator::Item, QueryEntityError>> { - entities.into_iter().map(|entity| self.get(entity)) + ) -> Result::Item>, QueryEntityError> { + // PERF: we could estimate the capacity and preallocate + // based on the size of the data in the query and the number of items in the iter + let mut data = Vec::new(); + for entity in entities { + data.push(self.get(entity)?); + } + + Ok(data.into_iter()) } /// Returns the query results for the ['BTreeSet'](std::collections::BTreeSet) of [`Entity`]s provided. @@ -834,23 +843,27 @@ where /// #[derive(Component, PartialEq, Debug)] /// struct A(u64); /// - /// let world = World::new(); + /// let mut world = World::new(); /// let entity_1 = world.spawn().insert(A(1)).id(); /// let entity_2 = world.spawn().insert(A(2)).id(); /// let entity_3 = world.spawn().insert(A(3)).id(); /// /// let query_state = world.query::<&mut A>(); - /// let a_query = Query::from_state(&mut world, &query_state); - /// let a_iterator = a_query.get_multiple_mut([entity_1, entity_3]).unwrap(); - /// let mut a_1 = a_iterator.next(); - /// let mut a_3 = a_iterator.next(); + /// let mut a_query = Query::from_state(&mut world, &query_state); + /// let mut a_iterator = a_query.get_multiple_mut([entity_1, entity_3]).unwrap(); + /// let mut a_1 = a_iterator.next().unwrap(); + /// let mut a_3 = a_iterator.next().unwrap(); /// /// *a_1 = A(11); /// *a_3 = A(33); /// + /// // Manually drop references so we can access the `World` again + /// std::mem::drop(a_iterator); + /// std::mem::drop(a_query); + /// /// assert_eq!(*world.get::(entity_1).unwrap(), A(11)); /// assert_eq!(*world.get::(entity_2).unwrap(), A(2)); - /// assert_eq!(*world.get::(entity_2).unwrap(), A(33)); + /// assert_eq!(*world.get::(entity_3).unwrap(), A(33)); /// ``` #[inline] pub fn get_multiple_mut( From 9cd5c234b039f9def55e8ed10adf01e83597fb17 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 15 Dec 2021 18:28:44 -0500 Subject: [PATCH 18/49] Improve performance by returning an iter of Results Avoids re-allocation and provides more granular failure data. Slightly more annoying API in the happy case. --- crates/bevy_ecs/src/system/query.rs | 48 +++++++++++++---------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 05ba89520a352..a465fc0370bf3 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -802,7 +802,7 @@ where /// /// let query_state = world.query::<&A>(); /// let a_query = Query::from_state(&mut world, &query_state); - /// let mut a_iterator = a_query.get_multiple([entity_3, entity_2, entity_1]).unwrap(); + /// let mut a_iterator = a_query.get_multiple([entity_3, entity_2, entity_1]).map(|i|i.unwrap()); /// assert_eq!(*a_iterator.next().unwrap(), A(3)); /// assert_eq!(*a_iterator.next().unwrap(), A(2)); /// assert_eq!(*a_iterator.next().unwrap(), A(1)); @@ -811,15 +811,8 @@ where pub fn get_multiple( &'s self, entities: impl IntoIterator, - ) -> Result::Item>, QueryEntityError> { - // PERF: we could estimate the capacity and preallocate - // based on the size of the data in the query and the number of items in the iter - let mut data = Vec::new(); - for entity in entities { - data.push(self.get(entity)?); - } - - Ok(data.into_iter()) + ) -> impl Iterator::Item, QueryEntityError>> { + entities.into_iter().map(|entity| self.get(entity)) } /// Returns the query results for the ['BTreeSet'](std::collections::BTreeSet) of [`Entity`]s provided. @@ -850,7 +843,7 @@ where /// /// let query_state = world.query::<&mut A>(); /// let mut a_query = Query::from_state(&mut world, &query_state); - /// let mut a_iterator = a_query.get_multiple_mut([entity_1, entity_3]).unwrap(); + /// let mut a_iterator = a_query.get_multiple_mut([entity_1, entity_3]).map(|i|i.unwrap()); /// let mut a_1 = a_iterator.next().unwrap(); /// let mut a_3 = a_iterator.next().unwrap(); /// @@ -869,10 +862,10 @@ where pub fn get_multiple_mut( &mut self, entities: impl IntoIterator, - ) -> Result::Item>, QueryEntityError> { + ) -> impl Iterator::Item, QueryEntityError>> { let entities_iter = entities.into_iter(); - // Preallocating the hash-set used to check uniqueness based on the expected maximum amount of space + // Preallocating the HashSet used to check uniqueness based on the expected maximum amount of space let (lower_bound, maybe_upper_bound) = entities_iter.size_hint(); let best_bound = if let Some(upper_bound) = maybe_upper_bound { upper_bound @@ -881,20 +874,23 @@ where }; let mut entities_seen = HashSet::with_capacity(best_bound * std::mem::size_of::()); - // PERF: we could estimate the capacity and preallocate - // based on the size of the data in the query and the number of items in the iter - let mut data = Vec::new(); - for entity in entities_iter { - if entities_seen.contains(&entity) { - return Err(QueryEntityError::AliasedMutability); - } else { - entities_seen.insert(entity); - unsafe { - data.push(self.get_unchecked(entity)?); + + // We must collect the results before converting back into an iterator + // in order to ensure that we don't have any dangling references to entities_seen that need to be evaluated later + let results: Vec::Item, QueryEntityError>> = entities_iter + .map(|entity| { + if entities_seen.contains(&entity) { + Err(QueryEntityError::AliasedMutability) + } else { + entities_seen.insert(entity); + + // SAFE: entities are checked for uniqueness using a HashSet + unsafe { self.get_unchecked(entity) } } - } - } - Ok(data.into_iter()) + }) + .collect(); + + results.into_iter() } /// Returns the query result for the given [`Entity`]. From aa5a23bae6fb60a51169e2a38824bff19bc8f3b3 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 15 Dec 2021 20:12:16 -0500 Subject: [PATCH 19/49] Avoid needless allocation in get_multiple_mut --- crates/bevy_ecs/src/system/query.rs | 56 ++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 17 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index a465fc0370bf3..6867cced77fa0 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -873,24 +873,15 @@ where lower_bound }; - let mut entities_seen = HashSet::with_capacity(best_bound * std::mem::size_of::()); + let entities_seen = HashSet::with_capacity(best_bound * std::mem::size_of::()); - // We must collect the results before converting back into an iterator - // in order to ensure that we don't have any dangling references to entities_seen that need to be evaluated later - let results: Vec::Item, QueryEntityError>> = entities_iter - .map(|entity| { - if entities_seen.contains(&entity) { - Err(QueryEntityError::AliasedMutability) - } else { - entities_seen.insert(entity); - - // SAFE: entities are checked for uniqueness using a HashSet - unsafe { self.get_unchecked(entity) } - } - }) - .collect(); - - results.into_iter() + // This is an iterator adaptor struct + // used to capture the value of `entities_seen` into the iterator returned + GetMultipleMut { + seen: entities_seen, + query: self, + iter: entities_iter, + } } /// Returns the query result for the given [`Entity`]. @@ -1252,6 +1243,37 @@ where } } +/// Iterator adaptor struct used for [`Query::get_multiple_mut`]('Query::get_multiple_mut`) +/// See https://stackoverflow.com/a/49813195 for more exposition +struct GetMultipleMut<'w, 's, 'q, Q: WorldQuery, F: WorldQuery, I> +where + F::Fetch: FilterFetch, +{ + seen: HashSet, + query: &'q Query<'w, 's, Q, F>, + iter: I, +} + +impl<'w, 's, 'q: 's, Q: WorldQuery, F: WorldQuery, I: Iterator> Iterator + for GetMultipleMut<'w, 's, 'q, Q, F, I> +where + F::Fetch: FilterFetch, +{ + type Item = Result<>::Item, QueryEntityError>; + + fn next(&mut self) -> Option { + Some({ + let entity = self.iter.next()?; + if self.seen.insert(entity) { + // SAFE: entities are checked for uniqueness using a HashSet + unsafe { Query::<'w, 's, Q, F>::get_unchecked(self.query, entity) } + } else { + Err(QueryEntityError::AliasedMutability) + } + }) + } +} + /// An error that occurs when retrieving a specific [`Entity`]'s component from a [`Query`] #[derive(Error, Debug)] pub enum QueryComponentError { From 5a4daf78e289d6ffeb3728321706b0da52ee1a0b Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 15 Dec 2021 20:17:09 -0500 Subject: [PATCH 20/49] Fix inappropriate lifetime constraint on get_unchecked --- crates/bevy_ecs/src/system/query.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 6867cced77fa0..7c7a99689b9d9 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -895,7 +895,7 @@ where /// this call does not result in multiple mutable references to the same component #[inline] pub unsafe fn get_unchecked( - &'s self, + &self, entity: Entity, ) -> Result<>::Item, QueryEntityError> { // SEMI-SAFE: system runs without conflicts with other systems. @@ -1254,7 +1254,7 @@ where iter: I, } -impl<'w, 's, 'q: 's, Q: WorldQuery, F: WorldQuery, I: Iterator> Iterator +impl<'w, 's, 'q, Q: WorldQuery, F: WorldQuery, I: Iterator> Iterator for GetMultipleMut<'w, 's, 'q, Q, F, I> where F::Fetch: FilterFetch, From 6ad192201ff58f44e7aa3375d051430574542850 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 16 Dec 2021 15:40:03 -0500 Subject: [PATCH 21/49] Allocate HashSet size correctly Co-authored-by: Mathspy --- crates/bevy_ecs/src/system/query.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 7c7a99689b9d9..25427df9bef43 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -873,7 +873,7 @@ where lower_bound }; - let entities_seen = HashSet::with_capacity(best_bound * std::mem::size_of::()); + let entities_seen = HashSet::with_capacity(best_bound); // This is an iterator adaptor struct // used to capture the value of `entities_seen` into the iterator returned From d1e262aec98364caa1577de82040109a63a792a3 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 6 Jan 2022 19:03:03 -0500 Subject: [PATCH 22/49] Use unwrap_or Co-authored-by: Nuutti Kotivuori --- crates/bevy_ecs/src/system/query.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 25427df9bef43..76ae75b617ec1 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -867,12 +867,7 @@ where // Preallocating the HashSet used to check uniqueness based on the expected maximum amount of space let (lower_bound, maybe_upper_bound) = entities_iter.size_hint(); - let best_bound = if let Some(upper_bound) = maybe_upper_bound { - upper_bound - } else { - lower_bound - }; - + let best_bound = maybe_upper_bound.unwrap_or(lower_bound); let entities_seen = HashSet::with_capacity(best_bound); // This is an iterator adaptor struct From 183910c4c38775b503b0776ddd3ad402a6083b48 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 14 Feb 2022 16:21:40 -0500 Subject: [PATCH 23/49] Fix CI --- crates/bevy_ecs/src/system/query.rs | 20 ++++++++++---------- crates/bevy_ecs/src/world/mod.rs | 4 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 76ae75b617ec1..1cda0f99c9111 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -235,11 +235,11 @@ use thiserror::Error; /// [`get_mut`](Self::get_mut) methods to access the query result for that particular entity. /// /// To access the data of exactly two specific entities at once, use [`get_pair`](Self::get_pair), -/// or ['get_pair_mut'](Self::get_pair_mut). -/// This is a surprisingly common pattern, and the API provided is both faster and more convenient than ['get_multiple_mut'](Self::get_multiple_mut). +/// or [`get_pair_mut`](Self::get_pair_mut). +/// This is a surprisingly common pattern, and the API provided is both faster and more convenient than [`get_multiple_mut`](Self::get_multiple_mut). /// /// If you require access to the data of multiple entities at once, -/// you can use the ['get_multiple'](Self::get_multiple) or ['get_multiple_mut'](Self::get_multiple_mut) methods. +/// you can use the [`get_multiple`](Self::get_multiple) or [`get_multiple_mut`](Self::get_multiple_mut) methods. /// /// ## Getting a single query result /// @@ -292,7 +292,7 @@ where /// As queries can modify the entity-component data in ways that could conflict dangerously, /// this method requires a mutable reference to the [`World`], /// ensuring only one query is active at once. - /// This can be quite restrictive: consider using ['SystemState::new'](bevy_ecs::system::SystemState::new) if this is a problem. + /// This can be quite restrictive: consider using [`SystemState::new`](bevy_ecs::system::SystemState::new) if this is a problem. /// /// # Example /// ```rust @@ -688,7 +688,7 @@ where /// returned instead. /// /// If you need to get the data from more than two specific entities at once, - /// use ['get_multiple'](Self::get_multiple) + /// use [`get_multiple`](Self::get_multiple) /// /// # Example /// ```rust @@ -731,7 +731,7 @@ where /// a [`QueryEntityError`] is returned instead. /// /// If you need to get the data from more than two specific entities at once, - /// use ['get_multiple_mut'](Self::get_multiple) + /// use [`get_multiple_mut`](Self::get_multiple) /// /// # Example /// ```rust @@ -787,7 +787,7 @@ where /// add [`Entity`] to your [`Query`]. /// /// If you need to get the data from exactly two specific entities at once, - /// use ['get_pair'](Self::get_pair). + /// use [`get_pair`](Self::get_pair). /// /// # Example /// ```rust @@ -815,7 +815,7 @@ where entities.into_iter().map(|entity| self.get(entity)) } - /// Returns the query results for the ['BTreeSet'](std::collections::BTreeSet) of [`Entity`]s provided. + /// Returns the query results for the [`BTreeSet`](std::collections::BTreeSet) of [`Entity`]s provided. /// /// These values follow the order of your input iterator (if any). /// In case of nonunique entities, a nonexisting entity or a mismatched component, @@ -828,7 +828,7 @@ where /// you can (carefully) call the unsafe [`get_unchecked`](Self::get_unchecked) method repeatedly instead. /// /// If you need to get the data from exactly two specific entities at once, - /// use ['get_pair_mut'](Self::get_pair_mut). + /// use [`get_pair_mut`](Self::get_pair_mut). /// /// # Example /// ```rust @@ -1239,7 +1239,7 @@ where } /// Iterator adaptor struct used for [`Query::get_multiple_mut`]('Query::get_multiple_mut`) -/// See https://stackoverflow.com/a/49813195 for more exposition +/// See for more exposition struct GetMultipleMut<'w, 's, 'q, Q: WorldQuery, F: WorldQuery, I> where F::Fetch: FilterFetch, diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 68a0ea45ed4c0..a8074d34ce229 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -20,8 +20,8 @@ use crate::{ use std::{ any::TypeId, fmt, - mem::ManuallyDrop, iter::IntoIterator, + mem::ManuallyDrop, sync::atomic::{AtomicU32, Ordering}, }; @@ -312,7 +312,7 @@ impl World { Some(unsafe { EntityMut::new(self, entity, location) }) } - /// Spawns a new [Entity] and returns a corresponding [EntityMut], which can be used + /// Spawns a new [`Entity`] and returns a corresponding [`EntityMut`], which can be used /// to add components to the entity or retrieve its id. /// /// ``` From d815de27ebeb34b193de0ebc70d271bb43b7c051 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 14 Feb 2022 17:16:30 -0500 Subject: [PATCH 24/49] Initial conversion to const_generics --- crates/bevy_ecs/src/system/query.rs | 151 +++++----------------------- 1 file changed, 27 insertions(+), 124 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 1cda0f99c9111..83663cd5fceb2 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -682,101 +682,6 @@ where } } - /// Returns the read-only query result for the given pair of [`Entity`]s. - /// - /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is - /// returned instead. - /// - /// If you need to get the data from more than two specific entities at once, - /// use [`get_multiple`](Self::get_multiple) - /// - /// # Example - /// ```rust - /// # use bevy_ecs::prelude::*; - /// #[derive(Component, PartialEq, Debug)] - /// struct Life(u64); - /// - /// let mut world = World::new(); - /// let entity_1 = world.spawn().insert(Life(1)).id(); - /// let entity_2 = world.spawn().insert(Life(2)).id(); - /// - /// let query_state = world.query::<&Life>(); - /// let life_query = Query::from_state(&mut world, &query_state); - /// let (entity_1_life, entity_2_life) = life_query.get_pair(entity_1, entity_2).unwrap(); - /// - /// assert_eq!(*entity_1_life, Life(1)); - /// assert_eq!(*entity_2_life, Life(2)); - /// ``` - #[inline] - pub fn get_pair( - &'s self, - entity_0: Entity, - entity_1: Entity, - ) -> Result< - ( - >::Item, - >::Item, - ), - QueryEntityError, - > { - let data_0 = self.get(entity_0)?; - let data_1 = self.get(entity_1)?; - - Ok((data_0, data_1)) - } - - /// Returns the query result for the given pair of [`Entity`]s. - /// - /// In case of nonunique entities, a nonexisting entity or mismatched component, - /// a [`QueryEntityError`] is returned instead. - /// - /// If you need to get the data from more than two specific entities at once, - /// use [`get_multiple_mut`](Self::get_multiple) - /// - /// # Example - /// ```rust - /// # use bevy_ecs::prelude::*; - /// #[derive(Component, PartialEq, Debug)] - /// struct Life(u64); - /// - /// let mut world = World::new(); - /// let entity_1 = world.spawn().insert(Life(1)).id(); - /// let entity_2 = world.spawn().insert(Life(2)).id(); - /// - /// let query_state = world.query::<&mut Life>(); - /// let life_query = Query::from_state(&mut world, &query_state); - /// let (mut entity_1_life, mut entity_2_life) = life_query.get_pair_mut(entity_1, entity_2).unwrap(); - /// - /// *entity_1_life = Life(0); - /// *entity_2_life = Life(100); - /// - /// assert_eq!(*entity_1_life, Life(0)); - /// assert_eq!(*entity_2_life, Life(100)); - /// ``` - #[inline] - pub fn get_pair_mut( - &'s self, - entity_0: Entity, - entity_1: Entity, - ) -> Result< - ( - >::Item, - >::Item, - ), - QueryEntityError, - > { - if entity_0 == entity_1 { - return Err(QueryEntityError::AliasedMutability); - } - - // SAFE: entities do not match - unsafe { - let data_0 = self.get_unchecked(entity_0)?; - let data_1 = self.get_unchecked(entity_1)?; - Ok((data_0, data_1)) - } - } - /// Returns the read-only query results for the iterator of [`Entity`]s provided. /// /// These values follow the order of your input iterator (if any). @@ -786,9 +691,6 @@ where /// If you need to verify the identity of each item returned, /// add [`Entity`] to your [`Query`]. /// - /// If you need to get the data from exactly two specific entities at once, - /// use [`get_pair`](Self::get_pair). - /// /// # Example /// ```rust /// # use bevy_ecs::prelude::*; @@ -808,9 +710,9 @@ where /// assert_eq!(*a_iterator.next().unwrap(), A(1)); /// ``` #[inline] - pub fn get_multiple( + pub fn get_multiple( &'s self, - entities: impl IntoIterator, + entities: [Entity; N], ) -> impl Iterator::Item, QueryEntityError>> { entities.into_iter().map(|entity| self.get(entity)) } @@ -827,9 +729,6 @@ where /// If you absolutely cannot afford the overhead of verifying uniqueness in this way, /// you can (carefully) call the unsafe [`get_unchecked`](Self::get_unchecked) method repeatedly instead. /// - /// If you need to get the data from exactly two specific entities at once, - /// use [`get_pair_mut`](Self::get_pair_mut). - /// /// # Example /// ```rust /// # use bevy_ecs::prelude::*; @@ -859,23 +758,20 @@ where /// assert_eq!(*world.get::(entity_3).unwrap(), A(33)); /// ``` #[inline] - pub fn get_multiple_mut( + pub fn get_multiple_mut( &mut self, - entities: impl IntoIterator, + entities: [Entity; N], ) -> impl Iterator::Item, QueryEntityError>> { - let entities_iter = entities.into_iter(); - // Preallocating the HashSet used to check uniqueness based on the expected maximum amount of space - let (lower_bound, maybe_upper_bound) = entities_iter.size_hint(); - let best_bound = maybe_upper_bound.unwrap_or(lower_bound); - let entities_seen = HashSet::with_capacity(best_bound); + let entities_seen = HashSet::with_capacity(N); // This is an iterator adaptor struct // used to capture the value of `entities_seen` into the iterator returned GetMultipleMut { seen: entities_seen, query: self, - iter: entities_iter, + index: 0, + array: entities, } } @@ -1240,32 +1136,39 @@ where /// Iterator adaptor struct used for [`Query::get_multiple_mut`]('Query::get_multiple_mut`) /// See for more exposition -struct GetMultipleMut<'w, 's, 'q, Q: WorldQuery, F: WorldQuery, I> +struct GetMultipleMut<'w, 's, 'q, Q: WorldQuery, F: WorldQuery, const N: usize> where F::Fetch: FilterFetch, { seen: HashSet, query: &'q Query<'w, 's, Q, F>, - iter: I, + index: usize, + array: [Entity; N], } -impl<'w, 's, 'q, Q: WorldQuery, F: WorldQuery, I: Iterator> Iterator - for GetMultipleMut<'w, 's, 'q, Q, F, I> +impl<'w, 's, 'q, Q: WorldQuery, F: WorldQuery, const N: usize> Iterator + for GetMultipleMut<'w, 's, 'q, Q, F, N> where F::Fetch: FilterFetch, { type Item = Result<>::Item, QueryEntityError>; fn next(&mut self) -> Option { - Some({ - let entity = self.iter.next()?; - if self.seen.insert(entity) { - // SAFE: entities are checked for uniqueness using a HashSet - unsafe { Query::<'w, 's, Q, F>::get_unchecked(self.query, entity) } - } else { - Err(QueryEntityError::AliasedMutability) - } - }) + if self.index < N { + self.index += 1; + Some({ + let entity = self.array[self.index]; + if self.seen.insert(entity) { + // SAFE: entities are checked for uniqueness using a HashSet + unsafe { Query::<'w, 's, Q, F>::get_unchecked(self.query, entity) } + } else { + Err(QueryEntityError::AliasedMutability) + } + }) + } else { + self.index += 1; + None + } } } From b2e499ea85aaf2fe7876121c1462cbd6f73f7eea Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 14 Feb 2022 18:02:59 -0500 Subject: [PATCH 25/49] Fix off-by-one error --- crates/bevy_ecs/src/system/query.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 83663cd5fceb2..7d055c4d3ce07 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1140,6 +1140,8 @@ struct GetMultipleMut<'w, 's, 'q, Q: WorldQuery, F: WorldQuery, const N: usize> where F::Fetch: FilterFetch, { + // PERF: we could probably make this faster in common cases using a PetitSet + // or other set optimized for small sizes seen: HashSet, query: &'q Query<'w, 's, Q, F>, index: usize, @@ -1155,9 +1157,10 @@ where fn next(&mut self) -> Option { if self.index < N { + let entity = self.array[self.index]; self.index += 1; Some({ - let entity = self.array[self.index]; + // Returns true if the entity was not already present in the HashSet if self.seen.insert(entity) { // SAFE: entities are checked for uniqueness using a HashSet unsafe { Query::<'w, 's, Q, F>::get_unchecked(self.query, entity) } From 1a7f1d35e1eaa339c7212e81d2b7d75f15734370 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 14 Feb 2022 18:17:58 -0500 Subject: [PATCH 26/49] Remove references to get_pair in docs --- crates/bevy_ecs/src/system/query.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 7d055c4d3ce07..7acba604f0c24 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -234,10 +234,6 @@ use thiserror::Error; /// If you have an [`Entity`] ID, you can use the [`get`](Self::get) or /// [`get_mut`](Self::get_mut) methods to access the query result for that particular entity. /// -/// To access the data of exactly two specific entities at once, use [`get_pair`](Self::get_pair), -/// or [`get_pair_mut`](Self::get_pair_mut). -/// This is a surprisingly common pattern, and the API provided is both faster and more convenient than [`get_multiple_mut`](Self::get_multiple_mut). -/// /// If you require access to the data of multiple entities at once, /// you can use the [`get_multiple`](Self::get_multiple) or [`get_multiple_mut`](Self::get_multiple_mut) methods. /// From 69fb1c9780b15c56e93260b107e2ca87bdb83916 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 14 Feb 2022 18:57:50 -0500 Subject: [PATCH 27/49] Fix doc links --- crates/bevy_ecs/src/system/query.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 7acba604f0c24..a219a3f2556e4 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -288,7 +288,7 @@ where /// As queries can modify the entity-component data in ways that could conflict dangerously, /// this method requires a mutable reference to the [`World`], /// ensuring only one query is active at once. - /// This can be quite restrictive: consider using [`SystemState::new`](bevy_ecs::system::SystemState::new) if this is a problem. + /// This can be quite restrictive: consider using [`SystemState::new`](crate::system::SystemState::new) if this is a problem. /// /// # Example /// ```rust From 81d6a6c1a032d131497fdbfd1459bf4fc66cdad2 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 16 Feb 2022 09:54:53 -0500 Subject: [PATCH 28/49] Update docs Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com> --- crates/bevy_ecs/src/system/query.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index a219a3f2556e4..0e0da1d516aab 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -678,9 +678,9 @@ where } } - /// Returns the read-only query results for the iterator of [`Entity`]s provided. + /// Returns the read-only query results for the provided Array of [`Entity`]s. /// - /// These values follow the order of your input iterator (if any). + /// These values follow the order of your input Array (if anyy). /// In case of a nonexisting entity or mismatched component, /// a [`QueryEntityError`] is returned instead. /// From 94ca870dde79fc710db428c89537b8925cfc8157 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 16 Feb 2022 09:55:03 -0500 Subject: [PATCH 29/49] Update docs Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com> --- crates/bevy_ecs/src/system/query.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 0e0da1d516aab..d6141d4aea198 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -713,9 +713,9 @@ where entities.into_iter().map(|entity| self.get(entity)) } - /// Returns the query results for the [`BTreeSet`](std::collections::BTreeSet) of [`Entity`]s provided. + /// Returns the query results for the provided Array of [`Entity`]s. /// - /// These values follow the order of your input iterator (if any). + /// These values follow the order of your input Array (if any). /// In case of nonunique entities, a nonexisting entity or a mismatched component, /// a [`QueryEntityError`] is returned instead. /// From a191c1eb2c5c7311edc4980875e7537e37e5f442 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 16 Feb 2022 09:55:30 -0500 Subject: [PATCH 30/49] Remove dead import Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com> --- crates/bevy_ecs/src/world/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index a8074d34ce229..ad037819e89f7 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -20,7 +20,6 @@ use crate::{ use std::{ any::TypeId, fmt, - iter::IntoIterator, mem::ManuallyDrop, sync::atomic::{AtomicU32, Ordering}, }; From 724d6f3ac2f8e3eb22ee07046897e205e7918531 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 16 Feb 2022 09:55:51 -0500 Subject: [PATCH 31/49] Fix iterator Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com> --- crates/bevy_ecs/src/system/query.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index d6141d4aea198..3532cfb353f3f 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1165,7 +1165,6 @@ where } }) } else { - self.index += 1; None } } From ccf903d308af54c21f3f3ea9afbcf3eac009f7ab Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 16 Feb 2022 09:57:33 -0500 Subject: [PATCH 32/49] Remove QueryState -> Query helper method --- crates/bevy_ecs/src/system/query.rs | 34 ----------------------------- 1 file changed, 34 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 3532cfb353f3f..1a145d9e1b5c8 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -279,40 +279,6 @@ where } } - /// Creates a new [`Query`] from a [`QueryState`] - /// - /// [`Query`] is a simply a borrow of [`QueryState`], - /// but when working directly with the [`World`] - /// we need to store the query's internal data somewhere, and so cannot return a [`Query`] directly. - /// - /// As queries can modify the entity-component data in ways that could conflict dangerously, - /// this method requires a mutable reference to the [`World`], - /// ensuring only one query is active at once. - /// This can be quite restrictive: consider using [`SystemState::new`](crate::system::SystemState::new) if this is a problem. - /// - /// # Example - /// ```rust - /// # use bevy_ecs::prelude::*; - /// #[derive(Component)] - /// struct PowerLevel(u64); - /// - /// let mut world = World::new(); - /// world.spawn().insert(PowerLevel(9001)); - /// - /// let query_state = world.query::<&PowerLevel>(); - /// let query = Query::from_state(&mut world, &query_state); - /// let power_level = query.single(); - /// - /// assert!(power_level.0 > 9000); - /// ``` - pub fn from_state(world: &'w mut World, state: &'s QueryState) -> Self { - let last_change_tick = world.last_change_tick(); - let change_tick = world.change_tick(); - - // SAFE: the `World` is borrowed mutably, so no other queries can have simultaneous access - unsafe { Self::new(world, state, last_change_tick, change_tick) } - } - /// Returns an [`Iterator`] over the query results. /// /// This can only return immutable data (mutable data will be cast to an immutable form). From 4e016232e2e0b767a2f791da3bd68f6a0f293280 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 16 Feb 2022 09:57:33 -0500 Subject: [PATCH 33/49] Revert "Remove QueryState -> Query helper method" This reverts commit ccf903d308af54c21f3f3ea9afbcf3eac009f7ab. --- crates/bevy_ecs/src/system/query.rs | 34 +++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 1a145d9e1b5c8..3532cfb353f3f 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -279,6 +279,40 @@ where } } + /// Creates a new [`Query`] from a [`QueryState`] + /// + /// [`Query`] is a simply a borrow of [`QueryState`], + /// but when working directly with the [`World`] + /// we need to store the query's internal data somewhere, and so cannot return a [`Query`] directly. + /// + /// As queries can modify the entity-component data in ways that could conflict dangerously, + /// this method requires a mutable reference to the [`World`], + /// ensuring only one query is active at once. + /// This can be quite restrictive: consider using [`SystemState::new`](crate::system::SystemState::new) if this is a problem. + /// + /// # Example + /// ```rust + /// # use bevy_ecs::prelude::*; + /// #[derive(Component)] + /// struct PowerLevel(u64); + /// + /// let mut world = World::new(); + /// world.spawn().insert(PowerLevel(9001)); + /// + /// let query_state = world.query::<&PowerLevel>(); + /// let query = Query::from_state(&mut world, &query_state); + /// let power_level = query.single(); + /// + /// assert!(power_level.0 > 9000); + /// ``` + pub fn from_state(world: &'w mut World, state: &'s QueryState) -> Self { + let last_change_tick = world.last_change_tick(); + let change_tick = world.change_tick(); + + // SAFE: the `World` is borrowed mutably, so no other queries can have simultaneous access + unsafe { Self::new(world, state, last_change_tick, change_tick) } + } + /// Returns an [`Iterator`] over the query results. /// /// This can only return immutable data (mutable data will be cast to an immutable form). From c6e32419b5a63a6b686a794a626b534f302605bd Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 16 Feb 2022 12:43:10 -0500 Subject: [PATCH 34/49] Improve docs for `Query::from_state` --- crates/bevy_ecs/src/system/query.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 3532cfb353f3f..27e1f87dae1a5 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -281,9 +281,10 @@ where /// Creates a new [`Query`] from a [`QueryState`] /// - /// [`Query`] is a simply a borrow of [`QueryState`], - /// but when working directly with the [`World`] - /// we need to store the query's internal data somewhere, and so cannot return a [`Query`] directly. + /// For performance reasons (and to enable change detection), queries cache internal state. + /// This data must be stored somewhere, + /// and so [`World::query`] must return a [`QueryState`], rather than a [`Query`]. + /// However, it can often be more convenient to work with a [`Query`] instead. /// /// As queries can modify the entity-component data in ways that could conflict dangerously, /// this method requires a mutable reference to the [`World`], From 189974ff2ed7d7d76ea0bae317a8613ee19dbd29 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Wed, 16 Feb 2022 14:36:18 -0500 Subject: [PATCH 35/49] Compile-fail test for Query::from_state Fix imports for compile-fail test Compile-fail test --- .../tests/ui/query_state_uniqueness_safety.rs | 19 +++++++++++++++++++ .../ui/query_state_uniqueness_safety.stderr | 11 +++++++++++ 2 files changed, 30 insertions(+) create mode 100644 crates/bevy_ecs_compile_fail_tests/tests/ui/query_state_uniqueness_safety.rs create mode 100644 crates/bevy_ecs_compile_fail_tests/tests/ui/query_state_uniqueness_safety.stderr diff --git a/crates/bevy_ecs_compile_fail_tests/tests/ui/query_state_uniqueness_safety.rs b/crates/bevy_ecs_compile_fail_tests/tests/ui/query_state_uniqueness_safety.rs new file mode 100644 index 0000000000000..1155a9f224874 --- /dev/null +++ b/crates/bevy_ecs_compile_fail_tests/tests/ui/query_state_uniqueness_safety.rs @@ -0,0 +1,19 @@ +use bevy_ecs::prelude::*; + +#[derive(Component, Eq, PartialEq, Debug)] +struct A(usize); + +fn main() { + let mut world = World::default(); + world.spawn().insert(A(1)); + + let first_query_state: QueryState<&mut A> = world.query(); + let second_query_state: QueryState<&mut A> = world.query(); + + let mut first_query = Query::from_state(&mut world, &first_query_state); + // This should fail to compile, as another query is already active + let mut second_query = Query::from_state(&mut world, &second_query_state); + + // This is a clear violation of no-aliased mutability + assert_eq!(*first_query.single_mut(), *second_query.single_mut()); +} diff --git a/crates/bevy_ecs_compile_fail_tests/tests/ui/query_state_uniqueness_safety.stderr b/crates/bevy_ecs_compile_fail_tests/tests/ui/query_state_uniqueness_safety.stderr new file mode 100644 index 0000000000000..b94eccc6dd07a --- /dev/null +++ b/crates/bevy_ecs_compile_fail_tests/tests/ui/query_state_uniqueness_safety.stderr @@ -0,0 +1,11 @@ +error[E0499]: cannot borrow `world` as mutable more than once at a time + --> tests/ui/query_state_uniqueness_safety.rs:15:46 + | +13 | let mut first_query = Query::from_state(&mut world, &first_query_state); + | ---------- first mutable borrow occurs here +14 | // This should fail to compile, as another query is already active +15 | let mut second_query = Query::from_state(&mut world, &second_query_state); + | ^^^^^^^^^^ second mutable borrow occurs here +... +18 | assert_eq!(*first_query.single_mut(), *second_query.single_mut()); + | ------------------------ first borrow later used here From ab74fd3bc828199fd087d4d7f7f74319a255fcf3 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 21 Mar 2022 12:45:46 -0400 Subject: [PATCH 36/49] Fix lifetimes per Boxy Co-authored-by: Boxy --- crates/bevy_ecs/src/system/query.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 27e1f87dae1a5..3afea5604497b 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -758,7 +758,7 @@ where pub fn get_multiple_mut( &mut self, entities: [Entity; N], - ) -> impl Iterator::Item, QueryEntityError>> { + ) -> impl Iterator>::Item, QueryEntityError>> { // Preallocating the HashSet used to check uniqueness based on the expected maximum amount of space let entities_seen = HashSet::with_capacity(N); @@ -1150,7 +1150,7 @@ impl<'w, 's, 'q, Q: WorldQuery, F: WorldQuery, const N: usize> Iterator where F::Fetch: FilterFetch, { - type Item = Result<>::Item, QueryEntityError>; + type Item = Result<>::Item, QueryEntityError>; fn next(&mut self) -> Option { if self.index < N { From 043abb112bb2e4d055d6fd89482250015afafe9f Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 21 Mar 2022 12:53:03 -0400 Subject: [PATCH 37/49] Fix lifetimes per Boxy Co-authored-by: Boxy --- crates/bevy_ecs/src/system/query.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 3afea5604497b..ce45aed624dc7 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -708,9 +708,9 @@ where /// ``` #[inline] pub fn get_multiple( - &'s self, + &self, entities: [Entity; N], - ) -> impl Iterator::Item, QueryEntityError>> { + ) -> impl Iterator>::Item, QueryEntityError>> { entities.into_iter().map(|entity| self.get(entity)) } From 7cb4330fd4c43869262e651ce31c10661b192f67 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 21 Mar 2022 12:54:03 -0400 Subject: [PATCH 38/49] Iterator should require a mutable reference Co-authored-by: Boxy --- crates/bevy_ecs/src/system/query.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index ce45aed624dc7..f2a16e669111a 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1140,7 +1140,7 @@ where // PERF: we could probably make this faster in common cases using a PetitSet // or other set optimized for small sizes seen: HashSet, - query: &'q Query<'w, 's, Q, F>, + query: &'q mut Query<'w, 's, Q, F>, index: usize, array: [Entity; N], } From b264ffc6cde0b8cd1b5e609fc17e4f160a6f90d3 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 21 Mar 2022 18:41:07 -0400 Subject: [PATCH 39/49] Mostly fix liftime errors Full credit to Boxy --- crates/bevy_ecs/src/system/query.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index f2a16e669111a..daf92b36b2ce9 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -623,7 +623,7 @@ where /// ``` #[inline] pub fn get( - &'s self, + &self, entity: Entity, ) -> Result<>::Item, QueryEntityError> { // SAFE: system runs without conflicts with other systems. @@ -710,7 +710,8 @@ where pub fn get_multiple( &self, entities: [Entity; N], - ) -> impl Iterator>::Item, QueryEntityError>> { + ) -> impl Iterator>::Item, QueryEntityError>> + { entities.into_iter().map(|entity| self.get(entity)) } @@ -758,7 +759,7 @@ where pub fn get_multiple_mut( &mut self, entities: [Entity; N], - ) -> impl Iterator>::Item, QueryEntityError>> { + ) -> GetMultipleMut { // Preallocating the HashSet used to check uniqueness based on the expected maximum amount of space let entities_seen = HashSet::with_capacity(N); @@ -1160,7 +1161,7 @@ where // Returns true if the entity was not already present in the HashSet if self.seen.insert(entity) { // SAFE: entities are checked for uniqueness using a HashSet - unsafe { Query::<'w, 's, Q, F>::get_unchecked(self.query, entity) } + unsafe { Query::<'q, 's, Q, F>::get_unchecked(self.query, entity) } } else { Err(QueryEntityError::AliasedMutability) } From be7ec0deada1c363dd5679849d0c897bd434695c Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 21 Mar 2022 19:00:05 -0400 Subject: [PATCH 40/49] Fix remaining lifetime error --- crates/bevy_ecs/src/system/query.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index daf92b36b2ce9..8bdba402ea08a 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -759,7 +759,7 @@ where pub fn get_multiple_mut( &mut self, entities: [Entity; N], - ) -> GetMultipleMut { + ) -> GetMultipleMut<'w, 's, '_, Q, F, N> { // Preallocating the HashSet used to check uniqueness based on the expected maximum amount of space let entities_seen = HashSet::with_capacity(N); @@ -1132,9 +1132,9 @@ where } } -/// Iterator adaptor struct used for [`Query::get_multiple_mut`]('Query::get_multiple_mut`) -/// See for more exposition -struct GetMultipleMut<'w, 's, 'q, Q: WorldQuery, F: WorldQuery, const N: usize> +/// An iterator adaptor struct used for [`Query::get_multiple_mut`]('Query::get_multiple_mut`) +// See for more exposition +pub struct GetMultipleMut<'w, 's, 'q, Q: WorldQuery, F: WorldQuery, const N: usize> where F::Fetch: FilterFetch, { From 1670877bbc814e0d72c56e605fbf1b862c6eccc7 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 21 Mar 2022 19:47:57 -0400 Subject: [PATCH 41/49] Change return types --- crates/bevy_ecs/src/query/state.rs | 2 +- crates/bevy_ecs/src/system/query.rs | 68 +++++------------------------ 2 files changed, 13 insertions(+), 57 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 9abff9da9e635..5fb8405d55234 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -733,7 +733,7 @@ where } /// An error that occurs when retrieving a specific [`Entity`]'s query result. -#[derive(Error, Debug)] +#[derive(Error, Debug, Clone, Copy)] pub enum QueryEntityError { #[error("The given entity does not have the requested component.")] QueryDoesNotMatch, diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 8bdba402ea08a..4eb3597bd4f12 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -2,13 +2,12 @@ use crate::{ component::Component, entity::Entity, query::{ - Fetch, FilterFetch, NopFetch, QueryCombinationIter, QueryEntityError, QueryIter, + self, Fetch, FilterFetch, NopFetch, QueryCombinationIter, QueryEntityError, QueryIter, QueryState, WorldQuery, }, world::{Mut, World}, }; use bevy_tasks::TaskPool; -use bevy_utils::{AHashExt, HashSet}; use std::{any::TypeId, fmt::Debug}; use thiserror::Error; @@ -710,9 +709,8 @@ where pub fn get_multiple( &self, entities: [Entity; N], - ) -> impl Iterator>::Item, QueryEntityError>> - { - entities.into_iter().map(|entity| self.get(entity)) + ) -> Result<[>::Item; N], QueryEntityError> { + todo!() } /// Returns the query results for the provided Array of [`Entity`]s. @@ -759,18 +757,16 @@ where pub fn get_multiple_mut( &mut self, entities: [Entity; N], - ) -> GetMultipleMut<'w, 's, '_, Q, F, N> { - // Preallocating the HashSet used to check uniqueness based on the expected maximum amount of space - let entities_seen = HashSet::with_capacity(N); - - // This is an iterator adaptor struct - // used to capture the value of `entities_seen` into the iterator returned - GetMultipleMut { - seen: entities_seen, - query: self, - index: 0, - array: entities, + ) -> Result<[>::Item; N], QueryEntityError> { + for entity_i in entities { + for entity_j in entities { + if entity_i == entity_j { + return Err(QueryEntityError::AliasedMutability); + } + } } + + todo!() } /// Returns the query result for the given [`Entity`]. @@ -1132,46 +1128,6 @@ where } } -/// An iterator adaptor struct used for [`Query::get_multiple_mut`]('Query::get_multiple_mut`) -// See for more exposition -pub struct GetMultipleMut<'w, 's, 'q, Q: WorldQuery, F: WorldQuery, const N: usize> -where - F::Fetch: FilterFetch, -{ - // PERF: we could probably make this faster in common cases using a PetitSet - // or other set optimized for small sizes - seen: HashSet, - query: &'q mut Query<'w, 's, Q, F>, - index: usize, - array: [Entity; N], -} - -impl<'w, 's, 'q, Q: WorldQuery, F: WorldQuery, const N: usize> Iterator - for GetMultipleMut<'w, 's, 'q, Q, F, N> -where - F::Fetch: FilterFetch, -{ - type Item = Result<>::Item, QueryEntityError>; - - fn next(&mut self) -> Option { - if self.index < N { - let entity = self.array[self.index]; - self.index += 1; - Some({ - // Returns true if the entity was not already present in the HashSet - if self.seen.insert(entity) { - // SAFE: entities are checked for uniqueness using a HashSet - unsafe { Query::<'q, 's, Q, F>::get_unchecked(self.query, entity) } - } else { - Err(QueryEntityError::AliasedMutability) - } - }) - } else { - None - } - } -} - /// An error that occurs when retrieving a specific [`Entity`]'s component from a [`Query`] #[derive(Error, Debug)] pub enum QueryComponentError { From 10dd5810d26490118e654146550328dbba501491 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 21 Mar 2022 19:49:21 -0400 Subject: [PATCH 42/49] Record which entity caused QueryEntityError errors --- crates/bevy_ecs/src/query/state.rs | 6 +++--- crates/bevy_ecs/src/system/query.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 5fb8405d55234..907e7290b70e6 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -228,7 +228,7 @@ 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()) @@ -738,7 +738,7 @@ pub enum QueryEntityError { #[error("The given entity does not have the requested component.")] QueryDoesNotMatch, #[error("The requested entity does not exist.")] - NoSuchEntity, + NoSuchEntity(Entity), #[error("The same entity was accessed mutably more than once.")] - AliasedMutability, + AliasedMutability(Entity), } diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 4eb3597bd4f12..9d519f0464cf8 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -761,7 +761,7 @@ where for entity_i in entities { for entity_j in entities { if entity_i == entity_j { - return Err(QueryEntityError::AliasedMutability); + return Err(QueryEntityError::AliasedMutability(entity_i)); } } } From 783b4f446f79806ea796401fdbbeb2714460998a Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 21 Mar 2022 19:50:36 -0400 Subject: [PATCH 43/49] Remove Query::from_state method --- crates/bevy_ecs/src/system/query.rs | 35 ----------------------------- 1 file changed, 35 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 9d519f0464cf8..456fbb34f9791 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -278,41 +278,6 @@ where } } - /// Creates a new [`Query`] from a [`QueryState`] - /// - /// For performance reasons (and to enable change detection), queries cache internal state. - /// This data must be stored somewhere, - /// and so [`World::query`] must return a [`QueryState`], rather than a [`Query`]. - /// However, it can often be more convenient to work with a [`Query`] instead. - /// - /// As queries can modify the entity-component data in ways that could conflict dangerously, - /// this method requires a mutable reference to the [`World`], - /// ensuring only one query is active at once. - /// This can be quite restrictive: consider using [`SystemState::new`](crate::system::SystemState::new) if this is a problem. - /// - /// # Example - /// ```rust - /// # use bevy_ecs::prelude::*; - /// #[derive(Component)] - /// struct PowerLevel(u64); - /// - /// let mut world = World::new(); - /// world.spawn().insert(PowerLevel(9001)); - /// - /// let query_state = world.query::<&PowerLevel>(); - /// let query = Query::from_state(&mut world, &query_state); - /// let power_level = query.single(); - /// - /// assert!(power_level.0 > 9000); - /// ``` - pub fn from_state(world: &'w mut World, state: &'s QueryState) -> Self { - let last_change_tick = world.last_change_tick(); - let change_tick = world.change_tick(); - - // SAFE: the `World` is borrowed mutably, so no other queries can have simultaneous access - unsafe { Self::new(world, state, last_change_tick, change_tick) } - } - /// Returns an [`Iterator`] over the query results. /// /// This can only return immutable data (mutable data will be cast to an immutable form). From a0d66322dadd581364190147be871927b3ae485b Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 21 Mar 2022 19:52:20 -0400 Subject: [PATCH 44/49] Doc improvements --- crates/bevy_ecs/src/system/query.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 456fbb34f9791..f49e46cdde891 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -645,8 +645,8 @@ where /// Returns the read-only query results for the provided Array of [`Entity`]s. /// - /// These values follow the order of your input Array (if anyy). - /// In case of a nonexisting entity or mismatched component, + /// These values follow the order of your input array. + /// In case of a nonexisting entity, /// a [`QueryEntityError`] is returned instead. /// /// If you need to verify the identity of each item returned, @@ -680,13 +680,10 @@ where /// Returns the query results for the provided Array of [`Entity`]s. /// - /// These values follow the order of your input Array (if any). - /// In case of nonunique entities, a nonexisting entity or a mismatched component, + /// These values follow the order of your input array. + /// In case of nonunique or nonexisting entities or a mismatched component, /// a [`QueryEntityError`] is returned instead. /// - /// If you need to verify the identity of each item returned, - /// add [`Entity`] to your [`Query`]. - /// /// If you absolutely cannot afford the overhead of verifying uniqueness in this way, /// you can (carefully) call the unsafe [`get_unchecked`](Self::get_unchecked) method repeatedly instead. /// From 9c74b6de2f18d5ccb537e676e1e2f4a52c690bec Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 21 Mar 2022 20:30:09 -0400 Subject: [PATCH 45/49] Move methods onto QueryState --- crates/bevy_ecs/src/query/state.rs | 83 +++++++++++++++++++++++++++++ crates/bevy_ecs/src/system/query.rs | 63 +--------------------- 2 files changed, 85 insertions(+), 61 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 907e7290b70e6..fa67d3e4ce188 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -190,6 +190,89 @@ where } } + /// Returns the read-only query results for the provided Array of [`Entity`]s. + /// + /// These values follow the order of your input array. + /// In case of a nonexisting entity, + /// a [`QueryEntityError`] is returned instead. + /// + /// # Example + /// ```rust + /// # use bevy_ecs::prelude::*; + /// #[derive(Component, PartialEq, Debug)] + /// struct A(u64); + /// + /// let mut world = World::new(); + /// let entity_1 = world.spawn().insert(A(1)).id(); + /// let entity_2 = world.spawn().insert(A(2)).id(); + /// let entity_3 = world.spawn().insert(A(3)).id(); + /// + /// let query_state = world.query::<&A>(); + /// let mut a_iterator = a_query.get_multiple([entity_3, entity_2, entity_1]).map(|i|i.unwrap()); + /// assert_eq!(*a_iterator.next().unwrap(), A(3)); + /// assert_eq!(*a_iterator.next().unwrap(), A(2)); + /// assert_eq!(*a_iterator.next().unwrap(), A(1)); + /// ``` + #[inline] + pub fn get_multiple( + &self, + entities: [Entity; N], + ) -> Result<[>::Item; N], QueryEntityError> { + todo!() + } + + /// Returns the query results for the provided Array of [`Entity`]s. + /// + /// These values follow the order of your input array. + /// In case of nonunique or nonexisting entities or a mismatched component, + /// a [`QueryEntityError`] is returned instead. + /// + /// If you absolutely cannot afford the overhead of verifying uniqueness in this way, + /// you can (carefully) call the unsafe [`get_unchecked`](Self::get_unchecked) method repeatedly instead. + /// + /// # Example + /// ```rust + /// # use bevy_ecs::prelude::*; + /// #[derive(Component, PartialEq, Debug)] + /// struct A(u64); + /// + /// let mut world = World::new(); + /// let entity_1 = world.spawn().insert(A(1)).id(); + /// let entity_2 = world.spawn().insert(A(2)).id(); + /// let entity_3 = world.spawn().insert(A(3)).id(); + /// + /// let query_state = world.query::<&mut A>(); + /// let mut a_iterator = a_query.get_multiple_mut([entity_1, entity_3]).map(|i|i.unwrap()); + /// let mut a_1 = a_iterator.next().unwrap(); + /// let mut a_3 = a_iterator.next().unwrap(); + /// + /// *a_1 = A(11); + /// *a_3 = A(33); + /// + /// // Manually drop references so we can access the `World` again + /// std::mem::drop(a_iterator); + /// std::mem::drop(a_query); + /// + /// assert_eq!(*world.get::(entity_1).unwrap(), A(11)); + /// assert_eq!(*world.get::(entity_2).unwrap(), A(2)); + /// assert_eq!(*world.get::(entity_3).unwrap(), A(33)); + /// ``` + #[inline] + pub fn get_multiple_mut( + &mut self, + entities: [Entity; N], + ) -> Result<[>::Item; N], QueryEntityError> { + for entity_i in entities { + for entity_j in entities { + if entity_i == entity_j { + return Err(QueryEntityError::AliasedMutability(entity_i)); + } + } + } + + todo!() + } + /// Gets the query result for the given [`World`] and [`Entity`]. /// /// # Safety diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index f49e46cdde891..b87460e8c49f6 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -648,34 +648,12 @@ where /// These values follow the order of your input array. /// In case of a nonexisting entity, /// a [`QueryEntityError`] is returned instead. - /// - /// If you need to verify the identity of each item returned, - /// add [`Entity`] to your [`Query`]. - /// - /// # Example - /// ```rust - /// # use bevy_ecs::prelude::*; - /// #[derive(Component, PartialEq, Debug)] - /// struct A(u64); - /// - /// let mut world = World::new(); - /// let entity_1 = world.spawn().insert(A(1)).id(); - /// let entity_2 = world.spawn().insert(A(2)).id(); - /// let entity_3 = world.spawn().insert(A(3)).id(); - /// - /// let query_state = world.query::<&A>(); - /// let a_query = Query::from_state(&mut world, &query_state); - /// let mut a_iterator = a_query.get_multiple([entity_3, entity_2, entity_1]).map(|i|i.unwrap()); - /// assert_eq!(*a_iterator.next().unwrap(), A(3)); - /// assert_eq!(*a_iterator.next().unwrap(), A(2)); - /// assert_eq!(*a_iterator.next().unwrap(), A(1)); - /// ``` #[inline] pub fn get_multiple( &self, entities: [Entity; N], ) -> Result<[>::Item; N], QueryEntityError> { - todo!() + self.state.get_multiple(entities) } /// Returns the query results for the provided Array of [`Entity`]s. @@ -686,49 +664,12 @@ where /// /// If you absolutely cannot afford the overhead of verifying uniqueness in this way, /// you can (carefully) call the unsafe [`get_unchecked`](Self::get_unchecked) method repeatedly instead. - /// - /// # Example - /// ```rust - /// # use bevy_ecs::prelude::*; - /// #[derive(Component, PartialEq, Debug)] - /// struct A(u64); - /// - /// let mut world = World::new(); - /// let entity_1 = world.spawn().insert(A(1)).id(); - /// let entity_2 = world.spawn().insert(A(2)).id(); - /// let entity_3 = world.spawn().insert(A(3)).id(); - /// - /// let query_state = world.query::<&mut A>(); - /// let mut a_query = Query::from_state(&mut world, &query_state); - /// let mut a_iterator = a_query.get_multiple_mut([entity_1, entity_3]).map(|i|i.unwrap()); - /// let mut a_1 = a_iterator.next().unwrap(); - /// let mut a_3 = a_iterator.next().unwrap(); - /// - /// *a_1 = A(11); - /// *a_3 = A(33); - /// - /// // Manually drop references so we can access the `World` again - /// std::mem::drop(a_iterator); - /// std::mem::drop(a_query); - /// - /// assert_eq!(*world.get::(entity_1).unwrap(), A(11)); - /// assert_eq!(*world.get::(entity_2).unwrap(), A(2)); - /// assert_eq!(*world.get::(entity_3).unwrap(), A(33)); - /// ``` #[inline] pub fn get_multiple_mut( &mut self, entities: [Entity; N], ) -> Result<[>::Item; N], QueryEntityError> { - for entity_i in entities { - for entity_j in entities { - if entity_i == entity_j { - return Err(QueryEntityError::AliasedMutability(entity_i)); - } - } - } - - todo!() + self.state.get_multiple_mut(entities) } /// Returns the query result for the given [`Entity`]. From aff9c652745edda3956912fe637e0994a59f2c97 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Mon, 21 Mar 2022 20:37:15 -0400 Subject: [PATCH 46/49] Add infallible variants --- crates/bevy_ecs/src/query/state.rs | 28 +++++++++++++++++++++++++++- crates/bevy_ecs/src/system/query.rs | 27 ++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index fa67d3e4ce188..d7c34a0087ee1 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -221,6 +221,19 @@ where todo!() } + /// Returns the read-only query items for the provided Array of [`Entity`]s. + /// + /// # Panics + /// + /// Panics if any entities do not exist. + #[inline] + pub fn multiple( + &self, + entities: [Entity; N], + ) -> [>::Item; N] { + self.get_multiple(entities).unwrap() + } + /// Returns the query results for the provided Array of [`Entity`]s. /// /// These values follow the order of your input array. @@ -261,7 +274,7 @@ where pub fn get_multiple_mut( &mut self, entities: [Entity; N], - ) -> Result<[>::Item; N], QueryEntityError> { + ) -> Result<[>::Item; N], QueryEntityError> { for entity_i in entities { for entity_j in entities { if entity_i == entity_j { @@ -273,6 +286,19 @@ where todo!() } + /// Returns the query items for the provided Array of [`Entity`]s. + /// + /// # Panics + /// + /// Panics if any entities do not exist, or any entities are repeated. + #[inline] + pub fn multiple_mut( + &self, + entities: [Entity; N], + ) -> [>::Item; N] { + self.get_multiple_mut(entities).unwrap() + } + /// Gets the query result for the given [`World`] and [`Entity`]. /// /// # Safety diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index b87460e8c49f6..cabb982d3ac43 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -656,6 +656,19 @@ where self.state.get_multiple(entities) } + /// Returns the read-only query items for the provided Array of [`Entity`]s. + /// + /// # Panics + /// + /// Panics if any entities do not exist. + #[inline] + pub fn multiple( + &self, + entities: [Entity; N], + ) -> [>::Item; N] { + self.state.multiple(entities) + } + /// Returns the query results for the provided Array of [`Entity`]s. /// /// These values follow the order of your input array. @@ -668,10 +681,22 @@ where pub fn get_multiple_mut( &mut self, entities: [Entity; N], - ) -> Result<[>::Item; N], QueryEntityError> { + ) -> Result<[>::Item; N], QueryEntityError> { self.state.get_multiple_mut(entities) } + /// Returns the query items for the provided Array of [`Entity`]s. + /// + /// # Panics + /// Panics if any entities do not exist, or any entities are repeated. + #[inline] + pub fn multiple_mut( + &self, + entities: [Entity; N], + ) -> [>::Item; N] { + self.state.multiple_mut(entities) + } + /// Returns the query result for the given [`Entity`]. /// /// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is From de5bda7fd1a9b18f4fbf8f841fb055ad51e02f37 Mon Sep 17 00:00:00 2001 From: Ellen Date: Tue, 22 Mar 2022 01:58:40 +0000 Subject: [PATCH 47/49] i did not realise querystate was so unbelievably MESSED Up --- crates/bevy_ecs/src/query/state.rs | 80 ++++++++++++++++++++--------- crates/bevy_ecs/src/system/query.rs | 20 ++++---- 2 files changed, 66 insertions(+), 34 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index d7c34a0087ee1..b74f609ec51e6 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -214,11 +214,12 @@ where /// assert_eq!(*a_iterator.next().unwrap(), A(1)); /// ``` #[inline] - pub fn get_multiple( - &self, + pub fn get_multiple<'s, 'w, const N: usize>( + &'s self, + world: &'w World, entities: [Entity; N], - ) -> Result<[>::Item; N], QueryEntityError> { - todo!() + ) -> [Result<>::Item, QueryEntityError>; N] { + entities.map(move |e| self.get_manual(world, e)) } /// Returns the read-only query items for the provided Array of [`Entity`]s. @@ -227,11 +228,48 @@ where /// /// Panics if any entities do not exist. #[inline] - pub fn multiple( - &self, + pub fn multiple<'s, 'w, const N: usize>( + &'s self, + world: &'w World, + entities: [Entity; N], + ) -> [>::Item; N] { + entities.map(|e| self.get_manual(world, e).unwrap()) + } + + // world aliased mutability blah blah + #[inline] + pub unsafe fn get_multiple_mut_unchecked<'s, 'w, const N: usize>( + &'s self, + world: &'w World, + entities: [Entity; N], + ) -> [Result<>::Item, QueryEntityError>; N] { + let mut idx = 0; + entities.map(move |e| { + entities[..idx] + .iter() + .find(|prev_e| **prev_e == e) + .ok_or(QueryEntityError::AliasedMutability(e))?; + idx += 1; + self.get_unchecked_manual(world, e) + }) + } + + // world aliased mutaiblity blah blah + #[inline] + pub unsafe fn multiple_mut_unchecked<'s, 'w, const N: usize>( + &'s self, + world: &'w World, entities: [Entity; N], - ) -> [>::Item; N] { - self.get_multiple(entities).unwrap() + ) -> [>::Item; N] { + let mut idx = 0; + entities.map(|e| { + entities[..idx] + .iter() + .find(|prev_e| **prev_e == e) + .unwrap_or_else(|| panic!()); + idx += 1; + self.get_unchecked(world, e).unwrap() + }) } /// Returns the query results for the provided Array of [`Entity`]s. @@ -271,19 +309,12 @@ where /// assert_eq!(*world.get::(entity_3).unwrap(), A(33)); /// ``` #[inline] - pub fn get_multiple_mut( - &mut self, + pub fn get_multiple_mut<'s, 'w, const N: usize>( + &'s self, + world: &'w mut World, entities: [Entity; N], - ) -> Result<[>::Item; N], QueryEntityError> { - for entity_i in entities { - for entity_j in entities { - if entity_i == entity_j { - return Err(QueryEntityError::AliasedMutability(entity_i)); - } - } - } - - todo!() + ) -> [Result<>::Item, QueryEntityError>; N] { + unsafe { self.get_multiple_mut_unchecked(world, entities) } } /// Returns the query items for the provided Array of [`Entity`]s. @@ -292,11 +323,12 @@ where /// /// Panics if any entities do not exist, or any entities are repeated. #[inline] - pub fn multiple_mut( - &self, + pub fn multiple_mut<'s, 'w, const N: usize>( + &'s self, + world: &'w mut World, entities: [Entity; N], - ) -> [>::Item; N] { - self.get_multiple_mut(entities).unwrap() + ) -> [>::Item; N] { + unsafe { self.multiple_mut_unchecked(world, entities) } } /// Gets the query result for the given [`World`] and [`Entity`]. diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index cabb982d3ac43..f54d794094caf 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -2,7 +2,7 @@ use crate::{ component::Component, entity::Entity, query::{ - self, Fetch, FilterFetch, NopFetch, QueryCombinationIter, QueryEntityError, QueryIter, + Fetch, FilterFetch, NopFetch, QueryCombinationIter, QueryEntityError, QueryIter, QueryState, WorldQuery, }, world::{Mut, World}, @@ -652,8 +652,8 @@ where pub fn get_multiple( &self, entities: [Entity; N], - ) -> Result<[>::Item; N], QueryEntityError> { - self.state.get_multiple(entities) + ) -> [Result<>::Item, QueryEntityError>; N] { + self.state.get_multiple(self.world, entities) } /// Returns the read-only query items for the provided Array of [`Entity`]s. @@ -665,8 +665,8 @@ where pub fn multiple( &self, entities: [Entity; N], - ) -> [>::Item; N] { - self.state.multiple(entities) + ) -> [>::Item; N] { + self.state.multiple(self.world, entities) } /// Returns the query results for the provided Array of [`Entity`]s. @@ -681,8 +681,8 @@ where pub fn get_multiple_mut( &mut self, entities: [Entity; N], - ) -> Result<[>::Item; N], QueryEntityError> { - self.state.get_multiple_mut(entities) + ) -> [Result<>::Item, QueryEntityError>; N] { + unsafe { self.state.get_multiple_mut_unchecked(self.world, entities) } } /// Returns the query items for the provided Array of [`Entity`]s. @@ -691,10 +691,10 @@ where /// Panics if any entities do not exist, or any entities are repeated. #[inline] pub fn multiple_mut( - &self, + &mut self, entities: [Entity; N], - ) -> [>::Item; N] { - self.state.multiple_mut(entities) + ) -> [>::Item; N] { + unsafe { self.state.multiple_mut_unchecked(self.world, entities) } } /// Returns the query result for the given [`Entity`]. From 52d57c1e8d1be6ac14ec780e5de2bba3ec09d031 Mon Sep 17 00:00:00 2001 From: Alice Date: Tue, 22 Mar 2022 02:21:02 -0400 Subject: [PATCH 48/49] Return a result of an array, rather than an array of results --- crates/bevy_ecs/src/query/state.rs | 89 +++++++++++++---------------- crates/bevy_ecs/src/system/query.rs | 8 +-- 2 files changed, 45 insertions(+), 52 deletions(-) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index b74f609ec51e6..97aa0f8146ab1 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -214,12 +214,20 @@ where /// assert_eq!(*a_iterator.next().unwrap(), A(1)); /// ``` #[inline] - pub fn get_multiple<'s, 'w, const N: usize>( + pub fn get_multiple<'w, 's, const N: usize>( &'s self, world: &'w World, entities: [Entity; N], - ) -> [Result<>::Item, QueryEntityError>; N] { - entities.map(move |e| self.get_manual(world, e)) + ) -> Result<[>::Item; N], QueryEntityError> { + let array_of_results = entities.map(move |e| self.get(world, e)); + + // If any of the entities were missing, return the first error + for result in array_of_results { + result?; + } + + // At this point, we're guaranteed that all results are okay + Ok(array_of_results.map(move |result| result.unwrap())) } /// Returns the read-only query items for the provided Array of [`Entity`]s. @@ -228,48 +236,12 @@ where /// /// Panics if any entities do not exist. #[inline] - pub fn multiple<'s, 'w, const N: usize>( + pub fn multiple<'w, 's, const N: usize>( &'s self, world: &'w World, entities: [Entity; N], ) -> [>::Item; N] { - entities.map(|e| self.get_manual(world, e).unwrap()) - } - - // world aliased mutability blah blah - #[inline] - pub unsafe fn get_multiple_mut_unchecked<'s, 'w, const N: usize>( - &'s self, - world: &'w World, - entities: [Entity; N], - ) -> [Result<>::Item, QueryEntityError>; N] { - let mut idx = 0; - entities.map(move |e| { - entities[..idx] - .iter() - .find(|prev_e| **prev_e == e) - .ok_or(QueryEntityError::AliasedMutability(e))?; - idx += 1; - self.get_unchecked_manual(world, e) - }) - } - - // world aliased mutaiblity blah blah - #[inline] - pub unsafe fn multiple_mut_unchecked<'s, 'w, const N: usize>( - &'s self, - world: &'w World, - entities: [Entity; N], - ) -> [>::Item; N] { - let mut idx = 0; - entities.map(|e| { - entities[..idx] - .iter() - .find(|prev_e| **prev_e == e) - .unwrap_or_else(|| panic!()); - idx += 1; - self.get_unchecked(world, e).unwrap() - }) + self.get_multiple(world, entities).unwrap() } /// Returns the query results for the provided Array of [`Entity`]s. @@ -309,12 +281,33 @@ where /// assert_eq!(*world.get::(entity_3).unwrap(), A(33)); /// ``` #[inline] - pub fn get_multiple_mut<'s, 'w, const N: usize>( - &'s self, + pub fn get_multiple_mut<'w, 's, const N: usize>( + &'s mut self, world: &'w mut World, entities: [Entity; N], - ) -> [Result<>::Item, QueryEntityError>; N] { - unsafe { self.get_multiple_mut_unchecked(world, entities) } + ) -> Result<[>::Item; N], QueryEntityError> { + // Brute force verification of uniqueness + for entity_i in entities { + for entity_j in entities { + if entity_i == entity_j { + return Err(QueryEntityError::AliasedMutability(entity_i)); + } + } + } + + // SAFE: the entities are checked for uniqueness above + // No other references to the query can be live, as this method takes &mut self + unsafe { + let array_of_results = entities.map(move |e| self.get_unchecked(world, e)); + + // If any of the entities were missing, return the first error + for result in array_of_results { + result?; + } + + // At this point, we're guaranteed that all results are okay + Ok(array_of_results.map(move |result| result.unwrap())) + } } /// Returns the query items for the provided Array of [`Entity`]s. @@ -323,12 +316,12 @@ where /// /// Panics if any entities do not exist, or any entities are repeated. #[inline] - pub fn multiple_mut<'s, 'w, const N: usize>( - &'s self, + pub fn multiple_mut<'w, 's, const N: usize>( + &'s mut self, world: &'w mut World, entities: [Entity; N], ) -> [>::Item; N] { - unsafe { self.multiple_mut_unchecked(world, entities) } + self.get_multiple_mut(world, entities).unwrap() } /// Gets the query result for the given [`World`] and [`Entity`]. diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index f54d794094caf..1021d6020332b 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -652,7 +652,7 @@ where pub fn get_multiple( &self, entities: [Entity; N], - ) -> [Result<>::Item, QueryEntityError>; N] { + ) -> Result<[>::Item; N], QueryEntityError> { self.state.get_multiple(self.world, entities) } @@ -681,8 +681,8 @@ where pub fn get_multiple_mut( &mut self, entities: [Entity; N], - ) -> [Result<>::Item, QueryEntityError>; N] { - unsafe { self.state.get_multiple_mut_unchecked(self.world, entities) } + ) -> Result<[>::Item; N], QueryEntityError> { + self.state.get_multiple_mut(self.world, entities) } /// Returns the query items for the provided Array of [`Entity`]s. @@ -694,7 +694,7 @@ where &mut self, entities: [Entity; N], ) -> [>::Item; N] { - unsafe { self.state.multiple_mut_unchecked(self.world, entities) } + self.state.multiple_mut(self.world, entities) } /// Returns the query result for the given [`Entity`]. From bee959e2a4eafaa7786576be7fa53dd49d2f6260 Mon Sep 17 00:00:00 2001 From: Alice Date: Tue, 22 Mar 2022 02:36:21 -0400 Subject: [PATCH 49/49] More comments, more safety --- crates/bevy_ecs/src/query/state.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 97aa0f8146ab1..8368fa1bb97d7 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -297,6 +297,7 @@ where // SAFE: the entities are checked for uniqueness above // No other references to the query can be live, as this method takes &mut self + // The World cannot be modified in other ways as we take &mut World unsafe { let array_of_results = entities.map(move |e| self.get_unchecked(world, e));