Skip to content

Commit aad2267

Browse files
committed
bevy_reflect: Relax bounds on Option<T> (#5658)
# Objective The reflection impls on `Option<T>` have the bound `T: Reflect + Clone`. This means that using `FromReflect` requires `Clone` even though we can normally get away with just `FromReflect`. ## Solution Update the bounds on `Option<T>` to match that of `Vec<T>`, where `T: FromReflect`. This helps remove a `Clone` implementation that may be undesired but added for the sole purpose of getting the code to compile. --- ## Changelog * Reflection on `Option<T>` now has `T` bound by `FromReflect` rather than `Reflect + Clone` * Added a `FromReflect` impl for `Instant` ## Migration Guide If using `Option<T>` with Bevy's reflection API, `T` now needs to implement `FromReflect` rather than just `Clone`. This can be achieved easily by simply deriving `FromReflect`: ```rust // OLD #[derive(Reflect, Clone)] struct Foo; let reflected: Box<dyn Reflect> = Box::new(Some(Foo)); // NEW #[derive(Reflect, FromReflect)] struct Foo; let reflected: Box<dyn Reflect> = Box::new(Some(Foo)); ``` > Note: You can still derive `Clone`, but it's not required in order to compile.
1 parent f9104b7 commit aad2267

File tree

3 files changed

+34
-13
lines changed

3 files changed

+34
-13
lines changed

crates/bevy_reflect/src/impls/std.rs

+30-9
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ impl_from_reflect_value!(String);
7676
impl_from_reflect_value!(HashSet<T: Hash + Eq + Clone + Send + Sync + 'static>);
7777
impl_from_reflect_value!(Range<T: Clone + Send + Sync + 'static>);
7878
impl_from_reflect_value!(Duration);
79+
impl_from_reflect_value!(Instant);
7980
impl_from_reflect_value!(NonZeroI128);
8081
impl_from_reflect_value!(NonZeroU128);
8182
impl_from_reflect_value!(NonZeroIsize);
@@ -585,13 +586,13 @@ impl Reflect for Cow<'static, str> {
585586
}
586587
}
587588

588-
impl<T: Reflect + Clone> GetTypeRegistration for Option<T> {
589+
impl<T: FromReflect> GetTypeRegistration for Option<T> {
589590
fn get_type_registration() -> TypeRegistration {
590591
TypeRegistration::of::<Option<T>>()
591592
}
592593
}
593594

594-
impl<T: Reflect + Clone> Enum for Option<T> {
595+
impl<T: FromReflect> Enum for Option<T> {
595596
fn field(&self, _name: &str) -> Option<&dyn Reflect> {
596597
None
597598
}
@@ -655,7 +656,7 @@ impl<T: Reflect + Clone> Enum for Option<T> {
655656
}
656657
}
657658

658-
impl<T: Reflect + Clone> Reflect for Option<T> {
659+
impl<T: FromReflect> Reflect for Option<T> {
659660
#[inline]
660661
fn type_name(&self) -> &str {
661662
std::any::type_name::<Self>()
@@ -741,7 +742,7 @@ impl<T: Reflect + Clone> Reflect for Option<T> {
741742

742743
#[inline]
743744
fn clone_value(&self) -> Box<dyn Reflect> {
744-
Box::new(self.clone())
745+
Box::new(Enum::clone_dynamic(self))
745746
}
746747

747748
fn reflect_hash(&self) -> Option<u64> {
@@ -753,7 +754,7 @@ impl<T: Reflect + Clone> Reflect for Option<T> {
753754
}
754755
}
755756

756-
impl<T: Reflect + Clone> FromReflect for Option<T> {
757+
impl<T: FromReflect> FromReflect for Option<T> {
757758
fn from_reflect(reflect: &dyn Reflect) -> Option<Self> {
758759
if let ReflectRef::Enum(dyn_enum) = reflect.reflect_ref() {
759760
match dyn_enum.variant_name() {
@@ -762,7 +763,7 @@ impl<T: Reflect + Clone> FromReflect for Option<T> {
762763
.field_at(0)
763764
.expect("Field at index 0 should exist")
764765
.clone_value();
765-
let field = field.take::<T>().unwrap_or_else(|_| {
766+
let field = T::from_reflect(field.as_ref()).unwrap_or_else(|| {
766767
panic!(
767768
"Field at index 0 should be of type {}",
768769
std::any::type_name::<T>()
@@ -783,7 +784,7 @@ impl<T: Reflect + Clone> FromReflect for Option<T> {
783784
}
784785
}
785786

786-
impl<T: Reflect + Clone> Typed for Option<T> {
787+
impl<T: FromReflect> Typed for Option<T> {
787788
fn type_info() -> &'static TypeInfo {
788789
static CELL: GenericTypeInfoCell = GenericTypeInfoCell::new();
789790
CELL.get_or_insert::<Self, _>(|| {
@@ -827,10 +828,12 @@ impl FromReflect for Cow<'static, str> {
827828

828829
#[cfg(test)]
829830
mod tests {
831+
use crate as bevy_reflect;
830832
use crate::{
831-
Enum, Reflect, ReflectSerialize, TypeInfo, TypeRegistry, Typed, VariantInfo, VariantType,
833+
Enum, FromReflect, Reflect, ReflectSerialize, TypeInfo, TypeRegistry, Typed, VariantInfo,
834+
VariantType,
832835
};
833-
use bevy_utils::HashMap;
836+
use bevy_utils::{HashMap, Instant};
834837
use std::f32::consts::{PI, TAU};
835838

836839
#[test]
@@ -939,6 +942,17 @@ mod tests {
939942
assert_eq!(Some(321), value);
940943
}
941944

945+
#[test]
946+
fn option_should_from_reflect() {
947+
#[derive(Reflect, FromReflect, PartialEq, Debug)]
948+
struct Foo(usize);
949+
950+
let expected = Some(Foo(123));
951+
let output = <Option<Foo> as FromReflect>::from_reflect(&expected).unwrap();
952+
953+
assert_eq!(expected, output);
954+
}
955+
942956
#[test]
943957
fn option_should_impl_typed() {
944958
type MyOption = Option<i32>;
@@ -979,4 +993,11 @@ mod tests {
979993
let forty_two: std::num::NonZeroUsize = crate::FromReflect::from_reflect(a).unwrap();
980994
assert_eq!(forty_two, std::num::NonZeroUsize::new(42).unwrap());
981995
}
996+
997+
#[test]
998+
fn instant_should_from_reflect() {
999+
let expected = Instant::now();
1000+
let output = <Instant as FromReflect>::from_reflect(&expected).unwrap();
1001+
assert_eq!(expected, output);
1002+
}
9821003
}

crates/bevy_render/src/camera/camera.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ use wgpu::Extent3d;
3232
/// You can overlay multiple cameras in a single window using viewports to create effects like
3333
/// split screen, minimaps, and character viewers.
3434
// TODO: remove reflect_value when possible
35-
#[derive(Reflect, Debug, Clone, Serialize, Deserialize)]
35+
#[derive(Reflect, FromReflect, Debug, Clone, Serialize, Deserialize)]
3636
#[reflect_value(Default, Serialize, Deserialize)]
3737
pub struct Viewport {
3838
/// The physical position to render this viewport to within the [`RenderTarget`] of this [`Camera`].
@@ -71,7 +71,7 @@ pub struct ComputedCameraValues {
7171
target_info: Option<RenderTargetInfo>,
7272
}
7373

74-
#[derive(Component, Debug, Reflect, Clone)]
74+
#[derive(Component, Debug, Reflect, FromReflect, Clone)]
7575
#[reflect(Component)]
7676
pub struct Camera {
7777
/// If set, this camera will render to the given [`Viewport`] rectangle within the configured [`RenderTarget`].

crates/bevy_time/src/time.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
use bevy_ecs::{reflect::ReflectResource, system::Resource};
2-
use bevy_reflect::Reflect;
2+
use bevy_reflect::{FromReflect, Reflect};
33
use bevy_utils::{Duration, Instant};
44

55
/// Tracks elapsed time since the last update and since the App has started
6-
#[derive(Resource, Reflect, Debug, Clone)]
6+
#[derive(Resource, Reflect, FromReflect, Debug, Clone)]
77
#[reflect(Resource)]
88
pub struct Time {
99
delta: Duration,

0 commit comments

Comments
 (0)