From f1f7e73712bf21ba7f6b6c37d3f38b6eb4561947 Mon Sep 17 00:00:00 2001 From: Fredia Huya-Kouadio Date: Thu, 26 Sep 2024 08:41:46 -0700 Subject: [PATCH] Remove the restriction on supported types for Godot Android plugins The Android plugin implementation is updated to use `JavaClassWrapper` which was fixed in https://github.com/godotengine/godot/pull/96182, thus removing the limitation on supported types. Note that `JavaClassWrapper` has also been updated in order to only provide access to public methods and constructor to GDScript. --- platform/android/api/api.cpp | 10 +- platform/android/api/java_class_wrapper.h | 13 +- platform/android/api/jni_singleton.h | 197 +++---------------- platform/android/java_class_wrapper.cpp | 10 +- platform/android/java_godot_lib_jni.cpp | 4 +- platform/android/plugin/godot_plugin_jni.cpp | 25 +-- 6 files changed, 51 insertions(+), 208 deletions(-) diff --git a/platform/android/api/api.cpp b/platform/android/api/api.cpp index 6920f801e524..078b9ab748f3 100644 --- a/platform/android/api/api.cpp +++ b/platform/android/api/api.cpp @@ -41,13 +41,11 @@ static JavaClassWrapper *java_class_wrapper = nullptr; void register_android_api() { #if !defined(ANDROID_ENABLED) - // On Android platforms, the `java_class_wrapper` instantiation and the - // `JNISingleton` registration occurs in + // On Android platforms, the `java_class_wrapper` instantiation occurs in // `platform/android/java_godot_lib_jni.cpp#Java_org_godotengine_godot_GodotLib_setup` - java_class_wrapper = memnew(JavaClassWrapper); // Dummy - GDREGISTER_CLASS(JNISingleton); + java_class_wrapper = memnew(JavaClassWrapper); #endif - + GDREGISTER_CLASS(JNISingleton); GDREGISTER_CLASS(JavaClass); GDREGISTER_CLASS(JavaObject); GDREGISTER_CLASS(JavaClassWrapper); @@ -108,7 +106,7 @@ Ref JavaObject::get_java_class() const { JavaClassWrapper *JavaClassWrapper::singleton = nullptr; -Ref JavaClassWrapper::wrap(const String &) { +Ref JavaClassWrapper::_wrap(const String &, bool) { return Ref(); } diff --git a/platform/android/api/java_class_wrapper.h b/platform/android/api/java_class_wrapper.h index 52df1644be8d..f008c097cf89 100644 --- a/platform/android/api/java_class_wrapper.h +++ b/platform/android/api/java_class_wrapper.h @@ -262,6 +262,8 @@ class JavaClassWrapper : public Object { bool _get_type_sig(JNIEnv *env, jobject obj, uint32_t &sig, String &strsig); #endif + Ref _wrap(const String &p_class, bool p_allow_private_methods_access); + static JavaClassWrapper *singleton; protected: @@ -270,15 +272,14 @@ class JavaClassWrapper : public Object { public: static JavaClassWrapper *get_singleton() { return singleton; } - Ref wrap(const String &p_class); + Ref wrap(const String &p_class) { + return _wrap(p_class, false); + } #ifdef ANDROID_ENABLED - Ref wrap_jclass(jclass p_class); - - JavaClassWrapper(jobject p_activity = nullptr); -#else - JavaClassWrapper(); + Ref wrap_jclass(jclass p_class, bool p_allow_private_methods_access = false); #endif + JavaClassWrapper(); }; #endif // JAVA_CLASS_WRAPPER_H diff --git a/platform/android/api/jni_singleton.h b/platform/android/api/jni_singleton.h index 06afc4eb782e..5e940819bcce 100644 --- a/platform/android/api/jni_singleton.h +++ b/platform/android/api/jni_singleton.h @@ -31,193 +31,53 @@ #ifndef JNI_SINGLETON_H #define JNI_SINGLETON_H +#include "java_class_wrapper.h" + #include "core/config/engine.h" #include "core/variant/variant.h" -#ifdef ANDROID_ENABLED -#include "jni_utils.h" -#endif - class JNISingleton : public Object { GDCLASS(JNISingleton, Object); -#ifdef ANDROID_ENABLED struct MethodData { - jmethodID method; Variant::Type ret_type; Vector argtypes; }; - jobject instance; RBMap method_map; -#endif + Ref wrapped_object; public: virtual Variant callp(const StringName &p_method, const Variant **p_args, int p_argcount, Callable::CallError &r_error) override { -#ifdef ANDROID_ENABLED - RBMap::Element *E = method_map.find(p_method); - - // Check the method we're looking for is in the JNISingleton map and that - // the arguments match. - bool call_error = !E || E->get().argtypes.size() != p_argcount; - if (!call_error) { - for (int i = 0; i < p_argcount; i++) { - if (!Variant::can_convert(p_args[i]->get_type(), E->get().argtypes[i])) { - call_error = true; - break; + if (wrapped_object.is_valid()) { + RBMap::Element *E = method_map.find(p_method); + + // Check the method we're looking for is in the JNISingleton map and that + // the arguments match. + bool call_error = !E || E->get().argtypes.size() != p_argcount; + if (!call_error) { + for (int i = 0; i < p_argcount; i++) { + if (!Variant::can_convert(p_args[i]->get_type(), E->get().argtypes[i])) { + call_error = true; + break; + } } } - } - - if (call_error) { - // The method is not in this map, defaulting to the regular instance calls. - return Object::callp(p_method, p_args, p_argcount, r_error); - } - - ERR_FAIL_NULL_V(instance, Variant()); - - r_error.error = Callable::CallError::CALL_OK; - - jvalue *v = nullptr; - if (p_argcount) { - v = (jvalue *)alloca(sizeof(jvalue) * p_argcount); - } - - JNIEnv *env = get_jni_env(); - - int res = env->PushLocalFrame(16); - - ERR_FAIL_COND_V(res != 0, Variant()); - - List to_erase; - for (int i = 0; i < p_argcount; i++) { - jvalret vr = _variant_to_jvalue(env, E->get().argtypes[i], p_args[i]); - v[i] = vr.val; - if (vr.obj) { - to_erase.push_back(vr.obj); + if (!call_error) { + return wrapped_object->callp(p_method, p_args, p_argcount, r_error); } } - Variant ret; - - switch (E->get().ret_type) { - case Variant::NIL: { - env->CallVoidMethodA(instance, E->get().method, v); - } break; - case Variant::BOOL: { - ret = env->CallBooleanMethodA(instance, E->get().method, v) == JNI_TRUE; - } break; - case Variant::INT: { - ret = env->CallIntMethodA(instance, E->get().method, v); - } break; - case Variant::FLOAT: { - ret = env->CallFloatMethodA(instance, E->get().method, v); - } break; - case Variant::STRING: { - jobject o = env->CallObjectMethodA(instance, E->get().method, v); - ret = jstring_to_string((jstring)o, env); - env->DeleteLocalRef(o); - } break; - case Variant::PACKED_STRING_ARRAY: { - jobjectArray arr = (jobjectArray)env->CallObjectMethodA(instance, E->get().method, v); - - ret = _jobject_to_variant(env, arr); - - env->DeleteLocalRef(arr); - } break; - case Variant::PACKED_INT32_ARRAY: { - jintArray arr = (jintArray)env->CallObjectMethodA(instance, E->get().method, v); - - int fCount = env->GetArrayLength(arr); - Vector sarr; - sarr.resize(fCount); - - int *w = sarr.ptrw(); - env->GetIntArrayRegion(arr, 0, fCount, w); - ret = sarr; - env->DeleteLocalRef(arr); - } break; - case Variant::PACKED_INT64_ARRAY: { - jlongArray arr = (jlongArray)env->CallObjectMethodA(instance, E->get().method, v); - - int fCount = env->GetArrayLength(arr); - Vector sarr; - sarr.resize(fCount); - - int64_t *w = sarr.ptrw(); - env->GetLongArrayRegion(arr, 0, fCount, w); - ret = sarr; - env->DeleteLocalRef(arr); - } break; - case Variant::PACKED_FLOAT32_ARRAY: { - jfloatArray arr = (jfloatArray)env->CallObjectMethodA(instance, E->get().method, v); - - int fCount = env->GetArrayLength(arr); - Vector sarr; - sarr.resize(fCount); - - float *w = sarr.ptrw(); - env->GetFloatArrayRegion(arr, 0, fCount, w); - ret = sarr; - env->DeleteLocalRef(arr); - } break; - case Variant::PACKED_FLOAT64_ARRAY: { - jdoubleArray arr = (jdoubleArray)env->CallObjectMethodA(instance, E->get().method, v); - - int fCount = env->GetArrayLength(arr); - Vector sarr; - sarr.resize(fCount); - - double *w = sarr.ptrw(); - env->GetDoubleArrayRegion(arr, 0, fCount, w); - ret = sarr; - env->DeleteLocalRef(arr); - } break; - case Variant::DICTIONARY: { - jobject obj = env->CallObjectMethodA(instance, E->get().method, v); - ret = _jobject_to_variant(env, obj); - env->DeleteLocalRef(obj); - - } break; - case Variant::OBJECT: { - jobject obj = env->CallObjectMethodA(instance, E->get().method, v); - ret = _jobject_to_variant(env, obj); - env->DeleteLocalRef(obj); - } break; - default: { - env->PopLocalFrame(nullptr); - ERR_FAIL_V(Variant()); - } break; - } - - while (to_erase.size()) { - env->DeleteLocalRef(to_erase.front()->get()); - to_erase.pop_front(); - } - - env->PopLocalFrame(nullptr); - - return ret; -#else // ANDROID_ENABLED - - // Defaulting to the regular instance calls. return Object::callp(p_method, p_args, p_argcount, r_error); -#endif } -#ifdef ANDROID_ENABLED - jobject get_instance() const { - return instance; + Ref get_wrapped_object() const { + return wrapped_object; } - void set_instance(jobject p_instance) { - instance = p_instance; - } - - void add_method(const StringName &p_name, jmethodID p_method, const Vector &p_args, Variant::Type p_ret_type) { + void add_method(const StringName &p_name, const Vector &p_args, Variant::Type p_ret_type) { MethodData md; - md.method = p_method; md.argtypes = p_args; md.ret_type = p_ret_type; method_map[p_name] = md; @@ -232,24 +92,15 @@ class JNISingleton : public Object { ADD_SIGNAL(mi); } -#endif + JNISingleton() {} - JNISingleton() { -#ifdef ANDROID_ENABLED - instance = nullptr; -#endif + JNISingleton(const Ref &p_wrapped_object) { + wrapped_object = p_wrapped_object; } ~JNISingleton() { -#ifdef ANDROID_ENABLED method_map.clear(); - if (instance) { - JNIEnv *env = get_jni_env(); - ERR_FAIL_NULL(env); - - env->DeleteGlobalRef(instance); - } -#endif + wrapped_object.unref(); } }; diff --git a/platform/android/java_class_wrapper.cpp b/platform/android/java_class_wrapper.cpp index c92717e92274..6bedbfd157a2 100644 --- a/platform/android/java_class_wrapper.cpp +++ b/platform/android/java_class_wrapper.cpp @@ -1120,7 +1120,7 @@ bool JavaClass::_convert_object_to_variant(JNIEnv *env, jobject obj, Variant &va return false; } -Ref JavaClassWrapper::wrap(const String &p_class) { +Ref JavaClassWrapper::_wrap(const String &p_class, bool p_allow_private_methods_access) { String class_name_dots = p_class.replace("/", "."); if (class_cache.has(class_name_dots)) { return class_cache[class_name_dots]; @@ -1175,7 +1175,7 @@ Ref JavaClassWrapper::wrap(const String &p_class) { jint mods = env->CallIntMethod(obj, is_constructor ? Constructor_getModifiers : Method_getModifiers); - if (!(mods & 0x0001)) { + if (!(mods & 0x0001) && (is_constructor || !p_allow_private_methods_access)) { env->DeleteLocalRef(obj); continue; //not public bye } @@ -1336,7 +1336,7 @@ Ref JavaClassWrapper::wrap(const String &p_class) { return java_class; } -Ref JavaClassWrapper::wrap_jclass(jclass p_class) { +Ref JavaClassWrapper::wrap_jclass(jclass p_class, bool p_allow_private_methods_access) { JNIEnv *env = get_jni_env(); ERR_FAIL_NULL_V(env, Ref()); @@ -1344,12 +1344,12 @@ Ref JavaClassWrapper::wrap_jclass(jclass p_class) { String class_name_string = jstring_to_string(class_name, env); env->DeleteLocalRef(class_name); - return wrap(class_name_string); + return _wrap(class_name_string, p_allow_private_methods_access); } JavaClassWrapper *JavaClassWrapper::singleton = nullptr; -JavaClassWrapper::JavaClassWrapper(jobject p_activity) { +JavaClassWrapper::JavaClassWrapper() { singleton = this; JNIEnv *env = get_jni_env(); diff --git a/platform/android/java_godot_lib_jni.cpp b/platform/android/java_godot_lib_jni.cpp index 6086f67a1e7d..1a256959cdb8 100644 --- a/platform/android/java_godot_lib_jni.cpp +++ b/platform/android/java_godot_lib_jni.cpp @@ -32,7 +32,6 @@ #include "android_input_handler.h" #include "api/java_class_wrapper.h" -#include "api/jni_singleton.h" #include "dir_access_jandroid.h" #include "display_server_android.h" #include "file_access_android.h" @@ -209,8 +208,7 @@ JNIEXPORT jboolean JNICALL Java_org_godotengine_godot_GodotLib_setup(JNIEnv *env TTS_Android::setup(p_godot_tts); - java_class_wrapper = memnew(JavaClassWrapper(godot_java->get_activity())); - GDREGISTER_CLASS(JNISingleton); + java_class_wrapper = memnew(JavaClassWrapper); return true; } diff --git a/platform/android/plugin/godot_plugin_jni.cpp b/platform/android/plugin/godot_plugin_jni.cpp index 75c8dd952818..fbea38eac00c 100644 --- a/platform/android/plugin/godot_plugin_jni.cpp +++ b/platform/android/plugin/godot_plugin_jni.cpp @@ -30,6 +30,7 @@ #include "godot_plugin_jni.h" +#include "api/java_class_wrapper.h" #include "api/jni_singleton.h" #include "jni_utils.h" #include "string_android.h" @@ -57,11 +58,15 @@ JNIEXPORT jboolean JNICALL Java_org_godotengine_godot_plugin_GodotPlugin_nativeR ERR_FAIL_COND_V(jni_singletons.has(singname), false); - JNISingleton *s = (JNISingleton *)ClassDB::instantiate("JNISingleton"); - s->set_instance(env->NewGlobalRef(obj)); - jni_singletons[singname] = s; + jclass java_class = env->GetObjectClass(obj); + Ref java_class_wrapped = JavaClassWrapper::get_singleton()->wrap_jclass(java_class, true); + env->DeleteLocalRef(java_class); - Engine::get_singleton()->add_singleton(Engine::Singleton(singname, s)); + Ref plugin_object = Ref(memnew(JavaObject(java_class_wrapped, obj))); + JNISingleton *plugin_singleton = memnew(JNISingleton(plugin_object)); + jni_singletons[singname] = plugin_singleton; + + Engine::get_singleton()->add_singleton(Engine::Singleton(singname, plugin_singleton)); return true; } @@ -75,7 +80,6 @@ JNIEXPORT void JNICALL Java_org_godotengine_godot_plugin_GodotPlugin_nativeRegis String mname = jstring_to_string(name, env); String retval = jstring_to_string(ret, env); Vector types; - String cs = "("; int stringCount = env->GetArrayLength(args); @@ -83,18 +87,9 @@ JNIEXPORT void JNICALL Java_org_godotengine_godot_plugin_GodotPlugin_nativeRegis jstring string = (jstring)env->GetObjectArrayElement(args, i); const String rawString = jstring_to_string(string, env); types.push_back(get_jni_type(rawString)); - cs += get_jni_sig(rawString); - } - - cs += ")"; - cs += get_jni_sig(retval); - jclass cls = env->GetObjectClass(s->get_instance()); - jmethodID mid = env->GetMethodID(cls, mname.ascii().get_data(), cs.ascii().get_data()); - if (!mid) { - print_line("Failed getting method ID " + mname); } - s->add_method(mname, mid, types, get_jni_type(retval)); + s->add_method(mname, types, get_jni_type(retval)); } JNIEXPORT void JNICALL Java_org_godotengine_godot_plugin_GodotPlugin_nativeRegisterSignal(JNIEnv *env, jclass clazz, jstring j_plugin_name, jstring j_signal_name, jobjectArray j_signal_param_types) {