mikktspace icon indicating copy to clipboard operation
mikktspace copied to clipboard

Enabling multiple features should not cause compilation errors

Open LPGhatguy opened this issue 2 years ago • 3 comments

Currently, mikktspace has an nalgebra feature (enabled by default) and a glam feature. This is discouraged as it means two libraries each depending on mikktspace might be incompatible with each other with no way for a user to fix it.

We ran into that incompatibility in our game, where the main game was depending on mikktspace with

mikktspace = { version = "0.3.0", default-features = false, features = ["glam"] }

but a crate that we depended on used

mikktspace = "0.3.0"

...causing a compilation error in mikktspace.

There are two pretty good approaches to solve this issue generally:

  1. Pick a single math library to use internally. This can help with maintainability. Math libraries, especially glam, aren't too heavy on compilation times so this might not be so bad.
  2. Inline the math functions into the crate. It looks like #23 is a step in that direction.

LPGhatguy avatar Apr 20 '23 03:04 LPGhatguy

Note that nalgebra is pretty heavy, so it probably shouldn't be used internally. It's also important not to have a math library in the public API, since they tend to break frequently and hence pose a major semver compatibility hazard.

Ralith avatar Jul 07 '23 00:07 Ralith

It's also important not to have a math library in the public API

I agree. If I've understood correctly, this is really the root cause of the issue. I suggest we use a neutral vector format for input and output instead.

The heavy library dependency is heavy but it's a least dependable. I'm beginning to grow tired of discussing which math libraries should and should not be used at this point.

alteous avatar Jul 12 '23 10:07 alteous

Yeah, the public API issue is by far the most important.

Ralith avatar Jul 12 '23 17:07 Ralith