From 4ff8d92ee625677bdd6bbcb577d722e31e88a4a3 Mon Sep 17 00:00:00 2001 From: Raul Santos Date: Thu, 13 Jul 2023 18:37:53 +0200 Subject: [PATCH] C#: Print error when MethodBind call fails --- modules/mono/editor/bindings_generator.cpp | 25 +++++++ modules/mono/mono_gd/gd_mono_internals.cpp | 85 ++++++++++++++++++++++ modules/mono/mono_gd/gd_mono_internals.h | 2 + 3 files changed, 112 insertions(+) diff --git a/modules/mono/editor/bindings_generator.cpp b/modules/mono/editor/bindings_generator.cpp index 2f06f3a9a66..b4661c39b9f 100644 --- a/modules/mono/editor/bindings_generator.cpp +++ b/modules/mono/editor/bindings_generator.cpp @@ -70,6 +70,7 @@ #define CS_FIELD_MEMORYOWN "memoryOwn" #define CS_PARAM_METHODBIND "method" #define CS_PARAM_INSTANCE "ptr" +#define CS_PARAM_CALLER "caller" #define CS_SMETHOD_GETINSTANCE "GetPtr" #define CS_METHOD_CALL "Call" @@ -87,6 +88,7 @@ #define C_NS_MONOINTERNALS "GDMonoInternals" #define C_METHOD_TIE_MANAGED_TO_UNMANAGED C_NS_MONOINTERNALS "::tie_managed_to_unmanaged" #define C_METHOD_UNMANAGED_GET_MANAGED C_NS_MONOUTILS "::unmanaged_get_managed" +#define C_METHOD_CHECK_CALL_ERROR C_NS_MONOINTERNALS "::check_call_error" #define C_NS_MONOMARSHAL "GDMonoMarshal" #define C_METHOD_MANAGED_TO_VARIANT C_NS_MONOMARSHAL "::mono_object_to_variant" @@ -774,6 +776,11 @@ void BindingsGenerator::_generate_method_icalls(const TypeInterface &p_itype) { i++; } + // Collect caller name for MethodBind + if (imethod.is_vararg) { + im_sig += ", string " CS_PARAM_CALLER; + } + String im_type_out = return_type->im_type_out; if (return_type->ret_as_byref_arg) { @@ -1658,6 +1665,10 @@ Error BindingsGenerator::_generate_cs_method(const BindingsGenerator::TypeInterf } } + if (p_imethod.is_vararg) { + icall_params += ", \"" + p_imethod.cname + "\""; + } + // Generate method { if (!p_imethod.is_virtual && !p_imethod.requires_object_call) { @@ -2002,6 +2013,11 @@ Error BindingsGenerator::_generate_glue_method(const BindingsGenerator::TypeInte i++; } + // Collect caller name for MethodBind + if (p_imethod.is_vararg) { + c_func_sig += ", MonoString* " CS_PARAM_CALLER; + } + if (return_type->ret_as_byref_arg) { c_func_sig += ", "; c_func_sig += return_type->c_type_in; @@ -2115,6 +2131,15 @@ Error BindingsGenerator::_generate_glue_method(const BindingsGenerator::TypeInte p_output.append(p_imethod.arguments.size() ? C_LOCAL_PTRCALL_ARGS ".ptr()" : "NULL"); p_output.append(", total_length, vcall_error);\n"); + p_output.append("#ifdef DEBUG_ENABLED\n"); + p_output.append("\tVariant instance_variant = Variant(" CS_PARAM_INSTANCE ");\n"); + p_output.append("\t" C_METHOD_CHECK_CALL_ERROR "("); + p_output.append(C_METHOD_MONOSTR_TO_GODOT "(" CS_PARAM_CALLER ")"); + p_output.append(", &instance_variant, "); + p_output.append(p_imethod.arguments.size() ? C_LOCAL_PTRCALL_ARGS ".ptr()" : "NULL"); + p_output.append(", total_length, vcall_error);\n"); + p_output.append("#endif // DEBUG_ENABLED\n"); + if (!ret_void) { // See the comment on the C_LOCAL_VARARG_RET declaration if (return_type->cname != name_cache.type_Variant) { diff --git a/modules/mono/mono_gd/gd_mono_internals.cpp b/modules/mono/mono_gd/gd_mono_internals.cpp index 4c9a6d52281..67c0841ef93 100644 --- a/modules/mono/mono_gd/gd_mono_internals.cpp +++ b/modules/mono/mono_gd/gd_mono_internals.cpp @@ -139,4 +139,89 @@ void gd_unhandled_exception_event(MonoException *p_exc) { args[0] = p_exc; mono_runtime_invoke(unhandled_exception_method, nullptr, (void **)args, nullptr); } + +#if DEBUG_ENABLED +static String _get_var_type(const Variant *p_var) { + String basestr; + + if (p_var->get_type() == Variant::OBJECT) { + Object *bobj = *p_var; + if (!bobj) { + if (p_var->is_invalid_object()) { + basestr = "previously freed instance"; + } else { + basestr = "null instance"; + } + } else { + if (bobj->get_script_instance()) { + basestr = bobj->get_class() + " (" + bobj->get_script_instance()->get_script()->get_path().get_file() + ")"; + } else { + basestr = bobj->get_class(); + } + } + + } else { + basestr = Variant::get_type_name(p_var->get_type()); + } + + return basestr; +} + +static String _get_call_where(const String &p_method, const Variant *p_instance, const Variant **argptrs, int argc) { + String methodstr = p_method; + String basestr = _get_var_type(p_instance); + + if (methodstr == "call") { + if (argc >= 1) { + methodstr = String(*argptrs[0]) + " (via call)"; + } + } else if (methodstr == "call_recursive" && basestr == "TreeItem") { + if (argc >= 1) { + methodstr = String(*argptrs[0]) + " (via TreeItem.call_recursive)"; + } + } + return "function '" + methodstr + "' in base '" + basestr + "'"; +} + +static String _get_call_error(const Variant::CallError &p_err, const String &p_where, const Variant **argptrs) { + String err_text; + + if (p_err.error == Variant::CallError::CALL_ERROR_INVALID_ARGUMENT) { + int errorarg = p_err.argument; + // Handle the Object to Object case separately as we don't have further class details. +#ifdef DEBUG_ENABLED + if (p_err.expected == Variant::OBJECT && argptrs[errorarg]->get_type() == p_err.expected) { + err_text = "Invalid type in " + p_where + ". The Object-derived class of argument " + itos(errorarg + 1) + " (" + _get_var_type(argptrs[errorarg]) + ") is not a subclass of the expected argument class."; + } else +#endif // DEBUG_ENABLED + { + err_text = "Invalid type in " + p_where + ". Cannot convert argument " + itos(errorarg + 1) + " from " + Variant::get_type_name(argptrs[errorarg]->get_type()) + " to " + Variant::get_type_name(p_err.expected) + "."; + } + } else if (p_err.error == Variant::CallError::CALL_ERROR_TOO_MANY_ARGUMENTS) { + err_text = "Invalid call to " + p_where + ". Expected " + itos(p_err.argument) + " arguments."; + } else if (p_err.error == Variant::CallError::CALL_ERROR_TOO_FEW_ARGUMENTS) { + err_text = "Invalid call to " + p_where + ". Expected " + itos(p_err.argument) + " arguments."; + } else if (p_err.error == Variant::CallError::CALL_ERROR_INVALID_METHOD) { + err_text = "Invalid call. Nonexistent " + p_where + "."; + } else if (p_err.error == Variant::CallError::CALL_ERROR_INSTANCE_IS_NULL) { + err_text = "Attempt to call " + p_where + " on a null instance."; + } else { + err_text = "Bug, call error: #" + itos(p_err.error); + } + + return err_text; +} + +void check_call_error(const String &p_method, const Variant *p_instance, const Variant **p_args, int p_arg_count, const Variant::CallError &p_error) { + if (p_error.error == Variant::CallError::CALL_OK) { + // The call was successful. + return; + } + + const String &where = _get_call_where(p_method, p_instance, p_args, p_arg_count); + ERR_PRINT(_get_call_error(p_error, where, p_args)); +} +#else +void check_call_error(const String &p_method, const Variant &p_instance, const Variant **p_args, int p_arg_count, const Variant::CallError &p_error) {} +#endif } // namespace GDMonoInternals diff --git a/modules/mono/mono_gd/gd_mono_internals.h b/modules/mono/mono_gd/gd_mono_internals.h index b9667cbd8af..d2d1c8cec09 100644 --- a/modules/mono/mono_gd/gd_mono_internals.h +++ b/modules/mono/mono_gd/gd_mono_internals.h @@ -47,6 +47,8 @@ void tie_managed_to_unmanaged(MonoObject *managed, Object *unmanaged); void unhandled_exception(MonoException *p_exc); void gd_unhandled_exception_event(MonoException *p_exc); + +void check_call_error(const String &p_method, const Variant *p_instance, const Variant **p_args, int p_arg_count, const Variant::CallError &p_error); } // namespace GDMonoInternals #endif // GD_MONO_INTERNALS_H