-
Notifications
You must be signed in to change notification settings - Fork 25
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 additional export options for Meta Quest features #68
Conversation
…keyboard, boundary mode
// Check for overlay keyboard | ||
bool _use_overlay_keyboard_option = _get_bool_option("meta_xr_features/use_overlay_keyboard"); | ||
if (_use_overlay_keyboard_option) { | ||
contents += " <uses-feature android:name=\"oculus.software.overlay_keyboard\" android:required=\"true\" />\n"; |
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.
Didn't realize the overlay keyboard required a feature
tag, I thought it was shown automatically.
Were you able to test if it shows when using Godot's text edit nodes with this feature
tag added?
Also, should we include the ability to make the feature optional with required=false
?
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.
Just to share my experience using this feature, I use the following feature tag (hard-coded in my AndroidManifest.xml):
<uses-feature android:name="oculus.software.overlay_keyboard" android:required="false"/>
I can confirm that it's shown automatically when using Godot's text edit nodes.
contents += " <uses-feature tools:node=\"replace\" android:name=\"com.oculus.feature.BOUNDARYLESS_APP\" android:required=\"true\" />\n"; | ||
} else if (boundary_mode == BOUNDARY_CONTEXTUAL_VALUE) { | ||
contents += " <uses-feature tools:node=\"replace\" android:name=\"com.oculus.feature.CONTEXTUAL_BOUNDARYLESS_APP\" android:required=\"true\" />\n"; | ||
} |
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.
For these features, we should provide the option to set the required
attribute to false
.
// Check for scene api | ||
bool use_scene_api = _get_bool_option("meta_xr_features/use_scene_api"); | ||
if (use_scene_api) { | ||
contents += " <uses-permission android:name=\"com.oculus.permission.USE_SCENE\" />\n"; |
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.
This should be moved within the #66 PR so we can keep all related changes together.
Also do we need additional logic to request the permission at runtime?
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.
When testing, I see the permission dialog automatically appears at launch. Not quite sure why though...
-
Does Godot automatically request all dangerous permissions an app requires at launch, or is that the app's responsibility (i.e. in context permissions)? Ideally we do the second and add an API to request via the plugin, if Godot isn't already handling this.
-
On Quest 2 and earlier this is declared as a "normal" permission, on Quest Pro and later it's a "dangerous" permission. There's some compat mechanism that either auto-grants or auto-requests for older apps. This could be kicking in if it's not (1), and if so I'd need to figure out how the system determines that the compat mechanism is needed for a given app and how to opt out so we can just request in context.
Will look through engine / plugin source for answers to (1) unless you know off the top of your head, and follow up on (2) when I'm back at the office
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.
For (1), yes dangerous permissions are automatically requested on start.
See https://github.com/godotengine/godot/blob/master/platform%2Fandroid%2Fjava%2Flib%2Fsrc%2Forg%2Fgodotengine%2Fgodot%2FGodotActivity.kt#L68
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.
Awesome, in that case I'll move just the Scene permission to #66 to keep all that together and call it done since the permission is already requested appropriately, and address remaining comments here.
Separate thought related to permissions: I see from the code pointer that "request at launch" isn't the only option:
// We exclude certain permissions from the set we request at startup, as they'll be // requested on demand based on use-cases.
I see this list is hardcoded to just the microphone permission. It may be a good idea to define a "request" mode for all dangerous permissions that the developer can pick from:
- request at launch (when the game launches)
- request on demand (upon first access, if we can detect this automatically)
- manual (when the game chooses to in its game logic)
Manual would be used for "contextual" permissions, where you tell the user "hey we're gonna request a permission and use your data for XYZ, please accept the following prompt" and then request (increasingly common in Android apps, generally recommended for user education).
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, that'd be a good improvement to the current logic. I'd imagine the request on demand
case would primarily be used by plugins / extensions so they can trigger the permission request when part of their functionality is used.
The engine does provide the ability to request permissions manually from the game logic, but that's moot because of the request all
logic on start, so that'd need to be updated accordingly.
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.
fyi @maunvz, I'm reverting the automatic permissions requests behavior in godotengine/godot#87080. So the PR will need to be updated so that the addon requests the necessary permissions when they're included in the manifest.
See #71 for reference.
_use_scene_api_option = _generate_export_option( | ||
"meta_xr_features/use_scene_api", | ||
"", | ||
Variant::Type::BOOL, | ||
PROPERTY_HINT_NONE, | ||
"", | ||
PROPERTY_USAGE_DEFAULT, | ||
false, | ||
false | ||
); |
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.
This can be removed since it's being added in #66
Superseded by #82 |
A lot of Meta Quest features are gated by AndroidManifest flags. Adding export options to support:
Experimental Features: https://developer.oculus.com/experimental/experimental-overview/
Overlay Keyboard: https://developer.oculus.com/documentation/unity/unity-keyboard-overlay/
Scene API: https://developer.oculus.com/documentation/native/native-spatial-data-perm/
Boundary Modes (can't find documentation, but already used by a few apps on the Quest Store)