Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - bevy_reflect: Simplify take-or-else-from_reflect operation #6566

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions crates/bevy_reflect/src/from_reflect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,26 @@ use crate::{FromType, Reflect};
pub trait FromReflect: Reflect + Sized {
/// Constructs a concrete instance of `Self` from a reflected value.
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.
Expand Down
98 changes: 45 additions & 53 deletions crates/bevy_reflect/src/impls/std.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,13 +214,11 @@ macro_rules! impl_reflect_for_veclike {

impl<T: FromReflect> List for $ty {
fn push(&mut self, value: Box<dyn Reflect>) {
let value = value.take::<T>().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);
}
Expand Down Expand Up @@ -391,21 +389,17 @@ impl<K: FromReflect + Eq + Hash, V: FromReflect> Map for HashMap<K, V> {
key: Box<dyn Reflect>,
value: Box<dyn Reflect>,
) -> Option<Box<dyn Reflect>> {
let key = key.take::<K>().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::<V>().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<dyn Reflect>)
Expand Down Expand Up @@ -811,25 +805,24 @@ impl<T: FromReflect> Reflect for Option<T> {
// 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::<Option<T>>()
)
})
.clone_value()
.take::<T>()
.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::<Option<T>>(),
std::any::type_name::<T>()
"Field in `Some` variant of {} should exist",
std::any::type_name::<Option<T>>()
)
})
});
.clone_value(),
)
.unwrap_or_else(|_| {
panic!(
"Field in `Some` variant of {} should be of type {}",
std::any::type_name::<Option<T>>(),
std::any::type_name::<T>()
)
});
*self = Some(field);
}
"None" => {
Expand Down Expand Up @@ -878,25 +871,24 @@ impl<T: FromReflect> FromReflect for Option<T> {
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::<Option<T>>()
)
})
.clone_value()
.take::<T>()
.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::<Option<T>>(),
std::any::type_name::<T>()
"Field in `Some` variant of {} should exist",
std::any::type_name::<Option<T>>()
)
})
});
.clone_value(),
)
.unwrap_or_else(|_| {
panic!(
"Field in `Some` variant of {} should be of type {}",
std::any::type_name::<Option<T>>(),
std::any::type_name::<T>()
)
});
Some(Some(field))
}
"None" => Some(None),
Expand Down