Feature/android support #101

piiertho · 2021-03-10T10:37:33Z

This adds android platform support.
This enable to export game. Only thing to do is the same as classic godot build.
Kotlin project Dex jars will be created if user enable android build in gradle project.
This modifies CI to add Android build for debug and release exports. Also build steps names have changed, explaining the expected but not run CI jobs.

raniejade · 2021-03-10T12:37:34Z

.github/workflows/check-pr-engine-editor-debug-and-tests.yaml

@@ -32,13 +32,16 @@ jobs:
  build-editor-debug:
    strategy:
      matrix:
-        os: [ubuntu-latest, macos-latest, windows-latest]
+        name: [ Linux, OSX, Windows ]


why the change?

Because on exports I had to rename it to not have confusions between x11 and android. So to keep it coherent, I renamed for all cpp jobs.

So this should contain Android as well?

There is no editor build for android.

raniejade · 2021-03-10T12:38:16Z

.github/workflows/check-pr-engine-editor-release.yaml

@@ -5,13 +5,16 @@ jobs:
  build-editor-release:
    strategy:
      matrix:
-        os: [ ubuntu-latest, macos-latest, windows-latest ]
+        name: [ Linux, OSX, Windows ]


same here - I don't think this is necessary.

raniejade · 2021-03-10T12:39:41Z

docs/src/doc/user-guide/exporting.md

+    godot {
+        isAndroidExportEnabled.set(true)
+        dxToolPath.set("${System.getenv("HOME")}/Android/Sdk/build-tools/30.0.3/dx")


can we use standard android env variables?

You mean ANDROID_HOME?

Hmm according to https://developer.android.com/studio/command-line/variables ANDROID_HOME is deprecated and replaced by ANDROID_SDK_ROOT. Will update the places in the docs and the commented line in build.gradle.kts of tests

raniejade · 2021-03-10T12:41:34Z

src/gd_kotlin.cpp

@@ -35,7 +40,33 @@ jni::JObject to_java_url(jni::Env& env, const String& bootstrapJar) {
    return url;
 }

+jni::JObject create_android_class_loader(jni::Env& env, const String& full_jar_path,


can we just inline this in create_class_loader? create_android_class_loader wont be called elsewhere right?

create_android_class_loader is also called in GDKotlin::init, while initializing bootstrap.

raniejade · 2021-03-10T12:51:52Z

src/jni/env.cpp

@@ -62,7 +59,7 @@ namespace jni {
        if (exception_check()) {
            exception_describe();
            exception_clear();
-            throw JniError("An exception has occurred!");
+            JVM_CRASH_COND_MSG(true, "An exception has occurred!")


A jvm exception shouldn't cause the engine to crash. If we can't use exceptions here - I suggest that we start implementing what C# did. Make the behaviour when an exception happens in jvm configurable. So either crash completely, or just log the message (maybe using ERR_FAIL_COND_MSG?). When in tool mode, we shouldn't crash to stop misbehaving plugins from crashing the engine.

I agree. But as we need to change this in more places in general, I suggest doing this in a separate PR.

I do not agree when I read code.
I'm not for checking exception every time we make a jvm call (which is the case for now).
I agree in debug we should make this check, tool should not crash. But in release mode I think we should think a bit more. Even if it is only a bool check, it is a bool check for every jvm method call.
Thing is our module is here to enable user to bring game logic. We should take the less cpu time possible, so that user have much for its game logic.
I think we should run flamegraph to see check_exception impact.

For now I made it crash only in debug when not in TOOL, otherwise it prints error. But we'll have to think a bit more to exception handling.

chippmann · 2021-03-10T15:11:22Z

src/kotlin_editor_export_plugin.cpp

-        ) != OK) {
-            LOG_ERROR("Cannot copy jre folder to export folder, please make sure you created a jre in project "
-                      "root folder using jlink.")
+    if (!p_path.ends_with(".apk")) {


This will not hold up anymore with 3.2.4!
As 3.2.4 will add bundle support. Not sure if just checking for aar as well is enough but most probably it is

godotengine/godot#42185
and godotengine/godot-proposals#1111 respecively

raniejade · 2021-03-10T23:31:41Z

src/gd_kotlin.cpp

@@ -35,8 +40,25 @@ jni::JObject to_java_url(jni::Env& env, const String& bootstrapJar) {
    return url;
 }

-jni::JObject create_class_loader(jni::Env& env, const String& bootstrapJar) {
-    jni::JObject url = to_java_url(env, bootstrapJar);
+jni::JObject create_class_loader(jni::Env& env, const String& full_jar_path, const jni::JObject& p_parent_loader) {


parent_class_loader is not being used in the non android path. As I've mentioned before URLClassLoader's constructor accepts a class loader as the second parameter :)

chippmann · 2021-03-11T05:55:04Z

src/kotlin_editor_export_plugin.cpp

-    String bootstrap_jar{"res://build/libs/godot-bootstrap.jar"};
+    String main_jar;
+    String bootstrap_jar;
+    if (p_path.ends_with(".apk")) {


Here aar is missing as well

… is dummy doing nothing

…tArgs and get ClassLoader from android platform

…n android, platform ready

…p side

piiertho requested review from chippmann, raniejade and CedNaru March 10, 2021 12:27

piiertho marked this pull request as ready for review March 10, 2021 12:27

raniejade requested changes Mar 10, 2021

View reviewed changes

chippmann requested changes Mar 10, 2021

View reviewed changes

piiertho requested review from raniejade and chippmann March 10, 2021 17:41

raniejade reviewed Mar 10, 2021

View reviewed changes

chippmann requested changes Mar 11, 2021

View reviewed changes

piiertho requested review from raniejade and chippmann March 11, 2021 08:25

chippmann approved these changes Mar 11, 2021

View reviewed changes

CedNaru approved these changes Mar 12, 2021

View reviewed changes

raniejade approved these changes Mar 12, 2021

View reviewed changes

piiertho and others added 15 commits March 13, 2021 09:54

Add jvm cpp distinction by platforms

3159f4d

Differentiate jvm init arguments for android and desktop, android one…

6d50751

… is dummy doing nothing

Remove exceptions in favor of error crash. Make android implem of Ini…

408fb9c

…tArgs and get ClassLoader from android platform

avoid copy jre for android export

6a7f44d

Exclude java 9+ module-info for android support

f8964d4

Make difference between dex jars and standard jars

0e6f72d

create class loader that load bootstrap jar for android

2f73b2e

Set bootstrap classloader as parent classloader for main jar loader o…

9606ab0

…n android, platform ready

Automate dex conversion and update docs

d3f6626

Update supported targets

681e0be

Fixed dex jar path in tests project

7fc3dde

First try to add android build to CI using debug export

cfb254a

Add android build to exports template Check PR CI

139463f

Remove android specific create class loader method

10fa9c3

Replace ANDROID_HOME with ANDROID_SDK_ROOT and update docs

af8793f

chippmann and others added 5 commits March 13, 2021 09:55

Remove println import

888152e

Make jvm exception crash engine only when in debug and not TOOL

5086273

Add aar to jre export in export plugin

0a0d5a6

Fix android export condition in export plugin

6f63cc8

Use parent class loader when creating a new desktop classloader on cp…

d34984c

…p side

piiertho force-pushed the feature/android-support branch from bfe8da4 to d34984c Compare March 13, 2021 08:56

piiertho merged commit 7bbf2b0 into master Mar 13, 2021

piiertho deleted the feature/android-support branch March 13, 2021 10:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/android support #101

Feature/android support #101

piiertho commented Mar 10, 2021 •

edited

Loading

raniejade Mar 10, 2021

piiertho Mar 10, 2021

raniejade Mar 10, 2021

piiertho Mar 10, 2021

raniejade Mar 10, 2021

raniejade Mar 10, 2021

chippmann Mar 10, 2021

chippmann Mar 10, 2021 •

edited

Loading

raniejade Mar 10, 2021

piiertho Mar 10, 2021 •

edited

Loading

raniejade Mar 10, 2021

chippmann Mar 10, 2021

piiertho Mar 10, 2021 •

edited

Loading

piiertho Mar 10, 2021 •

edited

Loading

chippmann Mar 10, 2021

chippmann Mar 10, 2021

raniejade Mar 10, 2021

chippmann Mar 11, 2021

Feature/android support #101

Feature/android support #101

Conversation

piiertho commented Mar 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chippmann Mar 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

piiertho Mar 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

piiertho Mar 10, 2021 • edited Loading

Choose a reason for hiding this comment

piiertho Mar 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

piiertho commented Mar 10, 2021 •

edited

Loading

chippmann Mar 10, 2021 •

edited

Loading

piiertho Mar 10, 2021 •

edited

Loading

piiertho Mar 10, 2021 •

edited

Loading

piiertho Mar 10, 2021 •

edited

Loading