bevy_reflect: Simplify take-or-else-from_reflect operation (#6566)
				
					
				
			# Objective
There are times where we want to simply take an owned `dyn Reflect` and cast it to a type `T`.
Currently, this involves doing:
```rust
let value = value.take::<T>().unwrap_or_else(|value| {
  T::from_reflect(&*value).unwrap_or_else(|| {
    panic!(
      "expected value of type {} to convert to type {}.",
      value.type_name(),
      std::any::type_name::<T>()
    )
  })
});
```
This is a common operation that could be easily be simplified.
## Solution
Add the `FromReflect::take_from_reflect` method. This first tries to `take` the value, calling `from_reflect` iff that fails.
```rust
let value = T::take_from_reflect(value).unwrap_or_else(|value| {
  panic!(
    "expected value of type {} to convert to type {}.",
    value.type_name(),
    std::any::type_name::<T>()
  )
});
```
Based on suggestion from @soqb on [Discord](https://discord.com/channels/691052431525675048/1002362493634629796/1041046880316043374).
---
## Changelog
- Add `FromReflect::take_from_reflect` method
			
			
This commit is contained in:
		
							parent
							
								
									9dd8fbc570
								
							
						
					
					
						commit
						229d6c686f
					
				| @ -13,6 +13,26 @@ use crate::{FromType, Reflect}; | |||||||
| pub trait FromReflect: Reflect + Sized { | pub trait FromReflect: Reflect + Sized { | ||||||
|     /// Constructs a concrete instance of `Self` from a reflected value.
 |     /// Constructs a concrete instance of `Self` from a reflected value.
 | ||||||
|     fn from_reflect(reflect: &dyn Reflect) -> Option<Self>; |     fn from_reflect(reflect: &dyn Reflect) -> Option<Self>; | ||||||
|  | 
 | ||||||
|  |     /// 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<Self>`)
 | ||||||
|  |     /// 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<dyn Reflect>) -> Result<Self, Box<dyn Reflect>> { | ||||||
|  |         match reflect.take::<Self>() { | ||||||
|  |             Ok(value) => Ok(value), | ||||||
|  |             Err(value) => match Self::from_reflect(value.as_ref()) { | ||||||
|  |                 None => Err(value), | ||||||
|  |                 Some(value) => Ok(value), | ||||||
|  |             }, | ||||||
|  |         } | ||||||
|  |     } | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| /// Type data that represents the [`FromReflect`] trait and allows it to be used dynamically.
 | /// Type data that represents the [`FromReflect`] trait and allows it to be used dynamically.
 | ||||||
|  | |||||||
| @ -230,13 +230,11 @@ macro_rules! impl_reflect_for_veclike { | |||||||
|             } |             } | ||||||
| 
 | 
 | ||||||
|             fn push(&mut self, value: Box<dyn Reflect>) { |             fn push(&mut self, value: Box<dyn Reflect>) { | ||||||
|                 let value = value.take::<T>().unwrap_or_else(|value| { |                 let value = T::take_from_reflect(value).unwrap_or_else(|value| { | ||||||
|                     T::from_reflect(&*value).unwrap_or_else(|| { |  | ||||||
|                     panic!( |                     panic!( | ||||||
|                         "Attempted to push invalid value of type {}.", |                         "Attempted to push invalid value of type {}.", | ||||||
|                         value.type_name() |                         value.type_name() | ||||||
|                     ) |                     ) | ||||||
|                     }) |  | ||||||
|                 }); |                 }); | ||||||
|                 $push(self, value); |                 $push(self, value); | ||||||
|             } |             } | ||||||
| @ -409,21 +407,17 @@ impl<K: FromReflect + Eq + Hash, V: FromReflect> Map for HashMap<K, V> { | |||||||
|         key: Box<dyn Reflect>, |         key: Box<dyn Reflect>, | ||||||
|         value: Box<dyn Reflect>, |         value: Box<dyn Reflect>, | ||||||
|     ) -> Option<Box<dyn Reflect>> { |     ) -> Option<Box<dyn Reflect>> { | ||||||
|         let key = key.take::<K>().unwrap_or_else(|key| { |         let key = K::take_from_reflect(key).unwrap_or_else(|key| { | ||||||
|             K::from_reflect(&*key).unwrap_or_else(|| { |  | ||||||
|             panic!( |             panic!( | ||||||
|                 "Attempted to insert invalid key of type {}.", |                 "Attempted to insert invalid key of type {}.", | ||||||
|                 key.type_name() |                 key.type_name() | ||||||
|             ) |             ) | ||||||
|             }) |  | ||||||
|         }); |         }); | ||||||
|         let value = value.take::<V>().unwrap_or_else(|value| { |         let value = V::take_from_reflect(value).unwrap_or_else(|value| { | ||||||
|             V::from_reflect(&*value).unwrap_or_else(|| { |  | ||||||
|             panic!( |             panic!( | ||||||
|                 "Attempted to insert invalid value of type {}.", |                 "Attempted to insert invalid value of type {}.", | ||||||
|                 value.type_name() |                 value.type_name() | ||||||
|             ) |             ) | ||||||
|             }) |  | ||||||
|         }); |         }); | ||||||
|         self.insert(key, value) |         self.insert(key, value) | ||||||
|             .map(|old_value| Box::new(old_value) as Box<dyn Reflect>) |             .map(|old_value| Box::new(old_value) as Box<dyn Reflect>) | ||||||
| @ -829,7 +823,8 @@ impl<T: FromReflect> Reflect for Option<T> { | |||||||
|                 // New variant -> perform a switch
 |                 // New variant -> perform a switch
 | ||||||
|                 match value.variant_name() { |                 match value.variant_name() { | ||||||
|                     "Some" => { |                     "Some" => { | ||||||
|                         let field = value |                         let field = T::take_from_reflect( | ||||||
|  |                             value | ||||||
|                                 .field_at(0) |                                 .field_at(0) | ||||||
|                                 .unwrap_or_else(|| { |                                 .unwrap_or_else(|| { | ||||||
|                                     panic!( |                                     panic!( | ||||||
| @ -837,16 +832,14 @@ impl<T: FromReflect> Reflect for Option<T> { | |||||||
|                                         std::any::type_name::<Option<T>>() |                                         std::any::type_name::<Option<T>>() | ||||||
|                                     ) |                                     ) | ||||||
|                                 }) |                                 }) | ||||||
|                             .clone_value() |                                 .clone_value(), | ||||||
|                             .take::<T>() |                         ) | ||||||
|                             .unwrap_or_else(|value| { |                         .unwrap_or_else(|_| { | ||||||
|                                 T::from_reflect(&*value).unwrap_or_else(|| { |  | ||||||
|                             panic!( |                             panic!( | ||||||
|                                 "Field in `Some` variant of {} should be of type {}", |                                 "Field in `Some` variant of {} should be of type {}", | ||||||
|                                 std::any::type_name::<Option<T>>(), |                                 std::any::type_name::<Option<T>>(), | ||||||
|                                 std::any::type_name::<T>() |                                 std::any::type_name::<T>() | ||||||
|                             ) |                             ) | ||||||
|                                 }) |  | ||||||
|                         }); |                         }); | ||||||
|                         *self = Some(field); |                         *self = Some(field); | ||||||
|                     } |                     } | ||||||
| @ -896,7 +889,8 @@ impl<T: FromReflect> FromReflect for Option<T> { | |||||||
|         if let ReflectRef::Enum(dyn_enum) = reflect.reflect_ref() { |         if let ReflectRef::Enum(dyn_enum) = reflect.reflect_ref() { | ||||||
|             match dyn_enum.variant_name() { |             match dyn_enum.variant_name() { | ||||||
|                 "Some" => { |                 "Some" => { | ||||||
|                     let field = dyn_enum |                     let field = T::take_from_reflect( | ||||||
|  |                         dyn_enum | ||||||
|                             .field_at(0) |                             .field_at(0) | ||||||
|                             .unwrap_or_else(|| { |                             .unwrap_or_else(|| { | ||||||
|                                 panic!( |                                 panic!( | ||||||
| @ -904,16 +898,14 @@ impl<T: FromReflect> FromReflect for Option<T> { | |||||||
|                                     std::any::type_name::<Option<T>>() |                                     std::any::type_name::<Option<T>>() | ||||||
|                                 ) |                                 ) | ||||||
|                             }) |                             }) | ||||||
|                         .clone_value() |                             .clone_value(), | ||||||
|                         .take::<T>() |                     ) | ||||||
|                         .unwrap_or_else(|value| { |                     .unwrap_or_else(|_| { | ||||||
|                             T::from_reflect(&*value).unwrap_or_else(|| { |  | ||||||
|                         panic!( |                         panic!( | ||||||
|                             "Field in `Some` variant of {} should be of type {}", |                             "Field in `Some` variant of {} should be of type {}", | ||||||
|                             std::any::type_name::<Option<T>>(), |                             std::any::type_name::<Option<T>>(), | ||||||
|                             std::any::type_name::<T>() |                             std::any::type_name::<T>() | ||||||
|                         ) |                         ) | ||||||
|                             }) |  | ||||||
|                     }); |                     }); | ||||||
|                     Some(Some(field)) |                     Some(Some(field)) | ||||||
|                 } |                 } | ||||||
|  | |||||||
		Loading…
	
		Reference in New Issue
	
	Block a user
	 Gino Valente
						Gino Valente