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

Use mint types in public API #18

Merged
merged 3 commits into from
Feb 25, 2024
Merged

Use mint types in public API #18

merged 3 commits into from
Feb 25, 2024

Conversation

junglie85
Copy link
Contributor

Implements #16. It still leaves the conversion between types looking a bit messy in the examples. Happy to discuss and see if we can rework this. I think it would be neater if macroquad exposed a similar mint feature.

@h3r2tic
Copy link
Owner

h3r2tic commented Jun 18, 2023

Apologies for the delay, had some busy times lately 😅

This looks great! Thank you for the hard work 💗 I've just tested your branch in Tiny Glade, and it only took a few changes in our 400loc camera module. All pretty much a handful of .into()s and temporary bindings for where we were previously using &mut to mutate some internal driver vectors.

Not too annoying overall, and it could be a win for folks trying to use other math libs.

I'm not sure whether this will actually help with the glam semver bounds -- in Tiny Glade we're actively combing our crates, and duplicates are denied via cargo-deny except where our hands are tied. For the sake of reducing crate duplicates, we'll probably still need to bump dolly for new versions of glam, but using mint should allow users to be lazier with their dependency combing, as they will be able to upgrade their glam separately, and not have to change all the code around dolly.

So considering all this, it's still probably a good idea to merge this PR -- and if it backfires, we'll re-evaluate 🙂

@h3r2tic
Copy link
Owner

h3r2tic commented Jun 18, 2023

I've cleaned up a Clippy warning, and also changed a few mint::Vector3 to be mint::Point3, as that might be relevant to math libs which have separate types for points and vectors.

@kanerogers
Copy link

Any updates on this one? :)

@junglie85
Copy link
Contributor Author

@h3r2tic I'm cleaning up my account and would like to delete my fork. Do you plan to merge these changes?

@h3r2tic
Copy link
Owner

h3r2tic commented Feb 18, 2024

I think I was initially waiting to see if you'd have any comments on my changes, but I guess I didn't indicate that :P Then it slipped under my radar - sorry about that. Will allocate some time this week to finalize this! 💚

@h3r2tic h3r2tic merged commit 48d621c into h3r2tic:main Feb 25, 2024
5 checks passed
@h3r2tic
Copy link
Owner

h3r2tic commented Feb 25, 2024

This is now released in 0.5.0 🚀 I've also done a pass to check the examples in the README, and add tests for them.

Thanks for your contribution, and once again apologies for the delay 🖖

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants