From fb324f0e89800cf4563f46ae7d9e7cc2dec4147a Mon Sep 17 00:00:00 2001 From: aecsocket <43144841+aecsocket@users.noreply.github.com> Date: Mon, 23 Sep 2024 18:24:28 +0100 Subject: [PATCH] `impl_reflect!` for EulerRot instead of treating it as an opaque value (#15349) # Objective Currently, Bevy implements reflection for `glam::EulerRot` using: ```rs impl_reflect_value!(::glam::EulerRot(Debug, Default, Deserialize, Serialize)); ``` Treating it as an opaque type. However, it's useful to expose the EulerRot enum variants directly, which I make use of from a drop down selection box in `bevy_egui`. This PR changes this to use `impl_reflect!`. **Importantly**, Bevy currently uses glam 0.28.0, in which `EulerRot` has just 6 variants. In glam 0.29.0, this is exanded to 24 variants, see https://github.com/bitshifter/glam-rs/commit/bb2ab05613e98e9cbf24693473a0c4e7beb04e87. When Bevy updates to 0.29.0, this reflect impl must also be updated to include the new variants. ## Solution Replaces the `impl_reflect_value!` with `impl_reflect!` and a handwritten version of `EulerRot` with the same variants. ## Testing Added a `tests` module to `glam.rs` to ensure that de/serialization works. However, my main concern is making sure that the number of enum variants matches glam's, which I'm not sure how to do using `Enum`. --- crates/bevy_reflect/src/impls/glam.rs | 81 ++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/crates/bevy_reflect/src/impls/glam.rs b/crates/bevy_reflect/src/impls/glam.rs index 8d54a5d5c9..8ae372dcbe 100644 --- a/crates/bevy_reflect/src/impls/glam.rs +++ b/crates/bevy_reflect/src/impls/glam.rs @@ -330,6 +330,85 @@ impl_reflect!( } ); -impl_reflect_value!(::glam::EulerRot(Debug, Default, Deserialize, Serialize)); +impl_reflect!( + #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[type_path = "glam"] + enum EulerRot { + ZYX, + ZXY, + YXZ, + YZX, + XYZ, + XZY, + } +); + impl_reflect_value!(::glam::BVec3A(Debug, Default, Deserialize, Serialize)); impl_reflect_value!(::glam::BVec4A(Debug, Default, Deserialize, Serialize)); + +#[cfg(test)] +mod tests { + use ron::{ + ser::{to_string_pretty, PrettyConfig}, + Deserializer, + }; + use serde::de::DeserializeSeed; + use static_assertions::assert_impl_all; + + use crate::{ + prelude::*, + serde::{ReflectDeserializer, ReflectSerializer}, + Enum, GetTypeRegistration, TypeRegistry, + }; + + use super::*; + + assert_impl_all!(EulerRot: Enum); + + #[test] + fn euler_rot_serialization() { + let v = EulerRot::YXZ; + + let mut registry = TypeRegistry::default(); + registry.register::(); + + let ser = ReflectSerializer::new(&v, ®istry); + + let config = PrettyConfig::default() + .new_line(String::from("\n")) + .indentor(String::from(" ")); + let output = to_string_pretty(&ser, config).unwrap(); + let expected = r#" +{ + "glam::EulerRot": YXZ, +}"#; + + assert_eq!(expected, format!("\n{output}")); + } + + #[test] + fn euler_rot_deserialization() { + let data = r#" +{ + "glam::EulerRot": XZY, +}"#; + + let mut registry = TypeRegistry::default(); + registry.add_registration(EulerRot::get_type_registration()); + + let de = ReflectDeserializer::new(®istry); + + let mut deserializer = + Deserializer::from_str(data).expect("Failed to acquire deserializer"); + + let dynamic_struct = de + .deserialize(&mut deserializer) + .expect("Failed to deserialize"); + + let mut result = EulerRot::default(); + + result.apply(dynamic_struct.as_partial_reflect()); + + assert_eq!(result, EulerRot::XZY); + } +}