Skip to content

Commit

Permalink
remove component and resource suffixes from reflect structs (#5219)
Browse files Browse the repository at this point in the history
# Objective

Remove suffixes from reflect component and resource methods to closer match bevy norms.

## Solution

removed suffixes and also fixed a spelling error

---
  • Loading branch information
oliverpauffley committed Jul 6, 2022
1 parent 2db1611 commit bf1ca81
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 79 deletions.
127 changes: 58 additions & 69 deletions crates/bevy_ecs/src/reflect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ use bevy_reflect::{
/// [`bevy_reflect::TypeRegistration::data`].
#[derive(Clone)]
pub struct ReflectComponent {
add_component: fn(&mut World, Entity, &dyn Reflect),
apply_component: fn(&mut World, Entity, &dyn Reflect),
remove_component: fn(&mut World, Entity),
reflect_component: fn(&World, Entity) -> Option<&dyn Reflect>,
reflect_component_mut: unsafe fn(&World, Entity) -> Option<ReflectMut>,
copy_component: fn(&World, &mut World, Entity, Entity),
add: fn(&mut World, Entity, &dyn Reflect),
apply: fn(&mut World, Entity, &dyn Reflect),
remove: fn(&mut World, Entity),
reflect: fn(&World, Entity) -> Option<&dyn Reflect>,
reflect_mut: unsafe fn(&World, Entity) -> Option<ReflectMut>,
copy: fn(&World, &mut World, Entity, Entity),
}

impl ReflectComponent {
Expand All @@ -32,45 +32,37 @@ impl ReflectComponent {
/// # Panics
///
/// Panics if there is no such entity.
pub fn add_component(&self, world: &mut World, entity: Entity, component: &dyn Reflect) {
(self.add_component)(world, entity, component);
pub fn add(&self, world: &mut World, entity: Entity, component: &dyn Reflect) {
(self.add)(world, entity, component);
}

/// Uses reflection to set the value of this [`Component`] type in the entity to the given value.
///
/// # Panics
///
/// Panics if there is no [`Component`] of the given type or the `entity` does not exist.
pub fn apply_component(&self, world: &mut World, entity: Entity, component: &dyn Reflect) {
(self.apply_component)(world, entity, component);
pub fn apply(&self, world: &mut World, entity: Entity, component: &dyn Reflect) {
(self.apply)(world, entity, component);
}

/// Removes this [`Component`] type from the entity. Does nothing if it doesn't exist.
///
/// # Panics
///
/// Panics if there is no [`Component`] of the given type or the `entity` does not exist.
pub fn remove_component(&self, world: &mut World, entity: Entity) {
(self.remove_component)(world, entity);
pub fn remove(&self, world: &mut World, entity: Entity) {
(self.remove)(world, entity);
}

/// Gets the value of this [`Component`] type from the entity as a reflected reference.
pub fn reflect_component<'a>(
&self,
world: &'a World,
entity: Entity,
) -> Option<&'a dyn Reflect> {
(self.reflect_component)(world, entity)
pub fn reflect<'a>(&self, world: &'a World, entity: Entity) -> Option<&'a dyn Reflect> {
(self.reflect)(world, entity)
}

/// Gets the value of this [`Component`] type from the entity as a mutable reflected reference.
pub fn reflect_component_mut<'a>(
&self,
world: &'a mut World,
entity: Entity,
) -> Option<ReflectMut<'a>> {
pub fn reflect_mut<'a>(&self, world: &'a mut World, entity: Entity) -> Option<ReflectMut<'a>> {
// SAFETY: unique world access
unsafe { (self.reflect_component_mut)(world, entity) }
unsafe { (self.reflect_mut)(world, entity) }
}

/// # Safety
Expand All @@ -79,27 +71,27 @@ impl ReflectComponent {
/// * 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_component_unchecked_mut<'a>(
pub unsafe fn reflect_unchecked_mut<'a>(
&self,
world: &'a World,
entity: Entity,
) -> Option<ReflectMut<'a>> {
(self.reflect_component_mut)(world, entity)
(self.reflect_mut)(world, entity)
}

/// Gets the value of this [`Component`] type from entity from `source_world` and [applies](Self::apply_component()) it to the value of this [`Component`] type in entity in `destination_world`.
/// 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
///
/// Panics if there is no [`Component`] of the given type or either entity does not exist.
pub fn copy_component(
pub fn copy(
&self,
source_world: &World,
destination_world: &mut World,
source_entity: Entity,
destination_entity: Entity,
) {
(self.copy_component)(
(self.copy)(
source_world,
destination_world,
source_entity,
Expand All @@ -111,35 +103,35 @@ impl ReflectComponent {
impl<C: Component + Reflect + FromWorld> FromType<C> for ReflectComponent {
fn from_type() -> Self {
ReflectComponent {
add_component: |world, entity, reflected_component| {
add: |world, entity, reflected_component| {
let mut component = C::from_world(world);
component.apply(reflected_component);
world.entity_mut(entity).insert(component);
},
apply_component: |world, entity, reflected_component| {
apply: |world, entity, reflected_component| {
let mut component = world.get_mut::<C>(entity).unwrap();
component.apply(reflected_component);
},
remove_component: |world, entity| {
remove: |world, entity| {
world.entity_mut(entity).remove::<C>();
},
copy_component: |source_world, destination_world, source_entity, destination_entity| {
copy: |source_world, destination_world, source_entity, destination_entity| {
let source_component = source_world.get::<C>(source_entity).unwrap();
let mut destination_component = C::from_world(destination_world);
destination_component.apply(source_component);
destination_world
.entity_mut(destination_entity)
.insert(destination_component);
},
reflect_component: |world, entity| {
reflect: |world, entity| {
world
.get_entity(entity)?
.get::<C>()
.map(|c| c as &dyn Reflect)
},
reflect_component_mut: |world, entity| {
// SAFETY: reflect_component_mut is an unsafe function pointer used by `reflect_component_unchecked_mut` which promises to never
// produce aliasing mutable references, and reflect_component_mut, which has mutable world access
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
unsafe {
world
.get_entity(entity)?
Expand All @@ -160,43 +152,43 @@ impl<C: Component + Reflect + FromWorld> FromType<C> for ReflectComponent {
/// [`bevy_reflect::TypeRegistration::data`].
#[derive(Clone)]
pub struct ReflectResource {
insert_resource: fn(&mut World, &dyn Reflect),
apply_resource: fn(&mut World, &dyn Reflect),
remove_resource: fn(&mut World),
reflect_resource: fn(&World) -> Option<&dyn Reflect>,
reflect_resource_unchecked_mut: unsafe fn(&World) -> Option<ReflectMut>,
copy_resource: fn(&World, &mut World),
insert: fn(&mut World, &dyn Reflect),
apply: fn(&mut World, &dyn Reflect),
remove: fn(&mut World),
reflect: fn(&World) -> Option<&dyn Reflect>,
reflect_unchecked_mut: unsafe fn(&World) -> Option<ReflectMut>,
copy: fn(&World, &mut World),
}

impl ReflectResource {
/// Insert a reflected [`Resource`] into the world like [`insert_resource()`](World::insert_resource).
pub fn insert_resource(&self, world: &mut World, resource: &dyn Reflect) {
(self.insert_resource)(world, resource);
/// Insert a reflected [`Resource`] into the world like [`insert()`](World::insert_resource).
pub fn insert(&self, world: &mut World, resource: &dyn Reflect) {
(self.insert)(world, resource);
}

/// Uses reflection to set the value of this [`Resource`] type in the world to the given value.
///
/// # Panics
///
/// Panics if there is no [`Resource`] of the given type.
pub fn apply_resource(&self, world: &mut World, resource: &dyn Reflect) {
(self.apply_resource)(world, resource);
pub fn apply(&self, world: &mut World, resource: &dyn Reflect) {
(self.apply)(world, resource);
}

/// Removes this [`Resource`] type from the world. Does nothing if it doesn't exist.
pub fn remove_resource(&self, world: &mut World) {
(self.remove_resource)(world);
pub fn remove(&self, world: &mut World) {
(self.remove)(world);
}

/// Gets the value of this [`Resource`] type from the world as a reflected reference.
pub fn reflect_resource<'a>(&self, world: &'a World) -> Option<&'a dyn Reflect> {
(self.reflect_resource)(world)
pub fn reflect<'a>(&self, world: &'a World) -> Option<&'a dyn Reflect> {
(self.reflect)(world)
}

/// Gets the value of this [`Resource`] type from the world as a mutable reflected reference.
pub fn reflect_resource_mut<'a>(&self, world: &'a mut World) -> Option<ReflectMut<'a>> {
pub fn reflect_mut<'a>(&self, world: &'a mut World) -> Option<ReflectMut<'a>> {
// SAFETY: unique world access
unsafe { (self.reflect_resource_unchecked_mut)(world) }
unsafe { (self.reflect_unchecked_mut)(world) }
}

/// # Safety
Expand All @@ -205,42 +197,39 @@ impl ReflectResource {
/// * 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 [`Resource`].
pub unsafe fn reflect_resource_unckecked_mut<'a>(
&self,
world: &'a World,
) -> Option<ReflectMut<'a>> {
pub unsafe fn reflect_unchecked_mut<'a>(&self, world: &'a World) -> Option<ReflectMut<'a>> {
// SAFETY: caller promises to uphold uniqueness guarantees
(self.reflect_resource_unchecked_mut)(world)
(self.reflect_unchecked_mut)(world)
}

/// Gets the value of this [`Resource`] type from `source_world` and [applies](Self::apply_resource()) it to the value of this [`Resource`] type in `destination_world`.
/// Gets the value of this [`Resource`] type from `source_world` and [applies](Self::apply()) it to the value of this [`Resource`] type in `destination_world`.
///
/// # Panics
///
/// Panics if there is no [`Resource`] of the given type.
pub fn copy_resource(&self, source_world: &World, destination_world: &mut World) {
(self.copy_resource)(source_world, destination_world);
pub fn copy(&self, source_world: &World, destination_world: &mut World) {
(self.copy)(source_world, destination_world);
}
}

impl<C: Resource + Reflect + FromWorld> FromType<C> for ReflectResource {
fn from_type() -> Self {
ReflectResource {
insert_resource: |world, reflected_resource| {
insert: |world, reflected_resource| {
let mut resource = C::from_world(world);
resource.apply(reflected_resource);
world.insert_resource(resource);
},
apply_resource: |world, reflected_resource| {
apply: |world, reflected_resource| {
let mut resource = world.resource_mut::<C>();
resource.apply(reflected_resource);
},
remove_resource: |world| {
remove: |world| {
world.remove_resource::<C>();
},
reflect_resource: |world| world.get_resource::<C>().map(|res| res as &dyn Reflect),
reflect_resource_unchecked_mut: |world| {
// SAFETY: all usages of `reflect_resource_unchecked_mut` guarantee that there is either a single mutable
reflect: |world| world.get_resource::<C>().map(|res| res as &dyn Reflect),
reflect_unchecked_mut: |world| {
// SAFETY: all usages of `reflect_unchecked_mut` guarantee that there is either a single mutable
// reference or multiple immutable ones alive at any given point
unsafe {
world
Expand All @@ -251,7 +240,7 @@ impl<C: Resource + Reflect + FromWorld> FromType<C> for ReflectResource {
})
}
},
copy_resource: |source_world, destination_world| {
copy: |source_world, destination_world| {
let source_resource = source_world.resource::<C>();
let mut destination_resource = C::from_world(destination_world);
destination_resource.apply(source_resource);
Expand Down
7 changes: 3 additions & 4 deletions crates/bevy_scene/src/dynamic_scene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ impl DynamicScene {
.and_then(|registration| registration.data::<ReflectComponent>());
if let Some(reflect_component) = reflect_component {
for (i, entity) in archetype.entities().iter().enumerate() {
if let Some(component) = reflect_component.reflect_component(world, *entity)
{
if let Some(component) = reflect_component.reflect(world, *entity) {
scene.entities[entities_offset + i]
.components
.push(component.clone_value());
Expand Down Expand Up @@ -117,9 +116,9 @@ impl DynamicScene {
.entity(entity)
.contains_type_id(registration.type_id())
{
reflect_component.apply_component(world, entity, &**component);
reflect_component.apply(world, entity, &**component);
} else {
reflect_component.add_component(world, entity, &**component);
reflect_component.add(world, entity, &**component);
}
}
}
Expand Down
7 changes: 1 addition & 6 deletions crates/bevy_scene/src/scene_spawner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,12 +199,7 @@ impl SceneSpawner {
}
})
})?;
reflect_component.copy_component(
&scene.world,
world,
*scene_entity,
entity,
);
reflect_component.copy(&scene.world, world, *scene_entity, entity);
}
}
}
Expand Down

0 comments on commit bf1ca81

Please sign in to comment.