-
-
Notifications
You must be signed in to change notification settings - Fork 4
Add Meta Prebuilt APK Install Tool #18
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?
Add Meta Prebuilt APK Install Tool #18
Conversation
8198f06 to
fc442f0
Compare
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.
Thanks! This looks great :-)
I've only got a couple notes/questions on the code
Otherwise, the only other important thing to note here is that this will only work with Godot 4.5 (because we need EditorExportPlugin::_update_android_prebuilt_manifest()) and so merging this will make the plugin 4.5+
We'll want to consider when we want to do this, or if we want to make two builds (one that's 4.3+ and another that's 4.5+), or maybe two major versions?
If we go with two builds, we could come up with a way to have a single codebase, where we wrap the new code with #if GODOT_VERSION_PATCH >= 5, which will check if the provided extension_api.json is for Godot 4.5 or later. Then we just need CI to make two builds with two different extension_api.jsons
build.gradle
Outdated
| if (godotDir == "") { | ||
| throw new GradleException("godot_dir property is required") | ||
| } |
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 probably check that the directory exists as well. And since this needs to be done in a bunch of places, it probably makes sense to move this check into a function
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.
Added checking that it exists / is a directory, and extracted it out to a verifyGodotDir() function.
1dc17a9 to
cf444dd
Compare
The previous logic only uses `ANDROID_ENABLED` to gate the Platform SDK logic, which causes it to be active when the addon is loaded in the Android / XR editor. The updated logic adds an editor specific build for the Android / XR editor which disables the Platform SDK logic.
cf444dd to
104e440
Compare
| <activity | ||
| android:name=".GodotApp" | ||
| android:exported="true" | ||
| tools:node="merge"> | ||
| <intent-filter> | ||
| <action android:name="android.intent.action.MAIN" /> | ||
| <category android:name="android.intent.category.LAUNCHER" /> | ||
|
|
||
| <!-- Enable access to OpenXR on Oculus mobile devices, no-op on other Android | ||
| platforms. --> | ||
| <category android:name="com.oculus.intent.category.VR" /> | ||
|
|
||
| <!-- OpenXR category tag to indicate the activity starts in an immersive OpenXR mode. | ||
| See https://registry.khronos.org/OpenXR/specs/1.0/html/xrspec.html#android-runtime-category. --> | ||
| <category android:name="org.khronos.openxr.intent.category.IMMERSIVE_HMD" /> | ||
| </intent-filter> | ||
| </activity> |
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 will need to be updated following the work in https://github.com/m4gr3d/godot/tree/add_front_door
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.
Can you provide more detail on this? That link doesn't appear to lead anywhere. 🙂
| "XRInterface", | ||
| "ZIPReader" | ||
| ] | ||
| } |
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.
Missing newline.
| break; | ||
| } | ||
|
|
||
| bool install_failed = 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.
install_failed doesn't seem to be updated in the following logic.
| overrides["gradle_build/min_sdk"] = "29"; // Android 10 | ||
| overrides["gradle_build/target_sdk"] = "32"; // Android 12 | ||
| } else { | ||
| overrides["gradle_build/use_gradle_build"] = 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.
Let's remove this given the progress being made on the gradle build and let's have the min_sdk and target_sdk reflect the values from above.
| } | ||
|
|
||
| if (_get_bool_option("meta_xr_features/quest_3_support")) { | ||
| supported_devices.append("quest3"); |
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.
| // Check for scene api. | ||
| bool use_scene_api = ProjectSettings::get_singleton()->get_setting_with_override("xr/openxr/extensions/meta/scene_api"); | ||
| if (use_scene_api) { | ||
| r_permissions.append("com.oculus.permission.USE_SCENE"); | ||
| } |
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 also check for environment depth as done in https://github.com/GodotVR/godot_openxr_vendors/blob/3b4fb43d4e88be334f0ce73244490216861f46fd/plugin/src/main/cpp/export/meta_export_plugin.cpp#L418
m4gr3d
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.
Some feedback to address, but it looks good overall.
This PR builds on top of the work in #15
For ease of review I've kept commits separate, including a commit that runs
.clang-formaton all of the c++ files. Some of the updates / additions to Fredia's work:generatePrebuiltApksnow serves as a standalone task, it won't be run when building the plugin.generatePrebuiltApksit needs to be pointed to a godot directory with-Pgodot_dir=path/to/godotmeta-export-template.zipfile in the project root, with the idea that we'll host them here in releases of the Godot Meta Toolkit.GODOT_META_TOOLKIT_VERSIONdefine in a new fileversion.h.