Skip to content

Commit

Permalink
Follow up on Todo in bevy_reflect_derive (bevyengine#7461)
Browse files Browse the repository at this point in the history
# Objective

Follow up on Todo in bevy_reflect_derive

## Solution

- Replaced all Instances that do the same as `ident_or_index` with a call to it.
- Only the following Line wasn't replaced, as it only wants the index, and not the ident:
[https://github.com/bevyengine/bevy/blob/69fc8c6b70dd350ef31eb260a1137a2d6c8a3c19/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs#L18](https://github.com/bevyengine/bevy/blob/69fc8c6b70dd350ef31eb260a1137a2d6c8a3c19/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs#L18)
  • Loading branch information
MinerSebas committed Feb 2, 2023
1 parent 6beb463 commit e5b5220
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 31 deletions.
25 changes: 6 additions & 19 deletions crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ use crate::derive_data::ReflectEnum;
use crate::enum_utility::{get_variant_constructors, EnumVariantConstructors};
use crate::field_attributes::DefaultBehavior;
use crate::fq_std::{FQAny, FQClone, FQDefault, FQOption};
use crate::utility::ident_or_index;
use crate::{ReflectMeta, ReflectStruct};
use proc_macro::TokenStream;
use proc_macro2::Span;
use quote::{quote, ToTokens};
use syn::{Field, Ident, Index, Lit, LitInt, LitStr, Member};
use syn::{Field, Ident, Lit, LitInt, LitStr, Member};

/// Implements `FromReflect` for the given struct
pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream {
Expand Down Expand Up @@ -104,8 +105,7 @@ fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> Token
#FQOption::Some(__this)
)
} else {
let MemberValuePair(ignored_members, ignored_values) =
get_ignored_fields(reflect_struct, is_tuple);
let MemberValuePair(ignored_members, ignored_values) = get_ignored_fields(reflect_struct);

quote!(
#FQOption::Some(
Expand Down Expand Up @@ -149,12 +149,12 @@ fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> Token
///
/// Each value of the `MemberValuePair` is a token stream that generates a
/// a default value for the ignored field.
fn get_ignored_fields(reflect_struct: &ReflectStruct, is_tuple: bool) -> MemberValuePair {
fn get_ignored_fields(reflect_struct: &ReflectStruct) -> MemberValuePair {
MemberValuePair::new(
reflect_struct
.ignored_fields()
.map(|field| {
let member = get_ident(field.data, field.index, is_tuple);
let member = ident_or_index(field.data.ident.as_ref(), field.index);

let value = match &field.attrs.default {
DefaultBehavior::Func(path) => quote! {#path()},
Expand Down Expand Up @@ -183,7 +183,7 @@ fn get_active_fields(
reflect_struct
.active_fields()
.map(|field| {
let member = get_ident(field.data, field.index, is_tuple);
let member = ident_or_index(field.data.ident.as_ref(), field.index);
let accessor = get_field_accessor(field.data, field.index, is_tuple);
let ty = field.data.ty.clone();

Expand Down Expand Up @@ -221,19 +221,6 @@ fn get_active_fields(
)
}

/// Returns the member for a given field of a struct or tuple struct.
fn get_ident(field: &Field, index: usize, is_tuple: bool) -> Member {
if is_tuple {
Member::Unnamed(Index::from(index))
} else {
field
.ident
.as_ref()
.map(|ident| Member::Named(ident.clone()))
.unwrap_or_else(|| Member::Unnamed(Index::from(index)))
}
}

/// Returns the accessor for a given field of a struct or tuple struct.
///
/// This differs from a member in that it needs to be a number for tuple structs
Expand Down
12 changes: 2 additions & 10 deletions crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use crate::fq_std::{FQAny, FQBox, FQDefault, FQOption, FQResult};
use crate::impls::impl_typed;
use crate::utility::extend_where_clause;
use crate::utility::{extend_where_clause, ident_or_index};
use crate::ReflectStruct;
use proc_macro::TokenStream;
use quote::{quote, ToTokens};
use syn::{Index, Member};

/// Implements `Struct`, `GetTypeRegistration`, and `Reflect` for the given derive data.
pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream {
Expand All @@ -26,14 +25,7 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream {
.collect::<Vec<String>>();
let field_idents = reflect_struct
.active_fields()
.map(|field| {
field
.data
.ident
.as_ref()
.map(|ident| Member::Named(ident.clone()))
.unwrap_or_else(|| Member::Unnamed(Index::from(field.index)))
})
.map(|field| ident_or_index(field.data.ident.as_ref(), field.index))
.collect::<Vec<_>>();
let field_types = reflect_struct.active_types();
let field_count = field_idents.len();
Expand Down
2 changes: 0 additions & 2 deletions crates/bevy_reflect/bevy_reflect_derive/src/utility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ pub(crate) struct ResultSifter<T> {
/// a tuple struct or a struct with named fields. If you don't have a field name,
/// it means you need to access the struct through an index.
pub(crate) fn ident_or_index(ident: Option<&Ident>, index: usize) -> Member {
// TODO(Quality) when #4761 is merged, code that does this should be replaced
// by a call to `ident_or_index`.
ident.map_or_else(
|| Member::Unnamed(index.into()),
|ident| Member::Named(ident.clone()),
Expand Down

0 comments on commit e5b5220

Please sign in to comment.