RollbackIdProvider needs a smarter solution

[gschup]

Is your feature request related to a problem? Please describe.
Currently, when the RollbackIdProvider reaches u32::MAX, it simply panics. Since no sensible application should have that many rollbackable entitites at the same time, there should be a smarter way to provide unique (within the rollback frame window) IDs for entities.

Additional context
The relevant code is found here.

[nicopap]

Why not use the Entity? Isn't it the perfect "unique identifier per entities"? Or am I missing something?

Is the rollback ID used as a mapping of entity->entity between different hosts? If so, I would discourage using RollbackIdProvider or warn users about entity mapping. Since a correct mapping of entities is easier when the id is derived from the actual identity of the spawned entity (if I spawn "wall3" manually, I know to give it a unique ID, while it seems non-trivial in case where I use an incremental counter).

[gschup]

When entities are respawned or deleted after a rollback (and newly created when replaying frames after a rollback), they will have a different Entity component compared to before, making impossible to keep track of them when having to rollback again.

For this, another id is necessary that is able to track entities over time. All of this is handled in the plugin, but the solution of handing out the networking Ids is very minimal currently.

The RollbackId is not meant to be consistent between clients.

[nicopap]

Thank you for the quick answer. I was looking at a way to get a unique id for the Rollback component in an immutable context. I ended up asking myself "how to get a unique id per entity?" And I had to ask here what were the design consideration for the Rollback id, since the question had "use Entity" as an obvious answer.

[zicklag]

Maybe we could use Bevy's Entities allocator. it's essentially there to solve a similar problem of needing to be able to allocate unique indexes for items, while still being able to free and re-use them later to avoid running out of u32's.

---

Side question: if the entities are different while rolling back and forward, does that mean I can't depend on entities uniquely representing my entities if I use this plugin?

For instance, if I had an Inventory component that was a struct Inventory(Vec<Entity>), would the Entity's stored in that component become invalidated after rolling back and forth? Or not really, because if I sync the Inventory component, as well as the items in it, then the that would follow the rewind and fast-forward, so if they ended up with new Entities in a replay, then they Inventory would also be updated accordingly.

( Yeah, I'm thinking that's fine after typing that out... )

[gschup]

To answer your side question:

GGRS rewinds by loading an old state and overwriting current values. When loading a snapshot, items that have been deleted will have to be recreated and thus get a new entity id. In the same time, your inventory will be overwritten with the entity id vector it had at that point in time. These IDs will then probably be invalid.

[bushrat011899]

Since a Bevy Entity contains a generational index, Bevy already guarantees it will never re-issue an Entity such that two independent entities have equal ID's. Because of that, why not use the Entity as a discriminant for the Rollback component, but hidden. With regards to this earlier comment, just don't use the Entity within a Rollback for anything other than the comparison and equality of two Rollback components.

Yes this means the Entity of an entity and the Entity within a Rollback might not be equal after a rollback, but I don't think that matters, since the integer currently used within Rollback isn't equal to the entity Entity either.

/// Add this component to all entities you want to be loaded/saved on rollback.
#[derive(Component, Hash, PartialEq, Eq, Clone, Copy, Debug)]
pub struct RollbackFlag(Entity);

/// An `EntityCommand` which adds a `RollbackFlag` component to an entity.
pub struct Rollback;

impl EntityCommand for Rollback {
    fn write(self, id: Entity, world: &mut World) {
        world.entity_mut(id).insert(RollbackFlag(id));
    }
}

/* snip */

app
  // ...
  .add_startup_system(|mut commands: Commands| {
      commands
          .spawn(TransformBundle::from_transform(Transform::from_xyz(
              1., 2., 3.,
          )))
          .add(Rollback);
  })
  // ...

This would defer the ID problem back to how Bevy itself handles running out of entity IDs, which seems like a more appropriate location to deal with this issue.

[gschup]

That sounds exactly like the smart solution this issue asked for!

[bushrat011899]

That sounds exactly like the smart solution this issue asked for!

Thanks! I've created a pull request with the change as I described above. Please feel free to make any changes you think are appropriate to better align with the rest of the project.