From faeccd7a09d6871abd332a46eab892612acd3620 Mon Sep 17 00:00:00 2001
From: Nathan Stocks <cleancut@github.com>
Date: Tue, 9 Mar 2021 23:39:41 +0000
Subject: [PATCH] Reflection cleanup (#1536)

This is an effort to provide the correct `#[reflect_value(...)]` attributes where they are needed.

Supersedes #1533 and resolves #1528.

---

I am working under the following assumptions (thanks to @bjorn3 and @Davier for advice here):

- Any `enum` that derives `Reflect` and one or more of { `Serialize`, `Deserialize`, `PartialEq`, `Hash` } needs a `#[reflect_value(...)]` attribute containing the same subset of { `Serialize`, `Deserialize`, `PartialEq`, `Hash` } that is present on the derive.
- Same as above for `struct` and `#[reflect(...)]`, respectively.
- If a `struct` is used as a component, it should also have `#[reflect(Component)]`
- All reflected types should be registered in their plugins

I treated the following as components (added `#[reflect(Component)]` if necessary):
- `bevy_render`
  - `struct RenderLayers`
- `bevy_transform`
  - `struct GlobalTransform`
  - `struct Parent`
  - `struct Transform`
- `bevy_ui`
  - `struct Style`

Not treated as components:
- `bevy_math`
  - `struct Size<T>`
  - `struct Rect<T>`
  - Note: The updates for `Size<T>` and `Rect<T>` in `bevy::math::geometry` required using @Davier's suggestion to add `+ PartialEq` to the trait bound. I then registered the specific types used over in `bevy_ui` such as `Size<Val>`, etc. in `bevy_ui`'s plugin, since `bevy::math` does not contain a plugin.
- `bevy_render`
  - `struct Color`
  - `struct PipelineSpecialization`
  - `struct ShaderSpecialization`
  - `enum PrimitiveTopology`
  - `enum IndexFormat`

Not Addressed:
- I am not searching for components in Bevy that are _not_ reflected. So if there are components that are not reflected that should be reflected, that will need to be figured out in another PR.
- I only added `#[reflect(...)]` or `#[reflect_value(...)]` entries for the set of four traits { `Serialize`, `Deserialize`, `PartialEq`, `Hash` } _if they were derived via `#[derive(...)]`_. I did not look for manual trait implementations of the same set of four, nor did I consider any traits outside the four.  Are those other possibilities something that needs to be looked into?
---
 crates/bevy_core/src/lib.rs                   |  3 ++
 crates/bevy_math/src/geometry.rs              | 30 +++++++++--------
 crates/bevy_reflect/src/impls/std.rs          | 32 +++++++++++++++++--
 crates/bevy_reflect/src/lib.rs                |  3 ++
 .../src/camera/visible_entities.rs            |  2 +-
 crates/bevy_render/src/color.rs               |  3 +-
 crates/bevy_render/src/lib.rs                 | 10 ++++--
 .../src/pipeline/pipeline_compiler.rs         |  4 ++-
 .../src/pipeline/state_descriptors.rs         |  4 ++-
 crates/bevy_sprite/src/lib.rs                 |  1 +
 .../src/components/global_transform.rs        |  2 +-
 .../bevy_transform/src/components/parent.rs   |  4 +--
 .../src/components/transform.rs               |  2 +-
 crates/bevy_ui/src/lib.rs                     | 16 ++++++++++
 crates/bevy_ui/src/ui_node.rs                 |  1 +
 15 files changed, 91 insertions(+), 26 deletions(-)

diff --git a/crates/bevy_core/src/lib.rs b/crates/bevy_core/src/lib.rs
index e26be88c13..13700a005b 100644
--- a/crates/bevy_core/src/lib.rs
+++ b/crates/bevy_core/src/lib.rs
@@ -18,6 +18,7 @@ pub mod prelude {
 
 use bevy_app::prelude::*;
 use bevy_ecs::{entity::Entity, system::IntoSystem};
+use bevy_utils::HashSet;
 use std::ops::Range;
 
 /// Adds core functionality to Apps.
@@ -36,6 +37,8 @@ impl Plugin for CorePlugin {
         app.init_resource::<Time>()
             .init_resource::<EntityLabels>()
             .init_resource::<FixedTimesteps>()
+            .register_type::<HashSet<String>>()
+            .register_type::<Option<String>>()
             .register_type::<Entity>()
             .register_type::<Name>()
             .register_type::<Labels>()
diff --git a/crates/bevy_math/src/geometry.rs b/crates/bevy_math/src/geometry.rs
index 96ebedd7dd..0a449e5d02 100644
--- a/crates/bevy_math/src/geometry.rs
+++ b/crates/bevy_math/src/geometry.rs
@@ -4,18 +4,19 @@ use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign};
 
 /// A two dimensional "size" as defined by a width and height
 #[derive(Copy, Clone, PartialEq, Debug, Reflect)]
-pub struct Size<T: Reflect = f32> {
+#[reflect(PartialEq)]
+pub struct Size<T: Reflect + PartialEq = f32> {
     pub width: T,
     pub height: T,
 }
 
-impl<T: Reflect> Size<T> {
+impl<T: Reflect + PartialEq> Size<T> {
     pub fn new(width: T, height: T) -> Self {
         Size { width, height }
     }
 }
 
-impl<T: Default + Reflect> Default for Size<T> {
+impl<T: Default + Reflect + PartialEq> Default for Size<T> {
     fn default() -> Self {
         Self {
             width: Default::default(),
@@ -26,14 +27,15 @@ impl<T: Default + Reflect> Default for Size<T> {
 
 /// A rect, as defined by its "side" locations
 #[derive(Copy, Clone, PartialEq, Debug, Reflect)]
-pub struct Rect<T: Reflect> {
+#[reflect(PartialEq)]
+pub struct Rect<T: Reflect + PartialEq> {
     pub left: T,
     pub right: T,
     pub top: T,
     pub bottom: T,
 }
 
-impl<T: Reflect> Rect<T> {
+impl<T: Reflect + PartialEq> Rect<T> {
     pub fn all(value: T) -> Self
     where
         T: Clone,
@@ -47,7 +49,7 @@ impl<T: Reflect> Rect<T> {
     }
 }
 
-impl<T: Default + Reflect> Default for Rect<T> {
+impl<T: Default + Reflect + PartialEq> Default for Rect<T> {
     fn default() -> Self {
         Self {
             left: Default::default(),
@@ -58,7 +60,7 @@ impl<T: Default + Reflect> Default for Rect<T> {
     }
 }
 
-impl<T: Reflect> Add<Vec2> for Size<T>
+impl<T: Reflect + PartialEq> Add<Vec2> for Size<T>
 where
     T: Add<f32, Output = T>,
 {
@@ -72,7 +74,7 @@ where
     }
 }
 
-impl<T: Reflect> AddAssign<Vec2> for Size<T>
+impl<T: Reflect + PartialEq> AddAssign<Vec2> for Size<T>
 where
     T: AddAssign<f32>,
 {
@@ -82,7 +84,7 @@ where
     }
 }
 
-impl<T: Reflect> Sub<Vec2> for Size<T>
+impl<T: Reflect + PartialEq> Sub<Vec2> for Size<T>
 where
     T: Sub<f32, Output = T>,
 {
@@ -96,7 +98,7 @@ where
     }
 }
 
-impl<T: Reflect> SubAssign<Vec2> for Size<T>
+impl<T: Reflect + PartialEq> SubAssign<Vec2> for Size<T>
 where
     T: SubAssign<f32>,
 {
@@ -106,7 +108,7 @@ where
     }
 }
 
-impl<T: Reflect> Mul<f32> for Size<T>
+impl<T: Reflect + PartialEq> Mul<f32> for Size<T>
 where
     T: Mul<f32, Output = T>,
 {
@@ -120,7 +122,7 @@ where
     }
 }
 
-impl<T: Reflect> MulAssign<f32> for Size<T>
+impl<T: Reflect + PartialEq> MulAssign<f32> for Size<T>
 where
     T: MulAssign<f32>,
 {
@@ -130,7 +132,7 @@ where
     }
 }
 
-impl<T: Reflect> Div<f32> for Size<T>
+impl<T: Reflect + PartialEq> Div<f32> for Size<T>
 where
     T: Div<f32, Output = T>,
 {
@@ -144,7 +146,7 @@ where
     }
 }
 
-impl<T: Reflect> DivAssign<f32> for Size<T>
+impl<T: Reflect + PartialEq> DivAssign<f32> for Size<T>
 where
     T: DivAssign<f32>,
 {
diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs
index 4d40e41c05..fe0b22dcf7 100644
--- a/crates/bevy_reflect/src/impls/std.rs
+++ b/crates/bevy_reflect/src/impls/std.rs
@@ -1,6 +1,6 @@
 use crate::{
-    map_partial_eq, serde::Serializable, DynamicMap, List, ListIter, Map, MapIter, Reflect,
-    ReflectDeserialize, ReflectMut, ReflectRef,
+    map_partial_eq, serde::Serializable, DynamicMap, FromType, GetTypeRegistration, List, ListIter,
+    Map, MapIter, Reflect, ReflectDeserialize, ReflectMut, ReflectRef, TypeRegistration,
 };
 
 use bevy_reflect_derive::impl_reflect_value;
@@ -112,6 +112,14 @@ impl<T: Reflect> Reflect for Vec<T> {
     }
 }
 
+impl<T: Reflect + for<'de> Deserialize<'de>> GetTypeRegistration for Vec<T> {
+    fn get_type_registration() -> TypeRegistration {
+        let mut registration = TypeRegistration::of::<Vec<T>>();
+        registration.insert::<ReflectDeserialize>(FromType::<Vec<T>>::from_type());
+        registration
+    }
+}
+
 impl<K: Reflect + Clone + Eq + Hash, V: Reflect + Clone> Map for HashMap<K, V> {
     fn get(&self, key: &dyn Reflect) -> Option<&dyn Reflect> {
         key.downcast_ref::<K>()
@@ -207,6 +215,18 @@ impl<K: Reflect + Clone + Eq + Hash, V: Reflect + Clone> Reflect for HashMap<K,
     }
 }
 
+impl<K, V> GetTypeRegistration for HashMap<K, V>
+where
+    K: Reflect + Clone + Eq + Hash + for<'de> Deserialize<'de>,
+    V: Reflect + Clone + for<'de> Deserialize<'de>,
+{
+    fn get_type_registration() -> TypeRegistration {
+        let mut registration = TypeRegistration::of::<HashMap<K, V>>();
+        registration.insert::<ReflectDeserialize>(FromType::<HashMap<K, V>>::from_type());
+        registration
+    }
+}
+
 impl Reflect for Cow<'static, str> {
     fn type_name(&self) -> &str {
         std::any::type_name::<Self>()
@@ -266,3 +286,11 @@ impl Reflect for Cow<'static, str> {
         Some(Serializable::Borrowed(self))
     }
 }
+
+impl GetTypeRegistration for Cow<'static, str> {
+    fn get_type_registration() -> TypeRegistration {
+        let mut registration = TypeRegistration::of::<Cow<'static, str>>();
+        registration.insert::<ReflectDeserialize>(FromType::<Cow<'static, str>>::from_type());
+        registration
+    }
+}
diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs
index 6d00e533f4..52be9d03f0 100644
--- a/crates/bevy_reflect/src/lib.rs
+++ b/crates/bevy_reflect/src/lib.rs
@@ -194,6 +194,7 @@ mod tests {
     #[test]
     fn reflect_complex_patch() {
         #[derive(Reflect, Eq, PartialEq, Debug)]
+        #[reflect(PartialEq)]
         struct Foo {
             a: u32,
             #[reflect(ignore)]
@@ -205,6 +206,7 @@ mod tests {
         }
 
         #[derive(Reflect, Eq, PartialEq, Debug)]
+        #[reflect(PartialEq)]
         struct Bar {
             x: u32,
         }
@@ -317,6 +319,7 @@ mod tests {
     #[test]
     fn reflect_take() {
         #[derive(Reflect, Debug, PartialEq)]
+        #[reflect(PartialEq)]
         struct Bar {
             x: u32,
         }
diff --git a/crates/bevy_render/src/camera/visible_entities.rs b/crates/bevy_render/src/camera/visible_entities.rs
index 940adab57a..689c8af162 100644
--- a/crates/bevy_render/src/camera/visible_entities.rs
+++ b/crates/bevy_render/src/camera/visible_entities.rs
@@ -43,7 +43,7 @@ pub type Layer = u8;
 ///
 /// Entities without this component belong to layer `0`.
 #[derive(Copy, Clone, Reflect, PartialEq, Eq, PartialOrd, Ord)]
-#[reflect(Component)]
+#[reflect(Component, PartialEq)]
 pub struct RenderLayers(LayerMask);
 
 impl std::fmt::Debug for RenderLayers {
diff --git a/crates/bevy_render/src/color.rs b/crates/bevy_render/src/color.rs
index cfa52adc56..64dbbe7d0a 100644
--- a/crates/bevy_render/src/color.rs
+++ b/crates/bevy_render/src/color.rs
@@ -7,7 +7,7 @@ use crate::{
 use bevy_asset::Handle;
 use bevy_core::{Byteable, Bytes};
 use bevy_math::{Vec3, Vec4};
-use bevy_reflect::Reflect;
+use bevy_reflect::{Reflect, ReflectDeserialize};
 use serde::{Deserialize, Serialize};
 use std::ops::{Add, AddAssign, Mul, MulAssign};
 
@@ -16,6 +16,7 @@ use std::ops::{Add, AddAssign, Mul, MulAssign};
 /// RGBA color in the Linear sRGB colorspace (often colloquially referred to as "linear", "RGB", or "linear RGB").
 #[repr(C)]
 #[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize, Reflect)]
+#[reflect(PartialEq, Serialize, Deserialize)]
 pub struct Color {
     red: f32,
     green: f32,
diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs
index 0c6326305c..18d80770e4 100644
--- a/crates/bevy_render/src/lib.rs
+++ b/crates/bevy_render/src/lib.rs
@@ -39,11 +39,12 @@ use bevy_app::prelude::*;
 use bevy_asset::{AddAsset, AssetStage};
 use bevy_ecs::schedule::StageLabel;
 use camera::{
-    ActiveCameras, Camera, OrthographicProjection, PerspectiveProjection, VisibleEntities,
+    ActiveCameras, Camera, DepthCalculation, OrthographicProjection, PerspectiveProjection,
+    RenderLayers, ScalingMode, VisibleEntities, WindowOrigin,
 };
 use pipeline::{
     IndexFormat, PipelineCompiler, PipelineDescriptor, PipelineSpecialization, PrimitiveTopology,
-    ShaderSpecialization,
+    ShaderSpecialization, VertexBufferLayout,
 };
 use render_graph::{
     base::{self, BaseRenderGraphConfig, MainPass},
@@ -125,6 +126,7 @@ impl Plugin for RenderPlugin {
         .add_asset::<Shader>()
         .add_asset::<PipelineDescriptor>()
         .register_type::<Camera>()
+        .register_type::<DepthCalculation>()
         .register_type::<Draw>()
         .register_type::<Visible>()
         .register_type::<RenderPipelines>()
@@ -137,6 +139,10 @@ impl Plugin for RenderPlugin {
         .register_type::<PrimitiveTopology>()
         .register_type::<IndexFormat>()
         .register_type::<PipelineSpecialization>()
+        .register_type::<RenderLayers>()
+        .register_type::<ScalingMode>()
+        .register_type::<VertexBufferLayout>()
+        .register_type::<WindowOrigin>()
         .init_resource::<ClearColor>()
         .init_resource::<RenderGraph>()
         .init_resource::<PipelineCompiler>()
diff --git a/crates/bevy_render/src/pipeline/pipeline_compiler.rs b/crates/bevy_render/src/pipeline/pipeline_compiler.rs
index 26c575bdce..ab031df61e 100644
--- a/crates/bevy_render/src/pipeline/pipeline_compiler.rs
+++ b/crates/bevy_render/src/pipeline/pipeline_compiler.rs
@@ -5,12 +5,13 @@ use crate::{
     shader::{Shader, ShaderError},
 };
 use bevy_asset::{Assets, Handle};
-use bevy_reflect::Reflect;
+use bevy_reflect::{Reflect, ReflectDeserialize};
 use bevy_utils::{HashMap, HashSet};
 use once_cell::sync::Lazy;
 use serde::{Deserialize, Serialize};
 
 #[derive(Clone, Eq, PartialEq, Debug, Reflect)]
+#[reflect(PartialEq)]
 pub struct PipelineSpecialization {
     pub shader_specialization: ShaderSpecialization,
     pub primitive_topology: PrimitiveTopology,
@@ -41,6 +42,7 @@ impl PipelineSpecialization {
 }
 
 #[derive(Clone, Eq, PartialEq, Debug, Default, Reflect, Serialize, Deserialize)]
+#[reflect(PartialEq, Serialize, Deserialize)]
 pub struct ShaderSpecialization {
     pub shader_defs: HashSet<String>,
 }
diff --git a/crates/bevy_render/src/pipeline/state_descriptors.rs b/crates/bevy_render/src/pipeline/state_descriptors.rs
index 093ac3a45c..cf9bf1c108 100644
--- a/crates/bevy_render/src/pipeline/state_descriptors.rs
+++ b/crates/bevy_render/src/pipeline/state_descriptors.rs
@@ -1,5 +1,5 @@
 use crate::texture::TextureFormat;
-use bevy_reflect::Reflect;
+use bevy_reflect::{Reflect, ReflectDeserialize};
 use serde::{Deserialize, Serialize};
 
 #[derive(Clone, Debug)]
@@ -89,6 +89,7 @@ pub enum CompareFunction {
 
 /// Describes how the VertexAttributes should be interpreted while rendering
 #[derive(Copy, Clone, Debug, Hash, Eq, PartialEq, Serialize, Deserialize, Reflect)]
+#[reflect_value(Serialize, Deserialize, PartialEq, Hash)]
 pub enum PrimitiveTopology {
     PointList = 0,
     LineList = 1,
@@ -227,6 +228,7 @@ impl Default for BlendOperation {
 }
 
 #[derive(Copy, Clone, Debug, Hash, Eq, PartialEq, Serialize, Deserialize, Reflect)]
+#[reflect_value(Hash, PartialEq, Serialize, Deserialize)]
 pub enum IndexFormat {
     Uint16 = 0,
     Uint32 = 1,
diff --git a/crates/bevy_sprite/src/lib.rs b/crates/bevy_sprite/src/lib.rs
index 3853604b71..1301229281 100644
--- a/crates/bevy_sprite/src/lib.rs
+++ b/crates/bevy_sprite/src/lib.rs
@@ -48,6 +48,7 @@ impl Plugin for SpritePlugin {
         app.add_asset::<ColorMaterial>()
             .add_asset::<TextureAtlas>()
             .register_type::<Sprite>()
+            .register_type::<SpriteResizeMode>()
             .add_system_to_stage(CoreStage::PostUpdate, sprite_system.system())
             .add_system_to_stage(
                 CoreStage::PostUpdate,
diff --git a/crates/bevy_transform/src/components/global_transform.rs b/crates/bevy_transform/src/components/global_transform.rs
index 803136cd78..bc8d556d93 100644
--- a/crates/bevy_transform/src/components/global_transform.rs
+++ b/crates/bevy_transform/src/components/global_transform.rs
@@ -5,7 +5,7 @@ use bevy_reflect::Reflect;
 use std::ops::Mul;
 
 #[derive(Debug, PartialEq, Clone, Copy, Reflect)]
-#[reflect(Component)]
+#[reflect(Component, PartialEq)]
 pub struct GlobalTransform {
     pub translation: Vec3,
     pub rotation: Quat,
diff --git a/crates/bevy_transform/src/components/parent.rs b/crates/bevy_transform/src/components/parent.rs
index 2018ef7038..f26dfba1dd 100644
--- a/crates/bevy_transform/src/components/parent.rs
+++ b/crates/bevy_transform/src/components/parent.rs
@@ -7,7 +7,7 @@ use bevy_reflect::Reflect;
 use std::ops::{Deref, DerefMut};
 
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Reflect)]
-#[reflect(Component, MapEntities)]
+#[reflect(Component, MapEntities, PartialEq)]
 pub struct Parent(pub Entity);
 
 // TODO: We need to impl either FromWorld or Default so Parent can be registered as Properties.
@@ -42,7 +42,7 @@ impl DerefMut for Parent {
 }
 
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Reflect)]
-#[reflect(Component, MapEntities)]
+#[reflect(Component, MapEntities, PartialEq)]
 pub struct PreviousParent(pub(crate) Entity);
 
 impl MapEntities for PreviousParent {
diff --git a/crates/bevy_transform/src/components/transform.rs b/crates/bevy_transform/src/components/transform.rs
index 67335c5aa4..781c71377a 100644
--- a/crates/bevy_transform/src/components/transform.rs
+++ b/crates/bevy_transform/src/components/transform.rs
@@ -5,7 +5,7 @@ use bevy_reflect::Reflect;
 use std::ops::Mul;
 
 #[derive(Debug, PartialEq, Clone, Copy, Reflect)]
-#[reflect(Component)]
+#[reflect(Component, PartialEq)]
 pub struct Transform {
     pub translation: Vec3,
     pub rotation: Quat,
diff --git a/crates/bevy_ui/src/lib.rs b/crates/bevy_ui/src/lib.rs
index 6f7c97537d..cdbafe2217 100644
--- a/crates/bevy_ui/src/lib.rs
+++ b/crates/bevy_ui/src/lib.rs
@@ -9,6 +9,7 @@ pub mod update;
 pub mod widget;
 
 pub use anchors::*;
+use bevy_math::{Rect, Size};
 use bevy_render::RenderStage;
 pub use flex::*;
 pub use focus::*;
@@ -47,6 +48,21 @@ pub mod system {
 impl Plugin for UiPlugin {
     fn build(&self, app: &mut AppBuilder) {
         app.init_resource::<FlexSurface>()
+            .register_type::<AlignContent>()
+            .register_type::<AlignItems>()
+            .register_type::<AlignSelf>()
+            .register_type::<Direction>()
+            .register_type::<Display>()
+            .register_type::<FlexDirection>()
+            .register_type::<FlexWrap>()
+            .register_type::<JustifyContent>()
+            .register_type::<Node>()
+            .register_type::<PositionType>()
+            .register_type::<Size<f32>>()
+            .register_type::<Size<Val>>()
+            .register_type::<Rect<Val>>()
+            .register_type::<Style>()
+            .register_type::<Val>()
             .add_stage_before(CoreStage::PostUpdate, UiStage::Ui, SystemStage::parallel())
             .add_system_to_stage(CoreStage::PreUpdate, ui_focus_system.system())
             // add these stages to front because these must run before transform update systems
diff --git a/crates/bevy_ui/src/ui_node.rs b/crates/bevy_ui/src/ui_node.rs
index af3525027e..73a86f8cfc 100644
--- a/crates/bevy_ui/src/ui_node.rs
+++ b/crates/bevy_ui/src/ui_node.rs
@@ -50,6 +50,7 @@ impl AddAssign<f32> for Val {
 }
 
 #[derive(Clone, PartialEq, Debug, Reflect)]
+#[reflect(Component, PartialEq)]
 pub struct Style {
     pub display: Display,
     pub position_type: PositionType,