Skip to content

Commit

Permalink
Fix inconsistency between Debug and serialized representation of Enti…
Browse files Browse the repository at this point in the history
…ty (#12469)

# Objective

Fixes #12139 

## Solution

- Derive `Debug` impl for `Entity`
- Add impl `Display` for `Entity`
- Add `entity_display` test to check the output contains all required
info

I decided to go with `0v0|1234` format as opposed to the `0v0[1234]`
which was initially discussed in the issue.

My rationale for this is that `[1234]` may be confused for index values,
which may be common in logs, and so searching for entities by text would
become harder. I figured `|1234` would help the entity IDs stand out
more.

Additionally, I'm a little concerned that this change is gonna break
existing logging for projects because `Debug` is now going to be a
multi-line output. But maybe this is ok.

We could implement `Debug` to be a single-line output, but then I don't
see why it would be different from `Display` at all.

@alice-i-cecile Let me know if we'd like to make any changes based on
these points.
  • Loading branch information
Zeenobit authored Mar 14, 2024
1 parent 325f0fd commit 7d816aa
Showing 1 changed file with 19 additions and 3 deletions.
22 changes: 19 additions & 3 deletions crates/bevy_ecs/src/entity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ type IdCursor = isize;
/// [`Query::get`]: crate::system::Query::get
/// [`World`]: crate::world::World
/// [SemVer]: https://semver.org/
#[derive(Clone, Copy)]
#[derive(Clone, Copy, Debug)]
#[cfg_attr(feature = "bevy_reflect", derive(Reflect))]
#[cfg_attr(
feature = "bevy_reflect",
Expand Down Expand Up @@ -384,9 +384,15 @@ impl<'de> Deserialize<'de> for Entity {
}
}

impl fmt::Debug 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()
)
}
}

Expand Down Expand Up @@ -1147,4 +1153,14 @@ mod tests {
assert_ne!(hash, first_hash);
}
}

#[test]
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));
}
}

0 comments on commit 7d816aa

Please sign in to comment.