Misc improvements to the GodotPlugin API

- Add overload method for `GodotPlugin#emitSignal(...)`
- Allow passing `null` values as signal arguments

Author: m4gr3d

platform/android/api/java_class_wrapper.h

@@ -68,6 +68,7 @@ class JavaClass : public RefCounted {
 	RBMap<StringName, Variant> constant_map;
 
 	struct MethodInfo {
+		bool _public = false;
 		bool _static = false;
 		bool _constructor = false;
 		Vector<uint32_t> param_types;
@@ -274,7 +275,7 @@ class JavaClassWrapper : public Object {
 
 	Ref<JavaObject> exception;
 
-	Ref<JavaClass> _wrap(const String &p_class, bool p_allow_non_public_methods_access);
+	Ref<JavaClass> _wrap(const String &p_class, bool p_allow_non_public_methods_access = false);
 
 	static JavaClassWrapper *singleton;
 
@@ -293,7 +294,7 @@ class JavaClassWrapper : public Object {
 	}
 
 #ifdef ANDROID_ENABLED
-	Ref<JavaClass> wrap_jclass(jclass p_class, bool p_allow_private_methods_access = false);
+	Ref<JavaClass> wrap_jclass(jclass p_class, bool p_allow_non_public_methods_access = false);
 #endif
 	JavaClassWrapper();
 };

platform/android/api/jni_singleton.h

@@ -60,31 +60,15 @@ class JNISingleton : public Object {
 		}
 
 		// Check the method we're looking for is in the JNISingleton map.
+		// This is done because JNISingletons register methods differently than wrapped JavaClass / JavaObject to allow
+		// for access to private methods annotated with the @UsedByGodot annotation.
+		// In the future, we should remove access to private methods and require that JNISingletons' methods exposed to
+		// GDScript be all public, similarly to what we do for wrapped JavaClass / JavaObject methods. Doing so will
+		// also allow dropping and deprecating the @UsedByGodot annotation.
 		RBMap<StringName, MethodData>::Element *E = method_map.find(p_method);
 		if (E) {
 			if (wrapped_object.is_valid()) {
-				// Check that the arguments match.
-				int method_arg_count = E->get().argtypes.size();
-				if (p_argcount < method_arg_count) {
-					r_error.error = Callable::CallError::CALL_ERROR_TOO_FEW_ARGUMENTS;
-				} else if (p_argcount > method_arg_count) {
-					r_error.error = Callable::CallError::CALL_ERROR_TOO_MANY_ARGUMENTS;
-				} else {
-					// Check the arguments are valid.
-					bool arguments_valid = true;
-					for (int i = 0; i < p_argcount; i++) {
-						if (!Variant::can_convert(p_args[i]->get_type(), E->get().argtypes[i])) {
-							arguments_valid = false;
-							break;
-						}
-					}
-
-					if (arguments_valid) {
-						return wrapped_object->callp(p_method, p_args, p_argcount, r_error);
-					} else {
-						r_error.error = Callable::CallError::CALL_ERROR_INVALID_ARGUMENT;
-					}
-				}
+				return wrapped_object->callp(p_method, p_args, p_argcount, r_error);
 			} else {
 				r_error.error = Callable::CallError::CALL_ERROR_INSTANCE_IS_NULL;
 			}

platform/android/java/lib/src/org/godotengine/godot/plugin/GodotPlugin.java

@@ -361,7 +361,7 @@ protected void runOnRenderThread(Runnable action) {
 	/**
 	 * Emit a registered Godot signal.
 	 * @param signalName Name of the signal to emit. It will be validated against the set of registered signals.
-	 * @param signalArgs Arguments used to populate the emitted signal. The arguments will be validated against the {@link SignalInfo} matching the registered signalName parameter.
+	 * @param signalArgs Arguments used to populate the emitted signal. The arguments will be validated against the registered {@link SignalInfo} matching the signalName parameter.
 	 */
 	protected void emitSignal(final String signalName, final Object... signalArgs) {
 		try {
@@ -380,6 +380,15 @@ protected void emitSignal(final String signalName, final Object... signalArgs) {
 		}
 	}
 
+	/**
+	 * Emit a registered Godot signal.
+	 * @param signal Signal to emit. It will be validated against the set of registered signals.
+	 * @param signalArgs Arguments used to populate the emitted signal. The arguments will be validated against the registered {@link SignalInfo} matching the signal parameter.
+	 */
+	protected void emitSignal(SignalInfo signal, final Object... signalArgs) {
+		emitSignal(signal.getName(), signalArgs);
+	}
+
 	/**
 	 * Emit a Godot signal.
 	 * @param godot Godot instance
@@ -402,7 +411,8 @@ public static void emitSignal(Godot godot, String pluginName, SignalInfo signalI
 
 			// Validate the argument's types.
 			for (int i = 0; i < signalParamTypes.length; i++) {
-				if (!signalParamTypes[i].isInstance(signalArgs[i])) {
+				Object signalArg = signalArgs[i];
+				if (signalArg != null && !signalParamTypes[i].isInstance(signalArg)) {
 					throw new IllegalArgumentException(
 							"Invalid type for argument #" + i + ". Should be of type " + signalParamTypes[i].getName());
 				}

platform/android/java_class_wrapper.cpp

@@ -199,7 +199,7 @@ bool JavaClass::_call_method(JavaObject *p_instance, const StringName &p_method,
 				case ARG_ARRAY_BIT | ARG_TYPE_CHARSEQUENCE: {
 					if (p_args[i]->get_type() == Variant::ARRAY) {
 						Array arr = *p_args[i];
-						if (arr.is_typed() && arr.get_typed_builtin() != Variant::STRING) {
+						if (arr.is_typed() && (arr.get_typed_builtin() != Variant::STRING || arr.get_typed_builtin() != Variant::STRING_NAME)) {
 							arg_expected = Variant::ARRAY;
 						}
 					} else if (p_args[i]->get_type() != Variant::PACKED_STRING_ARRAY) {
@@ -1527,6 +1527,7 @@ Ref<JavaClass> JavaClassWrapper::_wrap(const String &p_class, bool p_allow_non_p
 		}
 
 		JavaClass::MethodInfo mi;
+		mi._public = is_public;
 		mi._static = (mods & 0x8) != 0;
 		mi._constructor = is_constructor;
 		bool valid = true;
@@ -1686,15 +1687,15 @@ Ref<JavaClass> JavaClassWrapper::_wrap(const String &p_class, bool p_allow_non_p
 	return java_class;
 }
 
-Ref<JavaClass> JavaClassWrapper::wrap_jclass(jclass p_class, bool p_allow_private_methods_access) {
+Ref<JavaClass> JavaClassWrapper::wrap_jclass(jclass p_class, bool p_allow_non_public_methods_access) {
 	JNIEnv *env = get_jni_env();
 	ERR_FAIL_NULL_V(env, Ref<JavaClass>());
 
 	jstring class_name = (jstring)env->CallObjectMethod(p_class, Class_getName);
 	String class_name_string = jstring_to_string(class_name, env);
 	env->DeleteLocalRef(class_name);
 
-	return _wrap(class_name_string, p_allow_private_methods_access);
+	return _wrap(class_name_string, p_allow_non_public_methods_access);
 }
 
 JavaClassWrapper *JavaClassWrapper::singleton = nullptr;

platform/android/jni_utils.cpp

@@ -55,15 +55,17 @@ Callable jcallable_to_callable(JNIEnv *p_env, jobject p_jcallable_obj) {
 	ERR_FAIL_NULL_V(p_env, Callable());
 
 	const Variant *callable_variant = nullptr;
-	jclass callable_class = jni_find_class(p_env, "org/godotengine/godot/variant/Callable");
-	if (callable_class && p_env->IsInstanceOf(p_jcallable_obj, callable_class)) {
-		jmethodID get_native_pointer = p_env->GetMethodID(callable_class, "getNativePointer", "()J");
-		jlong native_callable = p_env->CallLongMethod(p_jcallable_obj, get_native_pointer);
+	if (p_jcallable_obj) {
+		jclass callable_class = jni_find_class(p_env, "org/godotengine/godot/variant/Callable");
+		if (callable_class && p_env->IsInstanceOf(p_jcallable_obj, callable_class)) {
+			jmethodID get_native_pointer = p_env->GetMethodID(callable_class, "getNativePointer", "()J");
+			jlong native_callable = p_env->CallLongMethod(p_jcallable_obj, get_native_pointer);
 
-		callable_variant = reinterpret_cast<const Variant *>(native_callable);
-	}
+			callable_variant = reinterpret_cast<const Variant *>(native_callable);
+		}
 
-	p_env->DeleteLocalRef(callable_class);
+		p_env->DeleteLocalRef(callable_class);
+	}
 
 	ERR_FAIL_NULL_V(callable_variant, Callable());
 	return *callable_variant;
@@ -73,16 +75,18 @@ String charsequence_to_string(JNIEnv *p_env, jobject p_charsequence) {
 	ERR_FAIL_NULL_V(p_env, String());
 
 	String result;
-	jclass bclass = jni_find_class(p_env, "java/lang/CharSequence");
-	if (bclass && p_env->IsInstanceOf(p_charsequence, bclass)) {
-		jmethodID to_string = p_env->GetMethodID(bclass, "toString", "()Ljava/lang/String;");
-		jstring obj_string = (jstring)p_env->CallObjectMethod(p_charsequence, to_string);
+	if (p_charsequence) {
+		jclass bclass = jni_find_class(p_env, "java/lang/CharSequence");
+		if (bclass && p_env->IsInstanceOf(p_charsequence, bclass)) {
+			jmethodID to_string = p_env->GetMethodID(bclass, "toString", "()Ljava/lang/String;");
+			jstring obj_string = (jstring)p_env->CallObjectMethod(p_charsequence, to_string);
+
+			result = jstring_to_string(obj_string, p_env);
+			p_env->DeleteLocalRef(obj_string);
+		}
 
-		result = jstring_to_string(obj_string, p_env);
-		p_env->DeleteLocalRef(obj_string);
+		p_env->DeleteLocalRef(bclass);
 	}
-
-	p_env->DeleteLocalRef(bclass);
 	return result;
 }
 

platform/android/plugin/godot_plugin_jni.cpp

@@ -128,7 +128,6 @@ JNIEXPORT void JNICALL Java_org_godotengine_godot_plugin_GodotPlugin_nativeEmitS
 
 	for (int i = 0; i < count; i++) {
 		jobject j_param = env->GetObjectArrayElement(j_signal_params, i);
-		ERR_FAIL_NULL(j_param);
 		memnew_placement(&variant_params[i], Variant(_jobject_to_variant(env, j_param)));
 		args[i] = &variant_params[i];
 		env->DeleteLocalRef(j_param);