Skip to content

Commit

Permalink
bevy_ecs: Special-case Entity::PLACEHOLDER formatting (#15839)
Browse files Browse the repository at this point in the history
# Objective

Oftentimes, users will store an entity on a component or resource. To
make this component/resource `Default`-able, they might initialize it
with `Entity::PLACEHOLDER`. This is sometimes done to avoid the need for
an `Option<Entity>`, especially if it complicates other logic.

For example, it's used in this `Selection` resource to denote "no
selection":

```rust
#[derive(Resource, Debug)]
struct Selection(Entity);

impl Default for Selection {
    fn default() -> Self {
        Self(Entity::PLACEHOLDER)
    }
}
```

The problem is that if we try to `Debug` the current `Selection`, we get
back: `4294967295v1#8589934591`. It's not immediately obvious whether or
not the entity is an actual entity or the placeholder.

Now while it doesn't take long to realize that this is in fact just the
value of `Entity::PLACEHOLDER`, it would be a lot clearer if this was
made explicit, especially for these particular use cases.

## Solution

This PR makes the `Debug` and `Display` impls for `Entity` return
`PLACEHOLDER` for the `Entity::PLACEHOLDER` constant.

~~Feel free to bikeshed the actual value returned here. I think
`PLACEHOLDER` on its own could work too.~~ Swapped to `PLACEHOLDER` from
`Entity::PLACEHOLDER`.

## Testing

You can test locally by running:

```
cargo test --package bevy_ecs
```

---

## Migration Guide

The `Debug` and `Display` impls for `Entity` now return `PLACEHOLDER`
for the `Entity::PLACEHOLDER` constant. If you had any code relying on
these values, you may need to account for this change.
  • Loading branch information
MrGVSV authored Oct 11, 2024
1 parent 5def6f2 commit da4e776
Showing 1 changed file with 30 additions and 13 deletions.
43 changes: 30 additions & 13 deletions crates/bevy_ecs/src/entity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,8 @@ impl<'de> Deserialize<'de> for Entity {
///
/// This takes the format: `{index}v{generation}#{bits}`.
///
/// For [`Entity::PLACEHOLDER`], this outputs `PLACEHOLDER`.
///
/// # Usage
///
/// Prefer to use this format for debugging and logging purposes. Because the output contains
Expand All @@ -416,22 +418,32 @@ impl<'de> Deserialize<'de> for Entity {
/// ```
impl fmt::Debug for Entity {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(
f,
"{}v{}#{}",
self.index(),
self.generation(),
self.to_bits()
)
if self == &Self::PLACEHOLDER {
write!(f, "PLACEHOLDER")
} else {
write!(
f,
"{}v{}#{}",
self.index(),
self.generation(),
self.to_bits()
)
}
}
}

/// Outputs the short entity identifier, including the index and generation.
///
/// This takes the format: `{index}v{generation}`.
///
/// For [`Entity::PLACEHOLDER`], this outputs `PLACEHOLDER`.
impl fmt::Display for Entity {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}v{}", self.index(), self.generation())
if self == &Self::PLACEHOLDER {
write!(f, "PLACEHOLDER")
} else {
write!(f, "{}v{}", self.index(), self.generation())
}
}
}

Expand Down Expand Up @@ -1195,16 +1207,21 @@ mod tests {
fn entity_debug() {
let entity = Entity::from_raw(42);
let string = format!("{:?}", entity);
assert!(string.contains("42"));
assert!(string.contains("v1"));
assert!(string.contains(format!("#{}", entity.to_bits()).as_str()));
assert_eq!(string, "42v1#4294967338");

let entity = Entity::PLACEHOLDER;
let string = format!("{:?}", entity);
assert_eq!(string, "PLACEHOLDER");
}

#[test]
fn entity_display() {
let entity = Entity::from_raw(42);
let string = format!("{}", entity);
assert!(string.contains("42"));
assert!(string.contains("v1"));
assert_eq!(string, "42v1");

let entity = Entity::PLACEHOLDER;
let string = format!("{}", entity);
assert_eq!(string, "PLACEHOLDER");
}
}

0 comments on commit da4e776

Please sign in to comment.