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: Relax bounds on Option<T> #5658

Closed
wants to merge 4 commits 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 30 additions & 9 deletions crates/bevy_reflect/src/impls/std.rs
Expand Up @@ -76,6 +76,7 @@ impl_from_reflect_value!(String);
impl_from_reflect_value!(HashSet<T: Hash + Eq + Clone + Send + Sync + 'static>);
impl_from_reflect_value!(Range<T: Clone + Send + Sync + 'static>);
impl_from_reflect_value!(Duration);
impl_from_reflect_value!(Instant);
impl_from_reflect_value!(NonZeroI128);
impl_from_reflect_value!(NonZeroU128);
impl_from_reflect_value!(NonZeroIsize);
Expand Down Expand Up @@ -585,13 +586,13 @@ impl Reflect for Cow<'static, str> {
}
}

impl<T: Reflect + Clone> GetTypeRegistration for Option<T> {
impl<T: FromReflect> GetTypeRegistration for Option<T> {
fn get_type_registration() -> TypeRegistration {
TypeRegistration::of::<Option<T>>()
}
}

impl<T: Reflect + Clone> Enum for Option<T> {
impl<T: FromReflect> Enum for Option<T> {
fn field(&self, _name: &str) -> Option<&dyn Reflect> {
None
}
Expand Down Expand Up @@ -655,7 +656,7 @@ impl<T: Reflect + Clone> Enum for Option<T> {
}
}

impl<T: Reflect + Clone> Reflect for Option<T> {
impl<T: FromReflect> Reflect for Option<T> {
#[inline]
fn type_name(&self) -> &str {
std::any::type_name::<Self>()
Expand Down Expand Up @@ -741,7 +742,7 @@ impl<T: Reflect + Clone> Reflect for Option<T> {

#[inline]
fn clone_value(&self) -> Box<dyn Reflect> {
Box::new(self.clone())
Box::new(Enum::clone_dynamic(self))
}

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

impl<T: Reflect + Clone> FromReflect for Option<T> {
impl<T: FromReflect> FromReflect for Option<T> {
fn from_reflect(reflect: &dyn Reflect) -> Option<Self> {
if let ReflectRef::Enum(dyn_enum) = reflect.reflect_ref() {
match dyn_enum.variant_name() {
Expand All @@ -762,7 +763,7 @@ impl<T: Reflect + Clone> FromReflect for Option<T> {
.field_at(0)
.expect("Field at index 0 should exist")
.clone_value();
let field = field.take::<T>().unwrap_or_else(|_| {
let field = T::from_reflect(field.as_ref()).unwrap_or_else(|| {
panic!(
"Field at index 0 should be of type {}",
std::any::type_name::<T>()
Expand All @@ -783,7 +784,7 @@ impl<T: Reflect + Clone> FromReflect for Option<T> {
}
}

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

#[cfg(test)]
mod tests {
use crate as bevy_reflect;
use crate::{
Enum, Reflect, ReflectSerialize, TypeInfo, TypeRegistry, Typed, VariantInfo, VariantType,
Enum, FromReflect, Reflect, ReflectSerialize, TypeInfo, TypeRegistry, Typed, VariantInfo,
VariantType,
};
use bevy_utils::HashMap;
use bevy_utils::{HashMap, Instant};
use std::f32::consts::{PI, TAU};

#[test]
Expand Down Expand Up @@ -939,6 +942,17 @@ mod tests {
assert_eq!(Some(321), value);
}

#[test]
fn option_should_from_reflect() {
#[derive(Reflect, FromReflect, PartialEq, Debug)]
struct Foo(usize);

let expected = Some(Foo(123));
let output = <Option<Foo> as FromReflect>::from_reflect(&expected).unwrap();

assert_eq!(expected, output);
}

#[test]
fn option_should_impl_typed() {
type MyOption = Option<i32>;
Expand Down Expand Up @@ -979,4 +993,11 @@ mod tests {
let forty_two: std::num::NonZeroUsize = crate::FromReflect::from_reflect(a).unwrap();
assert_eq!(forty_two, std::num::NonZeroUsize::new(42).unwrap());
}

#[test]
fn instant_should_from_reflect() {
let expected = Instant::now();
let output = <Instant as FromReflect>::from_reflect(&expected).unwrap();
assert_eq!(expected, output);
}
}
5 changes: 3 additions & 2 deletions crates/bevy_render/src/camera/camera.rs
Expand Up @@ -19,6 +19,7 @@ use bevy_ecs::{
};
use bevy_math::{Mat4, UVec2, Vec2, Vec3};
use bevy_reflect::prelude::*;
use bevy_reflect::FromReflect;
use bevy_transform::components::GlobalTransform;
use bevy_utils::HashSet;
use bevy_window::{WindowCreated, WindowId, WindowResized, Windows};
Expand All @@ -32,7 +33,7 @@ use wgpu::Extent3d;
/// You can overlay multiple cameras in a single window using viewports to create effects like
/// split screen, minimaps, and character viewers.
// TODO: remove reflect_value when possible
#[derive(Reflect, Debug, Clone, Serialize, Deserialize)]
#[derive(Reflect, FromReflect, Debug, Clone, Serialize, Deserialize)]
#[reflect_value(Default, Serialize, Deserialize)]
pub struct Viewport {
/// The physical position to render this viewport to within the [`RenderTarget`] of this [`Camera`].
Expand Down Expand Up @@ -71,7 +72,7 @@ pub struct ComputedCameraValues {
target_info: Option<RenderTargetInfo>,
}

#[derive(Component, Debug, Reflect, Clone)]
#[derive(Component, Debug, Reflect, FromReflect, Clone)]
#[reflect(Component)]
pub struct Camera {
/// If set, this camera will render to the given [`Viewport`] rectangle within the configured [`RenderTarget`].
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_time/src/time.rs
@@ -1,9 +1,9 @@
use bevy_ecs::{reflect::ReflectResource, system::Resource};
use bevy_reflect::Reflect;
use bevy_reflect::{FromReflect, Reflect};
use bevy_utils::{Duration, Instant};

/// Tracks elapsed time since the last update and since the App has started
#[derive(Resource, Reflect, Debug, Clone)]
#[derive(Resource, Reflect, FromReflect, Debug, Clone)]
#[reflect(Resource)]
pub struct Time {
delta: Duration,
Expand Down