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

Add feature to disable egui builtin fonts #110

Merged
merged 2 commits into from
Jul 30, 2022
Merged

Conversation

iTitus
Copy link
Contributor

@iTitus iTitus commented Jul 2, 2022

Closes #107

@mvlabat
Copy link
Owner

mvlabat commented Jul 17, 2022

Hey! Thanks for the PR. Do you think it makes sense to wrap the rest of the features the same way? I'm not really sure how libraries usually approach that. Users can as well add needed features by specifying egui as a direct dependency and adding them there. But I don't know whether it's a good UX.

@iTitus
Copy link
Contributor Author

iTitus commented Jul 17, 2022

This is only for features in egui that one wants to disable.
By disabling all default features and putting them behind our default features we give the user a way to choose.

One can always add features, but removing is impossible.

@mvlabat
Copy link
Owner

mvlabat commented Jul 17, 2022

Yeah, with that I absolutely agree. bevy_egui users should have a way to run egui without default features.
I was wondering specifically about non-default features, whether it's a common practice to wrap them as well.

As for this feature, I'll merge it as soon as I start preparing the new release for Bevy 0.8.

@iTitus
Copy link
Contributor Author

iTitus commented Jul 17, 2022

I think non-default features only get wrapped if the package makes explicit use of them.

@mvlabat mvlabat merged commit 2669599 into mvlabat:main Jul 30, 2022
@iTitus iTitus deleted the patch-1 branch July 30, 2022 22:31
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.

Add feature to disable egui builtin fonts
2 participants