From cd544fd86b0aca99bf31e6f0e3a248d3e4195d49 Mon Sep 17 00:00:00 2001 From: Fredia Huya-Kouadio Date: Wed, 7 Sep 2022 14:21:34 -0700 Subject: [PATCH] Fix issue causing the project manager to crash because of missing path argument In the process, the initialization logic is updated to show an error message and gracefully close the engine when setup errors occur. --- editor/editor_node.cpp | 8 +++-- .../android/java/lib/res/values/strings.xml | 2 ++ .../lib/src/org/godotengine/godot/Godot.java | 35 +++++++++++++++---- .../src/org/godotengine/godot/GodotLib.java | 4 +-- platform/android/java_godot_lib_jni.cpp | 13 +++---- platform/android/java_godot_lib_jni.h | 4 +-- platform/android/java_godot_wrapper.cpp | 9 ++--- platform/android/java_godot_wrapper.h | 2 +- 8 files changed, 54 insertions(+), 23 deletions(-) diff --git a/editor/editor_node.cpp b/editor/editor_node.cpp index a2231f4601a..6b5e9820675 100644 --- a/editor/editor_node.cpp +++ b/editor/editor_node.cpp @@ -3256,8 +3256,12 @@ void EditorNode::_discard_changes(const String &p_str) { for (const String &a : Main::get_forwardable_cli_arguments(Main::CLI_SCOPE_TOOL)) { args.push_back(a); } - args.push_back("--path"); - args.push_back(exec.get_base_dir()); + + String exec_base_dir = exec.get_base_dir(); + if (!exec_base_dir.is_empty()) { + args.push_back("--path"); + args.push_back(exec_base_dir); + } args.push_back("--project-manager"); Error err = OS::get_singleton()->create_instance(args); diff --git a/platform/android/java/lib/res/values/strings.xml b/platform/android/java/lib/res/values/strings.xml index 010006b81ea..f5a4ab1071b 100644 --- a/platform/android/java/lib/res/values/strings.xml +++ b/platform/android/java/lib/res/values/strings.xml @@ -12,6 +12,8 @@ Resume Download Cancel Cancel Verification + Error! + Unable to setup the Godot Engine! Aborting… diff --git a/platform/android/java/lib/src/org/godotengine/godot/Godot.java b/platform/android/java/lib/src/org/godotengine/godot/Godot.java index 28e689e63af..ad912205fd4 100644 --- a/platform/android/java/lib/src/org/godotengine/godot/Godot.java +++ b/platform/android/java/lib/src/org/godotengine/godot/Godot.java @@ -57,6 +57,7 @@ import android.content.SharedPreferences.Editor; import android.content.pm.ConfigurationInfo; import android.content.pm.PackageManager; import android.content.pm.PackageManager.NameNotFoundException; +import android.content.res.Resources; import android.graphics.Point; import android.graphics.Rect; import android.hardware.Sensor; @@ -69,6 +70,7 @@ import android.os.Environment; import android.os.Messenger; import android.os.VibrationEffect; import android.os.Vibrator; +import android.util.Log; import android.view.Display; import android.view.LayoutInflater; import android.view.Surface; @@ -85,6 +87,8 @@ import android.widget.TextView; import androidx.annotation.CallSuper; import androidx.annotation.Keep; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import androidx.annotation.StringRes; import androidx.fragment.app.Fragment; import com.google.android.vending.expansion.downloader.DownloadProgressInfo; @@ -105,6 +109,8 @@ import java.util.List; import java.util.Locale; public class Godot extends Fragment implements SensorEventListener, IDownloaderClient { + private static final String TAG = Godot.class.getSimpleName(); + private IStub mDownloaderClientStub; private TextView mStatusText; private TextView mProgressFraction; @@ -250,7 +256,7 @@ public class Godot extends Fragment implements SensorEventListener, IDownloaderC * Used by the native code (java_godot_lib_jni.cpp) to complete initialization of the GLSurfaceView view and renderer. */ @Keep - private void onVideoInit() { + private boolean onVideoInit() { final Activity activity = getActivity(); containerLayout = new FrameLayout(activity); containerLayout.setLayoutParams(new LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT)); @@ -262,7 +268,11 @@ public class Godot extends Fragment implements SensorEventListener, IDownloaderC // ...add to FrameLayout containerLayout.addView(editText); - GodotLib.setup(command_line); + if (!GodotLib.setup(command_line)) { + Log.e(TAG, "Unable to setup the Godot engine! Aborting..."); + alert(R.string.error_engine_setup_message, R.string.text_error_title, this::forceQuit); + return false; + } final String videoDriver = GodotLib.getGlobal("rendering/driver/driver_name"); if (videoDriver.equals("vulkan")) { @@ -303,6 +313,7 @@ public class Godot extends Fragment implements SensorEventListener, IDownloaderC } } } + return true; } public void setKeepScreenOn(final boolean p_enabled) { @@ -344,13 +355,27 @@ public class Godot extends Fragment implements SensorEventListener, IDownloaderC } public void alert(final String message, final String title) { + alert(message, title, null); + } + + private void alert(@StringRes int messageResId, @StringRes int titleResId, @Nullable Runnable okCallback) { + Resources res = getResources(); + alert(res.getString(messageResId), res.getString(titleResId), okCallback); + } + + private void alert(final String message, final String title, @Nullable Runnable okCallback) { final Activity activity = getActivity(); runOnUiThread(() -> { AlertDialog.Builder builder = new AlertDialog.Builder(activity); builder.setMessage(message).setTitle(title); builder.setPositiveButton( "OK", - (dialog, id) -> dialog.cancel()); + (dialog, id) -> { + if (okCallback != null) { + okCallback.run(); + } + dialog.cancel(); + }); AlertDialog dialog = builder.create(); dialog.show(); }); @@ -471,7 +496,7 @@ public class Godot extends Fragment implements SensorEventListener, IDownloaderC mMagnetometer = mSensorManager.getDefaultSensor(Sensor.TYPE_MAGNETIC_FIELD); mGyroscope = mSensorManager.getDefaultSensor(Sensor.TYPE_GYROSCOPE); - GodotLib.initialize(activity, + godot_initialized = GodotLib.initialize(activity, this, activity.getAssets(), io, @@ -482,8 +507,6 @@ public class Godot extends Fragment implements SensorEventListener, IDownloaderC tts); result_callback = null; - - godot_initialized = true; } @Override diff --git a/platform/android/java/lib/src/org/godotengine/godot/GodotLib.java b/platform/android/java/lib/src/org/godotengine/godot/GodotLib.java index f855fc6cf66..6d3c8ebff8a 100644 --- a/platform/android/java/lib/src/org/godotengine/godot/GodotLib.java +++ b/platform/android/java/lib/src/org/godotengine/godot/GodotLib.java @@ -54,7 +54,7 @@ public class GodotLib { /** * Invoked on the main thread to initialize Godot native layer. */ - public static native void initialize(Activity activity, + public static native boolean initialize(Activity activity, Godot p_instance, AssetManager p_asset_manager, GodotIO godotIO, @@ -74,7 +74,7 @@ public class GodotLib { * Invoked on the GL thread to complete setup for the Godot native layer logic. * @param p_cmdline Command line arguments used to configure Godot native layer components. */ - public static native void setup(String[] p_cmdline); + public static native boolean setup(String[] p_cmdline); /** * Invoked on the GL thread when the underlying Android surface has changed size. diff --git a/platform/android/java_godot_lib_jni.cpp b/platform/android/java_godot_lib_jni.cpp index 422c05e5cec..43239a477b4 100644 --- a/platform/android/java_godot_lib_jni.cpp +++ b/platform/android/java_godot_lib_jni.cpp @@ -79,7 +79,7 @@ JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_setVirtualKeyboardHei } } -JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_initialize(JNIEnv *env, jclass clazz, jobject p_activity, jobject p_godot_instance, jobject p_asset_manager, jobject p_godot_io, jobject p_net_utils, jobject p_directory_access_handler, jobject p_file_access_handler, jboolean p_use_apk_expansion, jobject p_godot_tts) { +JNIEXPORT jboolean JNICALL Java_org_godotengine_godot_GodotLib_initialize(JNIEnv *env, jclass clazz, jobject p_activity, jobject p_godot_instance, jobject p_asset_manager, jobject p_godot_io, jobject p_net_utils, jobject p_directory_access_handler, jobject p_file_access_handler, jboolean p_use_apk_expansion, jobject p_godot_tts) { JavaVM *jvm; env->GetJavaVM(&jvm); @@ -100,7 +100,7 @@ JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_initialize(JNIEnv *en os_android = new OS_Android(godot_java, godot_io_java, p_use_apk_expansion); - godot_java->on_video_init(env); + return godot_java->on_video_init(env); } JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_ondestroy(JNIEnv *env, jclass clazz) { @@ -123,7 +123,7 @@ JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_ondestroy(JNIEnv *env } } -JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_setup(JNIEnv *env, jclass clazz, jobjectArray p_cmdline) { +JNIEXPORT jboolean JNICALL Java_org_godotengine_godot_GodotLib_setup(JNIEnv *env, jclass clazz, jobjectArray p_cmdline) { setup_android_thread(); const char **cmdline = nullptr; @@ -133,10 +133,10 @@ JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_setup(JNIEnv *env, jc cmdlen = env->GetArrayLength(p_cmdline); if (cmdlen) { cmdline = (const char **)memalloc((cmdlen + 1) * sizeof(const char *)); - ERR_FAIL_NULL_MSG(cmdline, "Out of memory."); + ERR_FAIL_NULL_V_MSG(cmdline, false, "Out of memory."); cmdline[cmdlen] = nullptr; j_cmdline = (jstring *)memalloc(cmdlen * sizeof(jstring)); - ERR_FAIL_NULL_MSG(j_cmdline, "Out of memory."); + ERR_FAIL_NULL_V_MSG(j_cmdline, false, "Out of memory."); for (int i = 0; i < cmdlen; i++) { jstring string = (jstring)env->GetObjectArrayElement(p_cmdline, i); @@ -161,11 +161,12 @@ JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_setup(JNIEnv *env, jc // Note: --help and --version return ERR_HELP, but this should be translated to 0 if exit codes are propagated. if (err != OK) { - return; // should exit instead and print the error + return false; } java_class_wrapper = memnew(JavaClassWrapper(godot_java->get_activity())); GDREGISTER_CLASS(JNISingleton); + return true; } JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_resize(JNIEnv *env, jclass clazz, jobject p_surface, jint p_width, jint p_height) { diff --git a/platform/android/java_godot_lib_jni.h b/platform/android/java_godot_lib_jni.h index 3c48ca04594..2a8bfcc4da7 100644 --- a/platform/android/java_godot_lib_jni.h +++ b/platform/android/java_godot_lib_jni.h @@ -37,9 +37,9 @@ // These functions can be called from within JAVA and are the means by which our JAVA implementation calls back into our C++ code. // See java/src/org/godotengine/godot/GodotLib.java for the JAVA side of this (yes that's why we have the long names) extern "C" { -JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_initialize(JNIEnv *env, jclass clazz, jobject p_activity, jobject p_godot_instance, jobject p_asset_manager, jobject p_godot_io, jobject p_net_utils, jobject p_directory_access_handler, jobject p_file_access_handler, jboolean p_use_apk_expansion, jobject p_godot_tts); +JNIEXPORT jboolean JNICALL Java_org_godotengine_godot_GodotLib_initialize(JNIEnv *env, jclass clazz, jobject p_activity, jobject p_godot_instance, jobject p_asset_manager, jobject p_godot_io, jobject p_net_utils, jobject p_directory_access_handler, jobject p_file_access_handler, jboolean p_use_apk_expansion, jobject p_godot_tts); JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_ondestroy(JNIEnv *env, jclass clazz); -JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_setup(JNIEnv *env, jclass clazz, jobjectArray p_cmdline); +JNIEXPORT jboolean JNICALL Java_org_godotengine_godot_GodotLib_setup(JNIEnv *env, jclass clazz, jobjectArray p_cmdline); JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_resize(JNIEnv *env, jclass clazz, jobject p_surface, jint p_width, jint p_height); JNIEXPORT void JNICALL Java_org_godotengine_godot_GodotLib_newcontext(JNIEnv *env, jclass clazz, jobject p_surface); JNIEXPORT jboolean JNICALL Java_org_godotengine_godot_GodotLib_step(JNIEnv *env, jclass clazz); diff --git a/platform/android/java_godot_wrapper.cpp b/platform/android/java_godot_wrapper.cpp index e3456fe4e4e..07b0d759215 100644 --- a/platform/android/java_godot_wrapper.cpp +++ b/platform/android/java_godot_wrapper.cpp @@ -58,7 +58,7 @@ GodotJavaWrapper::GodotJavaWrapper(JNIEnv *p_env, jobject p_activity, jobject p_ } // get some Godot method pointers... - _on_video_init = p_env->GetMethodID(godot_class, "onVideoInit", "()V"); + _on_video_init = p_env->GetMethodID(godot_class, "onVideoInit", "()Z"); _restart = p_env->GetMethodID(godot_class, "restart", "()V"); _finish = p_env->GetMethodID(godot_class, "forceQuit", "()V"); _set_keep_screen_on = p_env->GetMethodID(godot_class, "setKeepScreenOn", "(Z)V"); @@ -125,14 +125,15 @@ GodotJavaViewWrapper *GodotJavaWrapper::get_godot_view() { return _godot_view; } -void GodotJavaWrapper::on_video_init(JNIEnv *p_env) { +bool GodotJavaWrapper::on_video_init(JNIEnv *p_env) { if (_on_video_init) { if (p_env == nullptr) { p_env = get_jni_env(); } - ERR_FAIL_NULL(p_env); - p_env->CallVoidMethod(godot_instance, _on_video_init); + ERR_FAIL_NULL_V(p_env, false); + return p_env->CallBooleanMethod(godot_instance, _on_video_init); } + return false; } void GodotJavaWrapper::on_godot_setup_completed(JNIEnv *p_env) { diff --git a/platform/android/java_godot_wrapper.h b/platform/android/java_godot_wrapper.h index bbf7c0ae339..a6c78531070 100644 --- a/platform/android/java_godot_wrapper.h +++ b/platform/android/java_godot_wrapper.h @@ -83,7 +83,7 @@ public: jobject get_class_loader(); GodotJavaViewWrapper *get_godot_view(); - void on_video_init(JNIEnv *p_env = nullptr); + bool on_video_init(JNIEnv *p_env = nullptr); void on_godot_setup_completed(JNIEnv *p_env = nullptr); void on_godot_main_loop_started(JNIEnv *p_env = nullptr); void restart(JNIEnv *p_env = nullptr);