From 51f9b2d6cf9ce30fc8eb327c869f1e86fbe285ae Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Tue, 21 May 2024 12:43:41 -0700 Subject: [PATCH] Change return type to Result --- .../derive/src/container_attributes.rs | 10 +- crates/bevy_reflect/derive/src/derive_data.rs | 61 +++++-- .../bevy_reflect/derive/src/enum_utility.rs | 167 +++++++++--------- .../bevy_reflect/derive/src/impls/values.rs | 4 +- crates/bevy_reflect/src/error.rs | 61 +++++++ crates/bevy_reflect/src/fields.rs | 17 ++ crates/bevy_reflect/src/impls/std.rs | 26 +-- crates/bevy_reflect/src/lib.rs | 63 ++++++- crates/bevy_reflect/src/reflect.rs | 11 +- crates/bevy_reflect/src/tuple.rs | 14 +- 10 files changed, 307 insertions(+), 127 deletions(-) create mode 100644 crates/bevy_reflect/src/error.rs diff --git a/crates/bevy_reflect/derive/src/container_attributes.rs b/crates/bevy_reflect/derive/src/container_attributes.rs index f93559da384aec..2b268c118eedd7 100644 --- a/crates/bevy_reflect/derive/src/container_attributes.rs +++ b/crates/bevy_reflect/derive/src/container_attributes.rs @@ -9,7 +9,7 @@ use crate::custom_attributes::CustomAttributes; use crate::derive_data::ReflectTraitToImpl; use crate::utility; use crate::utility::terminated_parser; -use bevy_macro_utils::fq_std::{FQAny, FQBox, FQClone, FQOption}; +use bevy_macro_utils::fq_std::{FQAny, FQBox, FQClone, FQOption, FQResult}; use proc_macro2::{Ident, Span}; use quote::quote_spanned; use syn::ext::IdentExt; @@ -541,14 +541,14 @@ impl ContainerAttributes { match &self.clone { &TraitImpl::Implemented(span) => Some(quote_spanned! {span=> #[inline] - fn reflect_clone(&self) -> #FQOption<#FQBox> { - #FQOption::Some(#FQBox::new(#FQClone::clone(self))) + fn reflect_clone(&self) -> #FQResult<#FQBox, #bevy_reflect_path::ReflectCloneError> { + #FQResult::Ok(#FQBox::new(#FQClone::clone(self))) } }), &TraitImpl::Custom(ref impl_fn, span) => Some(quote_spanned! {span=> #[inline] - fn reflect_clone(&self) -> #FQOption<#FQBox> { - #FQOption::Some(#FQBox::new(#impl_fn(self))) + fn reflect_clone(&self) -> #FQResult<#FQBox, #bevy_reflect_path::ReflectCloneError> { + #FQResult::Ok(#FQBox::new(#impl_fn(self))) } }), TraitImpl::NotImplemented => None, diff --git a/crates/bevy_reflect/derive/src/derive_data.rs b/crates/bevy_reflect/derive/src/derive_data.rs index ef75d03ab689dc..dffb84090a442d 100644 --- a/crates/bevy_reflect/derive/src/derive_data.rs +++ b/crates/bevy_reflect/derive/src/derive_data.rs @@ -14,7 +14,7 @@ use crate::{ utility, REFLECT_ATTRIBUTE_NAME, REFLECT_VALUE_ATTRIBUTE_NAME, TYPE_NAME_ATTRIBUTE_NAME, TYPE_PATH_ATTRIBUTE_NAME, }; -use bevy_macro_utils::fq_std::{FQBox, FQClone, FQOption}; +use bevy_macro_utils::fq_std::{FQBox, FQClone, FQOption, FQResult}; use syn::punctuated::Punctuated; use syn::spanned::Spanned; use syn::{ @@ -520,6 +520,20 @@ impl<'a> StructField<'a> { info } + + /// Returns a token stream for generating a `FieldId` for this field. + pub fn field_id(&self, bevy_reflect_path: &Path) -> proc_macro2::TokenStream { + match &self.data.ident { + Some(ident) => { + let name = ident.to_string(); + quote!(#bevy_reflect_path::FieldId::Named(#name)) + } + None => { + let index = self.declaration_index; + quote!(#bevy_reflect_path::FieldId::Unnamed(#index)) + } + } + } } impl<'a> ReflectStruct<'a> { @@ -635,16 +649,41 @@ impl<'a> ReflectStruct<'a> { let mut tokens = proc_macro2::TokenStream::new(); for field in self.fields() { + let field_ty = &field.data.ty; let member = ident_or_index(field.data.ident.as_ref(), field.declaration_index); match &field.attrs.clone { CloneBehavior::Default => { - if field.attrs.ignore.is_ignored() { - return None; - } + let value = if field.attrs.ignore.is_ignored() { + let field_id = field.field_id(bevy_reflect_path); + + quote! { + return #FQResult::Err(#bevy_reflect_path::ReflectCloneError::FieldNotClonable { + field: #field_id, + variant: #FQOption::None, + container_type_path: ::std::borrow::Cow::Borrowed( + ::type_path() + ) + }) + } + } else { + quote! { + #bevy_reflect_path::Reflect::reflect_clone(&self.#member)? + .take() + .map_err(|value| #bevy_reflect_path::ReflectCloneError::FailedDowncast { + expected: ::std::borrow::Cow::Borrowed( + <#field_ty as #bevy_reflect_path::TypePath>::type_path() + ), + received: ::std::borrow::Cow::Owned( + #bevy_reflect_path::DynamicTypePath::reflect_type_path(&*value) + .to_string() + ), + })? + } + }; tokens.extend(quote! { - #member: #bevy_reflect_path::Reflect::reflect_clone(&self.#member)?.take().ok()?, + #member: #value, }); } CloneBehavior::Trait => { @@ -662,8 +701,9 @@ impl<'a> ReflectStruct<'a> { Some(quote! { #[inline] - fn reflect_clone(&self) -> #FQOption<#FQBox> { - #FQOption::Some(#FQBox::new(Self { + fn reflect_clone(&self) -> #FQResult<#FQBox, #bevy_reflect_path::ReflectCloneError> { + #[allow(unreachable_code)] + #FQResult::Ok(#FQBox::new(Self { #tokens })) } @@ -772,12 +812,13 @@ impl<'a> ReflectEnum<'a> { variant_patterns, variant_constructors, .. - } = ReflectCloneVariantBuilder::new(self).build(&this)?; + } = ReflectCloneVariantBuilder::new(self).build(&this); Some(quote! { #[inline] - fn reflect_clone(&#this) -> #FQOption<#FQBox> { - #FQOption::Some(#FQBox::new(match #this { + fn reflect_clone(&self) -> #FQResult<#FQBox, #bevy_reflect_path::ReflectCloneError> { + #[allow(unreachable_code)] + #FQResult::Ok(#FQBox::new(match #this { #(#variant_patterns => #variant_constructors),* })) } diff --git a/crates/bevy_reflect/derive/src/enum_utility.rs b/crates/bevy_reflect/derive/src/enum_utility.rs index 571a52f0b1147b..0302f27f4aa138 100644 --- a/crates/bevy_reflect/derive/src/enum_utility.rs +++ b/crates/bevy_reflect/derive/src/enum_utility.rs @@ -1,7 +1,7 @@ use crate::derive_data::StructField; use crate::field_attributes::{CloneBehavior, DefaultBehavior}; use crate::{derive_data::ReflectEnum, utility::ident_or_index}; -use bevy_macro_utils::fq_std::{FQClone, FQDefault, FQOption}; +use bevy_macro_utils::fq_std::{FQClone, FQDefault, FQOption, FQResult}; use proc_macro2::{Ident, TokenStream}; use quote::{format_ident, quote}; @@ -34,9 +34,6 @@ pub(crate) struct VariantField<'a, 'b> { /// Trait used to control how enum variants are built. pub(crate) trait VariantBuilder: Sized { - /// The variant output data. - type Output; - /// Returns the enum data. fn reflect_enum(&self) -> &ReflectEnum; @@ -121,83 +118,79 @@ pub(crate) trait VariantBuilder: Sized { /// Returns a token stream that constructs an instance of an ignored field. /// - /// If the ignored field cannot be adequately handled, `None` should be returned. - /// /// # Parameters /// * `field`: The field to access - fn on_ignored_field(&self, field: VariantField) -> Option { - Some(match &field.field.attrs.default { + fn on_ignored_field(&self, field: VariantField) -> TokenStream { + match &field.field.attrs.default { DefaultBehavior::Func(path) => quote! { #path() }, _ => quote! { #FQDefault::default() }, - }) + } } /// Builds the enum variant output data. - fn build(&self, this: &Ident) -> Self::Output; -} + fn build(&self, this: &Ident) -> EnumVariantOutputData { + let variants = self.reflect_enum().variants(); -fn build(builder: &V, this: &Ident) -> Option { - let variants = builder.reflect_enum().variants(); + let mut variant_names = Vec::with_capacity(variants.len()); + let mut variant_patterns = Vec::with_capacity(variants.len()); + let mut variant_constructors = Vec::with_capacity(variants.len()); - let mut variant_names = Vec::with_capacity(variants.len()); - let mut variant_patterns = Vec::with_capacity(variants.len()); - let mut variant_constructors = Vec::with_capacity(variants.len()); + for variant in variants { + let variant_ident = &variant.data.ident; + let variant_name = variant_ident.to_string(); + let variant_path = self.reflect_enum().get_unit(variant_ident); - for variant in variants { - let variant_ident = &variant.data.ident; - let variant_name = variant_ident.to_string(); - let variant_path = builder.reflect_enum().get_unit(variant_ident); + let fields = variant.fields(); - let fields = variant.fields(); + let mut field_patterns = Vec::with_capacity(fields.len()); + let mut field_constructors = Vec::with_capacity(fields.len()); - let mut field_patterns = Vec::with_capacity(fields.len()); - let mut field_constructors = Vec::with_capacity(fields.len()); + for field in fields { + let member = ident_or_index(field.data.ident.as_ref(), field.declaration_index); + let alias = format_ident!("_{}", member); - for field in fields { - let member = ident_or_index(field.data.ident.as_ref(), field.declaration_index); - let alias = format_ident!("_{}", member); + let variant_field = VariantField { + alias: &alias, + variant_name: &variant_name, + field, + }; - let variant_field = VariantField { - alias: &alias, - variant_name: &variant_name, - field, - }; + let value = if field.attrs.ignore.is_ignored() { + self.on_ignored_field(variant_field) + } else { + self.on_active_field(this, variant_field) + }; - let value = if field.attrs.ignore.is_ignored() { - builder.on_ignored_field(variant_field)? - } else { - builder.on_active_field(this, variant_field) - }; + field_patterns.push(quote! { + #member: #alias + }); - field_patterns.push(quote! { - #member: #alias - }); + field_constructors.push(quote! { + #member: #value + }); + } - field_constructors.push(quote! { - #member: #value - }); - } + let pattern = quote! { + #variant_path { #( #field_patterns ),* } + }; - let pattern = quote! { - #variant_path { #( #field_patterns ),* } - }; + let constructor = quote! { + #variant_path { + #( #field_constructors ),* + } + }; - let constructor = quote! { - #variant_path { - #( #field_constructors ),* - } - }; + variant_names.push(variant_name); + variant_patterns.push(pattern); + variant_constructors.push(constructor); + } - variant_names.push(variant_name); - variant_patterns.push(pattern); - variant_constructors.push(constructor); + EnumVariantOutputData { + variant_names, + variant_patterns, + variant_constructors, + } } - - Some(EnumVariantOutputData { - variant_names, - variant_patterns, - variant_constructors, - }) } /// Generates the enum variant output data needed to build the `FromReflect::from_reflect` implementation. @@ -212,8 +205,6 @@ impl<'a> FromReflectVariantBuilder<'a> { } impl<'a> VariantBuilder for FromReflectVariantBuilder<'a> { - type Output = EnumVariantOutputData; - fn reflect_enum(&self) -> &ReflectEnum { self.reflect_enum } @@ -232,10 +223,6 @@ impl<'a> VariantBuilder for FromReflectVariantBuilder<'a> { <#field_ty as #bevy_reflect_path::FromReflect>::from_reflect(#alias)? } } - - fn build(&self, this: &Ident) -> Self::Output { - build(self, this).expect("internal bevy_reflect error: ignored fields should be handled") - } } /// Generates the enum variant output data needed to build the `Reflect::try_apply` implementation. @@ -250,8 +237,6 @@ impl<'a> TryApplyVariantBuilder<'a> { } impl<'a> VariantBuilder for TryApplyVariantBuilder<'a> { - type Output = EnumVariantOutputData; - fn reflect_enum(&self) -> &ReflectEnum { self.reflect_enum } @@ -294,10 +279,6 @@ impl<'a> VariantBuilder for TryApplyVariantBuilder<'a> { })? } } - - fn build(&self, this: &Ident) -> Self::Output { - build(self, this).expect("internal bevy_reflect error: ignored fields should be handled") - } } /// Generates the enum variant output data needed to build the `Reflect::reflect_clone` implementation. @@ -312,7 +293,6 @@ impl<'a> ReflectCloneVariantBuilder<'a> { } impl<'a> VariantBuilder for ReflectCloneVariantBuilder<'a> { - type Output = Option; fn reflect_enum(&self) -> &ReflectEnum { self.reflect_enum } @@ -331,11 +311,22 @@ impl<'a> VariantBuilder for ReflectCloneVariantBuilder<'a> { let bevy_reflect_path = self.reflect_enum.meta().bevy_reflect_path(); let alias = field.alias; + let field_ty = &field.field.data.ty; match &field.field.attrs.clone { CloneBehavior::Default => { quote! { - #bevy_reflect_path::Reflect::reflect_clone(#alias)?.take().ok()? + #bevy_reflect_path::Reflect::reflect_clone(#alias)? + .take() + .map_err(|value| #bevy_reflect_path::ReflectCloneError::FailedDowncast { + expected: ::std::borrow::Cow::Borrowed( + <#field_ty as #bevy_reflect_path::TypePath>::type_path() + ), + received: ::std::borrow::Cow::Owned( + #bevy_reflect_path::DynamicTypePath::reflect_type_path(&*value) + .to_string() + ), + })? } } CloneBehavior::Trait => { @@ -355,19 +346,27 @@ impl<'a> VariantBuilder for ReflectCloneVariantBuilder<'a> { self.construct_field(field) } - fn on_ignored_field(&self, field: VariantField) -> Option { + fn on_ignored_field(&self, field: VariantField) -> TokenStream { + let bevy_reflect_path = self.reflect_enum.meta().bevy_reflect_path(); + let variant_name = field.variant_name; let alias = field.alias; match &field.field.attrs.clone { - CloneBehavior::Default => None, - CloneBehavior::Trait => Some(quote! { - #FQClone::clone(#alias) - }), - CloneBehavior::Func(clone_fn) => Some(quote! { #clone_fn() }), - } - } + CloneBehavior::Default => { + let field_id = field.field.field_id(bevy_reflect_path); - fn build(&self, this: &Ident) -> Self::Output { - build(self, this) + quote! { + return #FQResult::Err( + #bevy_reflect_path::ReflectCloneError::FieldNotClonable { + field: #field_id, + variant: #FQOption::Some(::std::borrow::Cow::Borrowed(#variant_name)), + container_type_path: ::std::borrow::Cow::Borrowed(::type_path()) + } + ) + } + } + CloneBehavior::Trait => quote! { #FQClone::clone(#alias) }, + CloneBehavior::Func(clone_fn) => quote! { #clone_fn() }, + } } } diff --git a/crates/bevy_reflect/derive/src/impls/values.rs b/crates/bevy_reflect/derive/src/impls/values.rs index ab18cfc585853c..70105ce1227f4a 100644 --- a/crates/bevy_reflect/derive/src/impls/values.rs +++ b/crates/bevy_reflect/derive/src/impls/values.rs @@ -86,8 +86,8 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> proc_macro2::TokenStream { } #[inline] - fn reflect_clone(&self) -> #FQOption<#FQBox> { - #FQOption::Some(#FQBox::new(#FQClone::clone(self))) + fn reflect_clone(&self) -> #FQResult<#FQBox, #bevy_reflect_path::ReflectCloneError> { + #FQResult::Ok(#FQBox::new(#FQClone::clone(self))) } #[inline] diff --git a/crates/bevy_reflect/src/error.rs b/crates/bevy_reflect/src/error.rs new file mode 100644 index 00000000000000..cc8193d8facd2b --- /dev/null +++ b/crates/bevy_reflect/src/error.rs @@ -0,0 +1,61 @@ +use crate::FieldId; +use alloc::borrow::Cow; +use thiserror::Error; + +/// An error that occurs when cloning a type via [`Reflect::reflect_clone`]. +/// +/// [`Reflect::reflect_clone`]: crate::Reflect::reflect_clone +#[derive(Clone, Debug, Error, PartialEq, Eq)] +pub enum ReflectCloneError { + /// The type does not have a custom implementation for [`Reflect::reflect_clone`]. + /// + /// [`Reflect::reflect_clone`]: crate::Reflect::reflect_clone + #[error("`Reflect::reflect_clone` not implemented for `{type_path}`")] + NotImplemented { type_path: Cow<'static, str> }, + /// The type cannot be cloned via [`Reflect::reflect_clone`]. + /// + /// This type should be returned when a type is intentionally opting out of reflection cloning. + /// + /// [`Reflect::reflect_clone`]: crate::Reflect::reflect_clone + #[error("`{type_path}` cannot be made clonable for `Reflect::reflect_clone`")] + NotClonable { type_path: Cow<'static, str> }, + /// The field cannot be cloned via [`Reflect::reflect_clone`]. + /// + /// When [deriving `Reflect`], this usually means that a field marked with `#[reflect(ignore)]` + /// is missing a `#[reflect(clone)]` attribute. + /// + /// This may be intentional if the field is not meant/able to be cloned. + /// + /// [`Reflect::reflect_clone`]: crate::Reflect::reflect_clone + /// [deriving `Reflect`]: derive@crate::Reflect + #[error( + "field `{}` cannot be made clonable for `Reflect::reflect_clone` (are you missing a `#[reflect(clone)]` attribute?)", + full_path(.field, .variant, .container_type_path) + )] + FieldNotClonable { + field: FieldId, + variant: Option>, + container_type_path: Cow<'static, str>, + }, + /// Could not downcast to the expected type. + /// + /// Realistically this should only occur when a type has incorrectly implemented [`Reflect`]. + /// + /// [`Reflect`]: crate::Reflect + #[error("expected downcast to `{expected}`, but received `{received}`")] + FailedDowncast { + expected: Cow<'static, str>, + received: Cow<'static, str>, + }, +} + +fn full_path( + field: &FieldId, + variant: &Option>, + container_type_path: &Cow<'static, str>, +) -> String { + match variant { + Some(variant) => format!("{}::{}::{}", container_type_path, variant, field), + None => format!("{}::{}", container_type_path, field), + } +} diff --git a/crates/bevy_reflect/src/fields.rs b/crates/bevy_reflect/src/fields.rs index 31855aeb782286..19614eb73bb0c7 100644 --- a/crates/bevy_reflect/src/fields.rs +++ b/crates/bevy_reflect/src/fields.rs @@ -1,5 +1,6 @@ use crate::attributes::{impl_custom_attribute_methods, CustomAttributes}; use crate::{Reflect, TypePath, TypePathTable}; +use core::fmt::{Debug, Display, Formatter}; use std::any::{Any, TypeId}; use std::sync::Arc; @@ -159,3 +160,19 @@ impl UnnamedField { impl_custom_attribute_methods!(self.custom_attributes, "field"); } + +/// A representation of a field's accessor. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum FieldId { + Named(&'static str), + Unnamed(usize), +} + +impl Display for FieldId { + fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { + match self { + Self::Named(name) => Display::fmt(name, f), + Self::Unnamed(index) => Display::fmt(index, f), + } + } +} diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index 8327ee920de894..9c16e8b4fe2fdd 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -6,9 +6,9 @@ use crate::{ self as bevy_reflect, impl_type_path, map_apply, map_partial_eq, map_try_apply, ApplyError, Array, ArrayInfo, ArrayIter, DynamicMap, DynamicTypePath, FromReflect, FromType, GetTypeRegistration, List, ListInfo, ListIter, Map, MapInfo, MapIter, Reflect, - ReflectDeserialize, ReflectFromPtr, ReflectFromReflect, ReflectKind, ReflectMut, ReflectOwned, - ReflectRef, ReflectSerialize, TypeInfo, TypePath, TypeRegistration, TypeRegistry, Typed, - ValueInfo, + ReflectCloneError, ReflectDeserialize, ReflectFromPtr, ReflectFromReflect, ReflectKind, + ReflectMut, ReflectOwned, ReflectRef, ReflectSerialize, TypeInfo, TypePath, TypeRegistration, + TypeRegistry, Typed, ValueInfo, }; use bevy_reflect_derive::{impl_reflect, impl_reflect_value}; use std::fmt; @@ -1219,8 +1219,8 @@ impl Reflect for Cow<'static, str> { Box::new(self.clone()) } - fn reflect_clone(&self) -> Option> { - Some(Box::new(self.clone())) + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Ok(Box::new(self.clone())) } fn reflect_hash(&self) -> Option { @@ -1408,8 +1408,8 @@ impl Reflect for Cow<'s Box::new(List::clone_dynamic(self)) } - fn reflect_clone(&self) -> Option> { - Some(Box::new(self.clone())) + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Ok(Box::new(self.clone())) } fn reflect_hash(&self) -> Option { @@ -1517,8 +1517,8 @@ impl Reflect for &'static str { Box::new(*self) } - fn reflect_clone(&self) -> Option> { - Some(Box::new(*self)) + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Ok(Box::new(*self)) } fn reflect_hash(&self) -> Option { @@ -1631,8 +1631,8 @@ impl Reflect for &'static Path { Box::new(*self) } - fn reflect_clone(&self) -> Option> { - Some(Box::new(*self)) + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Ok(Box::new(*self)) } fn reflect_hash(&self) -> Option { @@ -1740,8 +1740,8 @@ impl Reflect for Cow<'static, Path> { Box::new(self.clone()) } - fn reflect_clone(&self) -> Option> { - Some(Box::new(self.clone())) + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Ok(Box::new(self.clone())) } fn reflect_hash(&self) -> Option { diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 6a18ec8046bb9f..52d91397b3663c 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -474,6 +474,7 @@ //! [derive `Reflect`]: derive@crate::Reflect mod array; +mod error; mod fields; mod from_reflect; mod list; @@ -530,6 +531,7 @@ pub mod prelude { pub use array::*; pub use enums::*; +pub use error::*; pub use fields::*; pub use from_reflect::*; pub use list::*; @@ -547,6 +549,7 @@ pub use bevy_reflect_derive::*; pub use erased_serde; extern crate alloc; +extern crate core; /// Exports used by the reflection macros. /// @@ -909,12 +912,70 @@ mod tests { #[test] fn should_not_clone_ignored_fields() { + // Tuple Struct #[derive(Reflect, Clone, Debug, PartialEq)] struct Foo(#[reflect(ignore)] usize); let foo = Foo(123); let clone = foo.reflect_clone(); - assert!(clone.is_none()); + assert_eq!( + clone.unwrap_err(), + ReflectCloneError::FieldNotClonable { + field: FieldId::Unnamed(0), + variant: None, + container_type_path: Cow::Borrowed(Foo::type_path()), + } + ); + + // Struct + #[derive(Reflect, Clone, Debug, PartialEq)] + struct Bar { + #[reflect(ignore)] + value: usize, + } + + let bar = Bar { value: 123 }; + let clone = bar.reflect_clone(); + assert_eq!( + clone.unwrap_err(), + ReflectCloneError::FieldNotClonable { + field: FieldId::Named("value"), + variant: None, + container_type_path: Cow::Borrowed(Bar::type_path()), + } + ); + + // Enum + #[derive(Reflect, Clone, Debug, PartialEq)] + enum Baz { + Tuple(#[reflect(ignore)] usize), + Struct { + #[reflect(ignore)] + value: usize, + }, + } + + let baz = Baz::Tuple(123); + let clone = baz.reflect_clone(); + assert_eq!( + clone.unwrap_err(), + ReflectCloneError::FieldNotClonable { + field: FieldId::Unnamed(0), + variant: Some(Cow::Borrowed("Tuple")), + container_type_path: Cow::Borrowed(Baz::type_path()), + } + ); + + let baz = Baz::Struct { value: 123 }; + let clone = baz.reflect_clone(); + assert_eq!( + clone.unwrap_err(), + ReflectCloneError::FieldNotClonable { + field: FieldId::Named("value"), + variant: Some(Cow::Borrowed("Struct")), + container_type_path: Cow::Borrowed(Baz::type_path()), + } + ); } #[test] diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index 92b2041c17a886..34524a94d082b3 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -1,8 +1,9 @@ use crate::{ array_debug, enum_debug, list_debug, map_debug, serde::Serializable, struct_debug, tuple_debug, - tuple_struct_debug, Array, DynamicTypePath, Enum, List, Map, Struct, Tuple, TupleStruct, - TypeInfo, TypePath, Typed, ValueInfo, + tuple_struct_debug, Array, DynamicTypePath, Enum, List, Map, ReflectCloneError, Struct, Tuple, + TupleStruct, TypeInfo, TypePath, Typed, ValueInfo, }; +use alloc::borrow::Cow; use std::{ any::{Any, TypeId}, fmt::Debug, @@ -334,8 +335,10 @@ pub trait Reflect: DynamicTypePath + Any + Send + Sync { /// let cloned = value.reflect_clone().unwrap(); /// assert!(cloned.is::<(i32, bool, f64)>()) /// ``` - fn reflect_clone(&self) -> Option> { - None + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Err(ReflectCloneError::NotImplemented { + type_path: Cow::Owned(self.reflect_type_path().to_string()), + }) } /// Returns a hash of the value (which includes the type). diff --git a/crates/bevy_reflect/src/tuple.rs b/crates/bevy_reflect/src/tuple.rs index ddb5824807d998..761a3258c6dd5e 100644 --- a/crates/bevy_reflect/src/tuple.rs +++ b/crates/bevy_reflect/src/tuple.rs @@ -1,11 +1,7 @@ use bevy_reflect_derive::impl_type_path; use bevy_utils::all_tuples; -use crate::{ - self as bevy_reflect, utility::GenericTypePathCell, ApplyError, FromReflect, - GetTypeRegistration, Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, TypePath, - TypeRegistration, TypeRegistry, Typed, UnnamedField, -}; +use crate::{self as bevy_reflect, utility::GenericTypePathCell, ApplyError, FromReflect, GetTypeRegistration, Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, TypePath, TypeRegistration, TypeRegistry, Typed, UnnamedField, ReflectCloneError}; use crate::{ReflectKind, TypePathTable}; use std::any::{Any, TypeId}; use std::fmt::{Debug, Formatter}; @@ -596,10 +592,12 @@ macro_rules! impl_reflect_tuple { Box::new(self.clone_dynamic()) } - fn reflect_clone(&self) -> Option> { - Some(Box::new(( + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Ok(Box::new(( $( - self.$index.reflect_clone()?.take::<$name>().ok()?, + self.$index.reflect_clone()? + .take::<$name>() + .expect("`Reflect::reflect_clone` should return the same type"), )* ))) }