-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
EntityRef/Mut get_components #13375
EntityRef/Mut get_components #13375
Changes from all commits
68f09ac
7b5d7e1
af6bcc1
5005365
8b8a75b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ use crate::{ | |
change_detection::MutUntyped, | ||
component::{Component, ComponentId, ComponentTicks, Components, StorageType}, | ||
entity::{Entities, Entity, EntityLocation}, | ||
query::{Access, DebugCheckedUnwrap}, | ||
query::{Access, DebugCheckedUnwrap, QueryData, ReadOnlyQueryData}, | ||
removal_detection::RemovedComponentEvents, | ||
storage::Storages, | ||
world::{Mut, World}, | ||
|
@@ -153,6 +153,17 @@ impl<'w> EntityRef<'w> { | |
// SAFETY: We have read-only access to all components of this entity. | ||
unsafe { self.0.get_by_id(component_id) } | ||
} | ||
|
||
/// Returns read-only components for the current entity that match the query `Q`. | ||
pub fn components<Q: ReadOnlyQueryData>(&self) -> Q::Item<'w> { | ||
self.get_components::<Q>().expect(QUERY_MISMATCH_ERROR) | ||
} | ||
|
||
/// Returns read-only components for the current entity that match the query `Q`. | ||
pub fn get_components<Q: ReadOnlyQueryData>(&self) -> Option<Q::Item<'w>> { | ||
// SAFETY: We have read-only access to all components of this entity. | ||
unsafe { self.0.get_components::<Q>() } | ||
} | ||
} | ||
|
||
impl<'w> From<EntityWorldMut<'w>> for EntityRef<'w> { | ||
|
@@ -348,6 +359,28 @@ impl<'w> EntityMut<'w> { | |
self.as_readonly().get() | ||
} | ||
|
||
/// Returns read-only components for the current entity that match the query `Q`. | ||
pub fn components<Q: ReadOnlyQueryData>(&self) -> Q::Item<'_> { | ||
self.get_components::<Q>().expect(QUERY_MISMATCH_ERROR) | ||
} | ||
|
||
/// Returns components for the current entity that match the query `Q`. | ||
pub fn components_mut<Q: QueryData>(&mut self) -> Q::Item<'_> { | ||
self.get_components_mut::<Q>().expect(QUERY_MISMATCH_ERROR) | ||
} | ||
|
||
/// Returns read-only components for the current entity that match the query `Q`. | ||
pub fn get_components<Q: ReadOnlyQueryData>(&self) -> Option<Q::Item<'_>> { | ||
// SAFETY: We have read-only access to all components of this entity. | ||
unsafe { self.0.get_components::<Q>() } | ||
} | ||
|
||
/// Returns components for the current entity that match the query `Q`. | ||
pub fn get_components_mut<Q: QueryData>(&mut self) -> Option<Q::Item<'_>> { | ||
// SAFETY: &mut self implies exclusive access for duration of returned value | ||
unsafe { self.0.get_components::<Q>() } | ||
} | ||
|
||
/// Consumes `self` and gets access to the component of type `T` with the | ||
/// world `'w` lifetime for the current entity. | ||
/// | ||
|
@@ -645,6 +678,31 @@ impl<'w> EntityWorldMut<'w> { | |
EntityRef::from(self).get() | ||
} | ||
|
||
/// Returns read-only components for the current entity that match the query `Q`. | ||
#[inline] | ||
pub fn components<Q: ReadOnlyQueryData>(&self) -> Q::Item<'_> { | ||
EntityRef::from(self).components::<Q>() | ||
} | ||
|
||
/// Returns components for the current entity that match the query `Q`. | ||
#[inline] | ||
pub fn components_mut<Q: QueryData>(&mut self) -> Q::Item<'_> { | ||
self.get_components_mut::<Q>().expect(QUERY_MISMATCH_ERROR) | ||
} | ||
|
||
/// Returns read-only components for the current entity that match the query `Q`. | ||
#[inline] | ||
pub fn get_components<Q: ReadOnlyQueryData>(&self) -> Option<Q::Item<'_>> { | ||
EntityRef::from(self).get_components::<Q>() | ||
} | ||
|
||
/// Returns components for the current entity that match the query `Q`. | ||
#[inline] | ||
pub fn get_components_mut<Q: QueryData>(&mut self) -> Option<Q::Item<'_>> { | ||
// SAFETY: &mut self implies exclusive access for duration of returned value | ||
unsafe { self.as_unsafe_entity_cell().get_components::<Q>() } | ||
} | ||
|
||
/// Consumes `self` and gets access to the component of type `T` with | ||
/// the world `'w` lifetime for the current entity. | ||
/// Returns `None` if the entity does not have a component of type `T`. | ||
|
@@ -1383,6 +1441,8 @@ impl<'w> EntityWorldMut<'w> { | |
} | ||
} | ||
|
||
const QUERY_MISMATCH_ERROR: &str = "Query does not match the current entity"; | ||
|
||
/// A view into a single entity and component in a world, which may either be vacant or occupied. | ||
/// | ||
/// This `enum` can only be constructed from the [`entry`] method on [`EntityWorldMut`]. | ||
|
@@ -1770,7 +1830,7 @@ impl<'w> FilteredEntityRef<'w> { | |
|
||
/// Returns an iterator over the component ids that are accessed by self. | ||
#[inline] | ||
pub fn components(&self) -> impl Iterator<Item = ComponentId> + '_ { | ||
pub fn accessed_components(&self) -> impl Iterator<Item = ComponentId> + '_ { | ||
self.access.reads_and_writes() | ||
} | ||
|
||
|
@@ -2024,7 +2084,7 @@ impl<'w> FilteredEntityMut<'w> { | |
|
||
/// Returns an iterator over the component ids that are accessed by self. | ||
#[inline] | ||
pub fn components(&self) -> impl Iterator<Item = ComponentId> + '_ { | ||
pub fn accessed_components(&self) -> impl Iterator<Item = ComponentId> + '_ { | ||
self.access.reads_and_writes() | ||
} | ||
|
||
|
@@ -2918,4 +2978,24 @@ mod tests { | |
|
||
assert_is_system(incompatible_system); | ||
} | ||
|
||
#[test] | ||
fn get_components() { | ||
#[derive(Component, PartialEq, Eq, Debug)] | ||
struct X(usize); | ||
|
||
#[derive(Component, PartialEq, Eq, Debug)] | ||
struct Y(usize); | ||
let mut world = World::default(); | ||
let e1 = world.spawn((X(7), Y(10))).id(); | ||
let e2 = world.spawn(X(8)).id(); | ||
let e3 = world.spawn_empty().id(); | ||
|
||
assert_eq!( | ||
Some((&X(7), &Y(10))), | ||
world.entity(e1).get_components::<(&X, &Y)>() | ||
); | ||
assert_eq!(None, world.entity(e2).get_components::<(&X, &Y)>()); | ||
assert_eq!(None, world.entity(e3).get_components::<(&X, &Y)>()); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you add a test to make sure it returns none or panics if you do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also should have one for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Haha I'm so glad you asked this question. The Access part of the Query infrastructure was what enforced this elsewhere, so this was allowed. We can't merge this as-is, and fixing it will almost certainly incur a significant perf cost. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't creating an empty access and calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would. It would just be dissatisfying from a perf perspective because Access allocates. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI I have a PR that removes the allocation from an empty Or maybe not.. calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could the allocation not just be removed in the meantime by using a smallvec that has 8 items or something? |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ use crate::{ | |
component::{ComponentId, ComponentTicks, Components, StorageType, Tick, TickCells}, | ||
entity::{Entities, Entity, EntityLocation}, | ||
prelude::Component, | ||
query::{DebugCheckedUnwrap, QueryData}, | ||
removal_detection::RemovedComponentEvents, | ||
storage::{Column, ComponentSparseSet, Storages}, | ||
system::{Res, Resource}, | ||
|
@@ -818,6 +819,48 @@ impl<'w> UnsafeEntityCell<'w> { | |
}) | ||
} | ||
} | ||
|
||
/// # Safety | ||
/// It is the callers responsibility to ensure that | ||
/// - the [`UnsafeEntityCell`] has permission to access the queried data mutably | ||
/// - no other references to the queried data exist at the same time | ||
pub(crate) unsafe fn get_components<Q: QueryData>(&self) -> Option<Q::Item<'w>> { | ||
let state = Q::get_state(self.world().components())?; | ||
let location = self.location(); | ||
// SAFETY: Location is guaranteed to exist | ||
let archetype = unsafe { | ||
self.world | ||
.archetypes() | ||
.get(location.archetype_id) | ||
.debug_checked_unwrap() | ||
}; | ||
if Q::matches_component_set(&state, &|id| archetype.contains(id)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could use |
||
// SAFETY: state was initialized above using the world passed into this function | ||
let mut fetch = unsafe { | ||
Q::init_fetch( | ||
self.world, | ||
&state, | ||
self.world.last_change_tick(), | ||
self.world.change_tick(), | ||
) | ||
}; | ||
// SAFETY: Table is guaranteed to exist | ||
let table = unsafe { | ||
self.world | ||
.storages() | ||
.tables | ||
.get(location.table_id) | ||
.debug_checked_unwrap() | ||
}; | ||
// SAFETY: Archetype and table are from the same world used to initialize state and fetch. | ||
// Table corresponds to archetype. State is the same state used to init fetch above. | ||
unsafe { Q::set_archetype(&mut fetch, &state, archetype, table) } | ||
// SAFETY: Called after set_archetype above. Entity and location are guaranteed to exist. | ||
unsafe { Some(Q::fetch(&mut fetch, self.id(), location.table_row)) } | ||
} else { | ||
None | ||
} | ||
} | ||
} | ||
|
||
impl<'w> UnsafeEntityCell<'w> { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should add some docs to explain why it might return
None
. Ditto for the other methods that return an option.