-
-
Notifications
You must be signed in to change notification settings - Fork 3
Android support #11
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?
Android support #11
Conversation
|
Thats awesome! Can you add at least a compile check for android into the CI aswell? |
|
@Leinnan Nice the CI passes. Thank you! I cannot test this due to a lack of android in my life. Give me a final confirmation that this is good to go and tested? |
| /// ``` | ||
| #[derive(Resource, Clone, Debug, Default, Reflect)] | ||
| #[reflect(Resource)] | ||
| pub struct IosSafeAreaResource { |
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.
IosSafeAreaResource should be renamed SafeAreaResource then ; and crate renamed ?
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.
@ThierryBerger I dont think crates.io allows a rename, no?
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.
yeah, probably spawn a new crate then..?
Another option is to create a "parent" crate, but I'm not sure maintenance is worth it (or include them in the same repository and it may be ok?)
- window_safe_area
- ios_safe_area
- android_safe_area
- windows_safe_area -> yeah naming is hard ok
(I also think such crate would be more useful if it could be not tied to bevy, that may be another subject, but if all we need is a window handle it makes sense, that would mean a new crate bevy_window_safe_area or an exposed feature...)
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.
@Leinnan how about we make it a workspace with three crates: bevy_ios_safearea, bevy_android_safearea (no ties to the former) and then an abstracting one over both that supports either bevy_device_safearea? This way we can rename the GitHub repo and do not need to rename crates but just add two
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.
@extrawurst I think one package is more than enough for it, but I leave it up to you
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.
the dependency hell alone makes me want these in two separate crates
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.
we could argue to start with bevy_android_safearea before doing the combined one right away
Adds support for updating the Safe area on Android as well.