Migrate Quat reflection strategy from "value" to "struct" (#10068)
				
					
				
			Adopted from #8954, co-authored by @pyrotechnick # Objective The Bevy ecosystem currently reflects `Quat` via "value" rather than the more appropriate "struct" strategy. This behaviour is inconsistent to that of similar types, i.e. `Vec3`. Additionally, employing the "value" strategy causes instances of `Quat` to be serialised as a sequence `[x, y, z, w]` rather than structures of shape `{ x, y, z, w }`. The [comments surrounding the applicable code](bec299fa6e/crates/bevy_reflect/src/impls/glam.rs (L254)) give context and historical reasons for this discrepancy: ``` // Quat fields are read-only (as of now), and reflection is currently missing // mechanisms for read-only fields. I doubt those mechanisms would be added, // so for now quaternions will remain as values. They are represented identically // to Vec4 and DVec4, so you may use those instead and convert between. ``` This limitation has [since been lifted by the upstream crate](374625163e), glam. ## Solution Migrating the reflect strategy of Quat from "value" to "struct" via replacing `impl_reflect_value` with `impl_reflect_struct` resolves the issue. ## Changelog Migrated `Quat` reflection strategy to "struct" from "value" Migration Guide Changed Quat serialization/deserialization from sequences `[x, y, z, w]` to structures `{ x, y, z, w }`. --------- Co-authored-by: pyrotechnick <13998+pyrotechnick@users.noreply.github.com> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
This commit is contained in:
		
							parent
							
								
									665dbcbb21
								
							
						
					
					
						commit
						e5f5ce5e97
					
				| @ -1,6 +1,5 @@ | |||||||
| use crate as bevy_reflect; | use crate as bevy_reflect; | ||||||
| use crate::prelude::ReflectDefault; | use crate::prelude::ReflectDefault; | ||||||
| use crate::{ReflectDeserialize, ReflectSerialize}; |  | ||||||
| use bevy_reflect_derive::{impl_reflect_struct, impl_reflect_value}; | use bevy_reflect_derive::{impl_reflect_struct, impl_reflect_value}; | ||||||
| use glam::*; | use glam::*; | ||||||
| 
 | 
 | ||||||
| @ -310,24 +309,26 @@ impl_reflect_struct!( | |||||||
|     } |     } | ||||||
| ); | ); | ||||||
| 
 | 
 | ||||||
| // Quat fields are read-only (as of now), and reflection is currently missing
 | impl_reflect_struct!( | ||||||
| // mechanisms for read-only fields. I doubt those mechanisms would be added,
 |     #[reflect(Debug, PartialEq, Default)] | ||||||
| // so for now quaternions will remain as values. They are represented identically
 |     #[type_path = "glam"] | ||||||
| // to Vec4 and DVec4, so you may use those instead and convert between.
 |     struct Quat { | ||||||
| impl_reflect_value!(::glam::Quat( |         x: f32, | ||||||
|     Debug, |         y: f32, | ||||||
|     PartialEq, |         z: f32, | ||||||
|     Serialize, |         w: f32, | ||||||
|     Deserialize, |     } | ||||||
|     Default | ); | ||||||
| )); | impl_reflect_struct!( | ||||||
| impl_reflect_value!(::glam::DQuat( |     #[reflect(Debug, PartialEq, Default)] | ||||||
|     Debug, |     #[type_path = "glam"] | ||||||
|     PartialEq, |     struct DQuat { | ||||||
|     Serialize, |         x: f64, | ||||||
|     Deserialize, |         y: f64, | ||||||
|     Default |         z: f64, | ||||||
| )); |         w: f64, | ||||||
|  |     } | ||||||
|  | ); | ||||||
| 
 | 
 | ||||||
| impl_reflect_value!(::glam::EulerRot(Debug, Default)); | impl_reflect_value!(::glam::EulerRot(Debug, Default)); | ||||||
| impl_reflect_value!(::glam::BVec3A(Debug, Default)); | impl_reflect_value!(::glam::BVec3A(Debug, Default)); | ||||||
|  | |||||||
| @ -575,7 +575,7 @@ pub mod __macro_exports { | |||||||
| #[allow(clippy::disallowed_types, clippy::approx_constant)] | #[allow(clippy::disallowed_types, clippy::approx_constant)] | ||||||
| mod tests { | mod tests { | ||||||
|     #[cfg(feature = "glam")] |     #[cfg(feature = "glam")] | ||||||
|     use ::glam::{vec3, Vec3}; |     use ::glam::{quat, vec3, Quat, Vec3}; | ||||||
|     use ::serde::{de::DeserializeSeed, Deserialize, Serialize}; |     use ::serde::{de::DeserializeSeed, Deserialize, Serialize}; | ||||||
|     use bevy_utils::HashMap; |     use bevy_utils::HashMap; | ||||||
|     use ron::{ |     use ron::{ | ||||||
| @ -1937,6 +1937,65 @@ bevy_reflect::tests::Test { | |||||||
|     mod glam { |     mod glam { | ||||||
|         use super::*; |         use super::*; | ||||||
| 
 | 
 | ||||||
|  |         #[test] | ||||||
|  |         fn quat_serialization() { | ||||||
|  |             let q = quat(1.0, 2.0, 3.0, 4.0); | ||||||
|  | 
 | ||||||
|  |             let mut registry = TypeRegistry::default(); | ||||||
|  |             registry.register::<f32>(); | ||||||
|  |             registry.register::<Quat>(); | ||||||
|  | 
 | ||||||
|  |             let ser = ReflectSerializer::new(&q, ®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::Quat": ( | ||||||
|  |         x: 1.0, | ||||||
|  |         y: 2.0, | ||||||
|  |         z: 3.0, | ||||||
|  |         w: 4.0, | ||||||
|  |     ), | ||||||
|  | }"#;
 | ||||||
|  | 
 | ||||||
|  |             assert_eq!(expected, format!("\n{output}")); | ||||||
|  |         } | ||||||
|  | 
 | ||||||
|  |         #[test] | ||||||
|  |         fn quat_deserialization() { | ||||||
|  |             let data = r#" | ||||||
|  | { | ||||||
|  |     "glam::Quat": ( | ||||||
|  |         x: 1.0, | ||||||
|  |         y: 2.0, | ||||||
|  |         z: 3.0, | ||||||
|  |         w: 4.0, | ||||||
|  |     ), | ||||||
|  | }"#;
 | ||||||
|  | 
 | ||||||
|  |             let mut registry = TypeRegistry::default(); | ||||||
|  |             registry.register::<Quat>(); | ||||||
|  |             registry.register::<f32>(); | ||||||
|  | 
 | ||||||
|  |             let de = UntypedReflectDeserializer::new(®istry); | ||||||
|  | 
 | ||||||
|  |             let mut deserializer = | ||||||
|  |                 ron::de::Deserializer::from_str(data).expect("Failed to acquire deserializer"); | ||||||
|  | 
 | ||||||
|  |             let dynamic_struct = de | ||||||
|  |                 .deserialize(&mut deserializer) | ||||||
|  |                 .expect("Failed to deserialize"); | ||||||
|  | 
 | ||||||
|  |             let mut result = Quat::default(); | ||||||
|  | 
 | ||||||
|  |             result.apply(&*dynamic_struct); | ||||||
|  | 
 | ||||||
|  |             assert_eq!(result, quat(1.0, 2.0, 3.0, 4.0)); | ||||||
|  |         } | ||||||
|  | 
 | ||||||
|         #[test] |         #[test] | ||||||
|         fn vec3_serialization() { |         fn vec3_serialization() { | ||||||
|             let v = vec3(12.0, 3.0, -6.9); |             let v = vec3(12.0, 3.0, -6.9); | ||||||
|  | |||||||
		Loading…
	
		Reference in New Issue
	
	Block a user
	 Trashtalk217
						Trashtalk217