From 3743a3a1730317042723c54f7bc51fe425a54b75 Mon Sep 17 00:00:00 2001 From: TehPers Date: Tue, 11 Oct 2022 00:10:32 -0700 Subject: [PATCH 01/10] Support multiple `#[reflect]` + improve errors --- .../src/container_attributes.rs | 115 +++++++++++++++--- .../bevy_reflect_derive/src/derive_data.rs | 46 ++++++- crates/bevy_reflect/src/lib.rs | 42 +++++++ 3 files changed, 177 insertions(+), 26 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs index 99513ed3acbd4..00a3e22b25bee 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs @@ -6,10 +6,11 @@ //! `#[reflect(PartialEq, Default, ...)]` and `#[reflect_value(PartialEq, Default, ...)]`. use crate::utility; -use proc_macro2::Ident; -use quote::quote; +use proc_macro2::{Ident, Span}; +use quote::quote_spanned; use syn::parse::{Parse, ParseStream}; use syn::punctuated::Punctuated; +use syn::spanned::Spanned; use syn::token::Comma; use syn::{Meta, NestedMeta, Path}; @@ -23,19 +24,39 @@ const HASH_ATTR: &str = "Hash"; // but useful to know exist nonetheless pub(crate) const REFLECT_DEFAULT: &str = "ReflectDefault"; +// The error message to show when a trait is specified multiple times +const CONFLICTING_IMPL_MESSAGE: &str = "conflicting reflected trait implementation"; + /// A marker for trait implementations registered via the `Reflect` derive macro. #[derive(Clone, Default)] pub(crate) enum TraitImpl { /// The trait is not registered as implemented. #[default] NotImplemented, + /// The trait is registered as implemented. - Implemented, + Implemented(Span), // TODO: This can be made to use `ExprPath` instead of `Ident`, allowing for fully qualified paths to be used /// The trait is registered with a custom function rather than an actual implementation. - Custom(Ident), + Custom(Ident, Span), } + +impl TraitImpl { + /// Merges this [`TraitImpl`] with another. + /// + /// An error is returned if neither value is [`TraitImpl::NotImplemented`]. + pub fn merge(self, other: TraitImpl) -> Result { + match (self, other) { + (TraitImpl::NotImplemented, value) => Ok(value), + (value, TraitImpl::NotImplemented) => Ok(value), + (_, TraitImpl::Implemented(span) | TraitImpl::Custom(_, span)) => { + Err(syn::Error::new(span, CONFLICTING_IMPL_MESSAGE)) + } + } + } +} + /// A collection of traits that have been registered for a reflected type. /// /// This keeps track of a few traits that are utilized internally for reflection @@ -107,7 +128,9 @@ pub(crate) struct ReflectTraits { impl ReflectTraits { /// Create a new [`ReflectTraits`] instance from a set of nested metas. - pub fn from_nested_metas(nested_metas: &Punctuated) -> Self { + pub fn from_nested_metas( + nested_metas: &Punctuated, + ) -> Result { let mut traits = ReflectTraits::default(); for nested_meta in nested_metas.iter() { match nested_meta { @@ -115,17 +138,35 @@ impl ReflectTraits { NestedMeta::Meta(Meta::Path(path)) => { // Get the first ident in the path (hopefully the path only contains one and not `std::hash::Hash`) let ident = if let Some(segment) = path.segments.iter().next() { - segment.ident.to_string() + &segment.ident } else { continue; }; + let ident_name = ident.to_string(); - match ident.as_str() { - DEBUG_ATTR => traits.debug = TraitImpl::Implemented, - PARTIAL_EQ_ATTR => traits.partial_eq = TraitImpl::Implemented, - HASH_ATTR => traits.hash = TraitImpl::Implemented, + // Track the span where the trait is implemented for future errors + let span = ident.span(); + + match ident_name.as_str() { + DEBUG_ATTR => { + traits.debug = traits.debug.merge(TraitImpl::Implemented(span))?; + } + PARTIAL_EQ_ATTR => { + traits.partial_eq = + traits.partial_eq.merge(TraitImpl::Implemented(span))?; + } + HASH_ATTR => { + traits.hash = traits.hash.merge(TraitImpl::Implemented(span))?; + } // We only track reflected idents for traits not considered special - _ => traits.idents.push(utility::get_reflect_ident(&ident)), + _ => { + // Create the reflect ident + // We set the span to the old ident so any compile errors point to that ident instead + let mut reflect_ident = utility::get_reflect_ident(&ident_name); + reflect_ident.set_span(span); + + add_unique_ident(&mut traits.idents, reflect_ident)?; + } } } // Handles `#[reflect( Hash(custom_hash_fn) )]` @@ -137,11 +178,14 @@ impl ReflectTraits { continue; }; + // Track the span where the trait is implemented for future errors + let span = ident.span(); + let list_meta = list.nested.iter().next(); if let Some(NestedMeta::Meta(Meta::Path(path))) = list_meta { if let Some(segment) = path.segments.iter().next() { // This should be the ident of the custom function - let trait_func_ident = TraitImpl::Custom(segment.ident.clone()); + let trait_func_ident = TraitImpl::Custom(segment.ident.clone(), span); match ident.as_str() { DEBUG_ATTR => traits.debug = trait_func_ident, PARTIAL_EQ_ATTR => traits.partial_eq = trait_func_ident, @@ -155,7 +199,7 @@ impl ReflectTraits { } } - traits + Ok(traits) } /// Returns true if the given reflected trait name (i.e. `ReflectDefault` for `Default`) @@ -174,7 +218,7 @@ impl ReflectTraits { /// If `Hash` was not registered, returns `None`. pub fn get_hash_impl(&self, bevy_reflect_path: &Path) -> Option { match &self.hash { - TraitImpl::Implemented => Some(quote! { + &TraitImpl::Implemented(span) => Some(quote_spanned! {span=> fn reflect_hash(&self) -> Option { use std::hash::{Hash, Hasher}; let mut hasher = #bevy_reflect_path::ReflectHasher::default(); @@ -183,7 +227,7 @@ impl ReflectTraits { Some(hasher.finish()) } }), - TraitImpl::Custom(impl_fn) => Some(quote! { + &TraitImpl::Custom(ref impl_fn, span) => Some(quote_spanned! {span=> fn reflect_hash(&self) -> Option { Some(#impl_fn(self)) } @@ -200,7 +244,7 @@ impl ReflectTraits { bevy_reflect_path: &Path, ) -> Option { match &self.partial_eq { - TraitImpl::Implemented => Some(quote! { + &TraitImpl::Implemented(span) => Some(quote_spanned! {span=> fn reflect_partial_eq(&self, value: &dyn #bevy_reflect_path::Reflect) -> Option { let value = value.as_any(); if let Some(value) = value.downcast_ref::() { @@ -210,7 +254,7 @@ impl ReflectTraits { } } }), - TraitImpl::Custom(impl_fn) => Some(quote! { + &TraitImpl::Custom(ref impl_fn, span) => Some(quote_spanned! {span=> fn reflect_partial_eq(&self, value: &dyn #bevy_reflect_path::Reflect) -> Option { Some(#impl_fn(self, value)) } @@ -224,12 +268,12 @@ impl ReflectTraits { /// If `Debug` was not registered, returns `None`. pub fn get_debug_impl(&self) -> Option { match &self.debug { - TraitImpl::Implemented => Some(quote! { + &TraitImpl::Implemented(span) => Some(quote_spanned! {span=> fn debug(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { std::fmt::Debug::fmt(self, f) } }), - TraitImpl::Custom(impl_fn) => Some(quote! { + &TraitImpl::Custom(ref impl_fn, span) => Some(quote_spanned! {span=> fn debug(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { #impl_fn(self, f) } @@ -237,11 +281,42 @@ impl ReflectTraits { TraitImpl::NotImplemented => None, } } + + /// Merges the trait implementations of this [`ReflectTraits`] with another one. + /// + /// An error is returned if the two [`ReflectTraits`] have conflicting implementations. + pub fn merge(self, other: ReflectTraits) -> Result { + Ok(ReflectTraits { + debug: self.debug.merge(other.debug)?, + hash: self.hash.merge(other.hash)?, + partial_eq: self.partial_eq.merge(other.partial_eq)?, + idents: { + let mut idents = self.idents; + for ident in other.idents { + add_unique_ident(&mut idents, ident)?; + } + idents + }, + }) + } } impl Parse for ReflectTraits { fn parse(input: ParseStream) -> syn::Result { let result = Punctuated::::parse_terminated(input)?; - Ok(ReflectTraits::from_nested_metas(&result)) + ReflectTraits::from_nested_metas(&result) + } +} + +/// Adds an identifier to a vector of identifiers if it is not already present. +/// +/// Returns an error if the identifier already exists in the list. +fn add_unique_ident(idents: &mut Vec, ident: Ident) -> Result<(), syn::Error> { + let ident_name = ident.to_string(); + if idents.iter().any(|i| i == ident_name.as_str()) { + return Err(syn::Error::new(ident.span(), CONFLICTING_IMPL_MESSAGE)); } + + idents.push(ident); + Ok(()) } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index 9fb4295ea7e70..2e791839145c9 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -108,28 +108,62 @@ pub(crate) enum EnumVariantFields<'a> { Unit, } +#[derive(Clone, Copy, PartialEq, Eq)] +enum ReflectType { + Normal, + Value, +} + impl<'a> ReflectDerive<'a> { pub fn from_input(input: &'a DeriveInput) -> Result { let mut traits = ReflectTraits::default(); // Should indicate whether `#[reflect_value]` was used - let mut force_reflect_value = false; + let mut reflect_type = None; for attribute in input.attrs.iter().filter_map(|attr| attr.parse_meta().ok()) { match attribute { Meta::List(meta_list) if meta_list.path.is_ident(REFLECT_ATTRIBUTE_NAME) => { - traits = ReflectTraits::from_nested_metas(&meta_list.nested); + if !matches!(reflect_type, None | Some(ReflectType::Normal)) { + return Err(syn::Error::new( + meta_list.span(), + format_args!("cannot use both `#[{REFLECT_ATTRIBUTE_NAME}]` and `#[{REFLECT_VALUE_ATTRIBUTE_NAME}]`"), + )); + } + + reflect_type = Some(ReflectType::Normal); + let new_traits = ReflectTraits::from_nested_metas(&meta_list.nested)?; + traits = traits.merge(new_traits)?; } Meta::List(meta_list) if meta_list.path.is_ident(REFLECT_VALUE_ATTRIBUTE_NAME) => { - force_reflect_value = true; - traits = ReflectTraits::from_nested_metas(&meta_list.nested); + if !matches!(reflect_type, None | Some(ReflectType::Value)) { + return Err(syn::Error::new( + meta_list.span(), + format_args!("cannot use both `#[{REFLECT_ATTRIBUTE_NAME}]` and `#[{REFLECT_VALUE_ATTRIBUTE_NAME}]`"), + )); + } + + reflect_type = Some(ReflectType::Value); + let new_traits = ReflectTraits::from_nested_metas(&meta_list.nested)?; + traits = traits.merge(new_traits)?; } Meta::Path(path) if path.is_ident(REFLECT_VALUE_ATTRIBUTE_NAME) => { - force_reflect_value = true; + if !matches!(reflect_type, None | Some(ReflectType::Value)) { + return Err(syn::Error::new( + path.span(), + format_args!("cannot use both `#[{REFLECT_ATTRIBUTE_NAME}]` and `#[{REFLECT_VALUE_ATTRIBUTE_NAME}]`"), + )); + } + + reflect_type = Some(ReflectType::Value); } _ => continue, } } - if force_reflect_value { + + // Use normal reflection if unspecified + let reflect_type = reflect_type.unwrap_or(ReflectType::Normal); + + if reflect_type == ReflectType::Value { return Ok(Self::Value(ReflectMeta::new( &input.ident, &input.generics, diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index a9e57d3fbfac0..fb47aeb6c6f68 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -948,6 +948,48 @@ bevy_reflect::tests::should_reflect_debug::Test { assert_eq!(expected, format!("\n{:#?}", reflected)); } + #[test] + fn multiple_reflect_lists() { + #[derive(Hash, PartialEq, Reflect)] + #[reflect(Debug, Hash)] + #[reflect(PartialEq)] + struct Foo(i32); + + impl Debug for Foo { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "Foo") + } + } + + let foo = Foo(123); + let foo: &dyn Reflect = &foo; + + assert!(foo.reflect_hash().is_some()); + assert_eq!(Some(true), foo.reflect_partial_eq(foo)); + assert_eq!("Foo".to_string(), format!("{foo:?}")); + } + + #[test] + fn multiple_reflect_value_lists() { + #[derive(Clone, Hash, PartialEq, Reflect)] + #[reflect_value(Debug, Hash)] + #[reflect_value(PartialEq)] + struct Foo(i32); + + impl Debug for Foo { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "Foo") + } + } + + let foo = Foo(123); + let foo: &dyn Reflect = &foo; + + assert!(foo.reflect_hash().is_some()); + assert_eq!(Some(true), foo.reflect_partial_eq(foo)); + assert_eq!("Foo".to_string(), format!("{foo:?}")); + } + #[cfg(feature = "glam")] mod glam { use super::*; From f1fb7e8d58775e6ef7e4336b7d4cbb8ed1e38d9d Mon Sep 17 00:00:00 2001 From: TehPers Date: Tue, 11 Oct 2022 00:38:37 -0700 Subject: [PATCH 02/10] Merge match arms --- .../bevy_reflect_derive/src/container_attributes.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs index 00a3e22b25bee..aadbf34577c10 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs @@ -48,8 +48,7 @@ impl TraitImpl { /// An error is returned if neither value is [`TraitImpl::NotImplemented`]. pub fn merge(self, other: TraitImpl) -> Result { match (self, other) { - (TraitImpl::NotImplemented, value) => Ok(value), - (value, TraitImpl::NotImplemented) => Ok(value), + (TraitImpl::NotImplemented, value) | (value, TraitImpl::NotImplemented) => Ok(value), (_, TraitImpl::Implemented(span) | TraitImpl::Custom(_, span)) => { Err(syn::Error::new(span, CONFLICTING_IMPL_MESSAGE)) } From b9e38ddda402b058be73a5f745abe9e4f9964bab Mon Sep 17 00:00:00 2001 From: TehPers Date: Tue, 11 Oct 2022 01:34:03 -0700 Subject: [PATCH 03/10] Update docs for `TraitImpl::merge` Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> --- .../bevy_reflect_derive/src/container_attributes.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs index aadbf34577c10..c3e63aaa9dc65 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs @@ -45,7 +45,9 @@ pub(crate) enum TraitImpl { impl TraitImpl { /// Merges this [`TraitImpl`] with another. /// - /// An error is returned if neither value is [`TraitImpl::NotImplemented`]. + /// Returns whichever value is not [`TraitImpl::NotImplemented`]. + /// If both values are [`TraitImpl::NotImplemented`], then that is returned. + /// Otherwise, an error is returned if neither value is [`TraitImpl::NotImplemented`]. pub fn merge(self, other: TraitImpl) -> Result { match (self, other) { (TraitImpl::NotImplemented, value) | (value, TraitImpl::NotImplemented) => Ok(value), From f6f8171781007f4b5c91df062a5d96329c1b828f Mon Sep 17 00:00:00 2001 From: TehPers Date: Tue, 11 Oct 2022 01:45:50 -0700 Subject: [PATCH 04/10] Rename `ReflectType` -> `ReflectMode` --- .../bevy_reflect_derive/src/derive_data.rs | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index 2e791839145c9..d0b6d5b45299b 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -108,9 +108,12 @@ pub(crate) enum EnumVariantFields<'a> { Unit, } +/// The method in which the type should be reflected. #[derive(Clone, Copy, PartialEq, Eq)] -enum ReflectType { +enum ReflectMode { + /// Reflect the type normally, providing information about all fields/variants. Normal, + /// Reflect the type as a value. Value, } @@ -118,52 +121,52 @@ impl<'a> ReflectDerive<'a> { pub fn from_input(input: &'a DeriveInput) -> Result { let mut traits = ReflectTraits::default(); // Should indicate whether `#[reflect_value]` was used - let mut reflect_type = None; + let mut reflect_mode = None; for attribute in input.attrs.iter().filter_map(|attr| attr.parse_meta().ok()) { match attribute { Meta::List(meta_list) if meta_list.path.is_ident(REFLECT_ATTRIBUTE_NAME) => { - if !matches!(reflect_type, None | Some(ReflectType::Normal)) { + if !matches!(reflect_mode, None | Some(ReflectMode::Normal)) { return Err(syn::Error::new( meta_list.span(), format_args!("cannot use both `#[{REFLECT_ATTRIBUTE_NAME}]` and `#[{REFLECT_VALUE_ATTRIBUTE_NAME}]`"), )); } - reflect_type = Some(ReflectType::Normal); + reflect_mode = Some(ReflectMode::Normal); let new_traits = ReflectTraits::from_nested_metas(&meta_list.nested)?; traits = traits.merge(new_traits)?; } Meta::List(meta_list) if meta_list.path.is_ident(REFLECT_VALUE_ATTRIBUTE_NAME) => { - if !matches!(reflect_type, None | Some(ReflectType::Value)) { + if !matches!(reflect_mode, None | Some(ReflectMode::Value)) { return Err(syn::Error::new( meta_list.span(), format_args!("cannot use both `#[{REFLECT_ATTRIBUTE_NAME}]` and `#[{REFLECT_VALUE_ATTRIBUTE_NAME}]`"), )); } - reflect_type = Some(ReflectType::Value); + reflect_mode = Some(ReflectMode::Value); let new_traits = ReflectTraits::from_nested_metas(&meta_list.nested)?; traits = traits.merge(new_traits)?; } Meta::Path(path) if path.is_ident(REFLECT_VALUE_ATTRIBUTE_NAME) => { - if !matches!(reflect_type, None | Some(ReflectType::Value)) { + if !matches!(reflect_mode, None | Some(ReflectMode::Value)) { return Err(syn::Error::new( path.span(), format_args!("cannot use both `#[{REFLECT_ATTRIBUTE_NAME}]` and `#[{REFLECT_VALUE_ATTRIBUTE_NAME}]`"), )); } - reflect_type = Some(ReflectType::Value); + reflect_mode = Some(ReflectMode::Value); } _ => continue, } } // Use normal reflection if unspecified - let reflect_type = reflect_type.unwrap_or(ReflectType::Normal); + let reflect_mode = reflect_mode.unwrap_or(ReflectMode::Normal); - if reflect_type == ReflectType::Value { + if reflect_mode == ReflectMode::Value { return Ok(Self::Value(ReflectMeta::new( &input.ident, &input.generics, From 94391a5767e4b72dfc2aebfa3a540ab99c07fdd6 Mon Sep 17 00:00:00 2001 From: TehPers Date: Tue, 11 Oct 2022 01:54:09 -0700 Subject: [PATCH 05/10] Allow full paths for custom reflected trait impls --- .../src/container_attributes.rs | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs index aadbf34577c10..1d7c223d03c14 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs @@ -37,9 +37,8 @@ pub(crate) enum TraitImpl { /// The trait is registered as implemented. Implemented(Span), - // TODO: This can be made to use `ExprPath` instead of `Ident`, allowing for fully qualified paths to be used /// The trait is registered with a custom function rather than an actual implementation. - Custom(Ident, Span), + Custom(Path, Span), } impl TraitImpl { @@ -182,15 +181,21 @@ impl ReflectTraits { let list_meta = list.nested.iter().next(); if let Some(NestedMeta::Meta(Meta::Path(path))) = list_meta { - if let Some(segment) = path.segments.iter().next() { - // This should be the ident of the custom function - let trait_func_ident = TraitImpl::Custom(segment.ident.clone(), span); - match ident.as_str() { - DEBUG_ATTR => traits.debug = trait_func_ident, - PARTIAL_EQ_ATTR => traits.partial_eq = trait_func_ident, - HASH_ATTR => traits.hash = trait_func_ident, - _ => {} + // This should be the ident of the custom function + let trait_func_ident = TraitImpl::Custom(path.clone(), span); + match ident.as_str() { + DEBUG_ATTR => { + traits.debug = { + traits.debug.merge(trait_func_ident)?; + } } + PARTIAL_EQ_ATTR => { + traits.partial_eq = traits.partial_eq.merge(trait_func_ident)?; + } + HASH_ATTR => { + traits.hash = traits.hash.merge(trait_func_ident)?; + } + _ => {} } } } From da763ad5b952aeb3e05c14e75cd34b90dde6a418 Mon Sep 17 00:00:00 2001 From: TehPers Date: Tue, 11 Oct 2022 01:59:56 -0700 Subject: [PATCH 06/10] Remove extra braces causing error --- .../bevy_reflect_derive/src/container_attributes.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs index 405fbe4e0506f..4955c58e5abbf 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs @@ -187,9 +187,7 @@ impl ReflectTraits { let trait_func_ident = TraitImpl::Custom(path.clone(), span); match ident.as_str() { DEBUG_ATTR => { - traits.debug = { - traits.debug.merge(trait_func_ident)?; - } + traits.debug = traits.debug.merge(trait_func_ident)?; } PARTIAL_EQ_ATTR => { traits.partial_eq = traits.partial_eq.merge(trait_func_ident)?; From 9192d3d64bf0835685500b3d1b3c3ee6ee5c32dd Mon Sep 17 00:00:00 2001 From: TehPers Date: Tue, 11 Oct 2022 12:45:48 -0700 Subject: [PATCH 07/10] Update conflict error message --- .../bevy_reflect_derive/src/container_attributes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs index 4955c58e5abbf..4102e4dd60a67 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs @@ -25,7 +25,7 @@ const HASH_ATTR: &str = "Hash"; pub(crate) const REFLECT_DEFAULT: &str = "ReflectDefault"; // The error message to show when a trait is specified multiple times -const CONFLICTING_IMPL_MESSAGE: &str = "conflicting reflected trait implementation"; +const CONFLICTING_IMPL_MESSAGE: &str = "conflicting type data registration"; /// A marker for trait implementations registered via the `Reflect` derive macro. #[derive(Clone, Default)] From c87f7d4a48313e6df3a6d31b66208bbbac78b2b1 Mon Sep 17 00:00:00 2001 From: TehPers Date: Tue, 11 Oct 2022 12:46:21 -0700 Subject: [PATCH 08/10] Remove extra derives on `ReflectMode` --- crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index d0b6d5b45299b..7114613649f5f 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -109,7 +109,7 @@ pub(crate) enum EnumVariantFields<'a> { } /// The method in which the type should be reflected. -#[derive(Clone, Copy, PartialEq, Eq)] +#[derive(PartialEq, Eq)] enum ReflectMode { /// Reflect the type normally, providing information about all fields/variants. Normal, From d5feec2515deb844b1c81dacb422028dab03f3bb Mon Sep 17 00:00:00 2001 From: TehPers Date: Tue, 11 Oct 2022 12:49:12 -0700 Subject: [PATCH 09/10] Rename const --- .../bevy_reflect_derive/src/container_attributes.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs index 4102e4dd60a67..f6d0f598fe714 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs @@ -24,8 +24,8 @@ const HASH_ATTR: &str = "Hash"; // but useful to know exist nonetheless pub(crate) const REFLECT_DEFAULT: &str = "ReflectDefault"; -// The error message to show when a trait is specified multiple times -const CONFLICTING_IMPL_MESSAGE: &str = "conflicting type data registration"; +// The error message to show when a trait/type is specified multiple times +const CONFLICTING_TYPE_DATA_MESSAGE: &str = "conflicting type data registration"; /// A marker for trait implementations registered via the `Reflect` derive macro. #[derive(Clone, Default)] @@ -51,7 +51,7 @@ impl TraitImpl { match (self, other) { (TraitImpl::NotImplemented, value) | (value, TraitImpl::NotImplemented) => Ok(value), (_, TraitImpl::Implemented(span) | TraitImpl::Custom(_, span)) => { - Err(syn::Error::new(span, CONFLICTING_IMPL_MESSAGE)) + Err(syn::Error::new(span, CONFLICTING_TYPE_DATA_MESSAGE)) } } } @@ -318,7 +318,7 @@ impl Parse for ReflectTraits { fn add_unique_ident(idents: &mut Vec, ident: Ident) -> Result<(), syn::Error> { let ident_name = ident.to_string(); if idents.iter().any(|i| i == ident_name.as_str()) { - return Err(syn::Error::new(ident.span(), CONFLICTING_IMPL_MESSAGE)); + return Err(syn::Error::new(ident.span(), CONFLICTING_TYPE_DATA_MESSAGE)); } idents.push(ident); From 2f55ef1fda3654fbe369295d439d62097622a7d2 Mon Sep 17 00:00:00 2001 From: TehPers Date: Tue, 11 Oct 2022 17:01:15 -0700 Subject: [PATCH 10/10] Update comment Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> --- .../bevy_reflect_derive/src/container_attributes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs index f6d0f598fe714..a6512e15d6e84 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs @@ -183,7 +183,7 @@ impl ReflectTraits { let list_meta = list.nested.iter().next(); if let Some(NestedMeta::Meta(Meta::Path(path))) = list_meta { - // This should be the ident of the custom function + // This should be the path of the custom function let trait_func_ident = TraitImpl::Custom(path.clone(), span); match ident.as_str() { DEBUG_ATTR => {