bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Togglable bit flag for Identifier

Open Bluefinger opened this issue 1 year ago • 3 comments

Objective

  • Provide the building block to enable togglable components using IDs.
  • Add another piece to the relations story.

Solution

This PR focuses on adding to the Identifier spec/type the ability to set and modify the IS_TOGGLABLE flag bit, for the primary purpose of building out the ability to "toggle" components assigned to a given entity (once they too are back by Identifiers/Entities). The PR does NOT include wiring this in as a full feature, but is instead focusing on just the Identifier bits.

The ability to toggle components on/off on an Entity allows the ability to have "temporary" removals instead of full removal, which would necessitate an archetype move. However, checking whether each component is toggled in a query would kill query perf, so this has to be a strictly opt-in feature. With this, a component that is set to be togglable would exist in an archetype for such, and would have a bitset to check against. And since the ID's are different, plain versions of that component would be distinguishable from togglable versions, so the checking of the bitset only applies to those that have been set to be such.

Note: Adding an additional flag to Identifier means that available generation space for Identifier/Entity is now 2^30 - 1, or 1,073,741,823 generations. The flags occupy the two most significant bits on the Identifier's high component. Due to the increasing complexity of managing the bit flags, I've included bitflags crate to wire some of the logic in and also to enable const-able operations.

The generation increment method has been modified to account for this, and also incorporates a micro-optimisation that seems only to be triggered for 30-bit values and less, as the same code for a 31-bit value resolves to the same ASM output. The optimisation now allows for one less instruction being emitted: https://rust.godbolt.org/z/vnnhr5cod

inc_entity_generation_by:
        and     edi, 1073741823
        and     esi, 1073741823
        lea     eax, [rsi + rdi]
        cmp     eax, 1073741824
        sbb     eax, -1
        and     eax, 1073741823
        ret

Some other optimisations include removing panic! paths from methods that will never panic, and preventing the inlining of panic pathways as a further optimisation for others (if something is panicking, it doesn't matter if that path is "slow").


Changelog

Added

  • Togglable bit flag for Identifier.

Changed

  • Micro-optimisations for Identifier/Entity methods

Migration Guide

Entity/Identifier layout has changed, so that the flags occupy 2 bits and the high value is now only 30 bits, so all code/assets relying on specific layout of Entity will need to be migrated/updated to the new layout.

Bluefinger avatar Jan 22 '24 14:01 Bluefinger

This is a building block for https://github.com/bevyengine/bevy/issues/11090, correct? Can you say more about how this builds towards relations (#3742)?

alice-i-cecile avatar Mar 17 '24 15:03 alice-i-cecile

I'm having some difficulties understanding the semantics here. If an entity is togglable (once we've wired this up), what does that mean? Should this be on/off or enabled/disabled instead?

The "hidden" naming was much clearer to me, but I agree that we shouldn't use that naming: it's very confusing when rendering visibility enters play.

alice-i-cecile avatar Mar 17 '24 15:03 alice-i-cecile

This is a building block for #11090, correct? Can you say more about how this builds towards relations (#3742)?

After some discussions on Discord, I think it became clearer that for entity disabling, that requires a different mechanism using a marker component. This PR would be more useful for component disabling, but being able to apply it conditionally so that only components that have their ID with the bit set would have their bitset checked for being disabled or not. A similar mechanism could be used to do opt-in change detection, though that probably needs another bit reserved (or maybe reusing this bit, but that I think @maniwani has a better idea on this). Overall, this would allow some features to enable/disable components on entities (useful for syncing/rollback/etc for networking and other uses).

Bluefinger avatar Mar 17 '24 16:03 Bluefinger