-
-
Notifications
You must be signed in to change notification settings - Fork 4k
Derive Macro for MapEntites (#17611) #18311
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
Conversation
/// } | ||
/// | ||
/// # #[derive(MapEntities)] | ||
/// # struct TestTuple(A); |
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.
An extra #
here
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.
to eliminate that double newline?
match &attr.meta { | ||
syn::Meta::Path(_) => continue 'fields, | ||
_ => panic!("just use the bare `#[skip_mapping]`"), | ||
} |
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.
just continue without checking! This is unnessesary code
fn map_struct(name: syn::Ident, fields: &Fields) -> proc_macro2::TokenStream { | ||
let mut map_entities = vec![]; | ||
|
||
'fields: for (i, field) in fields.iter().enumerate() { |
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.
'fields: for (i, field) in fields.iter().enumerate() { | |
for (i, field) in fields.iter().enumerate() { |
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.
the label is needed here, because later, we iterate over all attributes to find a "skip_mapping".
we could use an attr.iter().find(|attr| attr.path().is_ident("skip_mapping")
to get rid of the label, but that hasnt made the code any easier to follow
match &attr.meta { | ||
syn::Meta::Path(_) => continue 'fields, | ||
_ => panic!("just use the bare `#[skip_mapping]`"), | ||
} |
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.
match &attr.meta { | |
syn::Meta::Path(_) => continue 'fields, | |
_ => panic!("just use the bare `#[skip_mapping]`"), | |
} | |
continue |
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.
here i check that the user has really put a #[skip_mapping]
and not #[skip_mapping(something random)]
it might not be necessary, but i prefer this tool to tell the user exactly how it wants to be used
let map_field = if let Some(field_name) = &field.ident { | ||
// Named field (struct) | ||
quote! { | ||
<#ty as bevy_ecs::entity::MapEntities>::map_entities(&mut self.#field_name, entity_mapper); | ||
} | ||
} else { | ||
// Unnamed field (tuple-like struct) | ||
let idx = syn::Index::from(i); | ||
quote! { | ||
<#ty as bevy_ecs::entity::MapEntities>::map_entities(&mut self.#idx, entity_mapper); | ||
} | ||
}; | ||
|
||
map_entities.push(map_field); |
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.
let map_field = if let Some(field_name) = &field.ident { | |
// Named field (struct) | |
quote! { | |
<#ty as bevy_ecs::entity::MapEntities>::map_entities(&mut self.#field_name, entity_mapper); | |
} | |
} else { | |
// Unnamed field (tuple-like struct) | |
let idx = syn::Index::from(i); | |
quote! { | |
<#ty as bevy_ecs::entity::MapEntities>::map_entities(&mut self.#idx, entity_mapper); | |
} | |
}; | |
map_entities.push(map_field); | |
let member = as_member(&field.ident, i); | |
map_entities.push(quote! { | |
<#ty as bevy_ecs::entity::MapEntities>::map_entities(&mut self.#member, entity_mapper); | |
}); | |
i revisited this pr, and when merging from main, i noticed that this pr has already been made obsolete by #18432 |
Objective
instead of writing boilerplate like this
users can now use the derive macro
MapEntities
.If any fields dont implement that trait, they can be skipped with a
#[skip_mapping]
attribute.Fixes #17611
Solution
My implementation is a heavily simplified version of @Liam19's version, as I didnt want to add too much complexity at once, and I am not familiar with that trait. So feedback is appreciated.
Since its possible that people want to derive enums as well, I have put the derive function in its own file, so that it has space to grow
Testing
Theres now an example in the documentation