diff --git a/Cargo.toml b/Cargo.toml index 7a0eec40f6455..4fc6699374fe6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,6 +315,10 @@ path = "examples/ecs/system_sets.rs" name = "timers" path = "examples/ecs/timers.rs" +[[example]] +name = "use_after_despawn" +path = "examples/ecs/use_after_despawn.rs" + # Games [[example]] name = "alien_cake_addict" diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index edef90da4939b..88345f3899218 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -740,6 +740,11 @@ fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize, Vec< impl Stage for SystemStage { fn run(&mut self, world: &mut World) { + if world.despawned.len() > 0 { + println!("despawned clear {}", world.despawned.len()); + world.despawned.clear(); + } + if let Some(world_id) = self.world_id { assert!( world.id() == world_id, diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index a1726920744c0..c0b037f355aa8 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -35,6 +35,7 @@ pub trait Command: Send + Sync + 'static { pub struct Commands<'w, 's> { queue: &'s mut CommandQueue, entities: &'w Entities, + pub(crate) name: &'static str, } impl<'w, 's> Commands<'w, 's> { @@ -43,9 +44,15 @@ impl<'w, 's> Commands<'w, 's> { Self { queue, entities: world.entities(), + name: "", } } + pub fn with_name(mut self, name: &'static str) -> Self { + self.name = name; + self + } + /// Creates a new empty [`Entity`] and returns an [`EntityCommands`] builder for it. /// /// To directly spawn an entity with a [`Bundle`] included, you can use @@ -346,6 +353,10 @@ impl<'w, 's, 'a> EntityCommands<'w, 's, 'a> { self.entity } + pub fn system_name(&self) -> &'static str { + self.commands.name + } + /// Adds a [`Bundle`] of components to the entity. /// /// # Example @@ -506,6 +517,7 @@ impl<'w, 's, 'a> EntityCommands<'w, 's, 'a> { pub fn despawn(&mut self) { self.commands.add(Despawn { entity: self.entity, + name: self.commands.name, }) } @@ -586,15 +598,19 @@ where #[derive(Debug)] pub struct Despawn { pub entity: Entity, + pub name: &'static str, } impl Command for Despawn { fn write(self, world: &mut World) { + println!("despawn from {}", self.name); + world.set_system_name(Some(self.name)); if !world.despawn(self.entity) { warn!("Could not despawn entity {:?} because it doesn't exist in this World.\n\ If this command was added to a newly spawned entity, ensure that you have not despawned that entity within the same stage.\n\ This may have occurred due to system order ambiguity, or if the spawning system has multiple command buffers", self.entity); } + world.set_system_name(None); } } @@ -611,9 +627,15 @@ where if let Some(mut entity) = world.get_entity_mut(self.entity) { entity.insert_bundle(self.bundle); } else { + let name = world + .despawned + .iter() + .find_map(|(e, n)| if *e == self.entity { Some(n) } else { None }) + .unwrap_or(&"unknown system"); panic!("Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World.\n\ + Entity was despawned in {}.\n\ If this command was added to a newly spawned entity, ensure that you have not despawned that entity within the same stage.\n\ - This may have occurred due to system order ambiguity, or if the spawning system has multiple command buffers", std::any::type_name::(), self.entity); + This may have occurred due to system order ambiguity, or if the spawning system has multiple command buffers", std::any::type_name::(), self.entity, name); } } } @@ -632,9 +654,15 @@ where if let Some(mut entity) = world.get_entity_mut(self.entity) { entity.insert(self.component); } else { + let name = world + .despawned + .iter() + .find_map(|(e, n)| if *e == self.entity { Some(n) } else { None }) + .unwrap_or(&"unknown system"); panic!("Could not add a component (of type `{}`) to entity {:?} because it doesn't exist in this World.\n\ + Entity was despawned in {}.\n\ If this command was added to a newly spawned entity, ensure that you have not despawned that entity within the same stage.\n\ - This may have occurred due to system order ambiguity, or if the spawning system has multiple command buffers", std::any::type_name::(), self.entity); + This may have occurred due to system order ambiguity, or if the spawning system has multiple command buffers", std::any::type_name::(), self.entity, name); } } } @@ -753,6 +781,25 @@ mod tests { assert_eq!(results2, vec![]); } + #[test] + #[should_panic(expected = "Entity was despawned in test commands")] + fn use_after_despawn() { + let mut world = World::default(); + let mut command_queue = CommandQueue::default(); + let entity = { + Commands::new(&mut command_queue, &world) + .spawn_bundle((1u32, 2u64)) + .id() + }; + command_queue.apply(&mut world); + { + let mut commands = Commands::new(&mut command_queue, &world).with_name("test commands"); + commands.entity(entity).despawn(); + commands.entity(entity).insert(5); + } + command_queue.apply(&mut world); + } + #[test] fn remove_components() { let mut world = World::default(); diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 6868c7fb978fe..884173d1433a4 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -519,11 +519,15 @@ impl<'w, 's> SystemParamFetch<'w, 's> for CommandQueue { #[inline] unsafe fn get_param( state: &'s mut Self, - _system_meta: &SystemMeta, + system_meta: &SystemMeta, world: &'w World, _change_tick: u32, ) -> Self::Item { - Commands::new(state, world) + let name: &'static str = match system_meta.name { + std::borrow::Cow::Borrowed(s) => s, + std::borrow::Cow::Owned(_) => "owned", + }; + Commands::new(state, world).with_name(name) } } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index c2bae02ca01da..147b1ad3d9d2e 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -411,6 +411,11 @@ impl<'w> EntityMut<'w> { let table_row; let moved_entity; { + if let Some(name) = world.system_name { + println!("world despawn from {}", name); + world.despawned.push((self.entity, name)); + } + let archetype = &mut world.archetypes[location.archetype_id]; for component_id in archetype.components() { let removed_components = world diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 441da301bda80..dc7b66cb227e0 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -83,6 +83,8 @@ pub struct World { main_thread_validator: MainThreadValidator, pub(crate) change_tick: AtomicU32, pub(crate) last_change_tick: u32, + pub(crate) despawned: Vec<(Entity, &'static str)>, + pub(crate) system_name: Option<&'static str>, } impl Default for World { @@ -101,6 +103,8 @@ impl Default for World { // are detected on first system runs and for direct world queries. change_tick: AtomicU32::new(1), last_change_tick: 0, + despawned: Vec::new(), + system_name: None, } } } @@ -117,6 +121,12 @@ impl World { World::default() } + #[inline] + pub fn set_system_name(&mut self, name: Option<&'static str>) { + println!("set system name {:?}", name); + self.system_name = name; + } + /// Retrieves this [`World`]'s unique ID #[inline] pub fn id(&self) -> WorldId { diff --git a/crates/bevy_transform/src/hierarchy/hierarchy.rs b/crates/bevy_transform/src/hierarchy/hierarchy.rs index 6701387fe953b..de470d9b90f71 100644 --- a/crates/bevy_transform/src/hierarchy/hierarchy.rs +++ b/crates/bevy_transform/src/hierarchy/hierarchy.rs @@ -9,6 +9,7 @@ use bevy_utils::tracing::debug; #[derive(Debug)] pub struct DespawnRecursive { entity: Entity, + name: &'static str, } pub fn despawn_with_children_recursive(world: &mut World, entity: Entity) { @@ -38,7 +39,10 @@ fn despawn_with_children_recursive_inner(world: &mut World, entity: Entity) { impl Command for DespawnRecursive { fn write(self, world: &mut World) { + println!("despawn recursive from {}", self.name); + world.set_system_name(Some(self.name)); despawn_with_children_recursive(world, self.entity); + world.set_system_name(None); } } @@ -51,7 +55,8 @@ impl<'w, 's, 'a> DespawnRecursiveExt for EntityCommands<'w, 's, 'a> { /// Despawns the provided entity and its children. fn despawn_recursive(mut self) { let entity = self.id(); - self.commands().add(DespawnRecursive { entity }); + let name = self.system_name(); + self.commands().add(DespawnRecursive { entity, name }); } } diff --git a/examples/ecs/use_after_despawn.rs b/examples/ecs/use_after_despawn.rs new file mode 100644 index 0000000000000..6e99b14b1f29f --- /dev/null +++ b/examples/ecs/use_after_despawn.rs @@ -0,0 +1,44 @@ +use bevy::prelude::*; + +fn main() { + App::new() + .add_startup_system(do_spawn) + // This system despawns the entity in the Update stage. + .add_system(do_despawn.system().label("despawn")) + // This system inserts into the entity in the Update stage. + // It is scheduled explicitly to run after do_despawn, so its Insert command + // will panic because in the meantime the Despawn command despawed the entity. + // + // Here it is simple. But it could be as complicated a schedule like + // the inserting system is some system of a third party plugin. + // About this imagined system you as a user, + // you don't immediately know it exists, + // you don't know when it is scheduled, + // you don't know if it is handling intermittent despawned entities well. + // But eventually the parallel schedule is run in a way where + // both systems, your despawner and the third party inserter, run in the same stage + // in an order where they clash. + // And when you don't know, which of my entity kinds was it? When was it despawned? + // How to reproduce it? + .add_system(do_insert.system().after("despawn")) + .run(); +} + +fn do_spawn(mut cmds: Commands) { + let entity = cmds.spawn().insert(5u32).id(); + println!("{:?} spawn", entity); +} + +fn do_despawn(query: Query>, mut cmds: Commands) { + for entity in query.iter() { + println!("{:?} despawn", entity); + cmds.entity(entity).despawn(); + } +} + +fn do_insert(query: Query>, mut cmds: Commands) { + for entity in query.iter() { + println!("{:?} insert", entity); + cmds.entity(entity).insert("panic".to_string()); + } +}