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 of glam types serialization #683

Merged
merged 1 commit into from
Jul 17, 2024
Merged

Conversation

ihor-tarasov
Copy link
Contributor

No description provided.

@not-fl3
Copy link
Owner

not-fl3 commented Jan 8, 2024

Note that cargo's feature are additive - you can add glam/serde to your own project and all the dependencies in the depenency tree will get serde feature!

@iiYese
Copy link

iiYese commented Jul 16, 2024

Note that cargo's feature are additive - you can add glam/serde to your own project and all the dependencies in the depenency tree will get serde feature!

This relies on unification which sucks because you have to match the glam versions. The latest release of macroquad is like 8 versions behind the latest glam version & the main branch is 1 version behind. The only place you can get that information the dep tree. This should be merged. It's a very trivial change for much better DX.

@not-fl3
Copy link
Owner

not-fl3 commented Jul 16, 2024

If we merge it, we force every single macroquad user to depend on serde and compile serde, syn, proc_macro and whatnot all the time.

I we do not merge it, each user have a choice to enable glam's serialization and which serialization library to use with glam if they choose so.

Considering macroquad's priorities, here I choose slightly less usability for serde users over slower compile time for everyone else.

@not-fl3 not-fl3 closed this Jul 16, 2024
@iiYese
Copy link

iiYese commented Jul 17, 2024

If we merge it, we force every single macroquad user to depend on serde and compile serde, syn, proc_macro and whatnot all the time.

I we do not merge it, each user have a choice to enable glam's serialization and which serialization library to use with glam if they choose so.

Considering macroquad's priorities, here I choose slightly less usability for serde users over slower compile time for everyone else.

Where did you get this notion? The whole point of feature flags is conditional compilation. Unless you set it as a default feature which this change does not do there is no effect on compile times. You are also not just choosing "slightly less usability for serde users" you are choosing slightly less usability for a significant amount of current and potential macroquad users because those venn diagrams intersect.

@not-fl3
Copy link
Owner

not-fl3 commented Jul 17, 2024

Ohhh, I missread the PR 🤦 it really is not a deafault feature, I agree, there is no harm in having an extra feature. Sorry!

@not-fl3 not-fl3 reopened this Jul 17, 2024
@not-fl3 not-fl3 merged commit 60e75d2 into not-fl3:master Jul 17, 2024
6 of 12 checks passed
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.

3 participants