Adding a World ID to `Entity`

LPGhatguy commented 1 year ago

Currently, Entity values are not tied to any specific World. This has benefits like supporting straightforward serialization.

It could be useful if Entity were given a world ID field, expanding it from

pub struct Entity {
    generation: NonZeroU32,
    id: u32,
}

to

pub struct Entity {
    generation: NonZeroU32,
    id: u32,
    world_id: u32,
}

This world ID would ensure that Entity values from one World would never be valid when used with another World.

Rationale

Recently, we uncovered a data loss bug in our game editor related to our hecs World being recreated. It went something like this:

The user opens a scene and selects an object. This spawns some editor gizmos in the world and stores a handful of Entity values to reference them.
The user opens a different scene. This creates a new World. The editor is still holding onto the Entity values from the previous scene.
The user selects a new object, causing the editor gizmos to be recreated. This deletes some of the entities in the current scene.

After finding this issue, I found that a number of our game systems occasionally store Entity values from a previous World, and then get confused when those handles now point to completely different objects.

We can patch issues like that individually, usually by flushing any stored state whenever we're also changing worlds. However, I think it'd be nice to have a holistic solution to the problem. I came to the conclusion that we had two decent options besides not storing Entity values:

Only ever create one hecs::World so that duplicate Entity values are never created.
Give hecs::Entity a World ID so that handles created from other worlds are invalid.

Option 1 is feasible, but complicates our serialization/deserialization code a bit. Additionally, while we don't currently ever have more than one World at a time, I think it would be a shame to fuse off that functionality.

I tried to prototype option 2 by wrapping hecs::Entity and all related APIs externally, but that turned into quite a mess.

I thought it would be a good idea to open this issue to get another read on this problem. Adding more data to Entity is likely overkill for a lot of use cases, but does eliminate issues like this very cleanly.

Pros

Entity handles from other worlds or that are stale will never access a new entity
Game code working with multiple worlds can store a single HashMap<Entity, T> for keeping extra state without mixing state between worlds

Cons

Entity becomes 12 bytes
World needs some way of getting a unique ID
Serializing Entity now includes extra data

Alternatives

Users can wrap Entity themselves and insert extra checks
Users can use a single World only
Users can avoid storing Entity references at all, instead using marker components and queries
hecs could expose an API to let multiple World objects share an Entity generation allocator

Ralith commented 1 year ago

This is an instance of a very general problem, and I'm not sure it makes sense for hecs to try to come up with magic to solve it. Is there a way you could wrap hecs::Entity for these uses without also wrapping many APIs? Maybe something like:

#[derive(Debug, Copy, Clone, Eq, PartialEq)]
struct UniversalEntity {
    world_id: u64,
    entity: hecs::Entity,
}

impl UniversalEntity {
    pub fn to_entity(&self, world_id: u64) -> Option<hecs::Entity> {
        (self.world_id == world_id).then_some(*self);
    }
}

Ensuring that Entity values stored outside of the World use UniversalEntity should be easy compared to never having such bugs to begin with, while not adding large amounts of maintenance load or complicating hecs itself.

adamreichold commented 1 year ago

As a little help, the existing World::id field could be made available since hecs already manages it?

Ralith / hecs