From 05b0f28ebfb0df975c91ab2c7f96ab00cbe9c912 Mon Sep 17 00:00:00 2001 From: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Date: Tue, 8 Oct 2024 19:56:35 -0700 Subject: [PATCH] bevy_scene: Use `FromReflect` on extracted resources (#15753) # Objective Fixes #15726 The extraction logic for components makes use of `FromReflect` to try and ensure we have a concrete type for serialization. However, we did not do the same for resources. The reason we're seeing this for the glam types is that #15174 also made a change to rely on the glam type's `Serialize` and `Deserialize` impls, which I don't think should have been merged (I'll put up a PR addressing this specifically soon). ## Solution Use `FromReflect` on extracted resources. ## Testing You can test locally by running: ``` cargo test --package bevy_scene ``` --- .../bevy_scene/src/dynamic_scene_builder.rs | 50 +++++++++++++++++-- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/crates/bevy_scene/src/dynamic_scene_builder.rs b/crates/bevy_scene/src/dynamic_scene_builder.rs index 894f300a9b651..208702dd81b3b 100644 --- a/crates/bevy_scene/src/dynamic_scene_builder.rs +++ b/crates/bevy_scene/src/dynamic_scene_builder.rs @@ -365,12 +365,19 @@ impl<'w> DynamicSceneBuilder<'w> { return None; } - let resource = type_registry - .get(type_id)? + let type_registration = type_registry.get(type_id)?; + + let resource = type_registration .data::()? .reflect(self.original_world)?; - self.extracted_resources - .insert(component_id, resource.clone_value()); + + let resource = type_registration + .data::() + .and_then(|fr| fr.from_reflect(resource.as_partial_reflect())) + .map(PartialReflect::into_partial_reflect) + .unwrap_or_else(|| resource.clone_value()); + + self.extracted_resources.insert(component_id, resource); Some(()) }; extract_and_push(); @@ -688,4 +695,39 @@ mod tests { assert_eq!(scene.resources.len(), 1); assert!(scene.resources[0].represents::()); } + + #[test] + fn should_use_from_reflect() { + #[derive(Resource, Component, Reflect)] + #[reflect(Resource, Component)] + struct SomeType(i32); + + let mut world = World::default(); + let atr = AppTypeRegistry::default(); + { + let mut register = atr.write(); + register.register::(); + } + world.insert_resource(atr); + + world.insert_resource(SomeType(123)); + let entity = world.spawn(SomeType(123)).id(); + + let scene = DynamicSceneBuilder::from_world(&world) + .extract_resources() + .extract_entities(vec![entity].into_iter()) + .build(); + + let component = &scene.entities[0].components[0]; + assert!(component + .try_as_reflect() + .expect("component should be concrete due to `FromReflect`") + .is::()); + + let resource = &scene.resources[0]; + assert!(resource + .try_as_reflect() + .expect("resource should be concrete due to `FromReflect`") + .is::()); + } }