Skip to content

Commit

Permalink
feat!: change #[from_entity_instance] to use references (#149)
Browse files Browse the repository at this point in the history
EntityInstance is a rather large struct, and cloning it requires allocating
for several fields. Despite this fact, each use of the #[from_entity_instance]
attribute was cloning the struct once, wasting CPU time for no good
reason - especially for Bundles that contain several #[from_entity_instance]
fields.

Change the attribute to work on From<&EntityInstance> instead of
From<EntityInstance>, avoiding the implicit clone in most cases. The clone
is still necessary when a plain EntityInstance field exists in a struct
deriving LdtkEntity, which is handled by having EntityInstance implement
From<&EntityInstance>.

While being a breaking change, fixing up users should be trivial - in
most cases no owned EntityInstance is needed at all, and where it is an
explicit clone can be inserted.
  • Loading branch information
neocturne committed Jan 9, 2023
1 parent 45303f3 commit 246880f
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 10 deletions.
10 changes: 5 additions & 5 deletions examples/platformer/components.rs
Expand Up @@ -24,8 +24,8 @@ pub struct SensorBundle {
pub rotation_constraints: LockedAxes,
}

impl From<EntityInstance> for ColliderBundle {
fn from(entity_instance: EntityInstance) -> ColliderBundle {
impl From<&EntityInstance> for ColliderBundle {
fn from(entity_instance: &EntityInstance) -> ColliderBundle {
let rotation_constraints = LockedAxes::ROTATION_LOCKED;

match entity_instance.identifier.as_ref() {
Expand Down Expand Up @@ -81,8 +81,8 @@ impl From<IntGridCell> for SensorBundle {
#[derive(Clone, Component, Debug, Eq, Default, PartialEq)]
pub struct Items(Vec<String>);

impl From<EntityInstance> for Items {
fn from(entity_instance: EntityInstance) -> Self {
impl From<&EntityInstance> for Items {
fn from(entity_instance: &EntityInstance) -> Self {
let mut items: Vec<String> = vec![];

if let Some(field_instance) = entity_instance
Expand Down Expand Up @@ -128,7 +128,7 @@ pub struct PlayerBundle {
pub climber: Climber,
pub ground_detection: GroundDetection,

// Build Items Component manually by using `impl From<EntityInstance>
// Build Items Component manually by using `impl From<&EntityInstance>`
#[from_entity_instance]
items: Items,

Expand Down
2 changes: 1 addition & 1 deletion macros/src/ldtk_entity.rs
Expand Up @@ -318,7 +318,7 @@ fn expand_from_entity_instance_attribute(
{
syn::Meta::Path(_) => {
quote! {
#field_name: <#field_type as From<bevy_ecs_ldtk::prelude::EntityInstance>>::from(entity_instance.clone()),
#field_name: <#field_type as From<&bevy_ecs_ldtk::prelude::EntityInstance>>::from(entity_instance),
}
}
_ => {
Expand Down
9 changes: 5 additions & 4 deletions src/app/ldtk_entity.rs
Expand Up @@ -207,19 +207,20 @@ use crate::app::register_ldtk_objects::RegisterLdtkObjects;
/// ```
///
/// ### `#[from_entity_instance]`
/// Indicates that a component or bundle that implements [From<EntityInstance>] should be created
/// Indicates that a component or bundle that implements [From<&EntityInstance>] should be created
/// using that conversion.
/// This allows for more modular and custom component construction, and for different structs that
/// contain the same component to have different constructions of that component, without having to
/// `impl LdtkEntity` for both of them.
/// It also allows you to have an [EntityInstance] field, since all types `T` implement `From<T>`.
/// It also allows you to have an [EntityInstance] field, since `EntityInstance` implements
/// `From<&EntityInstance>`.
/// ```
/// # use bevy::prelude::*;
/// # use bevy_ecs_ldtk::prelude::*;
/// # #[derive(Component, Default)]
/// # struct Sellable { value: i32 }
/// impl From<EntityInstance> for Sellable {
/// fn from(entity_instance: EntityInstance) -> Sellable {
/// impl From<&EntityInstance> for Sellable {
/// fn from(entity_instance: &EntityInstance) -> Sellable {
/// let sell_value = match entity_instance.identifier.as_str() {
/// "gem" => 1000,
/// "nickel" => 5,
Expand Down
7 changes: 7 additions & 0 deletions src/ldtk/mod.rs
Expand Up @@ -1023,6 +1023,13 @@ pub struct EntityInstance {
pub width: i32,
}

// Allow using #[from_entity_instance] to add EntityInstance fields when deriving LdtkEntity
impl From<&EntityInstance> for EntityInstance {
fn from(value: &EntityInstance) -> Self {
value.clone()
}
}

/// This object is used in Field Instances to describe an EntityRef value.
#[derive(Eq, PartialEq, Debug, Default, Clone, Serialize, Deserialize)]
pub struct FieldInstanceEntityReference {
Expand Down

0 comments on commit 246880f

Please sign in to comment.