From eb2dddacd7867c4b15e7779ebb6cd68ca19a614f Mon Sep 17 00:00:00 2001 From: John Payne <20407779+johngpayne@users.noreply.github.com> Date: Sat, 8 Jun 2024 22:35:51 +0100 Subject: [PATCH 1/8] Add a new Display impl for DebugName which displays the Name for entities which have one or the entity's own Display impl for entities which don't. --- crates/bevy_core/src/name.rs | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/crates/bevy_core/src/name.rs b/crates/bevy_core/src/name.rs index dfa4a11c118bd..3d3a5f7983eb4 100644 --- a/crates/bevy_core/src/name.rs +++ b/crates/bevy_core/src/name.rs @@ -100,13 +100,18 @@ impl std::fmt::Debug for Name { /// for (name, mut score) in &mut scores { /// score.0 += 1.0; /// if score.0.is_nan() { -/// bevy_utils::tracing::error!("Score for {:?} is invalid", name); +/// // use the Display impl to return either the Name, +/// // or Entity's Display impl for entities which don't +/// // have one. +/// // You can still use the Debug impl it is just quite verbose. +/// bevy_utils::tracing::error!("Score for {} is invalid", name); /// } /// } /// } /// # bevy_ecs::system::assert_is_system(increment_score); /// ``` #[derive(QueryData)] +#[query_data(derive(Debug))] pub struct DebugName { /// A [`Name`] that the entity might have that is displayed if available. pub name: Option<&'static Name>, @@ -114,12 +119,12 @@ pub struct DebugName { pub entity: Entity, } -impl<'a> std::fmt::Debug for DebugNameItem<'a> { +impl<'a> std::fmt::Display for DebugNameItem<'a> { #[inline(always)] fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { match self.name { - Some(name) => write!(f, "{:?} ({:?})", &name, &self.entity), - None => std::fmt::Debug::fmt(&self.entity, f), + Some(name) => std::fmt::Display::fmt(name, f), + None => std::fmt::Display::fmt(&self.entity, f), } } } @@ -198,3 +203,24 @@ impl Deref for Name { self.name.as_ref() } } + +#[cfg(test)] +mod tests { + use super::*; + use bevy_ecs::world::World; + + #[test] + fn test_display_of_debug_name() { + let mut world = World::new(); + let e1 = world.spawn_empty().id(); + let name = Name::new("MyName"); + let e2 = world.spawn(name.clone()).id(); + let mut query = world.query::(); + let d1 = query.get(&world, e1).unwrap(); + let d2 = query.get(&world, e2).unwrap(); + // DebugName Display for entities without a Name should be Display impl of the Entity + assert_eq!(d1.to_string(), format!("{e1}")); + // DebugName Display for entiites with a Name should be the Name + assert_eq!(d2.to_string(), "MyName"); + } +} From 38d14fa02cf89efa8b847f5384a47220088abaf9 Mon Sep 17 00:00:00 2001 From: John Payne <20407779+johngpayne@users.noreply.github.com> Date: Sat, 8 Jun 2024 22:48:50 +0100 Subject: [PATCH 2/8] Returned Display for Entity to just {index}v{generation} as the bits part was redundant (it was generation << 32 | index). This might be contraversial and isn't required for the DebugName change. --- crates/bevy_ecs/src/entity/mod.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index d104eb4777eb9..9af972172a0f3 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -392,13 +392,7 @@ impl<'de> Deserialize<'de> for Entity { impl fmt::Display for Entity { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "{}v{}|{}", - self.index(), - self.generation(), - self.to_bits() - ) + write!(f, "{}v{}", self.index(), self.generation()) } } @@ -1162,9 +1156,7 @@ mod tests { fn entity_display() { let entity = Entity::from_raw(42); let string = format!("{}", entity); - let bits = entity.to_bits().to_string(); assert!(string.contains("42")); assert!(string.contains("v1")); - assert!(string.contains(&bits)); } } From fac703e60264c61bbeeb01e9439e9d34d157e57e Mon Sep 17 00:00:00 2001 From: John Payne <20407779+johngpayne@users.noreply.github.com> Date: Sun, 9 Jun 2024 09:57:17 +0100 Subject: [PATCH 3/8] Revert "Returned Display for Entity to just {index}v{generation} as the bits part was redundant (it was generation << 32 | index). This might be contraversial and isn't required for the DebugName change." This reverts commit 38d14fa02cf89efa8b847f5384a47220088abaf9. --- crates/bevy_ecs/src/entity/mod.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 9af972172a0f3..d104eb4777eb9 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -392,7 +392,13 @@ impl<'de> Deserialize<'de> for Entity { impl fmt::Display for Entity { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}v{}", self.index(), self.generation()) + write!( + f, + "{}v{}|{}", + self.index(), + self.generation(), + self.to_bits() + ) } } @@ -1156,7 +1162,9 @@ mod tests { fn entity_display() { let entity = Entity::from_raw(42); let string = format!("{}", entity); + let bits = entity.to_bits().to_string(); assert!(string.contains("42")); assert!(string.contains("v1")); + assert!(string.contains(&bits)); } } From 3e02519bb303c335b16ed22ad4b4e918ff54add3 Mon Sep 17 00:00:00 2001 From: John Payne <20407779+johngpayne@users.noreply.github.com> Date: Sun, 9 Jun 2024 10:03:47 +0100 Subject: [PATCH 4/8] Use {index}v{generation} in the Display implementation of DebugName as the Display for entities is more verbose than this. --- crates/bevy_core/src/name.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/bevy_core/src/name.rs b/crates/bevy_core/src/name.rs index 3d3a5f7983eb4..f823cdbcbd22c 100644 --- a/crates/bevy_core/src/name.rs +++ b/crates/bevy_core/src/name.rs @@ -100,9 +100,9 @@ impl std::fmt::Debug for Name { /// for (name, mut score) in &mut scores { /// score.0 += 1.0; /// if score.0.is_nan() { -/// // use the Display impl to return either the Name, -/// // or Entity's Display impl for entities which don't -/// // have one. +/// // use the Display impl to return either the Name +/// // where there is one, or {index}v{generation} +/// // for entities which don't have a Name. /// // You can still use the Debug impl it is just quite verbose. /// bevy_utils::tracing::error!("Score for {} is invalid", name); /// } @@ -124,7 +124,7 @@ impl<'a> std::fmt::Display for DebugNameItem<'a> { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { match self.name { Some(name) => std::fmt::Display::fmt(name, f), - None => std::fmt::Display::fmt(&self.entity, f), + None => write!(f, "{}v{}", self.entity.index(), self.entity.generation()), } } } @@ -218,8 +218,8 @@ mod tests { let mut query = world.query::(); let d1 = query.get(&world, e1).unwrap(); let d2 = query.get(&world, e2).unwrap(); - // DebugName Display for entities without a Name should be Display impl of the Entity - assert_eq!(d1.to_string(), format!("{e1}")); + // DebugName Display for entities without a Name should be {index}v{generation} + assert_eq!(d1.to_string(), "0v1"); // DebugName Display for entiites with a Name should be the Name assert_eq!(d2.to_string(), "MyName"); } From e591f6bb43f4e8d093f7ff2196dbf61aaffdb7a0 Mon Sep 17 00:00:00 2001 From: John Payne <20407779+johngpayne@users.noreply.github.com> Date: Sun, 9 Jun 2024 10:06:50 +0100 Subject: [PATCH 5/8] cargo fmt --all --- crates/bevy_core/src/name.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_core/src/name.rs b/crates/bevy_core/src/name.rs index f823cdbcbd22c..899e148f8d9fb 100644 --- a/crates/bevy_core/src/name.rs +++ b/crates/bevy_core/src/name.rs @@ -101,7 +101,7 @@ impl std::fmt::Debug for Name { /// score.0 += 1.0; /// if score.0.is_nan() { /// // use the Display impl to return either the Name -/// // where there is one, or {index}v{generation} +/// // where there is one, or {index}v{generation} /// // for entities which don't have a Name. /// // You can still use the Debug impl it is just quite verbose. /// bevy_utils::tracing::error!("Score for {} is invalid", name); From d70135471666e8c1ceab2989d144ca1666b00aad Mon Sep 17 00:00:00 2001 From: Gagnus <20407779+gagnus@users.noreply.github.com> Date: Sat, 22 Jun 2024 10:48:18 +0100 Subject: [PATCH 6/8] Update crates/bevy_core/src/name.rs review comment Co-authored-by: Jan Hohenheim --- crates/bevy_core/src/name.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_core/src/name.rs b/crates/bevy_core/src/name.rs index 899e148f8d9fb..35ecebd9736a7 100644 --- a/crates/bevy_core/src/name.rs +++ b/crates/bevy_core/src/name.rs @@ -104,7 +104,7 @@ impl std::fmt::Debug for Name { /// // where there is one, or {index}v{generation} /// // for entities which don't have a Name. /// // You can still use the Debug impl it is just quite verbose. -/// bevy_utils::tracing::error!("Score for {} is invalid", name); +/// bevy_utils::tracing::error!("Score for {name} is invalid"); /// } /// } /// } From a7c199fd13b8f5e2900d8d102bae653f413db757 Mon Sep 17 00:00:00 2001 From: Gagnus <20407779+gagnus@users.noreply.github.com> Date: Sat, 22 Jun 2024 10:48:34 +0100 Subject: [PATCH 7/8] Update crates/bevy_core/src/name.rs review fix Co-authored-by: Andres O. Vela --- crates/bevy_core/src/name.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_core/src/name.rs b/crates/bevy_core/src/name.rs index 35ecebd9736a7..678efeb631e53 100644 --- a/crates/bevy_core/src/name.rs +++ b/crates/bevy_core/src/name.rs @@ -220,7 +220,7 @@ mod tests { let d2 = query.get(&world, e2).unwrap(); // DebugName Display for entities without a Name should be {index}v{generation} assert_eq!(d1.to_string(), "0v1"); - // DebugName Display for entiites with a Name should be the Name + // DebugName Display for entities with a Name should be the Name assert_eq!(d2.to_string(), "MyName"); } } From b3baea36c21f34c331d41b5465bbfe86e39c2fa3 Mon Sep 17 00:00:00 2001 From: John Payne <20407779+johngpayne@users.noreply.github.com> Date: Sat, 22 Jun 2024 10:54:01 +0100 Subject: [PATCH 8/8] Suggestion on better documentation --- crates/bevy_core/src/name.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/bevy_core/src/name.rs b/crates/bevy_core/src/name.rs index 678efeb631e53..c3012224ad795 100644 --- a/crates/bevy_core/src/name.rs +++ b/crates/bevy_core/src/name.rs @@ -100,16 +100,17 @@ impl std::fmt::Debug for Name { /// for (name, mut score) in &mut scores { /// score.0 += 1.0; /// if score.0.is_nan() { -/// // use the Display impl to return either the Name -/// // where there is one, or {index}v{generation} -/// // for entities which don't have a Name. -/// // You can still use the Debug impl it is just quite verbose. /// bevy_utils::tracing::error!("Score for {name} is invalid"); /// } /// } /// } /// # bevy_ecs::system::assert_is_system(increment_score); /// ``` +/// +/// # Implementation +/// +/// The `Display` impl for `DebugName` returns the `Name` where there is one +/// or {index}v{generation} for entities without one. #[derive(QueryData)] #[query_data(derive(Debug))] pub struct DebugName {