Ralith / hecs

A handy ECS
Apache License 2.0
967 stars 82 forks source link

Implementation of `Ord` on `Entity` #317

Closed LPGhatguy closed 6 months ago

LPGhatguy commented 1 year ago

Currently, the Ord implementation on Entity behaves the same way as the to_bits representation of the entity: sorted by generation first, then by ID. That parity is nice, but I am unsure if this is the right default order.

I think it might make sense to sort entities by their ID first and then their generation. This makes the behavior of data structures like BTreeMap with entity keys behave more deterministically.

In our game, we have a WireNetwork component that holds a BTreeMap<EntityHandle, Connections>, where EntityHandle is a new-typed wrapper around Entity with custom serialization behavior. We were deriving Ord on that type, but I was surprised when the entries in the map would get shuffled around. This ended up being due to generations sometimes being different between runs, reordering the map!

We were able to solve that issue by manually implementing Ord to compare the entity's ID first. Doing that made me wonder whether the same change would be right for hecs itself.

Ralith commented 1 year ago

That sounds like a reasonable change to me! I wonder if we should flip the bit representation around to match?

Ralith commented 6 months ago

Fixed by #357.