From f895ce8a76a42d545bf7201f5016c6e46c731ad7 Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Mon, 2 Jan 2023 22:51:10 +0530 Subject: [PATCH 01/21] Update `from_reflect` to return a Result --- .../bevy_reflect_derive/src/from_reflect.rs | 44 +++++---- crates/bevy_reflect/src/from_reflect.rs | 32 ++++++- crates/bevy_reflect/src/impls/smallvec.rs | 16 ++-- crates/bevy_reflect/src/impls/std.rs | 91 ++++++++++++------- crates/bevy_reflect/src/lib.rs | 8 +- crates/bevy_reflect/src/tuple.rs | 12 +-- 6 files changed, 134 insertions(+), 69 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs b/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs index aab2545b54989..8870d4c9ff025 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs @@ -2,7 +2,7 @@ use crate::container_attributes::REFLECT_DEFAULT; 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::fq_std::{FQAny, FQClone, FQDefault, FQOption, FQResult}; use crate::{ReflectMeta, ReflectStruct}; use proc_macro::TokenStream; use proc_macro2::Span; @@ -26,8 +26,8 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream { let (impl_generics, ty_generics, where_clause) = meta.generics().split_for_impl(); TokenStream::from(quote! { impl #impl_generics #bevy_reflect_path::FromReflect for #type_name #ty_generics #where_clause { - fn from_reflect(reflect: &dyn #bevy_reflect_path::Reflect) -> #FQOption { - #FQOption::Some(#FQClone::clone(::downcast_ref::<#type_name #ty_generics>(::as_any(reflect))?)) + fn from_reflect(reflect: &dyn #bevy_reflect_path::Reflect) -> #FQResult { + #FQResult::Ok(#FQClone::clone(::downcast_ref::<#type_name #ty_generics>(::as_any(reflect)).ok_or(#bevy_reflect_path::FromReflectError{ from_type_name: #bevy_reflect_path::Reflect::type_name(reflect), to_type: #bevy_reflect_path::ReflectType::Value })?)) } } }) @@ -35,7 +35,7 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream { /// Implements `FromReflect` for the given enum type pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream { - let fqoption = FQOption.into_token_stream(); + let fqresult = FQResult.into_token_stream(); let type_name = reflect_enum.meta().type_name(); let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path(); @@ -50,14 +50,14 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream { reflect_enum.meta().generics().split_for_impl(); TokenStream::from(quote! { impl #impl_generics #bevy_reflect_path::FromReflect for #type_name #ty_generics #where_clause { - fn from_reflect(#ref_value: &dyn #bevy_reflect_path::Reflect) -> #FQOption { + fn from_reflect(#ref_value: &dyn #bevy_reflect_path::Reflect) -> #FQResult { if let #bevy_reflect_path::ReflectRef::Enum(#ref_value) = #bevy_reflect_path::Reflect::reflect_ref(#ref_value) { match #bevy_reflect_path::Enum::variant_name(#ref_value) { - #(#variant_names => #fqoption::Some(#variant_constructors),)* + #(#variant_names => #fqresult::Ok(#variant_constructors),)* name => panic!("variant with name `{}` does not exist on enum `{}`", name, ::core::any::type_name::()), } } else { - #FQOption::None + #FQResult::Err(#bevy_reflect_path::FromReflectError{ from_type_name: #bevy_reflect_path::Reflect::type_name(#ref_value), to_type: #bevy_reflect_path::ReflectType::Enum }) } } } @@ -75,7 +75,7 @@ impl MemberValuePair { } fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> TokenStream { - let fqoption = FQOption.into_token_stream(); + let fqresult = FQResult.into_token_stream(); let struct_name = reflect_struct.meta().type_name(); let generics = reflect_struct.meta().generics(); @@ -92,23 +92,29 @@ fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> Token let MemberValuePair(active_members, active_values) = get_active_fields(reflect_struct, &ref_struct, &ref_struct_type, is_tuple); + let error = if is_tuple { + quote!(#bevy_reflect_path::FromReflectError{ from_type_name: #bevy_reflect_path::Reflect::type_name(reflect), to_type: #bevy_reflect_path::ReflectType::TupleStruct }) + } else { + quote!(#bevy_reflect_path::FromReflectError{ from_type_name: #bevy_reflect_path::Reflect::type_name(reflect), to_type: #bevy_reflect_path::ReflectType::Struct }) + }; + let constructor = if reflect_struct.meta().traits().contains(REFLECT_DEFAULT) { quote!( let mut __this: Self = #FQDefault::default(); #( - if let #fqoption::Some(__field) = #active_values() { + if let #fqresult::Ok(__field) = #active_values() { // Iff field exists -> use its value __this.#active_members = __field; } )* - #FQOption::Some(__this) + #FQResult::Ok(__this) ) } else { let MemberValuePair(ignored_members, ignored_values) = get_ignored_fields(reflect_struct, is_tuple); quote!( - #FQOption::Some( + #FQResult::Ok( Self { #(#active_members: #active_values()?,)* #(#ignored_members: #ignored_values,)* @@ -134,11 +140,11 @@ fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> Token TokenStream::from(quote! { impl #impl_generics #bevy_reflect_path::FromReflect for #struct_name #ty_generics #where_from_reflect_clause { - fn from_reflect(reflect: &dyn #bevy_reflect_path::Reflect) -> #FQOption { + fn from_reflect(reflect: &dyn #bevy_reflect_path::Reflect) -> #FQResult { if let #bevy_reflect_path::ReflectRef::#ref_struct_type(#ref_struct) = #bevy_reflect_path::Reflect::reflect_ref(reflect) { #constructor } else { - #FQOption::None + #FQResult::Err(#error) } } } @@ -179,6 +185,12 @@ fn get_active_fields( ) -> MemberValuePair { let bevy_reflect_path = reflect_struct.meta().bevy_reflect_path(); + let error = if is_tuple { + quote!(#bevy_reflect_path::FromReflectError{ from_type_name: #bevy_reflect_path::Reflect::type_name(reflect), to_type: #bevy_reflect_path::ReflectType::TupleStruct }) + } else { + quote!(#bevy_reflect_path::FromReflectError{ from_type_name: #bevy_reflect_path::Reflect::type_name(reflect), to_type: #bevy_reflect_path::ReflectType::Struct }) + }; + MemberValuePair::new( reflect_struct .active_fields() @@ -197,7 +209,7 @@ fn get_active_fields( if let #FQOption::Some(field) = #get_field { <#ty as #bevy_reflect_path::FromReflect>::from_reflect(field) } else { - #FQOption::Some(#path()) + #FQResult::Ok(#path()) } ) }, @@ -206,12 +218,12 @@ fn get_active_fields( if let #FQOption::Some(field) = #get_field { <#ty as #bevy_reflect_path::FromReflect>::from_reflect(field) } else { - #FQOption::Some(#FQDefault::default()) + #FQResult::Ok(#FQDefault::default()) } ) }, DefaultBehavior::Required => quote! { - (|| <#ty as #bevy_reflect_path::FromReflect>::from_reflect(#get_field?)) + (|| <#ty as #bevy_reflect_path::FromReflect>::from_reflect(#get_field.ok_or(#error)?)) }, }; diff --git a/crates/bevy_reflect/src/from_reflect.rs b/crates/bevy_reflect/src/from_reflect.rs index e9df08aefc459..11d49dd4be6f7 100644 --- a/crates/bevy_reflect/src/from_reflect.rs +++ b/crates/bevy_reflect/src/from_reflect.rs @@ -1,4 +1,5 @@ use crate::{FromType, Reflect}; +use thiserror::Error; /// A trait for types which can be constructed from a reflected type. /// @@ -12,7 +13,29 @@ use crate::{FromType, Reflect}; /// reflected value. pub trait FromReflect: Reflect + Sized { /// Constructs a concrete instance of `Self` from a reflected value. - fn from_reflect(reflect: &dyn Reflect) -> Option; + fn from_reflect(reflect: &dyn Reflect) -> Result; +} + +/// An enum representing all the kinds of types that a value implementing [`Reflect`] could be. +/// This enum is primarily used in [`FromReflectError`]. +#[derive(Debug)] +pub enum ReflectType { + Struct, + TupleStruct, + Tuple, + List, + Array, + Map, + Enum, + Value, +} + +/// An Error for failed conversion of reflected type to original type in [`FromReflect::from_reflect`]. +#[derive(Error, Debug)] +#[error("The reflected type of `{}` cannot be converted to {:?}", .from_type_name, .to_type)] +pub struct FromReflectError<'a> { + pub from_type_name: &'a str, + pub to_type: ReflectType, } /// Type data that represents the [`FromReflect`] trait and allows it to be used dynamically. @@ -67,7 +90,7 @@ pub trait FromReflect: Reflect + Sized { /// [`DynamicEnum`]: crate::DynamicEnum #[derive(Clone)] pub struct ReflectFromReflect { - from_reflect: fn(&dyn Reflect) -> Option>, + from_reflect: fn(&dyn Reflect) -> Result, FromReflectError>, } impl ReflectFromReflect { @@ -76,7 +99,10 @@ impl ReflectFromReflect { /// This will convert the object to a concrete type if it wasn't already, and return /// the value as `Box`. #[allow(clippy::wrong_self_convention)] - pub fn from_reflect(&self, reflect_value: &dyn Reflect) -> Option> { + pub fn from_reflect<'a>( + &'a self, + reflect_value: &'a dyn Reflect, + ) -> Result, FromReflectError> { (self.from_reflect)(reflect_value) } } diff --git a/crates/bevy_reflect/src/impls/smallvec.rs b/crates/bevy_reflect/src/impls/smallvec.rs index f44957175f384..a3f4ae9e1534c 100644 --- a/crates/bevy_reflect/src/impls/smallvec.rs +++ b/crates/bevy_reflect/src/impls/smallvec.rs @@ -3,8 +3,9 @@ use std::any::Any; use crate::utility::GenericTypeInfoCell; use crate::{ - Array, ArrayIter, FromReflect, FromType, GetTypeRegistration, List, ListInfo, Reflect, - ReflectFromPtr, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, TypeRegistration, Typed, + Array, ArrayIter, FromReflect, FromReflectError, FromType, GetTypeRegistration, List, ListInfo, + Reflect, ReflectFromPtr, ReflectMut, ReflectOwned, ReflectRef, ReflectType, TypeInfo, + TypeRegistration, Typed, }; impl Array for SmallVec @@ -51,7 +52,7 @@ where { fn push(&mut self, value: Box) { let value = value.take::().unwrap_or_else(|value| { - ::Item::from_reflect(&*value).unwrap_or_else(|| { + ::Item::from_reflect(&*value).unwrap_or_else(|_| { panic!( "Attempted to push invalid value of type {}.", value.type_name() @@ -146,15 +147,18 @@ impl FromReflect for SmallVec where T::Item: FromReflect, { - fn from_reflect(reflect: &dyn Reflect) -> Option { + fn from_reflect(reflect: &dyn Reflect) -> Result { if let ReflectRef::List(ref_list) = reflect.reflect_ref() { let mut new_list = Self::with_capacity(ref_list.len()); for field in ref_list.iter() { new_list.push(::Item::from_reflect(field)?); } - Some(new_list) + Ok(new_list) } else { - None + Err(FromReflectError { + from_type_name: reflect.type_name(), + to_type: ReflectType::List, + }) } } } diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index ca7ad9c8710e9..b997348da8fd2 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -1,14 +1,13 @@ use crate::std_traits::ReflectDefault; +use crate::utility::{GenericTypeInfoCell, NonGenericTypeInfoCell}; use crate::{self as bevy_reflect, ReflectFromPtr, ReflectOwned}; use crate::{ map_apply, map_partial_eq, Array, ArrayInfo, ArrayIter, DynamicEnum, DynamicMap, Enum, - EnumInfo, FromReflect, FromType, GetTypeRegistration, List, ListInfo, Map, MapInfo, MapIter, - Reflect, ReflectDeserialize, ReflectMut, ReflectRef, ReflectSerialize, TupleVariantInfo, - TypeInfo, TypeRegistration, Typed, UnitVariantInfo, UnnamedField, ValueInfo, VariantFieldIter, - VariantInfo, VariantType, + EnumInfo, FromReflect, FromReflectError, FromType, GetTypeRegistration, List, ListInfo, Map, + MapInfo, MapIter, Reflect, ReflectDeserialize, ReflectMut, ReflectRef, ReflectSerialize, + ReflectType, TupleVariantInfo, TypeInfo, TypeRegistration, Typed, UnitVariantInfo, + UnnamedField, ValueInfo, VariantFieldIter, VariantInfo, VariantType, }; - -use crate::utility::{GenericTypeInfoCell, NonGenericTypeInfoCell}; use bevy_reflect_derive::{impl_from_reflect_value, impl_reflect_value}; use bevy_utils::{Duration, Instant}; use bevy_utils::{HashMap, HashSet}; @@ -215,7 +214,7 @@ macro_rules! impl_reflect_for_veclike { impl List for $ty { fn push(&mut self, value: Box) { let value = value.take::().unwrap_or_else(|value| { - T::from_reflect(&*value).unwrap_or_else(|| { + T::from_reflect(&*value).unwrap_or_else(|_| { panic!( "Attempted to push invalid value of type {}.", value.type_name() @@ -313,15 +312,18 @@ macro_rules! impl_reflect_for_veclike { } impl FromReflect for $ty { - fn from_reflect(reflect: &dyn Reflect) -> Option { + fn from_reflect(reflect: &dyn Reflect) -> Result { if let ReflectRef::List(ref_list) = reflect.reflect_ref() { let mut new_list = Self::with_capacity(ref_list.len()); for field in ref_list.iter() { $push(&mut new_list, T::from_reflect(field)?); } - Some(new_list) + Ok(new_list) } else { - None + Err(FromReflectError { + from_type_name: reflect.type_name(), + to_type: ReflectType::List, + }) } } } @@ -392,7 +394,7 @@ impl Map for HashMap { value: Box, ) -> Option> { let key = key.take::().unwrap_or_else(|key| { - K::from_reflect(&*key).unwrap_or_else(|| { + K::from_reflect(&*key).unwrap_or_else(|_| { panic!( "Attempted to insert invalid key of type {}.", key.type_name() @@ -400,7 +402,7 @@ impl Map for HashMap { }) }); let value = value.take::().unwrap_or_else(|value| { - V::from_reflect(&*value).unwrap_or_else(|| { + V::from_reflect(&*value).unwrap_or_else(|_| { panic!( "Attempted to insert invalid value of type {}.", value.type_name() @@ -415,7 +417,7 @@ impl Map for HashMap { let mut from_reflect = None; key.downcast_ref::() .or_else(|| { - from_reflect = K::from_reflect(key); + from_reflect = K::from_reflect(key).ok(); from_reflect.as_ref() }) .and_then(|key| self.remove(key)) @@ -507,7 +509,7 @@ where } impl FromReflect for HashMap { - fn from_reflect(reflect: &dyn Reflect) -> Option { + fn from_reflect(reflect: &dyn Reflect) -> Result { if let ReflectRef::Map(ref_map) = reflect.reflect_ref() { let mut new_map = Self::with_capacity(ref_map.len()); for (key, value) in ref_map.iter() { @@ -515,9 +517,12 @@ impl FromReflect for HashMap { let new_value = V::from_reflect(value)?; new_map.insert(new_key, new_value); } - Some(new_map) + Ok(new_map) } else { - None + Err(FromReflectError { + from_type_name: reflect.type_name(), + to_type: ReflectType::Map, + }) } } } @@ -637,15 +642,21 @@ impl Reflect for [T; N] { } impl FromReflect for [T; N] { - fn from_reflect(reflect: &dyn Reflect) -> Option { + fn from_reflect(reflect: &dyn Reflect) -> Result { if let ReflectRef::Array(ref_array) = reflect.reflect_ref() { let mut temp_vec = Vec::with_capacity(ref_array.len()); for field in ref_array.iter() { temp_vec.push(T::from_reflect(field)?); } - temp_vec.try_into().ok() + temp_vec.try_into().map_err(|_| FromReflectError { + from_type_name: reflect.type_name(), + to_type: ReflectType::Array, + }) } else { - None + Err(FromReflectError { + from_type_name: reflect.type_name(), + to_type: ReflectType::Array, + }) } } } @@ -822,7 +833,7 @@ impl Reflect for Option { .clone_value() .take::() .unwrap_or_else(|value| { - T::from_reflect(&*value).unwrap_or_else(|| { + T::from_reflect(&*value).unwrap_or_else(|_| { panic!( "Field in `Some` variant of {} should be of type {}", std::any::type_name::>(), @@ -874,7 +885,7 @@ impl Reflect for Option { } impl FromReflect for Option { - fn from_reflect(reflect: &dyn Reflect) -> Option { + fn from_reflect(reflect: &dyn Reflect) -> Result { if let ReflectRef::Enum(dyn_enum) = reflect.reflect_ref() { match dyn_enum.variant_name() { "Some" => { @@ -889,7 +900,7 @@ impl FromReflect for Option { .clone_value() .take::() .unwrap_or_else(|value| { - T::from_reflect(&*value).unwrap_or_else(|| { + T::from_reflect(&*value).unwrap_or_else(|_| { panic!( "Field in `Some` variant of {} should be of type {}", std::any::type_name::>(), @@ -897,9 +908,9 @@ impl FromReflect for Option { ) }) }); - Some(Some(field)) + Ok(Some(field)) } - "None" => Some(None), + "None" => Ok(None), name => panic!( "variant with name `{}` does not exist on enum `{}`", name, @@ -907,7 +918,10 @@ impl FromReflect for Option { ), } } else { - None + Err(FromReflectError { + from_type_name: reflect.type_name(), + to_type: ReflectType::Enum, + }) } } } @@ -1025,13 +1039,15 @@ impl GetTypeRegistration for Cow<'static, str> { } impl FromReflect for Cow<'static, str> { - fn from_reflect(reflect: &dyn crate::Reflect) -> Option { - Some( - reflect - .as_any() - .downcast_ref::>()? - .clone(), - ) + fn from_reflect(reflect: &dyn crate::Reflect) -> Result { + Ok(reflect + .as_any() + .downcast_ref::>() + .ok_or(FromReflectError { + from_type_name: reflect.type_name(), + to_type: ReflectType::Value, + })? + .clone()) } } @@ -1131,8 +1147,15 @@ impl GetTypeRegistration for &'static Path { } impl FromReflect for &'static Path { - fn from_reflect(reflect: &dyn crate::Reflect) -> Option { - reflect.as_any().downcast_ref::().copied() + fn from_reflect(reflect: &dyn crate::Reflect) -> Result { + reflect + .as_any() + .downcast_ref::() + .copied() + .ok_or(FromReflectError { + from_type_name: reflect.type_name(), + to_type: ReflectType::Value, + }) } } diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index dcf2d85861722..2919b088dd9a3 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -308,9 +308,9 @@ mod tests { }; let dyn_struct = DynamicStruct::default(); - let my_struct = ::from_reflect(&dyn_struct); + let my_struct = ::from_reflect(&dyn_struct).unwrap(); - assert_eq!(Some(expected), my_struct); + assert_eq!(expected, my_struct); } #[test] @@ -338,9 +338,9 @@ mod tests { }; let dyn_struct = DynamicStruct::default(); - let my_struct = ::from_reflect(&dyn_struct); + let my_struct = ::from_reflect(&dyn_struct).unwrap(); - assert_eq!(Some(expected), my_struct); + assert_eq!(expected, my_struct); } #[test] diff --git a/crates/bevy_reflect/src/tuple.rs b/crates/bevy_reflect/src/tuple.rs index cebabaf3ffc9d..b9591fae325c4 100644 --- a/crates/bevy_reflect/src/tuple.rs +++ b/crates/bevy_reflect/src/tuple.rs @@ -1,7 +1,7 @@ use crate::utility::NonGenericTypeInfoCell; use crate::{ - DynamicInfo, FromReflect, GetTypeRegistration, Reflect, ReflectMut, ReflectOwned, ReflectRef, - TypeInfo, TypeRegistration, Typed, UnnamedField, + DynamicInfo, FromReflect, FromReflectError, GetTypeRegistration, Reflect, ReflectMut, + ReflectOwned, ReflectRef, ReflectType, TypeInfo, TypeRegistration, Typed, UnnamedField, }; use std::any::{Any, TypeId}; use std::fmt::{Debug, Formatter}; @@ -588,17 +588,17 @@ macro_rules! impl_reflect_tuple { impl<$($name: FromReflect),*> FromReflect for ($($name,)*) { - fn from_reflect(reflect: &dyn Reflect) -> Option { + fn from_reflect(reflect: &dyn Reflect) -> Result { if let ReflectRef::Tuple(_ref_tuple) = reflect.reflect_ref() { - Some( + Ok( ( $( - <$name as FromReflect>::from_reflect(_ref_tuple.field($index)?)?, + <$name as FromReflect>::from_reflect(_ref_tuple.field($index).ok_or(FromReflectError{ from_type_name: reflect.type_name(), to_type: ReflectType::Tuple })?)?, )* ) ) } else { - None + Err(FromReflectError{ from_type_name: reflect.type_name(), to_type: ReflectType::Tuple }) } } } From 0d73862327703ec62cd5e479b855f98c9cffe507 Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Sat, 14 Jan 2023 11:14:15 +0530 Subject: [PATCH 02/21] Added `ReflectKind` and Added more elaborate errors for `FromReflectError` --- .../bevy_reflect_derive/src/enum_utility.rs | 54 +++++- .../bevy_reflect_derive/src/from_reflect.rs | 97 +++++++--- .../bevy_reflect_derive/src/impls/enums.rs | 4 + .../bevy_reflect_derive/src/impls/structs.rs | 4 + .../src/impls/tuple_structs.rs | 4 + .../bevy_reflect_derive/src/impls/values.rs | 4 + crates/bevy_reflect/src/array.rs | 9 +- crates/bevy_reflect/src/enums/dynamic_enum.rs | 7 +- crates/bevy_reflect/src/from_reflect.rs | 25 +-- crates/bevy_reflect/src/impls/smallvec.rs | 13 +- crates/bevy_reflect/src/impls/std.rs | 125 +++++++++---- crates/bevy_reflect/src/lib.rs | 170 ++++++++++++++++++ crates/bevy_reflect/src/list.rs | 9 +- crates/bevy_reflect/src/map.rs | 8 +- crates/bevy_reflect/src/reflect.rs | 23 ++- crates/bevy_reflect/src/struct_trait.rs | 8 +- crates/bevy_reflect/src/tuple.rs | 35 +++- crates/bevy_reflect/src/tuple_struct.rs | 8 +- crates/bevy_reflect/src/type_info.rs | 5 +- crates/bevy_reflect/src/utility.rs | 11 +- 20 files changed, 522 insertions(+), 101 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs index b8b94c693358d..f90428bd5c4d3 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs @@ -1,4 +1,4 @@ -use crate::fq_std::FQDefault; +use crate::fq_std::{FQBox, FQDefault}; use crate::{ derive_data::{EnumVariantFields, ReflectEnum}, utility::ident_or_index, @@ -53,18 +53,62 @@ pub(crate) fn get_variant_constructors( ); quote!(.expect(#type_err_message)) } else { - quote!(?) + match &field.data.ident { + Some(ident) => { + let name = ident.to_string(); + quote!(.map_err(|err| #bevy_reflect_path::FromReflectError::FieldError { + from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value), + from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value), + to_type: ::type_info(), + field: #name, + source: #FQBox::new(err), + })?) + }, + None => quote!(.map_err(|err| #bevy_reflect_path::FromReflectError::IndexError { + from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value), + from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value), + to_type: ::type_info(), + index: #reflect_index, + source: #FQBox::new(err), + })?) + } }; let field_accessor = match &field.data.ident { Some(ident) => { let name = ident.to_string(); - quote!(.field(#name)) + if can_panic { + quote!(.field(#name)) + } else { + quote!(.field(#name) + .ok_or_else(|| #bevy_reflect_path::FromReflectError::MissingField { + from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value), + from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value), + to_type: ::type_info(), + field: #name, + }) + ) + } + } + None => if can_panic { + quote!(.field_at(#reflect_index)) + } else { + quote!(.field_at(#reflect_index) + .ok_or_else(|| #bevy_reflect_path::FromReflectError::MissingIndex { + from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value), + from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value), + to_type: ::type_info(), + index: #reflect_index, + }) + ) } - None => quote!(.field_at(#reflect_index)), }; reflect_index += 1; let missing_field_err_message = format!("the field {error_repr} was not declared"); - let accessor = quote!(#field_accessor .expect(#missing_field_err_message)); + let accessor = if can_panic { + quote!(#field_accessor .expect(#missing_field_err_message)) + } else { + quote!(#field_accessor?) + }; quote! { #bevy_reflect_path::FromReflect::from_reflect(#ref_value #accessor) #unwrapper diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs b/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs index 8870d4c9ff025..564e8aa363a6e 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs @@ -2,7 +2,7 @@ use crate::container_attributes::REFLECT_DEFAULT; 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, FQResult}; +use crate::fq_std::{FQAny, FQBox, FQClone, FQDefault, FQResult}; use crate::{ReflectMeta, ReflectStruct}; use proc_macro::TokenStream; use proc_macro2::Span; @@ -27,7 +27,14 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream { TokenStream::from(quote! { impl #impl_generics #bevy_reflect_path::FromReflect for #type_name #ty_generics #where_clause { fn from_reflect(reflect: &dyn #bevy_reflect_path::Reflect) -> #FQResult { - #FQResult::Ok(#FQClone::clone(::downcast_ref::<#type_name #ty_generics>(::as_any(reflect)).ok_or(#bevy_reflect_path::FromReflectError{ from_type_name: #bevy_reflect_path::Reflect::type_name(reflect), to_type: #bevy_reflect_path::ReflectType::Value })?)) + #FQResult::Ok(#FQClone::clone( + ::downcast_ref::<#type_name #ty_generics>(::as_any(reflect)) + .ok_or_else(|| #bevy_reflect_path::FromReflectError::InvalidType { + from_type: #bevy_reflect_path::Reflect::get_type_info(reflect), + from_kind: #bevy_reflect_path::Reflect::reflect_kind(reflect), + to_type: ::type_info(), + })? + )) } } }) @@ -36,6 +43,7 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream { /// Implements `FromReflect` for the given enum type pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream { let fqresult = FQResult.into_token_stream(); + let fqbox = FQBox.into_token_stream(); let type_name = reflect_enum.meta().type_name(); let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path(); @@ -53,11 +61,28 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream { fn from_reflect(#ref_value: &dyn #bevy_reflect_path::Reflect) -> #FQResult { if let #bevy_reflect_path::ReflectRef::Enum(#ref_value) = #bevy_reflect_path::Reflect::reflect_ref(#ref_value) { match #bevy_reflect_path::Enum::variant_name(#ref_value) { - #(#variant_names => #fqresult::Ok(#variant_constructors),)* - name => panic!("variant with name `{}` does not exist on enum `{}`", name, ::core::any::type_name::()), + #(#variant_names => (|| #fqresult::Ok(#variant_constructors))().map_err(|err| { + #bevy_reflect_path::FromReflectError::VariantError { + from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value), + from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value), + to_type: ::type_info(), + variant: #variant_names.to_string(), + source: #fqbox::new(err), + } + }),)* + name => #FQResult::Err(#bevy_reflect_path::FromReflectError::MissingVariant { + from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value), + from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value), + to_type: ::type_info(), + variant: name.to_string(), + }), } } else { - #FQResult::Err(#bevy_reflect_path::FromReflectError{ from_type_name: #bevy_reflect_path::Reflect::type_name(#ref_value), to_type: #bevy_reflect_path::ReflectType::Enum }) + #FQResult::Err(#bevy_reflect_path::FromReflectError::InvalidType { + from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value), + from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value), + to_type: ::type_info(), + }) } } } @@ -92,11 +117,11 @@ fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> Token let MemberValuePair(active_members, active_values) = get_active_fields(reflect_struct, &ref_struct, &ref_struct_type, is_tuple); - let error = if is_tuple { - quote!(#bevy_reflect_path::FromReflectError{ from_type_name: #bevy_reflect_path::Reflect::type_name(reflect), to_type: #bevy_reflect_path::ReflectType::TupleStruct }) - } else { - quote!(#bevy_reflect_path::FromReflectError{ from_type_name: #bevy_reflect_path::Reflect::type_name(reflect), to_type: #bevy_reflect_path::ReflectType::Struct }) - }; + let error = quote!(#bevy_reflect_path::FromReflectError::InvalidType { + from_type: #bevy_reflect_path::Reflect::get_type_info(reflect), + from_kind: #bevy_reflect_path::Reflect::reflect_kind(reflect), + to_type: ::type_info(), + }); let constructor = if reflect_struct.meta().traits().contains(REFLECT_DEFAULT) { quote!( @@ -185,12 +210,6 @@ fn get_active_fields( ) -> MemberValuePair { let bevy_reflect_path = reflect_struct.meta().bevy_reflect_path(); - let error = if is_tuple { - quote!(#bevy_reflect_path::FromReflectError{ from_type_name: #bevy_reflect_path::Reflect::type_name(reflect), to_type: #bevy_reflect_path::ReflectType::TupleStruct }) - } else { - quote!(#bevy_reflect_path::FromReflectError{ from_type_name: #bevy_reflect_path::Reflect::type_name(reflect), to_type: #bevy_reflect_path::ReflectType::Struct }) - }; - MemberValuePair::new( reflect_struct .active_fields() @@ -199,15 +218,49 @@ fn get_active_fields( let accessor = get_field_accessor(field.data, field.index, is_tuple); let ty = field.data.ty.clone(); + let missing_error = if is_tuple { + quote!(|| #bevy_reflect_path::FromReflectError::MissingIndex { + from_type: #bevy_reflect_path::Reflect::get_type_info(reflect), + from_kind: #bevy_reflect_path::Reflect::reflect_kind(reflect), + to_type: ::type_info(), + index: #accessor, + }) + } else { + quote!(|| #bevy_reflect_path::FromReflectError::MissingField { + from_type: #bevy_reflect_path::Reflect::get_type_info(reflect), + from_kind: #bevy_reflect_path::Reflect::reflect_kind(reflect), + to_type: ::type_info(), + field: #accessor, + }) + }; + let get_field = quote! { - #bevy_reflect_path::#struct_type::field(#dyn_struct_name, #accessor) + #bevy_reflect_path::#struct_type::field(#dyn_struct_name, #accessor).ok_or_else(#missing_error) + }; + + let error = if is_tuple { + quote!(|err| #bevy_reflect_path::FromReflectError::IndexError { + from_type: #bevy_reflect_path::Reflect::get_type_info(reflect), + from_kind: #bevy_reflect_path::Reflect::reflect_kind(reflect), + to_type: ::type_info(), + index: #accessor, + source: #FQBox::new(err), + }) + } else { + quote!(|err| #bevy_reflect_path::FromReflectError::FieldError { + from_type: #bevy_reflect_path::Reflect::get_type_info(reflect), + from_kind: #bevy_reflect_path::Reflect::reflect_kind(reflect), + to_type: ::type_info(), + field: #accessor, + source: #FQBox::new(err), + }) }; let value = match &field.attrs.default { DefaultBehavior::Func(path) => quote! { (|| - if let #FQOption::Some(field) = #get_field { - <#ty as #bevy_reflect_path::FromReflect>::from_reflect(field) + if let #FQResult::OK(field) = #get_field { + <#ty as #bevy_reflect_path::FromReflect>::from_reflect(field).map_err(#error) } else { #FQResult::Ok(#path()) } @@ -215,15 +268,15 @@ fn get_active_fields( }, DefaultBehavior::Default => quote! { (|| - if let #FQOption::Some(field) = #get_field { - <#ty as #bevy_reflect_path::FromReflect>::from_reflect(field) + if let #FQResult::Ok(field) = #get_field { + <#ty as #bevy_reflect_path::FromReflect>::from_reflect(field).map_err(#error) } else { #FQResult::Ok(#FQDefault::default()) } ) }, DefaultBehavior::Required => quote! { - (|| <#ty as #bevy_reflect_path::FromReflect>::from_reflect(#get_field.ok_or(#error)?)) + (|| <#ty as #bevy_reflect_path::FromReflect>::from_reflect(#get_field?).map_err(#error)) }, }; diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs index ce163092fac5a..e80aa56a00af8 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs @@ -262,6 +262,10 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream { } } + fn reflect_kind(&self) -> #bevy_reflect_path::ReflectKind { + #bevy_reflect_path::ReflectKind::Enum + } + fn reflect_ref(&self) -> #bevy_reflect_path::ReflectRef { #bevy_reflect_path::ReflectRef::Enum(self) } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs index eefffd3ec0506..c2ef406764ead 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs @@ -224,6 +224,10 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream { } } + fn reflect_kind(&self) -> #bevy_reflect_path::ReflectKind { + #bevy_reflect_path::ReflectKind::Struct + } + fn reflect_ref(&self) -> #bevy_reflect_path::ReflectRef { #bevy_reflect_path::ReflectRef::Struct(self) } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs index da187a3ca8a0b..c6a2eedd95feb 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs @@ -185,6 +185,10 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream { } } + fn reflect_kind(&self) -> #bevy_reflect_path::ReflectKind { + #bevy_reflect_path::ReflectKind::TupleStruct + } + fn reflect_ref(&self) -> #bevy_reflect_path::ReflectRef { #bevy_reflect_path::ReflectRef::TupleStruct(self) } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/values.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/values.rs index 5d87027d1a25a..7d764525f7f5d 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/values.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/values.rs @@ -101,6 +101,10 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream { #FQResult::Ok(()) } + fn reflect_kind(&self) -> #bevy_reflect_path::ReflectKind { + #bevy_reflect_path::ReflectKind::Value + } + fn reflect_ref(&self) -> #bevy_reflect_path::ReflectRef { #bevy_reflect_path::ReflectRef::Value(self) } diff --git a/crates/bevy_reflect/src/array.rs b/crates/bevy_reflect/src/array.rs index 438a135c040a3..158c5cb5aef2a 100644 --- a/crates/bevy_reflect/src/array.rs +++ b/crates/bevy_reflect/src/array.rs @@ -1,6 +1,6 @@ use crate::{ - utility::NonGenericTypeInfoCell, DynamicInfo, Reflect, ReflectMut, ReflectOwned, ReflectRef, - TypeInfo, Typed, + utility::NonGenericTypeInfoCell, DynamicInfo, Reflect, ReflectKind, ReflectMut, ReflectOwned, + ReflectRef, TypeInfo, Typed, }; use std::{ any::{Any, TypeId}, @@ -222,6 +222,11 @@ impl Reflect for DynamicArray { Ok(()) } + #[inline] + fn reflect_kind(&self) -> ReflectKind { + ReflectKind::Array + } + #[inline] fn reflect_ref(&self) -> ReflectRef { ReflectRef::Array(self) diff --git a/crates/bevy_reflect/src/enums/dynamic_enum.rs b/crates/bevy_reflect/src/enums/dynamic_enum.rs index fd3a8585b763f..3ff0520d76c46 100644 --- a/crates/bevy_reflect/src/enums/dynamic_enum.rs +++ b/crates/bevy_reflect/src/enums/dynamic_enum.rs @@ -1,7 +1,7 @@ use crate::utility::NonGenericTypeInfoCell; use crate::{ enum_debug, enum_hash, enum_partial_eq, DynamicInfo, DynamicStruct, DynamicTuple, Enum, - Reflect, ReflectMut, ReflectOwned, ReflectRef, Struct, Tuple, TypeInfo, Typed, + Reflect, ReflectKind, ReflectMut, ReflectOwned, ReflectRef, Struct, Tuple, TypeInfo, Typed, VariantFieldIter, VariantType, }; use std::any::Any; @@ -386,6 +386,11 @@ impl Reflect for DynamicEnum { Ok(()) } + #[inline] + fn reflect_kind(&self) -> ReflectKind { + ReflectKind::Enum + } + #[inline] fn reflect_ref(&self) -> ReflectRef { ReflectRef::Enum(self) diff --git a/crates/bevy_reflect/src/from_reflect.rs b/crates/bevy_reflect/src/from_reflect.rs index 11d49dd4be6f7..1a129f99f1111 100644 --- a/crates/bevy_reflect/src/from_reflect.rs +++ b/crates/bevy_reflect/src/from_reflect.rs @@ -1,5 +1,4 @@ -use crate::{FromType, Reflect}; -use thiserror::Error; +use crate::{FromReflectError, FromType, Reflect}; /// A trait for types which can be constructed from a reflected type. /// @@ -16,28 +15,6 @@ pub trait FromReflect: Reflect + Sized { fn from_reflect(reflect: &dyn Reflect) -> Result; } -/// An enum representing all the kinds of types that a value implementing [`Reflect`] could be. -/// This enum is primarily used in [`FromReflectError`]. -#[derive(Debug)] -pub enum ReflectType { - Struct, - TupleStruct, - Tuple, - List, - Array, - Map, - Enum, - Value, -} - -/// An Error for failed conversion of reflected type to original type in [`FromReflect::from_reflect`]. -#[derive(Error, Debug)] -#[error("The reflected type of `{}` cannot be converted to {:?}", .from_type_name, .to_type)] -pub struct FromReflectError<'a> { - pub from_type_name: &'a str, - pub to_type: ReflectType, -} - /// Type data that represents the [`FromReflect`] trait and allows it to be used dynamically. /// /// `FromReflect` allows dynamic types (e.g. [`DynamicStruct`], [`DynamicEnum`], etc.) to be converted diff --git a/crates/bevy_reflect/src/impls/smallvec.rs b/crates/bevy_reflect/src/impls/smallvec.rs index a3f4ae9e1534c..a4cab68f73001 100644 --- a/crates/bevy_reflect/src/impls/smallvec.rs +++ b/crates/bevy_reflect/src/impls/smallvec.rs @@ -4,7 +4,7 @@ use std::any::Any; use crate::utility::GenericTypeInfoCell; use crate::{ Array, ArrayIter, FromReflect, FromReflectError, FromType, GetTypeRegistration, List, ListInfo, - Reflect, ReflectFromPtr, ReflectMut, ReflectOwned, ReflectRef, ReflectType, TypeInfo, + Reflect, ReflectFromPtr, ReflectKind, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, TypeRegistration, Typed, }; @@ -112,6 +112,10 @@ where Ok(()) } + fn reflect_kind(&self) -> ReflectKind { + ReflectKind::List + } + fn reflect_ref(&self) -> ReflectRef { ReflectRef::List(self) } @@ -155,9 +159,10 @@ where } Ok(new_list) } else { - Err(FromReflectError { - from_type_name: reflect.type_name(), - to_type: ReflectType::List, + Err(FromReflectError::InvalidType { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), }) } } diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index b997348da8fd2..b20ca6a10e9c9 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -4,8 +4,8 @@ use crate::{self as bevy_reflect, ReflectFromPtr, ReflectOwned}; use crate::{ map_apply, map_partial_eq, Array, ArrayInfo, ArrayIter, DynamicEnum, DynamicMap, Enum, EnumInfo, FromReflect, FromReflectError, FromType, GetTypeRegistration, List, ListInfo, Map, - MapInfo, MapIter, Reflect, ReflectDeserialize, ReflectMut, ReflectRef, ReflectSerialize, - ReflectType, TupleVariantInfo, TypeInfo, TypeRegistration, Typed, UnitVariantInfo, + MapInfo, MapIter, Reflect, ReflectDeserialize, ReflectKind, ReflectMut, ReflectRef, + ReflectSerialize, TupleVariantInfo, TypeInfo, TypeRegistration, Typed, UnitVariantInfo, UnnamedField, ValueInfo, VariantFieldIter, VariantInfo, VariantType, }; use bevy_reflect_derive::{impl_from_reflect_value, impl_reflect_value}; @@ -271,6 +271,10 @@ macro_rules! impl_reflect_for_veclike { Ok(()) } + fn reflect_kind(&self) -> ReflectKind { + ReflectKind::List + } + fn reflect_ref(&self) -> ReflectRef { ReflectRef::List(self) } @@ -315,14 +319,24 @@ macro_rules! impl_reflect_for_veclike { fn from_reflect(reflect: &dyn Reflect) -> Result { if let ReflectRef::List(ref_list) = reflect.reflect_ref() { let mut new_list = Self::with_capacity(ref_list.len()); - for field in ref_list.iter() { - $push(&mut new_list, T::from_reflect(field)?); + for (idx, field) in ref_list.iter().enumerate() { + $push( + &mut new_list, + T::from_reflect(field).map_err(|err| FromReflectError::IndexError { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), + index: idx, + source: Box::new(err), + })?, + ); } Ok(new_list) } else { - Err(FromReflectError { - from_type_name: reflect.type_name(), - to_type: ReflectType::List, + Err(FromReflectError::InvalidType { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), }) } } @@ -468,6 +482,10 @@ impl Reflect for HashMap { Ok(()) } + fn reflect_kind(&self) -> ReflectKind { + ReflectKind::Map + } + fn reflect_ref(&self) -> ReflectRef { ReflectRef::Map(self) } @@ -513,15 +531,27 @@ impl FromReflect for HashMap { if let ReflectRef::Map(ref_map) = reflect.reflect_ref() { let mut new_map = Self::with_capacity(ref_map.len()); for (key, value) in ref_map.iter() { - let new_key = K::from_reflect(key)?; - let new_value = V::from_reflect(value)?; + let new_key = K::from_reflect(key).map_err(|err| FromReflectError::KeyError { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), + source: Box::new(err), + })?; + let new_value = + V::from_reflect(value).map_err(|err| FromReflectError::ValueError { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), + source: Box::new(err), + })?; new_map.insert(new_key, new_value); } Ok(new_map) } else { - Err(FromReflectError { - from_type_name: reflect.type_name(), - to_type: ReflectType::Map, + Err(FromReflectError::InvalidType { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), }) } } @@ -610,6 +640,11 @@ impl Reflect for [T; N] { Ok(()) } + #[inline] + fn reflect_kind(&self) -> ReflectKind { + ReflectKind::Array + } + #[inline] fn reflect_ref(&self) -> ReflectRef { ReflectRef::Array(self) @@ -645,17 +680,32 @@ impl FromReflect for [T; N] { fn from_reflect(reflect: &dyn Reflect) -> Result { if let ReflectRef::Array(ref_array) = reflect.reflect_ref() { let mut temp_vec = Vec::with_capacity(ref_array.len()); - for field in ref_array.iter() { - temp_vec.push(T::from_reflect(field)?); + for (idx, field) in ref_array.iter().enumerate() { + temp_vec.push(T::from_reflect(field).map_err(|err| { + FromReflectError::IndexError { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), + index: idx, + source: Box::new(err), + } + })?); } - temp_vec.try_into().map_err(|_| FromReflectError { - from_type_name: reflect.type_name(), - to_type: ReflectType::Array, - }) + let temp_vec_len = temp_vec.len(); + temp_vec + .try_into() + .map_err(|_| FromReflectError::InvalidSize { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), + from_len: temp_vec_len, + to_len: N, + }) } else { - Err(FromReflectError { - from_type_name: reflect.type_name(), - to_type: ReflectType::Array, + Err(FromReflectError::InvalidType { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), }) } } @@ -858,6 +908,10 @@ impl Reflect for Option { Ok(()) } + fn reflect_kind(&self) -> ReflectKind { + ReflectKind::Enum + } + fn reflect_ref(&self) -> ReflectRef { ReflectRef::Enum(self) } @@ -918,9 +972,10 @@ impl FromReflect for Option { ), } } else { - Err(FromReflectError { - from_type_name: reflect.type_name(), - to_type: ReflectType::Enum, + Err(FromReflectError::InvalidType { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), }) } } @@ -988,6 +1043,10 @@ impl Reflect for Cow<'static, str> { Ok(()) } + fn reflect_kind(&self) -> ReflectKind { + ReflectKind::Value + } + fn reflect_ref(&self) -> ReflectRef { ReflectRef::Value(self) } @@ -1043,9 +1102,10 @@ impl FromReflect for Cow<'static, str> { Ok(reflect .as_any() .downcast_ref::>() - .ok_or(FromReflectError { - from_type_name: reflect.type_name(), - to_type: ReflectType::Value, + .ok_or_else(|| FromReflectError::InvalidType { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), })? .clone()) } @@ -1098,6 +1158,10 @@ impl Reflect for &'static Path { Ok(()) } + fn reflect_kind(&self) -> ReflectKind { + ReflectKind::Value + } + fn reflect_ref(&self) -> ReflectRef { ReflectRef::Value(self) } @@ -1152,9 +1216,10 @@ impl FromReflect for &'static Path { .as_any() .downcast_ref::() .copied() - .ok_or(FromReflectError { - from_type_name: reflect.type_name(), - to_type: ReflectType::Value, + .ok_or_else(|| FromReflectError::InvalidType { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), }) } } diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 2919b088dd9a3..33d52c1643ec9 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -64,6 +64,176 @@ pub use type_uuid::*; pub use bevy_reflect_derive::*; pub use erased_serde; +use thiserror::Error; + +/// An Error for failed conversion of reflected type to original type in [`FromReflect::from_reflect`]. +/// +/// Within variants `FieldError`, `IndexError`, `VariantError`, `KeyError` and `ValueError`; +/// [`Error::source`](std::error::Error::source) must be used to trace the underlying error. +#[derive(Error, Debug)] +pub enum FromReflectError { + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` of kind {} due to mismatched types or kinds", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), self.display_to_kind())] + InvalidType { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to source type having length of {} and target type having length of {}", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .from_len, .to_len)] + InvalidSize { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + from_len: usize, + to_len: usize, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing field `{}`", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .field)] + MissingField { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + field: &'static str, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing value at index {}", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] + MissingIndex { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + index: usize, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing variant `{}`", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .variant)] + MissingVariant { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + variant: String, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the field `{}`", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .field)] + FieldError { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + field: &'static str, + source: Box, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the value at index `{}`", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] + IndexError { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + index: usize, + source: Box, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the variant `{}`", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .variant)] + VariantError { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + variant: String, + source: Box, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in a key of the Map", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name())] + KeyError { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + source: Box, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in a value of the Map", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name())] + ValueError { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + source: Box, + }, +} + +impl FromReflectError { + /// Returns the [`TypeInfo`] of the source type + pub fn from_type(&self) -> &'static TypeInfo { + match self { + Self::InvalidType { from_type, .. } + | Self::InvalidSize { from_type, .. } + | Self::MissingField { from_type, .. } + | Self::MissingIndex { from_type, .. } + | Self::MissingVariant { from_type, .. } + | Self::FieldError { from_type, .. } + | Self::IndexError { from_type, .. } + | Self::VariantError { from_type, .. } + | Self::KeyError { from_type, .. } + | Self::ValueError { from_type, .. } => from_type, + } + } + + /// Returns the [`TypeInfo`] of the target type + pub fn to_type(&self) -> &'static TypeInfo { + match self { + Self::InvalidType { to_type, .. } + | Self::InvalidSize { to_type, .. } + | Self::MissingField { to_type, .. } + | Self::MissingIndex { to_type, .. } + | Self::MissingVariant { to_type, .. } + | Self::FieldError { to_type, .. } + | Self::IndexError { to_type, .. } + | Self::VariantError { to_type, .. } + | Self::KeyError { to_type, .. } + | Self::ValueError { to_type, .. } => to_type, + } + } + + /// Returns the [`ReflectKind`] of the source type + pub fn from_kind(&self) -> ReflectKind { + *match self { + Self::InvalidType { from_kind, .. } + | Self::InvalidSize { from_kind, .. } + | Self::MissingField { from_kind, .. } + | Self::MissingIndex { from_kind, .. } + | Self::MissingVariant { from_kind, .. } + | Self::FieldError { from_kind, .. } + | Self::IndexError { from_kind, .. } + | Self::VariantError { from_kind, .. } + | Self::KeyError { from_kind, .. } + | Self::ValueError { from_kind, .. } => from_kind, + } + } + + /// Returns the "kind" of source type for display purposes + fn display_from_kind(&self) -> String { + let prefix = if let TypeInfo::Dynamic(_) = self.from_type() { + "(Dynamic)" + } else { + "" + }; + + format!("{}{:?}", prefix, self.from_kind()) + } + + /// Returns the "kind" of target type for display purposes + fn display_to_kind(&self) -> String { + match self.to_type() { + TypeInfo::Struct(_) => "Struct", + TypeInfo::TupleStruct(_) => "TupleStruct", + TypeInfo::Tuple(_) => "Tuple", + TypeInfo::List(_) => "List", + TypeInfo::Array(_) => "Array", + TypeInfo::Map(_) => "Map", + TypeInfo::Enum(_) => "Enum", + TypeInfo::Value(_) => "Value", + TypeInfo::Dynamic(_) => "Dynamic", + } + .to_string() + } +} + #[doc(hidden)] pub mod __macro_exports { use crate::Uuid; diff --git a/crates/bevy_reflect/src/list.rs b/crates/bevy_reflect/src/list.rs index 6a2c2a8767381..63ce24b8d6a7b 100644 --- a/crates/bevy_reflect/src/list.rs +++ b/crates/bevy_reflect/src/list.rs @@ -3,8 +3,8 @@ use std::fmt::{Debug, Formatter}; use crate::utility::NonGenericTypeInfoCell; use crate::{ - Array, ArrayIter, DynamicArray, DynamicInfo, FromReflect, Reflect, ReflectMut, ReflectOwned, - ReflectRef, TypeInfo, Typed, + Array, ArrayIter, DynamicArray, DynamicInfo, FromReflect, Reflect, ReflectKind, ReflectMut, + ReflectOwned, ReflectRef, TypeInfo, Typed, }; /// An ordered, mutable list of [Reflect] items. This corresponds to types like [`std::vec::Vec`]. @@ -245,6 +245,11 @@ impl Reflect for DynamicList { Ok(()) } + #[inline] + fn reflect_kind(&self) -> ReflectKind { + ReflectKind::List + } + #[inline] fn reflect_ref(&self) -> ReflectRef { ReflectRef::List(self) diff --git a/crates/bevy_reflect/src/map.rs b/crates/bevy_reflect/src/map.rs index f5e4a8695f2a2..8270d1dd1c571 100644 --- a/crates/bevy_reflect/src/map.rs +++ b/crates/bevy_reflect/src/map.rs @@ -5,7 +5,9 @@ use std::hash::Hash; use bevy_utils::{Entry, HashMap}; use crate::utility::NonGenericTypeInfoCell; -use crate::{DynamicInfo, Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, Typed}; +use crate::{ + DynamicInfo, Reflect, ReflectKind, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, Typed, +}; /// An ordered mapping between [`Reflect`] values. /// @@ -311,6 +313,10 @@ impl Reflect for DynamicMap { Ok(()) } + fn reflect_kind(&self) -> ReflectKind { + ReflectKind::Map + } + fn reflect_ref(&self) -> ReflectRef { ReflectRef::Map(self) } diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index 3857d8f3cd84b..f7ecfd8273bc4 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -11,6 +11,22 @@ use std::{ use crate::utility::NonGenericTypeInfoCell; pub use bevy_utils::AHasher as ReflectHasher; +/// A simple enumeration of "kinds" of type, without any associated object. +/// +/// A `ReflectKind` is obtained via [`Reflect::reflect_kind`]. +#[derive(Copy, Clone, Debug)] +pub enum ReflectKind { + Struct, + TupleStruct, + Tuple, + List, + Array, + Map, + Enum, + Value, + Dynamic, +} + /// An immutable enumeration of "kinds" of reflected type. /// /// Each variant contains a trait object with methods specific to a kind of @@ -149,7 +165,12 @@ pub trait Reflect: Any + Send + Sync { /// containing the trait object. fn set(&mut self, value: Box) -> Result<(), Box>; - /// Returns an enumeration of "kinds" of type. + /// Returns a simple enumeration of "kinds" of type, without any associated object. + /// + /// See [`ReflectKind`]. + fn reflect_kind(&self) -> ReflectKind; + + /// Returns an immutable enumeration of "kinds" of type. /// /// See [`ReflectRef`]. fn reflect_ref(&self) -> ReflectRef; diff --git a/crates/bevy_reflect/src/struct_trait.rs b/crates/bevy_reflect/src/struct_trait.rs index 4b8485cbb5b3e..896a765be756d 100644 --- a/crates/bevy_reflect/src/struct_trait.rs +++ b/crates/bevy_reflect/src/struct_trait.rs @@ -1,6 +1,7 @@ use crate::utility::NonGenericTypeInfoCell; use crate::{ - DynamicInfo, NamedField, Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, Typed, + DynamicInfo, NamedField, Reflect, ReflectKind, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, + Typed, }; use bevy_utils::{Entry, HashMap}; use std::fmt::{Debug, Formatter}; @@ -424,6 +425,11 @@ impl Reflect for DynamicStruct { Box::new(self.clone_dynamic()) } + #[inline] + fn reflect_kind(&self) -> ReflectKind { + ReflectKind::Struct + } + #[inline] fn reflect_ref(&self) -> ReflectRef { ReflectRef::Struct(self) diff --git a/crates/bevy_reflect/src/tuple.rs b/crates/bevy_reflect/src/tuple.rs index b9591fae325c4..d2a2edfe8318c 100644 --- a/crates/bevy_reflect/src/tuple.rs +++ b/crates/bevy_reflect/src/tuple.rs @@ -1,7 +1,7 @@ use crate::utility::NonGenericTypeInfoCell; use crate::{ - DynamicInfo, FromReflect, FromReflectError, GetTypeRegistration, Reflect, ReflectMut, - ReflectOwned, ReflectRef, ReflectType, TypeInfo, TypeRegistration, Typed, UnnamedField, + DynamicInfo, FromReflect, FromReflectError, GetTypeRegistration, Reflect, ReflectKind, + ReflectMut, ReflectOwned, ReflectRef, TypeInfo, TypeRegistration, Typed, UnnamedField, }; use std::any::{Any, TypeId}; use std::fmt::{Debug, Formatter}; @@ -336,6 +336,11 @@ impl Reflect for DynamicTuple { Box::new(self.clone_dynamic()) } + #[inline] + fn reflect_kind(&self) -> ReflectKind { + ReflectKind::Tuple + } + #[inline] fn reflect_ref(&self) -> ReflectRef { ReflectRef::Tuple(self) @@ -546,6 +551,10 @@ macro_rules! impl_reflect_tuple { Ok(()) } + fn reflect_kind(&self) -> ReflectKind { + ReflectKind::Tuple + } + fn reflect_ref(&self) -> ReflectRef { ReflectRef::Tuple(self) } @@ -593,12 +602,30 @@ macro_rules! impl_reflect_tuple { Ok( ( $( - <$name as FromReflect>::from_reflect(_ref_tuple.field($index).ok_or(FromReflectError{ from_type_name: reflect.type_name(), to_type: ReflectType::Tuple })?)?, + <$name as FromReflect>::from_reflect( + _ref_tuple.field($index) + .ok_or_else(|| FromReflectError::MissingIndex { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), + index: $index, + })? + ).map_err(|err| FromReflectError::IndexError { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), + index: $index, + source: Box::new(err), + })?, )* ) ) } else { - Err(FromReflectError{ from_type_name: reflect.type_name(), to_type: ReflectType::Tuple }) + Err(FromReflectError::InvalidType { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info() + }) } } } diff --git a/crates/bevy_reflect/src/tuple_struct.rs b/crates/bevy_reflect/src/tuple_struct.rs index e8c52bdadddbd..bd547b5a926ac 100644 --- a/crates/bevy_reflect/src/tuple_struct.rs +++ b/crates/bevy_reflect/src/tuple_struct.rs @@ -1,6 +1,7 @@ use crate::utility::NonGenericTypeInfoCell; use crate::{ - DynamicInfo, Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, Typed, UnnamedField, + DynamicInfo, Reflect, ReflectKind, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, Typed, + UnnamedField, }; use std::any::{Any, TypeId}; use std::fmt::{Debug, Formatter}; @@ -327,6 +328,11 @@ impl Reflect for DynamicTupleStruct { Box::new(self.clone_dynamic()) } + #[inline] + fn reflect_kind(&self) -> ReflectKind { + ReflectKind::TupleStruct + } + #[inline] fn reflect_ref(&self) -> ReflectRef { ReflectRef::TupleStruct(self) diff --git a/crates/bevy_reflect/src/type_info.rs b/crates/bevy_reflect/src/type_info.rs index 42e011c14915f..18315a66f814b 100644 --- a/crates/bevy_reflect/src/type_info.rs +++ b/crates/bevy_reflect/src/type_info.rs @@ -23,7 +23,9 @@ use std::any::{Any, TypeId}; /// /// ``` /// # use std::any::Any; -/// # use bevy_reflect::{NamedField, Reflect, ReflectMut, ReflectOwned, ReflectRef, StructInfo, TypeInfo, ValueInfo}; +/// # use bevy_reflect::{ +/// # NamedField, Reflect, ReflectKind, ReflectMut, ReflectOwned, ReflectRef, StructInfo, TypeInfo, ValueInfo +/// # }; /// # use bevy_reflect::utility::NonGenericTypeInfoCell; /// use bevy_reflect::Typed; /// @@ -58,6 +60,7 @@ use std::any::{Any, TypeId}; /// # fn as_reflect_mut(&mut self) -> &mut dyn Reflect { todo!() } /// # fn apply(&mut self, value: &dyn Reflect) { todo!() } /// # fn set(&mut self, value: Box) -> Result<(), Box> { todo!() } +/// # fn reflect_kind(&self) -> ReflectKind { todo!() } /// # fn reflect_ref(&self) -> ReflectRef { todo!() } /// # fn reflect_mut(&mut self) -> ReflectMut { todo!() } /// # fn reflect_owned(self: Box) -> ReflectOwned { todo!() } diff --git a/crates/bevy_reflect/src/utility.rs b/crates/bevy_reflect/src/utility.rs index 8474bcb6e8595..960281f36ed99 100644 --- a/crates/bevy_reflect/src/utility.rs +++ b/crates/bevy_reflect/src/utility.rs @@ -16,7 +16,9 @@ use std::any::{Any, TypeId}; /// /// ``` /// # use std::any::Any; -/// # use bevy_reflect::{NamedField, Reflect, ReflectMut, ReflectOwned, ReflectRef, StructInfo, Typed, TypeInfo}; +/// # use bevy_reflect::{ +/// # NamedField, Reflect, ReflectKind, ReflectMut, ReflectOwned, ReflectRef, StructInfo, Typed, TypeInfo +/// # }; /// use bevy_reflect::utility::NonGenericTypeInfoCell; /// /// struct Foo { @@ -45,6 +47,7 @@ use std::any::{Any, TypeId}; /// # fn as_reflect_mut(&mut self) -> &mut dyn Reflect { todo!() } /// # fn apply(&mut self, value: &dyn Reflect) { todo!() } /// # fn set(&mut self, value: Box) -> Result<(), Box> { todo!() } +/// # fn reflect_kind(&self) -> ReflectKind { todo!() } /// # fn reflect_ref(&self) -> ReflectRef { todo!() } /// # fn reflect_mut(&mut self) -> ReflectMut { todo!() } /// # fn reflect_owned(self: Box) -> ReflectOwned { todo!() } @@ -81,7 +84,10 @@ impl NonGenericTypeInfoCell { /// /// ``` /// # use std::any::Any; -/// # use bevy_reflect::{Reflect, ReflectMut, ReflectOwned, ReflectRef, TupleStructInfo, Typed, TypeInfo, UnnamedField}; +/// # use bevy_reflect::{ +/// # Reflect, ReflectKind, ReflectMut, ReflectOwned, ReflectRef, TupleStructInfo, Typed, +/// # TypeInfo, UnnamedField, +/// # }; /// use bevy_reflect::utility::GenericTypeInfoCell; /// /// struct Foo(T); @@ -108,6 +114,7 @@ impl NonGenericTypeInfoCell { /// # fn as_reflect_mut(&mut self) -> &mut dyn Reflect { todo!() } /// # fn apply(&mut self, value: &dyn Reflect) { todo!() } /// # fn set(&mut self, value: Box) -> Result<(), Box> { todo!() } +/// # fn reflect_kind(&self) -> ReflectKind { todo!() } /// # fn reflect_ref(&self) -> ReflectRef { todo!() } /// # fn reflect_mut(&mut self) -> ReflectMut { todo!() } /// # fn reflect_owned(self: Box) -> ReflectOwned { todo!() } From 7888122004570fb80fb31eece2f96f4a8a167211 Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Sat, 14 Jan 2023 13:11:49 +0530 Subject: [PATCH 03/21] Resolve merge conflicts with 229d6c6 --- crates/bevy_reflect/src/from_reflect.rs | 17 +++++ crates/bevy_reflect/src/impls/std.rs | 98 ++++++++++++------------- 2 files changed, 62 insertions(+), 53 deletions(-) diff --git a/crates/bevy_reflect/src/from_reflect.rs b/crates/bevy_reflect/src/from_reflect.rs index 1a129f99f1111..7ea0538599364 100644 --- a/crates/bevy_reflect/src/from_reflect.rs +++ b/crates/bevy_reflect/src/from_reflect.rs @@ -13,6 +13,23 @@ use crate::{FromReflectError, FromType, Reflect}; pub trait FromReflect: Reflect + Sized { /// Constructs a concrete instance of `Self` from a reflected value. fn from_reflect(reflect: &dyn Reflect) -> Result; + + /// Attempts to downcast the given value to `Self` using, + /// constructing the value using [`from_reflect`] if that fails. + /// + /// This method is more efficient than using [`from_reflect`] for cases where + /// the given value is likely a boxed instance of `Self` (i.e. `Box`) + /// rather than a boxed dynamic type (e.g. [`DynamicStruct`], [`DynamicList`], etc.). + /// + /// [`from_reflect`]: Self::from_reflect + /// [`DynamicStruct`]: crate::DynamicStruct + /// [`DynamicList`]: crate::DynamicList + fn take_from_reflect(reflect: Box) -> Result> { + match reflect.take::() { + Ok(value) => Ok(value), + Err(value) => Self::from_reflect(value.as_ref()).map_err(|_| value), + } + } } /// Type data that represents the [`FromReflect`] trait and allows it to be used dynamically. diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index b20ca6a10e9c9..76a9dc213596b 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -213,13 +213,11 @@ macro_rules! impl_reflect_for_veclike { impl List for $ty { fn push(&mut self, value: Box) { - let value = value.take::().unwrap_or_else(|value| { - T::from_reflect(&*value).unwrap_or_else(|_| { - panic!( - "Attempted to push invalid value of type {}.", - value.type_name() - ) - }) + let value = T::take_from_reflect(value).unwrap_or_else(|value| { + panic!( + "Attempted to push invalid value of type {}.", + value.type_name() + ) }); $push(self, value); } @@ -407,21 +405,17 @@ impl Map for HashMap { key: Box, value: Box, ) -> Option> { - let key = key.take::().unwrap_or_else(|key| { - K::from_reflect(&*key).unwrap_or_else(|_| { - panic!( - "Attempted to insert invalid key of type {}.", - key.type_name() - ) - }) + let key = K::take_from_reflect(key).unwrap_or_else(|key| { + panic!( + "Attempted to insert invalid key of type {}.", + key.type_name() + ) }); - let value = value.take::().unwrap_or_else(|value| { - V::from_reflect(&*value).unwrap_or_else(|_| { - panic!( - "Attempted to insert invalid value of type {}.", - value.type_name() - ) - }) + let value = V::take_from_reflect(value).unwrap_or_else(|value| { + panic!( + "Attempted to insert invalid value of type {}.", + value.type_name() + ) }); self.insert(key, value) .map(|old_value| Box::new(old_value) as Box) @@ -872,25 +866,24 @@ impl Reflect for Option { // New variant -> perform a switch match value.variant_name() { "Some" => { - let field = value - .field_at(0) - .unwrap_or_else(|| { - panic!( - "Field in `Some` variant of {} should exist", - std::any::type_name::>() - ) - }) - .clone_value() - .take::() - .unwrap_or_else(|value| { - T::from_reflect(&*value).unwrap_or_else(|_| { + let field = T::take_from_reflect( + value + .field_at(0) + .unwrap_or_else(|| { panic!( - "Field in `Some` variant of {} should be of type {}", - std::any::type_name::>(), - std::any::type_name::() + "Field in `Some` variant of {} should exist", + std::any::type_name::>() ) }) - }); + .clone_value(), + ) + .unwrap_or_else(|_| { + panic!( + "Field in `Some` variant of {} should be of type {}", + std::any::type_name::>(), + std::any::type_name::() + ) + }); *self = Some(field); } "None" => { @@ -943,25 +936,24 @@ impl FromReflect for Option { if let ReflectRef::Enum(dyn_enum) = reflect.reflect_ref() { match dyn_enum.variant_name() { "Some" => { - let field = dyn_enum - .field_at(0) - .unwrap_or_else(|| { - panic!( - "Field in `Some` variant of {} should exist", - std::any::type_name::>() - ) - }) - .clone_value() - .take::() - .unwrap_or_else(|value| { - T::from_reflect(&*value).unwrap_or_else(|_| { + let field = T::take_from_reflect( + dyn_enum + .field_at(0) + .unwrap_or_else(|| { panic!( - "Field in `Some` variant of {} should be of type {}", - std::any::type_name::>(), - std::any::type_name::() + "Field in `Some` variant of {} should exist", + std::any::type_name::>() ) }) - }); + .clone_value(), + ) + .unwrap_or_else(|_| { + panic!( + "Field in `Some` variant of {} should be of type {}", + std::any::type_name::>(), + std::any::type_name::() + ) + }); Ok(Some(field)) } "None" => Ok(None), From 2d46959ac56e36a2f469f7a0a9829bc002fe8cc1 Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Sat, 14 Jan 2023 15:16:43 +0530 Subject: [PATCH 04/21] Fix compilation issue --- crates/bevy_reflect/src/impls/smallvec.rs | 2 +- crates/bevy_reflect/src/impls/std.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_reflect/src/impls/smallvec.rs b/crates/bevy_reflect/src/impls/smallvec.rs index ad4435832532c..264d6a984f11e 100644 --- a/crates/bevy_reflect/src/impls/smallvec.rs +++ b/crates/bevy_reflect/src/impls/smallvec.rs @@ -52,7 +52,7 @@ where { fn insert(&mut self, index: usize, value: Box) { let value = value.take::().unwrap_or_else(|value| { - ::Item::from_reflect(&*value).unwrap_or_else(|| { + ::Item::from_reflect(&*value).unwrap_or_else(|_| { panic!( "Attempted to insert invalid value of type {}.", value.type_name() diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index 2a809ac66d937..f774dc18e2289 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -214,7 +214,7 @@ macro_rules! impl_reflect_for_veclike { impl List for $ty { fn insert(&mut self, index: usize, value: Box) { let value = value.take::().unwrap_or_else(|value| { - T::from_reflect(&*value).unwrap_or_else(|| { + T::from_reflect(&*value).unwrap_or_else(|_| { panic!( "Attempted to insert invalid value of type {}.", value.type_name() From 78e7adbde55b214686c76fb89269e7a478ca84c4 Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Sat, 14 Jan 2023 15:41:22 +0530 Subject: [PATCH 05/21] Fix spelling error --- crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs b/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs index 564e8aa363a6e..5d3ff6a8c05d5 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs @@ -259,7 +259,7 @@ fn get_active_fields( let value = match &field.attrs.default { DefaultBehavior::Func(path) => quote! { (|| - if let #FQResult::OK(field) = #get_field { + if let #FQResult::Ok(field) = #get_field { <#ty as #bevy_reflect_path::FromReflect>::from_reflect(field).map_err(#error) } else { #FQResult::Ok(#path()) From 861a5fd4a19960844458e7a68fcdf46549e763aa Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Thu, 19 Jan 2023 01:17:26 +0530 Subject: [PATCH 06/21] Make `take_from_reflect` return `FromReflectError` also --- crates/bevy_reflect/src/from_reflect.rs | 11 ++++++----- crates/bevy_reflect/src/impls/std.rs | 6 +++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/crates/bevy_reflect/src/from_reflect.rs b/crates/bevy_reflect/src/from_reflect.rs index 7ea0538599364..cc328a1f80070 100644 --- a/crates/bevy_reflect/src/from_reflect.rs +++ b/crates/bevy_reflect/src/from_reflect.rs @@ -24,11 +24,12 @@ pub trait FromReflect: Reflect + Sized { /// [`from_reflect`]: Self::from_reflect /// [`DynamicStruct`]: crate::DynamicStruct /// [`DynamicList`]: crate::DynamicList - fn take_from_reflect(reflect: Box) -> Result> { - match reflect.take::() { - Ok(value) => Ok(value), - Err(value) => Self::from_reflect(value.as_ref()).map_err(|_| value), - } + fn take_from_reflect( + reflect: Box, + ) -> Result, FromReflectError)> { + reflect + .take::() + .or_else(|value| Self::from_reflect(value.as_ref()).map_err(|err| (value, err))) } } diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index f774dc18e2289..e3c7897cfef56 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -229,7 +229,7 @@ macro_rules! impl_reflect_for_veclike { } fn push(&mut self, value: Box) { - let value = T::take_from_reflect(value).unwrap_or_else(|value| { + let value = T::take_from_reflect(value).unwrap_or_else(|(value, _)| { panic!( "Attempted to push invalid value of type {}.", value.type_name() @@ -423,13 +423,13 @@ impl Map for HashMap { key: Box, value: Box, ) -> Option> { - let key = K::take_from_reflect(key).unwrap_or_else(|key| { + let key = K::take_from_reflect(key).unwrap_or_else(|(key, _)| { panic!( "Attempted to insert invalid key of type {}.", key.type_name() ) }); - let value = V::take_from_reflect(value).unwrap_or_else(|value| { + let value = V::take_from_reflect(value).unwrap_or_else(|(value, _)| { panic!( "Attempted to insert invalid value of type {}.", value.type_name() From 271a18e5121d0d5c7adbcb369bc5905f16ca0637 Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Thu, 19 Jan 2023 01:18:01 +0530 Subject: [PATCH 07/21] Remove panics in `FromReflect` impl for `Option` --- crates/bevy_reflect/src/impls/std.rs | 55 +++++++++++++++++----------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index e3c7897cfef56..6b86a96135034 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -954,32 +954,43 @@ impl FromReflect for Option { if let ReflectRef::Enum(dyn_enum) = reflect.reflect_ref() { match dyn_enum.variant_name() { "Some" => { - let field = T::take_from_reflect( - dyn_enum - .field_at(0) - .unwrap_or_else(|| { - panic!( - "Field in `Some` variant of {} should exist", - std::any::type_name::>() - ) - }) - .clone_value(), - ) - .unwrap_or_else(|_| { - panic!( - "Field in `Some` variant of {} should be of type {}", - std::any::type_name::>(), - std::any::type_name::() + // The closure is only created to provide a scope for `?` operator. + let field = (|| { + T::take_from_reflect( + dyn_enum + .field_at(0) + .ok_or_else(|| FromReflectError::MissingIndex { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), + index: 0, + })? + .clone_value(), ) - }); + .map_err(|(_, err)| FromReflectError::IndexError { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), + index: 0, + source: Box::new(err), + }) + })() + .map_err(|err| FromReflectError::VariantError { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), + variant: "Some".to_string(), + source: Box::new(err), + })?; Ok(Some(field)) } "None" => Ok(None), - name => panic!( - "variant with name `{}` does not exist on enum `{}`", - name, - std::any::type_name::() - ), + name => Err(FromReflectError::MissingVariant { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), + variant: name.to_string(), + }), } } else { Err(FromReflectError::InvalidType { From aa0b3c6234ae6f1ee4da402e226223b256b31b11 Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Thu, 19 Jan 2023 14:06:10 +0530 Subject: [PATCH 08/21] Remove `Dynamic` from `ReflectKind` --- crates/bevy_reflect/src/reflect.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index f7ecfd8273bc4..836cf5622096d 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -24,7 +24,6 @@ pub enum ReflectKind { Map, Enum, Value, - Dynamic, } /// An immutable enumeration of "kinds" of reflected type. From 695796b6e6ecbcec14fb2d9943f7b0d81f1e84e1 Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Thu, 19 Jan 2023 14:30:25 +0530 Subject: [PATCH 09/21] Add `PartialEq`, `Eq` and `Hash` to `ReflectKind` --- crates/bevy_reflect/src/reflect.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index 836cf5622096d..ca17c72c6f99b 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -14,7 +14,7 @@ pub use bevy_utils::AHasher as ReflectHasher; /// A simple enumeration of "kinds" of type, without any associated object. /// /// A `ReflectKind` is obtained via [`Reflect::reflect_kind`]. -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub enum ReflectKind { Struct, TupleStruct, From 5b7bc1416e264e22ffc1691557400ed172be00db Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Thu, 19 Jan 2023 14:44:02 +0530 Subject: [PATCH 10/21] Update `display_from_kind` to return `&str` instead of `String` --- crates/bevy_reflect/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 33d52c1643ec9..c77d842d97cda 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -218,7 +218,7 @@ impl FromReflectError { } /// Returns the "kind" of target type for display purposes - fn display_to_kind(&self) -> String { + fn display_to_kind(&self) -> &str { match self.to_type() { TypeInfo::Struct(_) => "Struct", TypeInfo::TupleStruct(_) => "TupleStruct", @@ -230,7 +230,6 @@ impl FromReflectError { TypeInfo::Value(_) => "Value", TypeInfo::Dynamic(_) => "Dynamic", } - .to_string() } } From 4ba849e0321e3e16a718a4d6c51ca706939e6596 Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Thu, 19 Jan 2023 14:50:29 +0530 Subject: [PATCH 11/21] Add period at the end in docs --- crates/bevy_reflect/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index c77d842d97cda..deefa604155ec 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -158,7 +158,7 @@ pub enum FromReflectError { } impl FromReflectError { - /// Returns the [`TypeInfo`] of the source type + /// Returns the [`TypeInfo`] of the source type. pub fn from_type(&self) -> &'static TypeInfo { match self { Self::InvalidType { from_type, .. } @@ -174,7 +174,7 @@ impl FromReflectError { } } - /// Returns the [`TypeInfo`] of the target type + /// Returns the [`TypeInfo`] of the target type. pub fn to_type(&self) -> &'static TypeInfo { match self { Self::InvalidType { to_type, .. } @@ -190,7 +190,7 @@ impl FromReflectError { } } - /// Returns the [`ReflectKind`] of the source type + /// Returns the [`ReflectKind`] of the source type. pub fn from_kind(&self) -> ReflectKind { *match self { Self::InvalidType { from_kind, .. } @@ -206,7 +206,7 @@ impl FromReflectError { } } - /// Returns the "kind" of source type for display purposes + /// Returns the "kind" of source type for display purposes. fn display_from_kind(&self) -> String { let prefix = if let TypeInfo::Dynamic(_) = self.from_type() { "(Dynamic)" @@ -217,7 +217,7 @@ impl FromReflectError { format!("{}{:?}", prefix, self.from_kind()) } - /// Returns the "kind" of target type for display purposes + /// Returns the "kind" of target type for display purposes. fn display_to_kind(&self) -> &str { match self.to_type() { TypeInfo::Struct(_) => "Struct", From 8c464956b2b330eff35f0dd079abb8a5a789ed52 Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Sat, 21 Jan 2023 02:06:29 +0530 Subject: [PATCH 12/21] Change `variant`'s type to `Cow<'static, str>` --- crates/bevy_reflect/bevy_reflect_derive/src/fq_std.rs | 7 +++++++ .../bevy_reflect/bevy_reflect_derive/src/from_reflect.rs | 7 ++++--- crates/bevy_reflect/src/impls/std.rs | 4 ++-- crates/bevy_reflect/src/lib.rs | 5 +++-- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/fq_std.rs b/crates/bevy_reflect/bevy_reflect_derive/src/fq_std.rs index 7a4e8e78bdc16..9faf9e7d2a46c 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/fq_std.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/fq_std.rs @@ -36,6 +36,7 @@ use quote::{quote, ToTokens}; pub(crate) struct FQAny; pub(crate) struct FQBox; pub(crate) struct FQClone; +pub(crate) struct FQCow; pub(crate) struct FQDefault; pub(crate) struct FQOption; pub(crate) struct FQResult; @@ -58,6 +59,12 @@ impl ToTokens for FQClone { } } +impl ToTokens for FQCow { + fn to_tokens(&self, tokens: &mut TokenStream) { + quote!(::std::borrow::Cow).to_tokens(tokens); + } +} + impl ToTokens for FQDefault { fn to_tokens(&self, tokens: &mut TokenStream) { quote!(::core::default::Default).to_tokens(tokens); diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs b/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs index 5d3ff6a8c05d5..153461425d647 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs @@ -2,7 +2,7 @@ use crate::container_attributes::REFLECT_DEFAULT; use crate::derive_data::ReflectEnum; use crate::enum_utility::{get_variant_constructors, EnumVariantConstructors}; use crate::field_attributes::DefaultBehavior; -use crate::fq_std::{FQAny, FQBox, FQClone, FQDefault, FQResult}; +use crate::fq_std::{FQAny, FQBox, FQClone, FQCow, FQDefault, FQResult}; use crate::{ReflectMeta, ReflectStruct}; use proc_macro::TokenStream; use proc_macro2::Span; @@ -44,6 +44,7 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream { pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream { let fqresult = FQResult.into_token_stream(); let fqbox = FQBox.into_token_stream(); + let fqcow = FQCow.into_token_stream(); let type_name = reflect_enum.meta().type_name(); let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path(); @@ -66,7 +67,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream { from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value), from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value), to_type: ::type_info(), - variant: #variant_names.to_string(), + variant: #fqcow::Borrowed(#variant_names), source: #fqbox::new(err), } }),)* @@ -74,7 +75,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream { from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value), from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value), to_type: ::type_info(), - variant: name.to_string(), + variant: #fqcow::Owned(name.to_string()), }), } } else { diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index 6b86a96135034..cfdd88b309ff8 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -979,7 +979,7 @@ impl FromReflect for Option { from_type: reflect.get_type_info(), from_kind: reflect.reflect_kind(), to_type: Self::type_info(), - variant: "Some".to_string(), + variant: Cow::Borrowed("Some"), source: Box::new(err), })?; Ok(Some(field)) @@ -989,7 +989,7 @@ impl FromReflect for Option { from_type: reflect.get_type_info(), from_kind: reflect.reflect_kind(), to_type: Self::type_info(), - variant: name.to_string(), + variant: Cow::Owned(name.to_string()), }), } } else { diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index deefa604155ec..5cba3880e6577 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -65,6 +65,7 @@ pub use bevy_reflect_derive::*; pub use erased_serde; use thiserror::Error; +use std::borrow::Cow; /// An Error for failed conversion of reflected type to original type in [`FromReflect::from_reflect`]. /// @@ -110,7 +111,7 @@ pub enum FromReflectError { from_type: &'static TypeInfo, from_kind: ReflectKind, to_type: &'static TypeInfo, - variant: String, + variant: Cow<'static, str>, }, #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the field `{}`", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .field)] @@ -136,7 +137,7 @@ pub enum FromReflectError { from_type: &'static TypeInfo, from_kind: ReflectKind, to_type: &'static TypeInfo, - variant: String, + variant: Cow<'static, str>, source: Box, }, #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in a key of the Map", From c7254ceac995fcc108bb6049730c27bff737a5bd Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Sat, 21 Jan 2023 17:29:06 +0530 Subject: [PATCH 13/21] Added description of "kind" in `ReflectKind` --- crates/bevy_reflect/src/lib.rs | 4 ++-- crates/bevy_reflect/src/reflect.rs | 33 +++++++++++++++++++----------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 5cba3880e6577..cb77ff7c9adcb 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -207,7 +207,7 @@ impl FromReflectError { } } - /// Returns the "kind" of source type for display purposes. + /// Returns the [kind](ReflectKind) of source type for display purposes. fn display_from_kind(&self) -> String { let prefix = if let TypeInfo::Dynamic(_) = self.from_type() { "(Dynamic)" @@ -218,7 +218,7 @@ impl FromReflectError { format!("{}{:?}", prefix, self.from_kind()) } - /// Returns the "kind" of target type for display purposes. + /// Returns the [kind](ReflectKind) of target type for display purposes. fn display_to_kind(&self) -> &str { match self.to_type() { TypeInfo::Struct(_) => "Struct", diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index ca17c72c6f99b..87e95222099d4 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -11,7 +11,13 @@ use std::{ use crate::utility::NonGenericTypeInfoCell; pub use bevy_utils::AHasher as ReflectHasher; -/// A simple enumeration of "kinds" of type, without any associated object. +/// A simple enumeration of [kinds](ReflectKind) of type, without any associated object. +/// +/// All types implementing [`Reflect`] are categorized into "kinds". They help to group types that +/// behave similarly and provide methods specific to its kind. These kinds directly correspond to +/// the traits [`Struct`], [`TupleStruct`], [`Tuple`], [`List`], [`Array`], [`Map`] and [`Enum`]; +/// which means that a type implementing any one of the above traits will be of the corresponding +/// kind. All the remaining types will be `ReflectKind::Value`. /// /// A `ReflectKind` is obtained via [`Reflect::reflect_kind`]. #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] @@ -26,7 +32,7 @@ pub enum ReflectKind { Value, } -/// An immutable enumeration of "kinds" of reflected type. +/// An immutable enumeration of [kinds](ReflectKind) of reflected type. /// /// Each variant contains a trait object with methods specific to a kind of /// type. @@ -43,7 +49,7 @@ pub enum ReflectRef<'a> { Value(&'a dyn Reflect), } -/// A mutable enumeration of "kinds" of reflected type. +/// A mutable enumeration of [kinds](ReflectKind) of reflected type. /// /// Each variant contains a trait object with methods specific to a kind of /// type. @@ -60,7 +66,7 @@ pub enum ReflectMut<'a> { Value(&'a mut dyn Reflect), } -/// An owned enumeration of "kinds" of reflected type. +/// An owned enumeration of [kinds](ReflectKind) of reflected type. /// /// Each variant contains a trait object with methods specific to a kind of /// type. @@ -79,8 +85,8 @@ pub enum ReflectOwned { /// A reflected Rust type. /// -/// Methods for working with particular kinds of Rust type are available using the [`Array`], [`List`], -/// [`Map`], [`Tuple`], [`TupleStruct`], [`Struct`], and [`Enum`] subtraits. +/// Methods for working with particular [kinds](ReflectKind) of Rust type are available using +/// the [`Array`], [`List`], [`Map`], [`Tuple`], [`TupleStruct`], [`Struct`], and [`Enum`] subtraits. /// /// When using `#[derive(Reflect)]` on a struct, tuple struct or enum, the suitable subtrait for that /// type (`Struct`, `TupleStruct` or `Enum`) is derived automatically. @@ -142,7 +148,7 @@ pub trait Reflect: Any + Send + Sync { /// Note that `Reflect` must be implemented manually for [`List`]s and /// [`Map`]s in order to achieve the correct semantics, as derived /// implementations will have the semantics for [`Struct`], [`TupleStruct`], [`Enum`] - /// or none of the above depending on the kind of type. For lists and maps, use the + /// or none of the above depending on the [kind] of type. For lists and maps, use the /// [`list_apply`] and [`map_apply`] helper functions when implementing this method. /// /// [`list_apply`]: crate::list_apply @@ -151,11 +157,13 @@ pub trait Reflect: Any + Send + Sync { /// # Panics /// /// Derived implementations of this method will panic: - /// - If the type of `value` is not of the same kind as `T` (e.g. if `T` is + /// - If the type of `value` is not of the same [kind] as `T` (e.g. if `T` is /// a `List`, while `value` is a `Struct`). /// - If `T` is any complex type and the corresponding fields or elements of /// `self` and `value` are not of the same type. /// - If `T` is a value type and `self` cannot be downcast to `T` + /// + /// [kind]: ReflectKind fn apply(&mut self, value: &dyn Reflect); /// Performs a type-checked assignment of a reflected value to this value. @@ -164,22 +172,23 @@ pub trait Reflect: Any + Send + Sync { /// containing the trait object. fn set(&mut self, value: Box) -> Result<(), Box>; - /// Returns a simple enumeration of "kinds" of type, without any associated object. + /// Returns a simple enumeration of [kinds](ReflectKind) of type, without any + /// associated object. /// /// See [`ReflectKind`]. fn reflect_kind(&self) -> ReflectKind; - /// Returns an immutable enumeration of "kinds" of type. + /// Returns an immutable enumeration of [kinds](ReflectKind) of type. /// /// See [`ReflectRef`]. fn reflect_ref(&self) -> ReflectRef; - /// Returns a mutable enumeration of "kinds" of type. + /// Returns a mutable enumeration of [kinds](ReflectKind) of type. /// /// See [`ReflectMut`]. fn reflect_mut(&mut self) -> ReflectMut; - /// Returns an owned enumeration of "kinds" of type. + /// Returns an owned enumeration of [kinds](ReflectKind) of type. /// /// See [`ReflectOwned`]. fn reflect_owned(self: Box) -> ReflectOwned; From 5f814aa9abde74fbfe2dbb4ba522183325da7443 Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Sat, 21 Jan 2023 17:40:11 +0530 Subject: [PATCH 14/21] Update docs to be specific to it's value --- crates/bevy_reflect/src/reflect.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index 87e95222099d4..cf5ca4b55eeb2 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -172,8 +172,7 @@ pub trait Reflect: Any + Send + Sync { /// containing the trait object. fn set(&mut self, value: Box) -> Result<(), Box>; - /// Returns a simple enumeration of [kinds](ReflectKind) of type, without any - /// associated object. + /// Returns the value's [kind](ReflectKind). /// /// See [`ReflectKind`]. fn reflect_kind(&self) -> ReflectKind; From b6f3c5d5c8ee5169aeae1d281166458ab8091b6d Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Sat, 21 Jan 2023 17:57:07 +0530 Subject: [PATCH 15/21] Move `FromReflectError` to from_reflect.rs --- crates/bevy_reflect/src/from_reflect.rs | 173 +++++++++++++++++++++++- crates/bevy_reflect/src/lib.rs | 170 ----------------------- 2 files changed, 172 insertions(+), 171 deletions(-) diff --git a/crates/bevy_reflect/src/from_reflect.rs b/crates/bevy_reflect/src/from_reflect.rs index cc328a1f80070..71d52934f00ff 100644 --- a/crates/bevy_reflect/src/from_reflect.rs +++ b/crates/bevy_reflect/src/from_reflect.rs @@ -1,4 +1,6 @@ -use crate::{FromReflectError, FromType, Reflect}; +use crate::{FromType, Reflect, ReflectKind, TypeInfo}; +use std::borrow::Cow; +use thiserror::Error; /// A trait for types which can be constructed from a reflected type. /// @@ -111,3 +113,172 @@ impl FromType for ReflectFromReflect { } } } + +/// An Error for failed conversion of reflected type to original type in [`FromReflect::from_reflect`]. +/// +/// Within variants `FieldError`, `IndexError`, `VariantError`, `KeyError` and `ValueError`; +/// [`Error::source`] must be used to trace the underlying error. +/// +/// [`Error::source`]: std::error::Error::source +#[derive(Error, Debug)] +pub enum FromReflectError { + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` of kind {} due to mismatched types or kinds", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), self.display_to_kind())] + InvalidType { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to source type having length of {} and target type having length of {}", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .from_len, .to_len)] + InvalidSize { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + from_len: usize, + to_len: usize, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing field `{}`", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .field)] + MissingField { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + field: &'static str, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing value at index {}", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] + MissingIndex { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + index: usize, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing variant `{}`", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .variant)] + MissingVariant { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + variant: Cow<'static, str>, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the field `{}`", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .field)] + FieldError { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + field: &'static str, + source: Box, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the value at index `{}`", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] + IndexError { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + index: usize, + source: Box, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the variant `{}`", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .variant)] + VariantError { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + variant: Cow<'static, str>, + source: Box, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in a key of the Map", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name())] + KeyError { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + source: Box, + }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in a value of the Map", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name())] + ValueError { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + source: Box, + }, +} + +impl FromReflectError { + /// Returns the [`TypeInfo`] of the source type. + pub fn from_type(&self) -> &'static TypeInfo { + match self { + Self::InvalidType { from_type, .. } + | Self::InvalidSize { from_type, .. } + | Self::MissingField { from_type, .. } + | Self::MissingIndex { from_type, .. } + | Self::MissingVariant { from_type, .. } + | Self::FieldError { from_type, .. } + | Self::IndexError { from_type, .. } + | Self::VariantError { from_type, .. } + | Self::KeyError { from_type, .. } + | Self::ValueError { from_type, .. } => from_type, + } + } + + /// Returns the [`TypeInfo`] of the target type. + pub fn to_type(&self) -> &'static TypeInfo { + match self { + Self::InvalidType { to_type, .. } + | Self::InvalidSize { to_type, .. } + | Self::MissingField { to_type, .. } + | Self::MissingIndex { to_type, .. } + | Self::MissingVariant { to_type, .. } + | Self::FieldError { to_type, .. } + | Self::IndexError { to_type, .. } + | Self::VariantError { to_type, .. } + | Self::KeyError { to_type, .. } + | Self::ValueError { to_type, .. } => to_type, + } + } + + /// Returns the [`ReflectKind`] of the source type. + pub fn from_kind(&self) -> ReflectKind { + *match self { + Self::InvalidType { from_kind, .. } + | Self::InvalidSize { from_kind, .. } + | Self::MissingField { from_kind, .. } + | Self::MissingIndex { from_kind, .. } + | Self::MissingVariant { from_kind, .. } + | Self::FieldError { from_kind, .. } + | Self::IndexError { from_kind, .. } + | Self::VariantError { from_kind, .. } + | Self::KeyError { from_kind, .. } + | Self::ValueError { from_kind, .. } => from_kind, + } + } + + /// Returns the [kind](ReflectKind) of source type for display purposes. + fn display_from_kind(&self) -> String { + let prefix = if let TypeInfo::Dynamic(_) = self.from_type() { + "(Dynamic)" + } else { + "" + }; + + format!("{}{:?}", prefix, self.from_kind()) + } + + /// Returns the [kind](ReflectKind) of target type for display purposes. + fn display_to_kind(&self) -> &str { + match self.to_type() { + TypeInfo::Struct(_) => "Struct", + TypeInfo::TupleStruct(_) => "TupleStruct", + TypeInfo::Tuple(_) => "Tuple", + TypeInfo::List(_) => "List", + TypeInfo::Array(_) => "Array", + TypeInfo::Map(_) => "Map", + TypeInfo::Enum(_) => "Enum", + TypeInfo::Value(_) => "Value", + TypeInfo::Dynamic(_) => "Dynamic", + } + } +} diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index cb77ff7c9adcb..2919b088dd9a3 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -64,176 +64,6 @@ pub use type_uuid::*; pub use bevy_reflect_derive::*; pub use erased_serde; -use thiserror::Error; -use std::borrow::Cow; - -/// An Error for failed conversion of reflected type to original type in [`FromReflect::from_reflect`]. -/// -/// Within variants `FieldError`, `IndexError`, `VariantError`, `KeyError` and `ValueError`; -/// [`Error::source`](std::error::Error::source) must be used to trace the underlying error. -#[derive(Error, Debug)] -pub enum FromReflectError { - #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` of kind {} due to mismatched types or kinds", - .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), self.display_to_kind())] - InvalidType { - from_type: &'static TypeInfo, - from_kind: ReflectKind, - to_type: &'static TypeInfo, - }, - #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to source type having length of {} and target type having length of {}", - .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .from_len, .to_len)] - InvalidSize { - from_type: &'static TypeInfo, - from_kind: ReflectKind, - to_type: &'static TypeInfo, - from_len: usize, - to_len: usize, - }, - #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing field `{}`", - .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .field)] - MissingField { - from_type: &'static TypeInfo, - from_kind: ReflectKind, - to_type: &'static TypeInfo, - field: &'static str, - }, - #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing value at index {}", - .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] - MissingIndex { - from_type: &'static TypeInfo, - from_kind: ReflectKind, - to_type: &'static TypeInfo, - index: usize, - }, - #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing variant `{}`", - .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .variant)] - MissingVariant { - from_type: &'static TypeInfo, - from_kind: ReflectKind, - to_type: &'static TypeInfo, - variant: Cow<'static, str>, - }, - #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the field `{}`", - .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .field)] - FieldError { - from_type: &'static TypeInfo, - from_kind: ReflectKind, - to_type: &'static TypeInfo, - field: &'static str, - source: Box, - }, - #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the value at index `{}`", - .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] - IndexError { - from_type: &'static TypeInfo, - from_kind: ReflectKind, - to_type: &'static TypeInfo, - index: usize, - source: Box, - }, - #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the variant `{}`", - .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .variant)] - VariantError { - from_type: &'static TypeInfo, - from_kind: ReflectKind, - to_type: &'static TypeInfo, - variant: Cow<'static, str>, - source: Box, - }, - #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in a key of the Map", - .from_type.type_name(), self.display_from_kind(), .to_type.type_name())] - KeyError { - from_type: &'static TypeInfo, - from_kind: ReflectKind, - to_type: &'static TypeInfo, - source: Box, - }, - #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in a value of the Map", - .from_type.type_name(), self.display_from_kind(), .to_type.type_name())] - ValueError { - from_type: &'static TypeInfo, - from_kind: ReflectKind, - to_type: &'static TypeInfo, - source: Box, - }, -} - -impl FromReflectError { - /// Returns the [`TypeInfo`] of the source type. - pub fn from_type(&self) -> &'static TypeInfo { - match self { - Self::InvalidType { from_type, .. } - | Self::InvalidSize { from_type, .. } - | Self::MissingField { from_type, .. } - | Self::MissingIndex { from_type, .. } - | Self::MissingVariant { from_type, .. } - | Self::FieldError { from_type, .. } - | Self::IndexError { from_type, .. } - | Self::VariantError { from_type, .. } - | Self::KeyError { from_type, .. } - | Self::ValueError { from_type, .. } => from_type, - } - } - - /// Returns the [`TypeInfo`] of the target type. - pub fn to_type(&self) -> &'static TypeInfo { - match self { - Self::InvalidType { to_type, .. } - | Self::InvalidSize { to_type, .. } - | Self::MissingField { to_type, .. } - | Self::MissingIndex { to_type, .. } - | Self::MissingVariant { to_type, .. } - | Self::FieldError { to_type, .. } - | Self::IndexError { to_type, .. } - | Self::VariantError { to_type, .. } - | Self::KeyError { to_type, .. } - | Self::ValueError { to_type, .. } => to_type, - } - } - - /// Returns the [`ReflectKind`] of the source type. - pub fn from_kind(&self) -> ReflectKind { - *match self { - Self::InvalidType { from_kind, .. } - | Self::InvalidSize { from_kind, .. } - | Self::MissingField { from_kind, .. } - | Self::MissingIndex { from_kind, .. } - | Self::MissingVariant { from_kind, .. } - | Self::FieldError { from_kind, .. } - | Self::IndexError { from_kind, .. } - | Self::VariantError { from_kind, .. } - | Self::KeyError { from_kind, .. } - | Self::ValueError { from_kind, .. } => from_kind, - } - } - - /// Returns the [kind](ReflectKind) of source type for display purposes. - fn display_from_kind(&self) -> String { - let prefix = if let TypeInfo::Dynamic(_) = self.from_type() { - "(Dynamic)" - } else { - "" - }; - - format!("{}{:?}", prefix, self.from_kind()) - } - - /// Returns the [kind](ReflectKind) of target type for display purposes. - fn display_to_kind(&self) -> &str { - match self.to_type() { - TypeInfo::Struct(_) => "Struct", - TypeInfo::TupleStruct(_) => "TupleStruct", - TypeInfo::Tuple(_) => "Tuple", - TypeInfo::List(_) => "List", - TypeInfo::Array(_) => "Array", - TypeInfo::Map(_) => "Map", - TypeInfo::Enum(_) => "Enum", - TypeInfo::Value(_) => "Value", - TypeInfo::Dynamic(_) => "Dynamic", - } - } -} - #[doc(hidden)] pub mod __macro_exports { use crate::Uuid; From 2e528a78992a08cd59bdfae805cc06e9b5feb718 Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Sat, 21 Jan 2023 21:23:37 +0530 Subject: [PATCH 16/21] Changed Error names to include NamedField and UnnamedField --- .../bevy_reflect_derive/src/enum_utility.rs | 8 ++-- .../bevy_reflect_derive/src/from_reflect.rs | 8 ++-- crates/bevy_reflect/src/from_reflect.rs | 39 +++++++++++++++---- crates/bevy_reflect/src/impls/std.rs | 4 +- 4 files changed, 41 insertions(+), 18 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs index f90428bd5c4d3..a44f4f8d95bb1 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs @@ -56,7 +56,7 @@ pub(crate) fn get_variant_constructors( match &field.data.ident { Some(ident) => { let name = ident.to_string(); - quote!(.map_err(|err| #bevy_reflect_path::FromReflectError::FieldError { + quote!(.map_err(|err| #bevy_reflect_path::FromReflectError::NamedFieldError { from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value), from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value), to_type: ::type_info(), @@ -64,7 +64,7 @@ pub(crate) fn get_variant_constructors( source: #FQBox::new(err), })?) }, - None => quote!(.map_err(|err| #bevy_reflect_path::FromReflectError::IndexError { + None => quote!(.map_err(|err| #bevy_reflect_path::FromReflectError::UnnamedFieldError { from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value), from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value), to_type: ::type_info(), @@ -80,7 +80,7 @@ pub(crate) fn get_variant_constructors( quote!(.field(#name)) } else { quote!(.field(#name) - .ok_or_else(|| #bevy_reflect_path::FromReflectError::MissingField { + .ok_or_else(|| #bevy_reflect_path::FromReflectError::MissingNamedField { from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value), from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value), to_type: ::type_info(), @@ -93,7 +93,7 @@ pub(crate) fn get_variant_constructors( quote!(.field_at(#reflect_index)) } else { quote!(.field_at(#reflect_index) - .ok_or_else(|| #bevy_reflect_path::FromReflectError::MissingIndex { + .ok_or_else(|| #bevy_reflect_path::FromReflectError::MissingUnnamedField { from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value), from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value), to_type: ::type_info(), diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs b/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs index 153461425d647..a2832470a09ba 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs @@ -220,14 +220,14 @@ fn get_active_fields( let ty = field.data.ty.clone(); let missing_error = if is_tuple { - quote!(|| #bevy_reflect_path::FromReflectError::MissingIndex { + quote!(|| #bevy_reflect_path::FromReflectError::MissingUnnamedField { from_type: #bevy_reflect_path::Reflect::get_type_info(reflect), from_kind: #bevy_reflect_path::Reflect::reflect_kind(reflect), to_type: ::type_info(), index: #accessor, }) } else { - quote!(|| #bevy_reflect_path::FromReflectError::MissingField { + quote!(|| #bevy_reflect_path::FromReflectError::MissingNamedField { from_type: #bevy_reflect_path::Reflect::get_type_info(reflect), from_kind: #bevy_reflect_path::Reflect::reflect_kind(reflect), to_type: ::type_info(), @@ -240,7 +240,7 @@ fn get_active_fields( }; let error = if is_tuple { - quote!(|err| #bevy_reflect_path::FromReflectError::IndexError { + quote!(|err| #bevy_reflect_path::FromReflectError::UnnamedFieldError { from_type: #bevy_reflect_path::Reflect::get_type_info(reflect), from_kind: #bevy_reflect_path::Reflect::reflect_kind(reflect), to_type: ::type_info(), @@ -248,7 +248,7 @@ fn get_active_fields( source: #FQBox::new(err), }) } else { - quote!(|err| #bevy_reflect_path::FromReflectError::FieldError { + quote!(|err| #bevy_reflect_path::FromReflectError::NamedFieldError { from_type: #bevy_reflect_path::Reflect::get_type_info(reflect), from_kind: #bevy_reflect_path::Reflect::reflect_kind(reflect), to_type: ::type_info(), diff --git a/crates/bevy_reflect/src/from_reflect.rs b/crates/bevy_reflect/src/from_reflect.rs index 71d52934f00ff..0938718943ab4 100644 --- a/crates/bevy_reflect/src/from_reflect.rs +++ b/crates/bevy_reflect/src/from_reflect.rs @@ -140,12 +140,20 @@ pub enum FromReflectError { }, #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing field `{}`", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .field)] - MissingField { + MissingNamedField { from_type: &'static TypeInfo, from_kind: ReflectKind, to_type: &'static TypeInfo, field: &'static str, }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing field at index {}", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] + MissingUnnamedField { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + index: usize, + }, #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing value at index {}", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] MissingIndex { @@ -164,13 +172,22 @@ pub enum FromReflectError { }, #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the field `{}`", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .field)] - FieldError { + NamedFieldError { from_type: &'static TypeInfo, from_kind: ReflectKind, to_type: &'static TypeInfo, field: &'static str, source: Box, }, + #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the field at index {}", + .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] + UnnamedFieldError { + from_type: &'static TypeInfo, + from_kind: ReflectKind, + to_type: &'static TypeInfo, + index: usize, + source: Box, + }, #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the value at index `{}`", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] IndexError { @@ -213,10 +230,12 @@ impl FromReflectError { match self { Self::InvalidType { from_type, .. } | Self::InvalidSize { from_type, .. } - | Self::MissingField { from_type, .. } + | Self::MissingNamedField { from_type, .. } + | Self::MissingUnnamedField { from_type, .. } | Self::MissingIndex { from_type, .. } | Self::MissingVariant { from_type, .. } - | Self::FieldError { from_type, .. } + | Self::NamedFieldError { from_type, .. } + | Self::UnnamedFieldError { from_type, .. } | Self::IndexError { from_type, .. } | Self::VariantError { from_type, .. } | Self::KeyError { from_type, .. } @@ -229,10 +248,12 @@ impl FromReflectError { match self { Self::InvalidType { to_type, .. } | Self::InvalidSize { to_type, .. } - | Self::MissingField { to_type, .. } + | Self::MissingNamedField { to_type, .. } + | Self::MissingUnnamedField { to_type, .. } | Self::MissingIndex { to_type, .. } | Self::MissingVariant { to_type, .. } - | Self::FieldError { to_type, .. } + | Self::NamedFieldError { to_type, .. } + | Self::UnnamedFieldError { to_type, .. } | Self::IndexError { to_type, .. } | Self::VariantError { to_type, .. } | Self::KeyError { to_type, .. } @@ -245,10 +266,12 @@ impl FromReflectError { *match self { Self::InvalidType { from_kind, .. } | Self::InvalidSize { from_kind, .. } - | Self::MissingField { from_kind, .. } + | Self::MissingNamedField { from_kind, .. } + | Self::MissingUnnamedField { from_kind, .. } | Self::MissingIndex { from_kind, .. } | Self::MissingVariant { from_kind, .. } - | Self::FieldError { from_kind, .. } + | Self::NamedFieldError { from_kind, .. } + | Self::UnnamedFieldError { from_kind, .. } | Self::IndexError { from_kind, .. } | Self::VariantError { from_kind, .. } | Self::KeyError { from_kind, .. } diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index cfdd88b309ff8..9272ca874c30e 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -959,7 +959,7 @@ impl FromReflect for Option { T::take_from_reflect( dyn_enum .field_at(0) - .ok_or_else(|| FromReflectError::MissingIndex { + .ok_or_else(|| FromReflectError::MissingUnnamedField { from_type: reflect.get_type_info(), from_kind: reflect.reflect_kind(), to_type: Self::type_info(), @@ -967,7 +967,7 @@ impl FromReflect for Option { })? .clone_value(), ) - .map_err(|(_, err)| FromReflectError::IndexError { + .map_err(|(_, err)| FromReflectError::UnnamedFieldError { from_type: reflect.get_type_info(), from_kind: reflect.reflect_kind(), to_type: Self::type_info(), From 8fc5732aa44b649b4609f93e0ffef915e925a751 Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Sun, 22 Jan 2023 01:08:23 +0530 Subject: [PATCH 17/21] Add docs to variants of `FromReflectError` --- crates/bevy_reflect/src/from_reflect.rs | 163 +++++++++++++++++++++++- 1 file changed, 161 insertions(+), 2 deletions(-) diff --git a/crates/bevy_reflect/src/from_reflect.rs b/crates/bevy_reflect/src/from_reflect.rs index 0938718943ab4..b88735123a8b3 100644 --- a/crates/bevy_reflect/src/from_reflect.rs +++ b/crates/bevy_reflect/src/from_reflect.rs @@ -116,110 +116,269 @@ impl FromType for ReflectFromReflect { /// An Error for failed conversion of reflected type to original type in [`FromReflect::from_reflect`]. /// -/// Within variants `FieldError`, `IndexError`, `VariantError`, `KeyError` and `ValueError`; -/// [`Error::source`] must be used to trace the underlying error. +/// In the error message, the kind of the source type may have a prefix "(Dynamic)" indicating that the +/// source is dynamic, i.e., [`DynamicStruct`], [`DynamicList`], etc. /// +/// Within variants `NamedFieldError`, `UnnamedFieldError`, `IndexError`, `VariantError`, `KeyError` and +/// `ValueError`; [`Error::source`] must be used to trace the underlying error. +/// +/// [`DynamicStruct`]: crate::DynamicStruct +/// [`DynamicList`]: crate::DynamicList /// [`Error::source`]: std::error::Error::source #[derive(Error, Debug)] pub enum FromReflectError { + /// The source and target types are of different types or [kinds](ReflectKind). #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` of kind {} due to mismatched types or kinds", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), self.display_to_kind())] InvalidType { + /// [`TypeInfo`] of the source type. from_type: &'static TypeInfo, + + /// [`ReflectKind`] of the source type. from_kind: ReflectKind, + + /// [`TypeInfo`] of the target type. to_type: &'static TypeInfo, }, + + /// The source and target types have different lengths. + /// + /// This error is given by types of [kind](ReflectKind) [`Array`](crate::Array). #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to source type having length of {} and target type having length of {}", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .from_len, .to_len)] InvalidSize { + /// [`TypeInfo`] of the source type. from_type: &'static TypeInfo, + + /// [`ReflectKind`] of the source type. from_kind: ReflectKind, + + /// [`TypeInfo`] of the target type. to_type: &'static TypeInfo, + + /// Length of the source type. from_len: usize, + + /// Length of the target type. to_len: usize, }, + + /// The source type did not have a field with name given by the parameter `field`. + /// + /// This error is given by types of [kind](ReflectKind) [`Struct`](crate::Struct) and + /// [`Enum`](crate::Enum). #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing field `{}`", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .field)] MissingNamedField { + /// [`TypeInfo`] of the source type. from_type: &'static TypeInfo, + + /// [`ReflectKind`] of the source type. from_kind: ReflectKind, + + /// [`TypeInfo`] of the target type. to_type: &'static TypeInfo, + + /// Name of missing field in source type. field: &'static str, }, + + /// The source type did not have a field at index given by the parameter `index`. + /// + /// This error is given by types of [kind](ReflectKind) [`TupleStruct`](crate::TupleStruct) and + /// [`Enum`](crate::Enum). #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing field at index {}", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] MissingUnnamedField { + /// [`TypeInfo`] of the source type. from_type: &'static TypeInfo, + + /// [`ReflectKind`] of the source type. from_kind: ReflectKind, + + /// [`TypeInfo`] of the target type. to_type: &'static TypeInfo, + + /// Index of missing field in source type. index: usize, }, + + /// The source type did not have a value at index given by the parameter `index`. + /// + /// This error is given by types of [kind](ReflectKind) [`Tuple`](crate::Tuple). #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing value at index {}", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] MissingIndex { + /// [`TypeInfo`] of the source type. from_type: &'static TypeInfo, + + /// [`ReflectKind`] of the source type. from_kind: ReflectKind, + + /// [`TypeInfo`] of the target type. to_type: &'static TypeInfo, + + /// Index of missing value in source type. index: usize, }, + + /// The target type did not have a variant with name given by the parameter `variant`. + /// + /// This error is given by types of [kind](ReflectKind) [`Enum`](crate::Enum). #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing variant `{}`", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .variant)] MissingVariant { + /// [`TypeInfo`] of the source type. from_type: &'static TypeInfo, + + /// [`ReflectKind`] of the source type. from_kind: ReflectKind, + + /// [`TypeInfo`] of the target type. to_type: &'static TypeInfo, + + /// Name of missing variant in target type. variant: Cow<'static, str>, }, + + /// An error has occurred in conversion of a field with name given by the parameter `field`. + /// + /// Use [`Error::source`](std::error::Error::source) to get the underlying error. + /// + /// This error is given by types of [kind](ReflectKind) [`Struct`](crate::Struct) and + /// [`Enum`](crate::Enum). #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the field `{}`", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .field)] NamedFieldError { + /// [`TypeInfo`] of the source type. from_type: &'static TypeInfo, + + /// [`ReflectKind`] of the source type. from_kind: ReflectKind, + + /// [`TypeInfo`] of the target type. to_type: &'static TypeInfo, + + /// Name of field where error occurred. field: &'static str, + + /// Underlying error in conversion of field. source: Box, }, + + /// An error has occurred in conversion of a field at index given by the parameter `index`. + /// + /// Use [`Error::source`](std::error::Error::source) to get the underlying error. + /// + /// This error is given by types of [kind](ReflectKind) [`TupleStruct`](crate::TupleStruct) + /// and [`Enum`](crate::Enum). #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the field at index {}", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] UnnamedFieldError { + /// [`TypeInfo`] of the source type. from_type: &'static TypeInfo, + + /// [`ReflectKind`] of the source type. from_kind: ReflectKind, + + /// [`TypeInfo`] of the target type. to_type: &'static TypeInfo, + + /// Index of field where error occurred. index: usize, + + /// Underlying error in conversion of field. source: Box, }, + + /// An error has occurred in conversion of a value at index given by the parameter `index`. + /// + /// Use [`Error::source`](std::error::Error::source) to get the underlying error. + /// + /// This error is given by types of [kind](ReflectKind) [`List`](crate::List) and + /// [`Enum`](crate::Enum). #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the value at index `{}`", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] IndexError { + /// [`TypeInfo`] of the source type. from_type: &'static TypeInfo, + + /// [`ReflectKind`] of the source type. from_kind: ReflectKind, + + /// [`TypeInfo`] of the target type. to_type: &'static TypeInfo, + + /// Index of value where error occurred. index: usize, + + /// Underlying error in conversion of value at the index. source: Box, }, + + /// An error has occurred in conversion of a variant with name given by the parameter `variant`. + /// + /// Use [`Error::source`](std::error::Error::source) to get the underlying error. + /// + /// This error is given by types of [kind](ReflectKind) [`Enum`](crate::Enum). #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the variant `{}`", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .variant)] VariantError { + /// [`TypeInfo`] of the source type. from_type: &'static TypeInfo, + + /// [`ReflectKind`] of the source type. from_kind: ReflectKind, + + /// [`TypeInfo`] of the target type. to_type: &'static TypeInfo, + + /// Name of variant where error occurred. variant: Cow<'static, str>, + + /// Underlying error in conversion of variant. source: Box, }, + + /// An error has occurred in conversion of a key of Map. + /// + /// Use [`Error::source`](std::error::Error::source) to get the underlying error. + /// + /// This error is given by types of [kind](ReflectKind) [`Map`](crate::Map). #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in a key of the Map", .from_type.type_name(), self.display_from_kind(), .to_type.type_name())] KeyError { + /// [`TypeInfo`] of the source type. from_type: &'static TypeInfo, + + /// [`ReflectKind`] of the source type. from_kind: ReflectKind, + + /// [`TypeInfo`] of the target type. to_type: &'static TypeInfo, + + /// Underlying error in conversion of a key of Map. source: Box, }, + + /// An error has occurred in conversion of a value of Map. + /// + /// Use [`Error::source`](std::error::Error::source) to get the underlying error. + /// + /// This error is given by types of [kind](ReflectKind) [`Map`](crate::Map). #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in a value of the Map", .from_type.type_name(), self.display_from_kind(), .to_type.type_name())] ValueError { + /// [`TypeInfo`] of the source type. from_type: &'static TypeInfo, + + /// [`ReflectKind`] of the source type. from_kind: ReflectKind, + + /// [`TypeInfo`] of the target type. to_type: &'static TypeInfo, + + /// Underlying error in conversion of a value of Map. source: Box, }, } From c5370d483c5c0f7c4c699f847a91701ef22b6a95 Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Sun, 22 Jan 2023 02:00:10 +0530 Subject: [PATCH 18/21] Add `IndexError` in `from_reflect` of `SmallVec` --- crates/bevy_reflect/src/impls/smallvec.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/bevy_reflect/src/impls/smallvec.rs b/crates/bevy_reflect/src/impls/smallvec.rs index 264d6a984f11e..736961fcd6dcf 100644 --- a/crates/bevy_reflect/src/impls/smallvec.rs +++ b/crates/bevy_reflect/src/impls/smallvec.rs @@ -170,8 +170,16 @@ where fn from_reflect(reflect: &dyn Reflect) -> Result { if let ReflectRef::List(ref_list) = reflect.reflect_ref() { let mut new_list = Self::with_capacity(ref_list.len()); - for field in ref_list.iter() { - new_list.push(::Item::from_reflect(field)?); + for (idx, field) in ref_list.iter().enumerate() { + new_list.push(::Item::from_reflect(field).map_err( + |err| FromReflectError::IndexError { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), + index: idx, + source: Box::new(err), + }, + )?); } Ok(new_list) } else { From 7e1d30f0c18818c61a1c574f035a4b381dd1a2e6 Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Sun, 22 Jan 2023 02:00:47 +0530 Subject: [PATCH 19/21] Rename `InvalidSize` to `InvalidLength` and cargo fmt --- crates/bevy_reflect/src/from_reflect.rs | 24 ++++++++++++------------ crates/bevy_reflect/src/impls/std.rs | 16 +++++++++------- crates/bevy_reflect/src/reflect.rs | 6 +++--- 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/crates/bevy_reflect/src/from_reflect.rs b/crates/bevy_reflect/src/from_reflect.rs index b88735123a8b3..9f3d1831cf937 100644 --- a/crates/bevy_reflect/src/from_reflect.rs +++ b/crates/bevy_reflect/src/from_reflect.rs @@ -116,7 +116,7 @@ impl FromType for ReflectFromReflect { /// An Error for failed conversion of reflected type to original type in [`FromReflect::from_reflect`]. /// -/// In the error message, the kind of the source type may have a prefix "(Dynamic)" indicating that the +/// In the error message, the kind of the source type may have a prefix "(Dynamic)" indicating that the /// source is dynamic, i.e., [`DynamicStruct`], [`DynamicList`], etc. /// /// Within variants `NamedFieldError`, `UnnamedFieldError`, `IndexError`, `VariantError`, `KeyError` and @@ -146,13 +146,13 @@ pub enum FromReflectError { /// This error is given by types of [kind](ReflectKind) [`Array`](crate::Array). #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to source type having length of {} and target type having length of {}", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .from_len, .to_len)] - InvalidSize { + InvalidLength { /// [`TypeInfo`] of the source type. from_type: &'static TypeInfo, /// [`ReflectKind`] of the source type. from_kind: ReflectKind, - + /// [`TypeInfo`] of the target type. to_type: &'static TypeInfo, @@ -192,7 +192,7 @@ pub enum FromReflectError { MissingUnnamedField { /// [`TypeInfo`] of the source type. from_type: &'static TypeInfo, - + /// [`ReflectKind`] of the source type. from_kind: ReflectKind, @@ -230,7 +230,7 @@ pub enum FromReflectError { MissingVariant { /// [`TypeInfo`] of the source type. from_type: &'static TypeInfo, - + /// [`ReflectKind`] of the source type. from_kind: ReflectKind, @@ -280,7 +280,7 @@ pub enum FromReflectError { /// [`ReflectKind`] of the source type. from_kind: ReflectKind, - + /// [`TypeInfo`] of the target type. to_type: &'static TypeInfo, @@ -295,7 +295,7 @@ pub enum FromReflectError { /// /// Use [`Error::source`](std::error::Error::source) to get the underlying error. /// - /// This error is given by types of [kind](ReflectKind) [`List`](crate::List) and + /// This error is given by types of [kind](ReflectKind) [`List`](crate::List) and /// [`Enum`](crate::Enum). #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the value at index `{}`", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] @@ -326,7 +326,7 @@ pub enum FromReflectError { VariantError { /// [`TypeInfo`] of the source type. from_type: &'static TypeInfo, - + /// [`ReflectKind`] of the source type. from_kind: ReflectKind, @@ -371,7 +371,7 @@ pub enum FromReflectError { ValueError { /// [`TypeInfo`] of the source type. from_type: &'static TypeInfo, - + /// [`ReflectKind`] of the source type. from_kind: ReflectKind, @@ -388,7 +388,7 @@ impl FromReflectError { pub fn from_type(&self) -> &'static TypeInfo { match self { Self::InvalidType { from_type, .. } - | Self::InvalidSize { from_type, .. } + | Self::InvalidLength { from_type, .. } | Self::MissingNamedField { from_type, .. } | Self::MissingUnnamedField { from_type, .. } | Self::MissingIndex { from_type, .. } @@ -406,7 +406,7 @@ impl FromReflectError { pub fn to_type(&self) -> &'static TypeInfo { match self { Self::InvalidType { to_type, .. } - | Self::InvalidSize { to_type, .. } + | Self::InvalidLength { to_type, .. } | Self::MissingNamedField { to_type, .. } | Self::MissingUnnamedField { to_type, .. } | Self::MissingIndex { to_type, .. } @@ -424,7 +424,7 @@ impl FromReflectError { pub fn from_kind(&self) -> ReflectKind { *match self { Self::InvalidType { from_kind, .. } - | Self::InvalidSize { from_kind, .. } + | Self::InvalidLength { from_kind, .. } | Self::MissingNamedField { from_kind, .. } | Self::MissingUnnamedField { from_kind, .. } | Self::MissingIndex { from_kind, .. } diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index 9272ca874c30e..81a2d96197da7 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -706,7 +706,7 @@ impl FromReflect for [T; N] { let temp_vec_len = temp_vec.len(); temp_vec .try_into() - .map_err(|_| FromReflectError::InvalidSize { + .map_err(|_| FromReflectError::InvalidLength { from_type: reflect.get_type_info(), from_kind: reflect.reflect_kind(), to_type: Self::type_info(), @@ -967,12 +967,14 @@ impl FromReflect for Option { })? .clone_value(), ) - .map_err(|(_, err)| FromReflectError::UnnamedFieldError { - from_type: reflect.get_type_info(), - from_kind: reflect.reflect_kind(), - to_type: Self::type_info(), - index: 0, - source: Box::new(err), + .map_err(|(_, err)| { + FromReflectError::UnnamedFieldError { + from_type: reflect.get_type_info(), + from_kind: reflect.reflect_kind(), + to_type: Self::type_info(), + index: 0, + source: Box::new(err), + } }) })() .map_err(|err| FromReflectError::VariantError { diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index cf5ca4b55eeb2..90504aba3262c 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -12,10 +12,10 @@ use crate::utility::NonGenericTypeInfoCell; pub use bevy_utils::AHasher as ReflectHasher; /// A simple enumeration of [kinds](ReflectKind) of type, without any associated object. -/// +/// /// All types implementing [`Reflect`] are categorized into "kinds". They help to group types that /// behave similarly and provide methods specific to its kind. These kinds directly correspond to -/// the traits [`Struct`], [`TupleStruct`], [`Tuple`], [`List`], [`Array`], [`Map`] and [`Enum`]; +/// the traits [`Struct`], [`TupleStruct`], [`Tuple`], [`List`], [`Array`], [`Map`] and [`Enum`]; /// which means that a type implementing any one of the above traits will be of the corresponding /// kind. All the remaining types will be `ReflectKind::Value`. /// @@ -86,7 +86,7 @@ pub enum ReflectOwned { /// A reflected Rust type. /// /// Methods for working with particular [kinds](ReflectKind) of Rust type are available using -/// the [`Array`], [`List`], [`Map`], [`Tuple`], [`TupleStruct`], [`Struct`], and [`Enum`] subtraits. +/// the [`Array`], [`List`], [`Map`], [`Tuple`], [`TupleStruct`], [`Struct`], and [`Enum`] subtraits. /// /// When using `#[derive(Reflect)]` on a struct, tuple struct or enum, the suitable subtrait for that /// type (`Struct`, `TupleStruct` or `Enum`) is derived automatically. From 53ace23f15248cf77e846fbee8d0ef77e3aa755e Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Sun, 22 Jan 2023 19:33:18 +0530 Subject: [PATCH 20/21] Added tests for FromReflectError --- crates/bevy_reflect/src/from_reflect.rs | 317 ++++++++++++++++++++++++ 1 file changed, 317 insertions(+) diff --git a/crates/bevy_reflect/src/from_reflect.rs b/crates/bevy_reflect/src/from_reflect.rs index 9f3d1831cf937..53f7cf3dbe399 100644 --- a/crates/bevy_reflect/src/from_reflect.rs +++ b/crates/bevy_reflect/src/from_reflect.rs @@ -464,3 +464,320 @@ impl FromReflectError { } } } + +#[cfg(test)] +mod tests { + use crate as bevy_reflect; + use crate::{ + DynamicEnum, DynamicList, DynamicMap, DynamicStruct, DynamicTuple, DynamicTupleStruct, + DynamicVariant, FromReflect, FromReflectError, Reflect, ReflectKind, TypeInfo, + }; + use bevy_utils::HashMap; + use std::borrow::Cow; + + #[test] + fn check_invalid_type() { + #[derive(Reflect, FromReflect)] + struct Rectangle { + height: i32, + width: i32, + } + + let result = Rectangle::from_reflect(&vec![1, 2, 3, 4, 5]); + + assert!( + matches!( + result, + Err(FromReflectError::InvalidType { + from_type: TypeInfo::List(_), + from_kind: ReflectKind::List, + to_type: TypeInfo::Struct(_), + }) + ), + "Incorrect error handling of FromReflectError::InvalidType" + ); + } + + #[test] + fn check_invalid_length() { + let result = <[i32; 10]>::from_reflect(&[1, 2, 3, 4, 5]); + + assert!( + matches!( + result, + Err(FromReflectError::InvalidLength { + from_type: TypeInfo::Array(_), + from_kind: ReflectKind::Array, + to_type: TypeInfo::Array(_), + from_len: 5, + to_len: 10, + }) + ), + "Incorrect error handling of FromReflectError::InvalidLength" + ); + } + + #[test] + fn check_missing_named_field() { + #[derive(Reflect, FromReflect)] + struct Rectangle { + height: i32, + width: i32, + } + + let mut dyn_struct = DynamicStruct::default(); + dyn_struct.insert("height", 5); + + let result = Rectangle::from_reflect(&dyn_struct); + + assert!( + matches!( + result, + Err(FromReflectError::MissingNamedField { + from_type: TypeInfo::Dynamic(_), + from_kind: ReflectKind::Struct, + to_type: TypeInfo::Struct(_), + field: "width", + }) + ), + "Incorrect error handling of FromReflectError::MissingNamedField" + ); + } + + #[test] + fn check_missing_unnamed_field() { + #[derive(Reflect, FromReflect)] + struct Rectangle(i32, i32); + + let mut dyn_tuple_struct = DynamicTupleStruct::default(); + dyn_tuple_struct.insert(5); + + let result = Rectangle::from_reflect(&dyn_tuple_struct); + + assert!( + matches!( + result, + Err(FromReflectError::MissingUnnamedField { + from_type: TypeInfo::Dynamic(_), + from_kind: ReflectKind::TupleStruct, + to_type: TypeInfo::TupleStruct(_), + index: 1, + }) + ), + "Incorrect error handling of FromReflectError::MissingUnnamedField" + ); + } + + #[test] + fn check_missing_index() { + let mut dyn_tuple = DynamicTuple::default(); + dyn_tuple.insert(5); + + let result = <(i32, i32)>::from_reflect(&dyn_tuple); + + assert!( + matches!( + result, + Err(FromReflectError::MissingIndex { + from_type: TypeInfo::Dynamic(_), + from_kind: ReflectKind::Tuple, + to_type: TypeInfo::Tuple(_), + index: 1, + }) + ), + "Incorrect error handling of FromReflectError::MissingIndex" + ); + } + + #[test] + fn check_missing_variant() { + #[derive(Reflect, FromReflect)] + enum Shape { + Point, + Circle(i32), + Rectangle { height: i32, width: i32 }, + } + + let dyn_enum = DynamicEnum::new("Shape", "None", DynamicVariant::Unit); + let result = Shape::from_reflect(&dyn_enum); + + assert!( + matches!( + result, + Err(FromReflectError::MissingVariant { + from_type: TypeInfo::Dynamic(_), + from_kind: ReflectKind::Enum, + to_type: TypeInfo::Enum(_), + variant: Cow::Owned(x), + }) if x.as_str() == "None" + ), + "Incorrect error handling of FromReflectError::MissingVariant" + ); + } + + #[test] + fn check_named_field_error() { + #[derive(Reflect, FromReflect)] + struct Rectangle { + height: i32, + width: i32, + } + + let mut dyn_struct = DynamicStruct::default(); + dyn_struct.insert("height", 5); + dyn_struct.insert("width", 3.2); + + let result = Rectangle::from_reflect(&dyn_struct); + + assert!( + matches!( + result, + Err(FromReflectError::NamedFieldError { + from_type: TypeInfo::Dynamic(_), + from_kind: ReflectKind::Struct, + to_type: TypeInfo::Struct(_), + field: "width", + source, + }) if matches!(*source, FromReflectError::InvalidType { .. }) + ), + "Incorrect error handling of FromReflectError::NamedFieldError" + ); + } + + #[test] + fn check_unnamed_field_error() { + #[derive(Reflect, FromReflect)] + struct Rectangle(i32, i32); + + let mut dyn_tuple_struct = DynamicTupleStruct::default(); + dyn_tuple_struct.insert(5); + dyn_tuple_struct.insert(3.2); + + let result = Rectangle::from_reflect(&dyn_tuple_struct); + + assert!( + matches!( + result, + Err(FromReflectError::UnnamedFieldError { + from_type: TypeInfo::Dynamic(_), + from_kind: ReflectKind::TupleStruct, + to_type: TypeInfo::TupleStruct(_), + index: 1, + source, + }) if matches!(*source, FromReflectError::InvalidType { .. }) + ), + "Incorrect error handling of FromReflectError::UnnamedFieldError" + ); + } + + #[test] + fn check_index_error() { + #[derive(Reflect, FromReflect)] + struct Rectangle(i32, i32); + + let mut dyn_list = DynamicList::default(); + dyn_list.push(1); + dyn_list.push(2); + dyn_list.push(3.2); + + let result = Vec::::from_reflect(&dyn_list); + + assert!( + matches!( + result, + Err(FromReflectError::IndexError { + from_type: TypeInfo::Dynamic(_), + from_kind: ReflectKind::List, + to_type: TypeInfo::List(_), + index: 2, + source, + }) if matches!(*source, FromReflectError::InvalidType { .. }) + ), + "Incorrect error handling of FromReflectError::IndexError" + ); + } + + #[test] + fn check_variant_error() { + #[derive(Reflect, FromReflect)] + enum Shape { + Point, + Circle(i32), + Rectangle { height: i32, width: i32 }, + } + + let mut dyn_struct = DynamicStruct::default(); + dyn_struct.insert("height", 5); + dyn_struct.insert("width", 3.2); + let dyn_enum = DynamicEnum::new("Shape", "Rectangle", DynamicVariant::Struct(dyn_struct)); + + let result = Shape::from_reflect(&dyn_enum); + + assert!( + matches!( + result, + Err(FromReflectError::VariantError { + from_type: TypeInfo::Dynamic(_), + from_kind: ReflectKind::Enum, + to_type: TypeInfo::Enum(_), + variant: Cow::Borrowed("Rectangle"), + source, + }) if matches!( + &*source, + FromReflectError::NamedFieldError { + from_type: TypeInfo::Dynamic(_), + from_kind: ReflectKind::Enum, + to_type: TypeInfo::Enum(_), + field: "width", + source: inner_source, + } if matches!(**inner_source, FromReflectError::InvalidType { .. }) + ) + ), + "Incorrect error handling of FromReflectError::VariantError" + ); + } + + #[test] + fn check_key_error() { + let mut dyn_map = DynamicMap::default(); + dyn_map.insert(String::from("a"), 5); + dyn_map.insert(9, 2); + + let result = HashMap::::from_reflect(&dyn_map); + + assert!( + matches!( + result, + Err(FromReflectError::KeyError { + from_type: TypeInfo::Dynamic(_), + from_kind: ReflectKind::Map, + to_type: TypeInfo::Map(_), + source, + }) if matches!(*source, FromReflectError::InvalidType { .. }) + ), + "Incorrect error handling of FromReflectError::KeyError" + ); + } + + #[test] + fn check_value_error() { + let mut dyn_map = DynamicMap::default(); + dyn_map.insert(String::from("a"), 5); + dyn_map.insert(String::from("b"), 3.2); + + let result = HashMap::::from_reflect(&dyn_map); + + assert!( + matches!( + result, + Err(FromReflectError::ValueError { + from_type: TypeInfo::Dynamic(_), + from_kind: ReflectKind::Map, + to_type: TypeInfo::Map(_), + source, + }) if matches!(*source, FromReflectError::InvalidType { .. }) + ), + "Incorrect error handling of FromReflectError::ValueError" + ); + } +} From 7974319fd55f48b192d31b19663ec7c01309bf75 Mon Sep 17 00:00:00 2001 From: Elbert Ronnie Date: Sun, 22 Jan 2023 20:23:40 +0530 Subject: [PATCH 21/21] Tuple should have fields instead of index --- crates/bevy_reflect/src/from_reflect.rs | 57 +++---------------------- crates/bevy_reflect/src/tuple.rs | 4 +- 2 files changed, 9 insertions(+), 52 deletions(-) diff --git a/crates/bevy_reflect/src/from_reflect.rs b/crates/bevy_reflect/src/from_reflect.rs index 53f7cf3dbe399..9d8bdd800049a 100644 --- a/crates/bevy_reflect/src/from_reflect.rs +++ b/crates/bevy_reflect/src/from_reflect.rs @@ -185,8 +185,8 @@ pub enum FromReflectError { /// The source type did not have a field at index given by the parameter `index`. /// - /// This error is given by types of [kind](ReflectKind) [`TupleStruct`](crate::TupleStruct) and - /// [`Enum`](crate::Enum). + /// This error is given by types of [kind](ReflectKind) [`TupleStruct`](crate::TupleStruct), + /// [`Tuple`](crate::Tuple) and [`Enum`](crate::Enum). #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing field at index {}", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] MissingUnnamedField { @@ -203,25 +203,6 @@ pub enum FromReflectError { index: usize, }, - /// The source type did not have a value at index given by the parameter `index`. - /// - /// This error is given by types of [kind](ReflectKind) [`Tuple`](crate::Tuple). - #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to a missing value at index {}", - .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] - MissingIndex { - /// [`TypeInfo`] of the source type. - from_type: &'static TypeInfo, - - /// [`ReflectKind`] of the source type. - from_kind: ReflectKind, - - /// [`TypeInfo`] of the target type. - to_type: &'static TypeInfo, - - /// Index of missing value in source type. - index: usize, - }, - /// The target type did not have a variant with name given by the parameter `variant`. /// /// This error is given by types of [kind](ReflectKind) [`Enum`](crate::Enum). @@ -270,8 +251,8 @@ pub enum FromReflectError { /// /// Use [`Error::source`](std::error::Error::source) to get the underlying error. /// - /// This error is given by types of [kind](ReflectKind) [`TupleStruct`](crate::TupleStruct) - /// and [`Enum`](crate::Enum). + /// This error is given by types of [kind](ReflectKind) [`TupleStruct`](crate::TupleStruct), + /// [`Tuple`](crate::Tuple) and [`Enum`](crate::Enum). #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the field at index {}", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] UnnamedFieldError { @@ -296,7 +277,7 @@ pub enum FromReflectError { /// Use [`Error::source`](std::error::Error::source) to get the underlying error. /// /// This error is given by types of [kind](ReflectKind) [`List`](crate::List) and - /// [`Enum`](crate::Enum). + /// [`Array`](crate::Array). #[error("The reflected type `{}` of kind {} cannot be converted to type `{}` due to an error in the value at index `{}`", .from_type.type_name(), self.display_from_kind(), .to_type.type_name(), .index)] IndexError { @@ -391,7 +372,6 @@ impl FromReflectError { | Self::InvalidLength { from_type, .. } | Self::MissingNamedField { from_type, .. } | Self::MissingUnnamedField { from_type, .. } - | Self::MissingIndex { from_type, .. } | Self::MissingVariant { from_type, .. } | Self::NamedFieldError { from_type, .. } | Self::UnnamedFieldError { from_type, .. } @@ -409,7 +389,6 @@ impl FromReflectError { | Self::InvalidLength { to_type, .. } | Self::MissingNamedField { to_type, .. } | Self::MissingUnnamedField { to_type, .. } - | Self::MissingIndex { to_type, .. } | Self::MissingVariant { to_type, .. } | Self::NamedFieldError { to_type, .. } | Self::UnnamedFieldError { to_type, .. } @@ -427,7 +406,6 @@ impl FromReflectError { | Self::InvalidLength { from_kind, .. } | Self::MissingNamedField { from_kind, .. } | Self::MissingUnnamedField { from_kind, .. } - | Self::MissingIndex { from_kind, .. } | Self::MissingVariant { from_kind, .. } | Self::NamedFieldError { from_kind, .. } | Self::UnnamedFieldError { from_kind, .. } @@ -469,8 +447,8 @@ impl FromReflectError { mod tests { use crate as bevy_reflect; use crate::{ - DynamicEnum, DynamicList, DynamicMap, DynamicStruct, DynamicTuple, DynamicTupleStruct, - DynamicVariant, FromReflect, FromReflectError, Reflect, ReflectKind, TypeInfo, + DynamicEnum, DynamicList, DynamicMap, DynamicStruct, DynamicTupleStruct, DynamicVariant, + FromReflect, FromReflectError, Reflect, ReflectKind, TypeInfo, }; use bevy_utils::HashMap; use std::borrow::Cow; @@ -568,27 +546,6 @@ mod tests { ); } - #[test] - fn check_missing_index() { - let mut dyn_tuple = DynamicTuple::default(); - dyn_tuple.insert(5); - - let result = <(i32, i32)>::from_reflect(&dyn_tuple); - - assert!( - matches!( - result, - Err(FromReflectError::MissingIndex { - from_type: TypeInfo::Dynamic(_), - from_kind: ReflectKind::Tuple, - to_type: TypeInfo::Tuple(_), - index: 1, - }) - ), - "Incorrect error handling of FromReflectError::MissingIndex" - ); - } - #[test] fn check_missing_variant() { #[derive(Reflect, FromReflect)] diff --git a/crates/bevy_reflect/src/tuple.rs b/crates/bevy_reflect/src/tuple.rs index d2a2edfe8318c..12a5b7ab386da 100644 --- a/crates/bevy_reflect/src/tuple.rs +++ b/crates/bevy_reflect/src/tuple.rs @@ -604,13 +604,13 @@ macro_rules! impl_reflect_tuple { $( <$name as FromReflect>::from_reflect( _ref_tuple.field($index) - .ok_or_else(|| FromReflectError::MissingIndex { + .ok_or_else(|| FromReflectError::MissingUnnamedField { from_type: reflect.get_type_info(), from_kind: reflect.reflect_kind(), to_type: Self::type_info(), index: $index, })? - ).map_err(|err| FromReflectError::IndexError { + ).map_err(|err| FromReflectError::UnnamedFieldError { from_type: reflect.get_type_info(), from_kind: reflect.reflect_kind(), to_type: Self::type_info(),