diff --git a/crates/bevy_ecs/src/entity/map_entities.rs b/crates/bevy_ecs/src/entity/map_entities.rs index 96045d3597..6429887c50 100644 --- a/crates/bevy_ecs/src/entity/map_entities.rs +++ b/crates/bevy_ecs/src/entity/map_entities.rs @@ -183,6 +183,9 @@ impl<'m> SceneEntityMapper<'m> { /// Creates a new [`SceneEntityMapper`], spawning a temporary base [`Entity`] in the provided [`World`] pub fn new(map: &'m mut EntityHashMap, world: &mut World) -> Self { + // We're going to be calling methods on `Entities` that require advance + // flushing, such as `alloc` and `free`. + world.flush_entities(); Self { map, // SAFETY: Entities data is kept in a valid state via `EntityMapper::world_scope` @@ -292,6 +295,23 @@ mod tests { ); } + #[test] + fn entity_mapper_no_panic() { + let mut world = World::new(); + // "Dirty" the `Entities`, requiring a flush afterward. + world.entities.reserve_entity(); + assert!(world.entities.needs_flush()); + + // Create and exercise a SceneEntityMapper - should not panic because it flushes + // `Entities` first. + SceneEntityMapper::world_scope(&mut Default::default(), &mut world, |_, m| { + m.map_entity(Entity::PLACEHOLDER); + }); + + // The SceneEntityMapper should leave `Entities` in a flushed state. + assert!(!world.entities.needs_flush()); + } + #[test] fn dyn_entity_mapper_object_safe() { assert_object_safe::(); diff --git a/crates/bevy_scene/src/dynamic_scene.rs b/crates/bevy_scene/src/dynamic_scene.rs index 7f094ce016..9845178623 100644 --- a/crates/bevy_scene/src/dynamic_scene.rs +++ b/crates/bevy_scene/src/dynamic_scene.rs @@ -209,14 +209,19 @@ where #[cfg(test)] mod tests { use bevy_ecs::{ + component::Component, entity::{Entity, EntityHashMap, EntityMapper, MapEntities}, - reflect::{AppTypeRegistry, ReflectMapEntitiesResource, ReflectResource}, + reflect::{ + AppTypeRegistry, ReflectComponent, ReflectMapEntities, ReflectMapEntitiesResource, + ReflectResource, + }, system::Resource, world::{Command, World}, }; use bevy_hierarchy::{AddChild, Parent}; use bevy_reflect::Reflect; + use crate::dynamic_scene::DynamicScene; use crate::dynamic_scene_builder::DynamicSceneBuilder; #[derive(Resource, Reflect, Debug)] @@ -348,4 +353,51 @@ mod tests { "something is wrong with the this test or the code reloading scenes since the relationship between scene entities is broken" ); } + + // Regression test for https://github.com/bevyengine/bevy/issues/14300 + // Fails before the fix in https://github.com/bevyengine/bevy/pull/15405 + #[test] + fn no_panic_in_map_entities_after_pending_entity_in_hook() { + #[derive(Default, Component, Reflect)] + #[reflect(Component)] + struct A; + + #[derive(Component, Reflect)] + #[reflect(Component, MapEntities)] + struct B(pub Entity); + + impl MapEntities for B { + fn map_entities(&mut self, entity_mapper: &mut M) { + self.0 = entity_mapper.map_entity(self.0); + } + } + + let reg = AppTypeRegistry::default(); + { + let mut reg_write = reg.write(); + reg_write.register::(); + reg_write.register::(); + } + + let mut scene_world = World::new(); + scene_world.insert_resource(reg.clone()); + scene_world.spawn((B(Entity::PLACEHOLDER), A)); + let scene = DynamicScene::from_world(&scene_world); + + let mut dst_world = World::new(); + dst_world + .register_component_hooks::() + .on_add(|mut world, _, _| { + world.commands().spawn_empty(); + }); + dst_world.insert_resource(reg.clone()); + + // Should not panic. + // Prior to fix, the `Entities::alloc` call in + // `EntityMapper::map_entity` would panic due to pending entities from the observer + // not having been flushed. + scene + .write_to_world(&mut dst_world, &mut Default::default()) + .unwrap(); + } }