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

Leverage Android archive (AAR) file for Godot custom build #31919

Merged
merged 2 commits into from
Sep 19, 2019

Conversation

m4gr3d
Copy link
Contributor

@m4gr3d m4gr3d commented Sep 3, 2019

As discussed in PR #31706, this PR leverages the Android library and its output file format (AAR file) to improve the Godot custom build process, and in the process the generation of the Godot prebuilt export templates.

This is done via the following steps:

  • The core of the Godot Android java layer is turned into an Android library:
    • Moved most of the source code under platfrom/android/java to platform/android/java/lib (responsible for the large number of changed files)
    • Updated the lib/build.gradle accordingly to reflect the new status as Android library. Additional gradle tasks were added to automatically generate the Godot Android shared (.so) files via scons when the library is built.
    • Updated the lib/AndroidManifest.xml file accordingly to reflect the Android library change.
    • Godot.java is made abstract to prevent its direct use as an activity.
  • A prebuiltApp binary module is added to replace the previous binary module responsible for generating the prebuilt export templates
    • The module depends on the Godot library for the core logic, and has its main Activity PrebuiltApp.java extends Godot.java
  • A customApp binary module is added which serves as template for Godot custom build process. The module depends on exported AAR files to access and use Godot's functionality.

To simplify the generation of the Godot prebuilt (android_debug.apk, android_release.apk) and custom build (android_source.zip) templates, a set of gradle tasks are added.
They can now be generated via the following steps:

  • cd platform/android/java
  • ./gradlew generateGodotTemplates

The resulting outputs (android_debug.apk, android_release.apk, android_source.zip) can be found under the Godot bin directory.

Misc cleanup: Removed the jetbrains ide setup directory as it's made obsolete by the new layout structure in platform/android/java.

@akien-mga
Copy link
Member

Awesome work!

You should be able to fix the style checks with this patch:

diff --git a/misc/hooks/pre-commit-clang-format b/misc/hooks/pre-commit-clang-format
index 3a0ac9f38..e309233a8 100755
--- a/misc/hooks/pre-commit-clang-format
+++ b/misc/hooks/pre-commit-clang-format
@@ -86,7 +86,7 @@ do
     if grep -q "thirdparty" <<< $file; then
         continue;
     fi
-    if grep -q "platform/android/java/src/com" <<< $file; then
+    if grep -q "platform/android/java/lib/src/com" <<< $file; then
         continue;
     fi
 
diff --git a/misc/travis/clang-format.sh b/misc/travis/clang-format.sh
index 48378281d..097b2a937 100755
--- a/misc/travis/clang-format.sh
+++ b/misc/travis/clang-format.sh
@@ -11,7 +11,7 @@ else
     RANGE=HEAD
 fi
 
-FILES=$(git diff-tree --no-commit-id --name-only -r $RANGE | grep -v thirdparty/ | grep -v platform/android/java/src/com/ | grep -E "\.(c|h|cpp|hpp|cc|hh|cxx|m|mm|inc|java|glsl)$")
+FILES=$(git diff-tree --no-commit-id --name-only -r $RANGE | grep -v thirdparty/ | grep -v platform/android/java/lib/src/com/ | grep -E "\.(c|h|cpp|hpp|cc|hh|cxx|m|mm|inc|java|glsl)$")
 echo "Checking files:\n$FILES"
 
 # create a random filename to store our generated patch

@akien-mga akien-mga added this to the 3.2 milestone Sep 3, 2019
@m4gr3d m4gr3d requested a review from neikeq as a code owner September 3, 2019 07:01
@@ -175,7 +175,7 @@ index e4b1b0f1c..36cd6aacf 100644
-import com.android.vending.expansion.downloader.R;
+// -- GODOT start --
+//import com.android.vending.expansion.downloader.R;
+import com.godot.game.R;
+import org.godotengine.godot.R;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the reference, I don't know why those changes were needed in the first place. Might be worth reviewing whether they actually are, and if so see if we can contribute a change upstream to make these files package-agnostic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes are needed because we're not using the library correctly. It's intended to be used as an android library, actually in a similar way to what I'm doing in this PR.
Directly adding the library source code into our own codebase is what's creating the need for the patch. I can look into cleanup that dependency later on.

@akien-mga
Copy link
Member

Could you squash commits together? (at least the style fixes, the removal of misc/ide/jetbrains can stay in its own commit)

@akien-mga
Copy link
Member

akien-mga commented Sep 3, 2019

This looks awesome from a quick overview, thanks a ton!

One concern is that there is now some amount of duplication around the AndroidManifest.xml and build.gradle for the various targets. I assume that whenever we want to do changes, we'd have to modify all those files now?
We could possibly generate them at build time from a common template, but I guess it might not be worth the trouble?

@akien-mga akien-mga requested review from reduz and a team and removed request for neikeq September 3, 2019 10:19
@m4gr3d
Copy link
Contributor Author

m4gr3d commented Sep 4, 2019

This looks awesome from a quick overview, thanks a ton!

One concern is that there is now some amount of duplication around the AndroidManifest.xml and build.gradle for the various targets. I assume that whenever we want to do changes, we'd have to modify all those files now?
We could possibly generate them at build time from a common template, but I guess it might not be worth the trouble?

The duplication was mostly with the customApp and prebuiltApp modules but I was able to coalesce them into a single module (app) and remove that issue.
Now we have one set of AndroidManifest.xml and build.gradle files for the lib module and another set for the app module.
The lib/AndroidManifest.xml and app/AndroidManifest.xml files have different contents and at build time the lib/AndroidManifest will be merge into the app/AndroidManifest via manifest merging.
Same for the gradle build files, and there configs that are shared between the two build files have been exported to the config.gradle file.

fhuya added 2 commits September 4, 2019 16:20
…(`lib`) and an application module (`app`).

The application module `app` serves double duties of providing the prebuilt Godot binaries ('android_debug.apk', 'android_release.apk') and the Godot custom build template ('android_source.zip').
@m4gr3d m4gr3d force-pushed the use_aar_for_custom_build branch from b299391 to f2d203a Compare September 4, 2019 23:22
@m4gr3d
Copy link
Contributor Author

m4gr3d commented Sep 4, 2019

Could you squash commits together? (at least the style fixes, the removal of misc/ide/jetbrains can stay in its own commit)

Done. the commits have been squashed together and the removal of misc/ide/jetbrains left in its own commit.

@m4gr3d m4gr3d requested a review from akien-mga September 8, 2019 22:25
@akien-mga akien-mga merged commit f9db6ad into godotengine:master Sep 19, 2019
@akien-mga
Copy link
Member

Thanks!

Sorry for the delay merging this, was on holidays :)

@m4gr3d
Copy link
Contributor Author

m4gr3d commented Sep 19, 2019

No problem, thanks!
Since this is merged, I'll be updating the documentation to highlight the new process for generating the Android build templates.
I've added issue #2773 for tracking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants