From 796628bc1a7c48c813347bcad7da40f6b454adcb Mon Sep 17 00:00:00 2001 From: Jerome Humbert Date: Sat, 9 Oct 2021 20:42:28 +0100 Subject: [PATCH 1/4] Add entity ID to expect() message Add the entity ID and generation to the expect() message of two world accessors, to make it easier to debug use-after-free issues. Coupled with e.g. bevy-inspector-egui which also displays the entity ID, this makes it much easier to identify what entity is being misused. --- crates/bevy_ecs/src/world/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index bd6c10e6a9079..5d031b46698c9 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -186,7 +186,7 @@ impl World { /// ``` #[inline] pub fn entity(&self, entity: Entity) -> EntityRef { - self.get_entity(entity).expect("Entity does not exist") + self.get_entity(entity).expect(&format!("Entity #{} does not exist (generation #{})", entity.id(), entity.generation())) } /// Retrieves an [EntityMut] that exposes read and write operations for the given `entity`. @@ -211,7 +211,7 @@ impl World { /// ``` #[inline] pub fn entity_mut(&mut self, entity: Entity) -> EntityMut { - self.get_entity_mut(entity).expect("Entity does not exist") + self.get_entity_mut(entity).expect(&format!("Entity #{} does not exist (generation #{})", entity.id(), entity.generation())) } /// Retrieves an [EntityRef] that exposes read-only operations for the given `entity`. From 40b202e17c01c542bff3b03e8fdd9c6dcc5d3bde Mon Sep 17 00:00:00 2001 From: Jerome Humbert Date: Sat, 9 Oct 2021 23:35:58 +0100 Subject: [PATCH 2/4] Remove allocation on non-failure case --- crates/bevy_ecs/src/world/mod.rs | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 5d031b46698c9..518c5734998f1 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -186,7 +186,14 @@ impl World { /// ``` #[inline] pub fn entity(&self, entity: Entity) -> EntityRef { - self.get_entity(entity).expect(&format!("Entity #{} does not exist (generation #{})", entity.id(), entity.generation())) + // Lazily evaluate panic!() via unwrap_or_else() to avoid allocation unless failure + self.get_entity(entity).unwrap_or_else(|| { + panic!( + "Entity #{} does not exist (generation #{})", + entity.id(), + entity.generation() + ) + }) } /// Retrieves an [EntityMut] that exposes read and write operations for the given `entity`. @@ -207,11 +214,18 @@ impl World { /// .id(); /// /// let mut position = world.entity_mut(entity).get_mut::().unwrap(); - /// position.x = 1.0; + /// position.x = 1.0;F /// ``` #[inline] pub fn entity_mut(&mut self, entity: Entity) -> EntityMut { - self.get_entity_mut(entity).expect(&format!("Entity #{} does not exist (generation #{})", entity.id(), entity.generation())) + // Lazily evaluate panic!() via unwrap_or_else() to avoid allocation unless failure + self.get_entity_mut(entity).unwrap_or_else(|| { + panic!( + "Entity #{} does not exist (generation #{})", + entity.id(), + entity.generation() + ) + }) } /// Retrieves an [EntityRef] that exposes read-only operations for the given `entity`. From e5827a67a9f3bc10a43a17b8932b91b3eb5dc577 Mon Sep 17 00:00:00 2001 From: Jerome Humbert Date: Sat, 9 Oct 2021 23:38:18 +0100 Subject: [PATCH 3/4] Fix typo --- crates/bevy_ecs/src/world/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 518c5734998f1..31a90d32dae47 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -214,7 +214,7 @@ impl World { /// .id(); /// /// let mut position = world.entity_mut(entity).get_mut::().unwrap(); - /// position.x = 1.0;F + /// position.x = 1.0; /// ``` #[inline] pub fn entity_mut(&mut self, entity: Entity) -> EntityMut { From be5e08a08ea0c1b408ddae7c9a2ad6df521616f4 Mon Sep 17 00:00:00 2001 From: Jerome Humbert Date: Sun, 10 Oct 2021 16:51:50 +0100 Subject: [PATCH 4/4] Use {:?} instead of custom format --- crates/bevy_ecs/src/world/mod.rs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 31a90d32dae47..0dc6a35f4ee51 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -187,13 +187,8 @@ impl World { #[inline] pub fn entity(&self, entity: Entity) -> EntityRef { // Lazily evaluate panic!() via unwrap_or_else() to avoid allocation unless failure - self.get_entity(entity).unwrap_or_else(|| { - panic!( - "Entity #{} does not exist (generation #{})", - entity.id(), - entity.generation() - ) - }) + self.get_entity(entity) + .unwrap_or_else(|| panic!("Entity {:?} does not exist", entity)) } /// Retrieves an [EntityMut] that exposes read and write operations for the given `entity`. @@ -219,13 +214,8 @@ impl World { #[inline] pub fn entity_mut(&mut self, entity: Entity) -> EntityMut { // Lazily evaluate panic!() via unwrap_or_else() to avoid allocation unless failure - self.get_entity_mut(entity).unwrap_or_else(|| { - panic!( - "Entity #{} does not exist (generation #{})", - entity.id(), - entity.generation() - ) - }) + self.get_entity_mut(entity) + .unwrap_or_else(|| panic!("Entity {:?} does not exist", entity)) } /// Retrieves an [EntityRef] that exposes read-only operations for the given `entity`.