Don't panic in scene::replicate for non-reflected components

projectharmonia · Sep 13, 2024 · 1cd4a0a · 1cd4a0a
1 parent c740527
commit 1cd4a0a
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- Ignore replicated components that don't have type registration or missing `#[reflect(Component)]` in `scene::replicate_into` instead of panicking.
+
 ## [0.28.2] - 2024-09-09
 
 ### Changed

diff --git a/src/scene.rs b/src/scene.rs
@@ -5,14 +5,12 @@ use crate::{core::replication_rules::ReplicationRules, Replicated};
 /**
 Fills scene with all replicated entities and their components.
 
+Components that are not registered using [`App::register_type`]
+or do not have `#[reflect(Component)]` will be skipped.
+
 Entities won't have the [`Replicated`] component.
 So on deserialization you need to insert it back if you want entities to continue to replicate.
 
-# Panics
-
-Panics if any replicated component is not registered using [`App::register_type`]
-or `#[reflect(Component)]` is missing.
-
 # Examples
 
 ```
@@ -83,22 +81,26 @@ pub fn replicate_into(scene: &mut DynamicScene, world: &World) {
                 let type_name = replicated_component.name();
                 let type_id = replicated_component
                     .type_id()
-                    .unwrap_or_else(|| panic!("{type_name} should have registered TypeId"));
-                let registration = registry
-                    .get(type_id)
-                    .unwrap_or_else(|| panic!("{type_name} should be registered"));
-                let reflect_component = registration
-                    .data::<ReflectComponent>()
-                    .unwrap_or_else(|| panic!("{type_name} should have reflect(Component)"));
+                    .unwrap_or_else(|| panic!("`{type_name}` should be a Rust type"));
+                let Some(registration) = registry.get(type_id) else {
+                    debug!("ignoring `{type_name}` because it's not registered");
+                    continue;
+                };
+                let Some(reflect_component) = registration.data::<ReflectComponent>() else {
+                    debug!("ignoring `{type_name}` because it's missing `#[reflect(Component)]`");
+                    continue;
+                };
 
                 for entity in archetype.entities() {
                     let component = reflect_component
                         .reflect(world.entity(entity.id()))
-                        .unwrap_or_else(|| panic!("entity should have {type_name}"));
+                        .unwrap_or_else(|| panic!("entity should have `{type_name}`"));
 
                     let components = entities
                         .get_mut(&entity.id())
                         .expect("all entities should be populated ahead of time");
+
+                    debug!("adding `{type_name}` to `{}`", entity.id());
                     components.push(component.clone_value());
                 }
             }

diff --git a/tests/scene.rs b/tests/scene.rs
@@ -7,9 +7,20 @@ fn replicated_entity() {
     let mut app = App::new();
     app.add_plugins(RepliconPlugins)
         .register_type::<DummyComponent>()
-        .replicate::<DummyComponent>();
+        .register_type::<NonReflectedComponent>()
+        .replicate::<DummyComponent>()
+        .replicate::<OtherReflectedComponent>() // Reflected, but the type is not registered.
+        .replicate::<NonReflectedComponent>();
 
-    let entity = app.world_mut().spawn((Replicated, DummyComponent)).id();
+    let entity = app
+        .world_mut()
+        .spawn((
+            Replicated,
+            DummyComponent,
+            OtherReflectedComponent,
+            NonReflectedComponent,
+        ))
+        .id();
 
     let mut scene = DynamicScene::default();
     scene::replicate_into(&mut scene, app.world());
@@ -19,7 +30,11 @@ fn replicated_entity() {
 
     let dyn_entity = &scene.entities[0];
     assert_eq!(dyn_entity.entity, entity);
-    assert_eq!(dyn_entity.components.len(), 1);
+    assert_eq!(
+        dyn_entity.components.len(),
+        1,
+        "entity should have only registered components with `#[reflect(Component)]`"
+    );
 }
 
 #[test]
@@ -94,3 +109,7 @@ struct DummyComponent;
 #[derive(Component, Default, Deserialize, Reflect, Serialize)]
 #[reflect(Component)]
 struct OtherReflectedComponent;
+
+/// Component that have `Reflect` derive, but without `#[reflect(Component)]`
+#[derive(Component, Default, Deserialize, Reflect, Serialize)]
+struct NonReflectedComponent;