-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Move bevy_window, bevy_input_focus, custom_cursor features to alternate feature collections
#22488
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
base: main
Are you sure you want to change the base?
Conversation
|
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
alice-i-cecile
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent, thanks for opening this up for review. Looking at it more closely, some thoughts:
bevy_input_focusandcustom_cursordefinitely do not belong indefault_app: these are far from the "core pieces most apps need" (e.g. servers do not want this).common_apidoes not feel like the right home for these, based on the description of it as being centered around scenes.bevy_input_focusbelongs underui_apiinstead: this is fundamentally a UI feature, even if you can use it for other things.- I'm not sure where to put the windowing features (
bevy_window,custom_cursor): they feel like they belong in their own collection, which is relied on by the various*_bevy_renderfeature collections.
I'm not immediately sure what the right organization is, but hopefully these observations are a useful starting point for others.
bevy_window, bevy_input_focus, custom_cursor features to common_api collecitionbevy_window, bevy_input_focus, custom_cursor features to alternate feature collections
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
|
For For |
This is a more sensible home. Annoying that it's winit-specific, but we do what we can.
Hmmm... This is probably a better home for now at least; this will cause it to be enabled where we need it without the addition of another feature collection. We'll see what the other reviewers have to say. |
|
I'm getting compilation failures testing with |
|
|
Related: Cargo Feature Collections #21472
Context:
- Me, https://discord.com/channels/691052431525675048/692572690833473578/1460424003486224517
- Alice, https://discord.com/channels/691052431525675048/692572690833473578/1460429142376845404
Solution
bevy_window,bevy_input_focus,custom_cursorfeatures fromdefault_appcollection tocommon_api.Testing
Confirmed that
default_appcollection no longer introduces the crates to the dependency tree:I'm not sure how cross-cutting changes to features have to be since I haven't worked with such a complicated feature set before, so it's possible I've omitted a change to some place that needs to be updated. Please point it out to me, thank you.