Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support mint #859

Open
parasyte opened this issue Feb 11, 2022 · 4 comments
Open

Support mint #859

parasyte opened this issue Feb 11, 2022 · 4 comments
Labels
c: core Component: core (mod core_types, object, log, init, ...) feature Adds functionality to the library good first issue
Milestone

Comments

@parasyte
Copy link
Contributor

I searched existing issues for mentions of mint and only found #594 and #810.

glam is used internally which is kind of nice, but I find myself wishing I could take advantage of it more often than not. Today I nearly wrote my own trait to clamp a Vector3. If mint support was available, I could use that to do the clamping et al. with glam or any other crate I prefer.

(In the end, I decided to normalize and scale the vector after checking its length squared. But I think having the option to use mint might still be useful in other situations.)

Potential drawbacks and alternatives

As mentioned in #810, using mint as a public export will increase the MSRV to 1.52.1 (It is currently 1.51 as of #833.)

An alternative to mint would be making Vector3::glam and friends public at the cost of also increasing the MSRV to 1.52.1 without the direct benefits of mint. Implementing Deref to and from the glam types would be practically the same thing with just a different interface.

Either of these will also re-raise the concern from #594 of requiring version bumps for the public crate exports.

Another alternative would be writing a whole bunch of instance methods on the core types to bring its feature set closer to glam. This would be a divergence from the Godot API for these classes.

@parasyte parasyte added the feature Adds functionality to the library label Feb 11, 2022
@Bromeon Bromeon added the c: core Component: core (mod core_types, object, log, init, ...) label Feb 11, 2022
@Bromeon Bromeon added this to the v0.10.1 milestone Feb 11, 2022
@Bromeon
Copy link
Member

Bromeon commented Feb 11, 2022

Thanks for bringing this up! Should have created a dedicated issue about it a while ago.

We could maybe add mint support in 0.10.1 (not the initial release) or later, possibly behind a mint feature gate. The version issues (MSRV + public-facing API) are something to consider indeed.

Out of curiosity, what are the glam features you're currently missing in the godot-rust vector types? You mentioned clamp, anything else?

@parasyte
Copy link
Contributor Author

Out of curiosity, what are the glam features you're currently missing in the godot-rust vector types? You mentioned clamp, anything else?

Good question. Since I'm not even using the glam methods right now, I don't have a great answer. Looking over the docs, I could see these additional methods being useful in some situations:

  • min
  • max
  • fract
  • exp
  • powf
  • recip

And in general, it has some nice trait implementations like Product, Sum, Rem, and RemAssign. I don't know if the last two ops were just overlooked, or if Godot just doesn't offer anything similar for vector modulus.

Beyond those, there are all of the swizzles! Which are really common in shader code and have some important uses in mesh generation, noise, and other applications.

@Bromeon
Copy link
Member

Bromeon commented Feb 12, 2022

One thing that makes glam a bit difficult to expose is that it keeps releasing semver-breaking versions very frequently. It already is at v0.20 and seems to have published a new minor version almost every month last year. That of course is also nice in terms of fast iteration and progress, but for the problems we've faced in the past it's not ideal.

On the other hand, mint seems considerably more stable, being at 0.5.9 since 2017. However, at a closer look it also occasionally introduces breaking changes in patch versions (e.g. layout change for #[repr(C)] in kvark/mint@8e33991, or increase of edition in kvark/mint@7a90a79). These are minor things though and likely don't affect most users.

@parasyte
Copy link
Contributor Author

parasyte commented Feb 12, 2022

I've had a few semver breaks in dependencies affect pixels, so I understand the concern. Mostly these have been pushing my MSRV which is not ideal, but I don't know how it impacts users in practice. (I almost always use a recent nightly compiler in my personal projects. Mostly out of habit, but also because I actually like using recently stabilized features when they are available.)

For libraries, it is completely desirable to maintain compatibility with older compilers as much as possible.

On the other hand, mint seems considerably more stable, being at 0.5.9 since 2017.

That's a nice bonus! If it were my call, I would lean toward mint over making glam public. Users can opt-in to use glam if they want, and mint should make Vector3 compatible with any glam type, regardless of which version of glam the user chooses to depend on. The other benefit IMHO is that the user could pick a completely different linear algebra library like ultraviolet and use that instead.

@Bromeon Bromeon modified the milestones: v0.10.1, v0.10.2 Jul 16, 2022
@Bromeon Bromeon modified the milestones: v0.10.2, v0.11.x Oct 1, 2022
@chitoyuu chitoyuu modified the milestones: v0.11.x, v0.11.1 Dec 6, 2022
@chitoyuu chitoyuu modified the milestones: v0.11.x, v0.12.0 Jan 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: core Component: core (mod core_types, object, log, init, ...) feature Adds functionality to the library good first issue
Projects
None yet
Development

No branches or pull requests

3 participants