fishfolk / bones

An easy-to-use game engine for making real games.
https://fishfolk.org/development/bones/introduction/
Other
212 stars 20 forks source link

fix: compiling with all features #360

Closed nelson137 closed 5 months ago

nelson137 commented 5 months ago

Fix an error cause by compiling with all features: cargo build --all-features.

Make the conditional compilation on the const BITSET_EXP declarations mutually exclusive so that any combination of the keysize* features only ever results in a single instance.

Note that the keysize16 declaration does not exclude all other keysizes since it is the default. If multiple keysize features are enabled, including 16, then the declaration for keysize16 will be the only one. If multiple keysize features are enabled, not including 16, then there will be no declarations, causing a compile error. This is fine since users should only enable one of these features at a time.

error[E0428]: the name `BITSET_EXP` is defined multiple times
  --> framework_crates/bones_ecs/src/bitset.rs:18:1
   |
16 | const BITSET_EXP: u32 = 16;
   | --------------------------- previous definition of the value `BITSET_EXP` here
17 | #[cfg(feature = "keysize20")]
18 | const BITSET_EXP: u32 = 20;
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `BITSET_EXP` redefined here
   |
   = note: `BITSET_EXP` must be defined only once in the value namespace of this module

error[E0428]: the name `BITSET_EXP` is defined multiple times
  --> framework_crates/bones_ecs/src/bitset.rs:20:1
   |
16 | const BITSET_EXP: u32 = 16;
   | --------------------------- previous definition of the value `BITSET_EXP` here
...
20 | const BITSET_EXP: u32 = 24;
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `BITSET_EXP` redefined here
   |
   = note: `BITSET_EXP` must be defined only once in the value namespace of this module

error[E0428]: the name `BITSET_EXP` is defined multiple times
  --> framework_crates/bones_ecs/src/bitset.rs:22:1
   |
16 | const BITSET_EXP: u32 = 16;
   | --------------------------- previous definition of the value `BITSET_EXP` here
...
22 | const BITSET_EXP: u32 = 32;
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `BITSET_EXP` redefined here
   |
   = note: `BITSET_EXP` must be defined only once in the value namespace of this module
MaxCWhitehead commented 5 months ago

This sounds alright to me, I think being able to compile with all features might be helpful to catch issues. @zicklag any thoughts / concerns?

I wish there was a cleaner way of exposing this constant to crate user instead of feature flags, but I couldn't think of anything (or anything that sounds better lol).

zicklag commented 5 months ago

This looks good to me. 👍️ In the long term I think we're going to switch to Roaring ( compressed ) bitmaps, which can have better performance and don't need to be configured for memory usage.

nelson137 commented 5 months ago

@zicklag do you mind creating an issue for that work?

zicklag commented 5 months ago

@nelson137 Here's an issue for the bitmap: https://github.com/fishfolk/bones/issues/363.