From 523d8ded7033df6365d49c5e79af719a5511a455 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sun, 4 Dec 2022 19:13:08 -0800 Subject: [PATCH 1/4] Expand ReflectComponentVtable --- crates/bevy_ecs/src/reflect.rs | 72 +++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/reflect.rs b/crates/bevy_ecs/src/reflect.rs index 88e25dfaf86b5..2787b51867414 100644 --- a/crates/bevy_ecs/src/reflect.rs +++ b/crates/bevy_ecs/src/reflect.rs @@ -5,7 +5,7 @@ use crate::{ component::Component, entity::{Entity, EntityMap, MapEntities, MapEntitiesError}, system::Resource, - world::{FromWorld, World}, + world::{EntityRef, EntityMut, FromWorld, World}, }; use bevy_reflect::{ impl_from_reflect_value, impl_reflect_value, FromType, Reflect, ReflectDeserialize, @@ -51,8 +51,14 @@ pub struct ReflectComponentFns { pub remove: fn(&mut World, Entity), /// Function pointer implementing [`ReflectComponent::reflect()`]. pub reflect: fn(&World, Entity) -> Option<&dyn Reflect>, + /// Function pointer implementing [`ReflectComponent::reflect_ref()`]. + pub reflect_ref: for<'a> fn(&EntityRef<'a>) -> Option<&'a dyn Reflect>, /// Function pointer implementing [`ReflectComponent::reflect_mut()`]. pub reflect_mut: unsafe fn(&World, Entity) -> Option>, + /// Function pointer implementing [`ReflectComponent::reflect_mut_ref()`]. + pub reflect_mut_ref: for<'a> fn(&'a mut EntityMut) -> Option>, + /// Function pointer implementing [`ReflectComponent::reflect_mut_unchecked_ref()`]. + pub reflect_mut_unchecked_ref: for<'a> unsafe fn(&'a EntityRef) -> Option>, /// Function pointer implementing [`ReflectComponent::copy()`]. pub copy: fn(&World, &mut World, Entity, Entity), } @@ -106,11 +112,22 @@ impl ReflectComponent { } /// Gets the value of this [`Component`] type from the entity as a reflected reference. + /// + /// If repeatedly fetching multiple components from the same entity, prefer using + /// [`ReflectComponent::reflect_ref`] where possible to avoid multiple entity location lookups. pub fn reflect<'a>(&self, world: &'a World, entity: Entity) -> Option<&'a dyn Reflect> { (self.0.reflect)(world, entity) } + /// Gets the value of this [`Component`] type from the entity as a reflected reference. + pub fn reflect_ref<'a>(&self, entity_ref: &EntityRef<'a>) -> Option<&'a dyn Reflect> { + (self.0.reflect_ref)(entity_ref) + } + /// Gets the value of this [`Component`] type from the entity as a mutable reflected reference. + /// + /// If repeatedly fetching multiple components from the same entity, prefer using + /// [`ReflectComponent::reflect_mut_ref`] where possible to avoid multiple entity location lookups. pub fn reflect_mut<'a>( &self, world: &'a mut World, @@ -120,6 +137,21 @@ impl ReflectComponent { unsafe { (self.0.reflect_mut)(world, entity) } } + /// Gets the value of this [`Component`] type from the entity as a mutable reflected reference. + pub fn reflect_mut_ref<'a>( + &self, + entity_mut: &'a mut EntityMut<'a> + ) -> Option> { + (self.0.reflect_mut_ref)(entity_mut) + } + + /// Gets the value of this [`Component`] type from the entity as a mutable reflected reference + /// without enforcing Rust's aliasing rules. + /// + /// If repeatedly fetching multiple components from the same entity, prefer using + /// [`ReflectComponent::reflect_unchecked_mut_ref`] where possible to avoid multiple entity + /// location lookups. + /// /// # Safety /// This method does not prevent you from having two mutable pointers to the same data, /// violating Rust's aliasing rules. To avoid this: @@ -134,6 +166,22 @@ impl ReflectComponent { (self.0.reflect_mut)(world, entity) } + /// Gets the value of this [`Component`] type from the entity as a mutable reflected reference + /// without enforcing Rust's aliasing rules. + /// + /// # Safety + /// This method does not prevent you from having two mutable pointers to the same data, + /// violating Rust's aliasing rules. To avoid this: + /// * Only call this method in an exclusive system to avoid sharing across threads (or use a + /// scheduler that enforces safe memory access). + /// * Don't call this method more than once in the same scope for a given [`Component`]. + pub unsafe fn reflect_unchecked_mut_ref<'a>( + &self, + entity_ref: &'a EntityRef<'a>, + ) -> Option> { + (self.0.reflect_mut_unchecked_ref)(entity_ref) + } + /// Gets the value of this [`Component`] type from entity from `source_world` and [applies](Self::apply()) it to the value of this [`Component`] type in entity in `destination_world`. /// /// # Panics @@ -208,6 +256,7 @@ impl FromType for ReflectComponent { .get::() .map(|c| c as &dyn Reflect) }, + reflect_ref: |entity_ref| entity_ref.get::().map(|c| c as &dyn Reflect), reflect_mut: |world, entity| { // SAFETY: reflect_mut is an unsafe function pointer used by `reflect_unchecked_mut` which promises to never // produce aliasing mutable references, and reflect_mut, which has mutable world access @@ -221,6 +270,27 @@ impl FromType for ReflectComponent { }) } }, + reflect_mut_ref: |entity_mut| { + entity_mut + .get_mut::() + .map(|c| Mut { + value: c.value as &mut dyn Reflect, + ticks: c.ticks, + }) + }, + reflect_mut_unchecked_ref: |entity_ref| { + let world = entity_ref.world(); + // SAFETY: reflect_mut_ref is an unsafe function pointer used by `reflect_unchecked_mut_ref ` which promises to never + // produce aliasing mutable references, and reflect_mut_ref, which has mutable world access + unsafe { + entity_ref + .get_unchecked_mut::(world.last_change_tick(), world.read_change_tick()) + .map(|c| Mut { + value: c.value as &mut dyn Reflect, + ticks: c.ticks, + }) + } + }, }) } } From e48450fd43911868aa943e5a36b43b83fb555610 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sun, 4 Dec 2022 20:13:50 -0800 Subject: [PATCH 2/4] Fix CI --- crates/bevy_ecs/src/reflect.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/reflect.rs b/crates/bevy_ecs/src/reflect.rs index 2787b51867414..5a4e7d5789a24 100644 --- a/crates/bevy_ecs/src/reflect.rs +++ b/crates/bevy_ecs/src/reflect.rs @@ -5,7 +5,7 @@ use crate::{ component::Component, entity::{Entity, EntityMap, MapEntities, MapEntitiesError}, system::Resource, - world::{EntityRef, EntityMut, FromWorld, World}, + world::{EntityMut, EntityRef, FromWorld, World}, }; use bevy_reflect::{ impl_from_reflect_value, impl_reflect_value, FromType, Reflect, ReflectDeserialize, @@ -57,7 +57,7 @@ pub struct ReflectComponentFns { pub reflect_mut: unsafe fn(&World, Entity) -> Option>, /// Function pointer implementing [`ReflectComponent::reflect_mut_ref()`]. pub reflect_mut_ref: for<'a> fn(&'a mut EntityMut) -> Option>, - /// Function pointer implementing [`ReflectComponent::reflect_mut_unchecked_ref()`]. + /// Function pointer implementing [`ReflectComponent::reflect_unchecked_mut_ref()`]. pub reflect_mut_unchecked_ref: for<'a> unsafe fn(&'a EntityRef) -> Option>, /// Function pointer implementing [`ReflectComponent::copy()`]. pub copy: fn(&World, &mut World, Entity, Entity), @@ -140,7 +140,7 @@ impl ReflectComponent { /// Gets the value of this [`Component`] type from the entity as a mutable reflected reference. pub fn reflect_mut_ref<'a>( &self, - entity_mut: &'a mut EntityMut<'a> + entity_mut: &'a mut EntityMut<'a>, ) -> Option> { (self.0.reflect_mut_ref)(entity_mut) } @@ -271,12 +271,10 @@ impl FromType for ReflectComponent { } }, reflect_mut_ref: |entity_mut| { - entity_mut - .get_mut::() - .map(|c| Mut { - value: c.value as &mut dyn Reflect, - ticks: c.ticks, - }) + entity_mut.get_mut::().map(|c| Mut { + value: c.value as &mut dyn Reflect, + ticks: c.ticks, + }) }, reflect_mut_unchecked_ref: |entity_ref| { let world = entity_ref.world(); From a73ba237831fdcab9a4fadb406638d06e784721e Mon Sep 17 00:00:00 2001 From: james7132 Date: Tue, 13 Dec 2022 18:26:19 -0800 Subject: [PATCH 3/4] Address review comments --- crates/bevy_ecs/src/reflect.rs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/crates/bevy_ecs/src/reflect.rs b/crates/bevy_ecs/src/reflect.rs index 5a4e7d5789a24..c109236cb639f 100644 --- a/crates/bevy_ecs/src/reflect.rs +++ b/crates/bevy_ecs/src/reflect.rs @@ -49,9 +49,7 @@ pub struct ReflectComponentFns { pub apply_or_insert: fn(&mut World, Entity, &dyn Reflect), /// Function pointer implementing [`ReflectComponent::remove()`]. pub remove: fn(&mut World, Entity), - /// Function pointer implementing [`ReflectComponent::reflect()`]. - pub reflect: fn(&World, Entity) -> Option<&dyn Reflect>, - /// Function pointer implementing [`ReflectComponent::reflect_ref()`]. + /// Function pointer implementing [`ReflectComponent::reflect()`] and [`ReflectComponent::reflect_ref()`]. pub reflect_ref: for<'a> fn(&EntityRef<'a>) -> Option<&'a dyn Reflect>, /// Function pointer implementing [`ReflectComponent::reflect_mut()`]. pub reflect_mut: unsafe fn(&World, Entity) -> Option>, @@ -116,7 +114,7 @@ impl ReflectComponent { /// If repeatedly fetching multiple components from the same entity, prefer using /// [`ReflectComponent::reflect_ref`] where possible to avoid multiple entity location lookups. pub fn reflect<'a>(&self, world: &'a World, entity: Entity) -> Option<&'a dyn Reflect> { - (self.0.reflect)(world, entity) + (self.0.reflect_ref)(&world.get_entity(entity)?) } /// Gets the value of this [`Component`] type from the entity as a reflected reference. @@ -250,12 +248,6 @@ impl FromType for ReflectComponent { .entity_mut(destination_entity) .insert(destination_component); }, - reflect: |world, entity| { - world - .get_entity(entity)? - .get::() - .map(|c| c as &dyn Reflect) - }, reflect_ref: |entity_ref| entity_ref.get::().map(|c| c as &dyn Reflect), reflect_mut: |world, entity| { // SAFETY: reflect_mut is an unsafe function pointer used by `reflect_unchecked_mut` which promises to never @@ -278,8 +270,8 @@ impl FromType for ReflectComponent { }, reflect_mut_unchecked_ref: |entity_ref| { let world = entity_ref.world(); - // SAFETY: reflect_mut_ref is an unsafe function pointer used by `reflect_unchecked_mut_ref ` which promises to never - // produce aliasing mutable references, and reflect_mut_ref, which has mutable world access + // SAFETY: reflect_mut_unchecked_ref is an unsafe function pointer used by `reflect_unchecked_mut_ref ` which promises to + // never produce aliasing mutable references unsafe { entity_ref .get_unchecked_mut::(world.last_change_tick(), world.read_change_tick()) From 228b1435a12ce54d6e30659a59e53552bf4e6221 Mon Sep 17 00:00:00 2001 From: james7132 Date: Tue, 13 Dec 2022 18:34:11 -0800 Subject: [PATCH 4/4] Formatting --- crates/bevy_ecs/src/reflect.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/reflect.rs b/crates/bevy_ecs/src/reflect.rs index c109236cb639f..d939ee6268892 100644 --- a/crates/bevy_ecs/src/reflect.rs +++ b/crates/bevy_ecs/src/reflect.rs @@ -270,7 +270,7 @@ impl FromType for ReflectComponent { }, reflect_mut_unchecked_ref: |entity_ref| { let world = entity_ref.world(); - // SAFETY: reflect_mut_unchecked_ref is an unsafe function pointer used by `reflect_unchecked_mut_ref ` which promises to + // SAFETY: reflect_mut_unchecked_ref is an unsafe function pointer used by `reflect_unchecked_mut_ref ` which promises to // never produce aliasing mutable references unsafe { entity_ref