[Merged by Bors] - Convenience method for entity naming #7186
Conversation
There was a problem hiding this comment.
I think that only working on exactly Query<&'w Name> types is too limiting: generally I want to combine this with other filters.
First question is would this make more sense to make a method on Name itself, or on Option<Name>? Unfortunately won't work because you don't have access to the entity information...
What about a custom WorldQuery type (just use the derive macro) that you can stick in your queries then?
alice-i-cecile
added
A-ECS
I feel like it doesn't make a bunch of sense since I feel like it'd be more misleading for an entity to have a
I'm a bit confused by this, do you mean something like this? #[derive(WorldQuery)]
struct DebugInfo {
names: Query<&Name>,
transforms: Query<&Transform>,
}
impl DebugInfo {
pub fn fmt(&self, entity: Entity) -> Box<dyn Debug> {
if let Some(name) = self.names.get(entity) { return Box::new(name.as_str()); }
if let Some(transform) = self.transforms.get(entity) { ... }
Box::new(entity)
}
} |
|
I meant something like: #[derive(WorldQuery)
pub struct EntityDebugInfo {
name: Option<&Name>,
entity: Entity,
}
pub enum EntityIdentifier {
Named(String),
Nameless(Entity),
}
// This struct is autogenerated as a query item with the WorldQuery derive macro
impl EntityDebugInfoItem {
fn identifier(&self) -> EntityIdentifier {
match self.name {
Some(name) => EntityIdentifier::Name(name),
None => EntityIdentifier::Nameless(self.entity),
}
}
}This would allow you to toss this into a |
|
Ah I see, ya I agree fully actually, that way we can even just impl Debug and it'd be just |
|
@alice-i-cecile Should be more in that vein now, also simplified the code a good bit. |
crates/bevy_core/src/name.rs
Outdated
| #[inline(always)] | ||
| fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { | ||
| match self.name { | ||
| Some(name) => std::fmt::Debug::fmt(&name, f), |
There was a problem hiding this comment.
maybe it would be worth printing out the self.entity too since Name isnt required to be a unique identifier
| @@ -76,6 +78,25 @@ impl std::fmt::Display for Name { | |||
| } | |||
| } | |||
|
|
|||
| /// Convenient query for giving a human friendly name to an entity. | |||
There was a problem hiding this comment.
Can you add a no_run doc test here to demonstrate how this is used?
There was a problem hiding this comment.
Actually helped me catch that I implemented Debug on DebugName instead of DebugNameItem, should we keep it no_run?
There was a problem hiding this comment.
Nah, it's fine to let it run.
# Objective
- Trying to make it easier to have a more user friendly debugging name for when you want to print out an entity.
## Solution
- Add a new `WorldQuery` struct `DebugName` to format the `Name` if the entity has one, otherwise formats the `Entity` id.
This means we can do this and get more descriptive errors without much more effort:
```rust
fn my_system(moving: Query<(DebugName, &mut Position, &Velocity)>) {
for (name, mut position, velocity) in &mut moving {
position += velocity;
if position.is_nan() {
error!("{:?} has an invalid position state", name);
}
}
}
```
---
## Changelog
- Added `DebugName` world query for more human friendly debug names of entities.
bors
bot
changed the title
# Objective
- Trying to make it easier to have a more user friendly debugging name for when you want to print out an entity.
## Solution
- Add a new `WorldQuery` struct `DebugName` to format the `Name` if the entity has one, otherwise formats the `Entity` id.
This means we can do this and get more descriptive errors without much more effort:
```rust
fn my_system(moving: Query<(DebugName, &mut Position, &Velocity)>) {
for (name, mut position, velocity) in &mut moving {
position += velocity;
if position.is_nan() {
error!("{:?} has an invalid position state", name);
}
}
}
```
---
## Changelog
- Added `DebugName` world query for more human friendly debug names of entities.
# Objective This is just an oversight on my part when I implemented this in #7186, there isn't much reason to print out the hash of a `Name` like it does currently: ``` Name { hash: 1608798714325729304, name: "Suzanne" } (7v0) ``` ## Solution Instead it would be better if we just printed out the string like so: ``` "Suzanne" (7v0) ``` As it conveys all of the information in a less cluttered and immediately intuitive way which was the original purpose of `DebugName`. Which I also think translates to `Name` as well since I mostly see it as a thin wrapper around a string. --------- Co-authored-by: Carter Anderson <[email protected]>
Objective
Solution
WorldQuerystructDebugNameto format theNameif the entity has one, otherwise formats theEntityid.This means we can do this and get more descriptive errors without much more effort:
Changelog
DebugNameworld query for more human friendly debug names of entities.