From 63369ec3066195a7d282d2345beecb50bfd7de3d Mon Sep 17 00:00:00 2001 From: Ignacio Etcheverry Date: Mon, 16 Oct 2017 03:54:23 +0200 Subject: [PATCH] Re-write SignalAwaiter implementation Old implementation had issues where you could only await on the same signal of the same source once. --- modules/mono/csharp_script.cpp | 44 +------------ modules/mono/csharp_script.h | 4 +- modules/mono/editor/bindings_generator.cpp | 4 -- modules/mono/mono_gd/gd_mono_utils.cpp | 4 +- modules/mono/mono_gd/gd_mono_utils.h | 4 +- modules/mono/signal_awaiter_utils.cpp | 75 ++++++++++++++++++++-- modules/mono/signal_awaiter_utils.h | 19 +++++- 7 files changed, 97 insertions(+), 57 deletions(-) diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp index fe78ce423c1..e0e53b03c7e 100644 --- a/modules/mono/csharp_script.cpp +++ b/modules/mono/csharp_script.cpp @@ -48,7 +48,7 @@ #include "mono_gd/gd_mono_marshal.h" #include "signal_awaiter_utils.h" -#define CACHED_STRING_NAME(m_var) (CSharpLanguage::get_singleton()->string_names.m_var) +#define CACHED_STRING_NAME(m_var) (CSharpLanguage::get_singleton()->get_string_names().m_var) static bool _create_project_solution_if_needed() { @@ -896,46 +896,6 @@ Variant CSharpInstance::call(const StringName &p_method, const Variant **p_args, } else { return Variant(); } - } else if (p_method == CACHED_STRING_NAME(_awaited_signal_callback)) { - // shitty hack.. - // TODO move to its own function, thx - - if (p_argcount < 1) { - r_error.error = Variant::CallError::CALL_ERROR_TOO_FEW_ARGUMENTS; - r_error.argument = 1; - return Variant(); - } - - Ref awaiter = *p_args[p_argcount - 1]; - - if (awaiter.is_null()) { - r_error.error = Variant::CallError::CALL_ERROR_INVALID_ARGUMENT; - r_error.argument = p_argcount - 1; - r_error.expected = Variant::OBJECT; - return Variant(); - } - - awaiter->set_completed(true); - - int extra_argc = p_argcount - 1; - MonoArray *extra_args = mono_array_new(SCRIPTS_DOMAIN, CACHED_CLASS_RAW(MonoObject), extra_argc); - - for (int i = 0; i < extra_argc; i++) { - MonoObject *boxed = GDMonoMarshal::variant_to_mono_object(*p_args[i]); - mono_array_set(extra_args, MonoObject *, i, boxed); - } - - GDMonoUtils::GodotObject__AwaitedSignalCallback thunk = CACHED_METHOD_THUNK(GodotObject, _AwaitedSignalCallback); - - MonoObject *ex = NULL; - thunk(mono_object, &extra_args, awaiter->get_target(), &ex); - - if (ex) { - mono_print_unhandled_exception(ex); - ERR_FAIL_V(Variant()); - } - - return Variant(); } top = top->get_parent_class(); @@ -1908,7 +1868,7 @@ bool ResourceFormatSaverCSharpScript::recognize(const RES &p_resource) const { CSharpLanguage::StringNameCache::StringNameCache() { - _awaited_signal_callback = StaticCString::create("_AwaitedSignalCallback"); + _signal_callback = StaticCString::create("_signal_callback"); _set = StaticCString::create("_set"); _get = StaticCString::create("_get"); _notification = StaticCString::create("_notification"); diff --git a/modules/mono/csharp_script.h b/modules/mono/csharp_script.h index 3fcc3bdf047..6b8475fb61e 100644 --- a/modules/mono/csharp_script.h +++ b/modules/mono/csharp_script.h @@ -225,7 +225,7 @@ class CSharpLanguage : public ScriptLanguage { struct StringNameCache { - StringName _awaited_signal_callback; + StringName _signal_callback; StringName _set; StringName _get; StringName _notification; @@ -242,6 +242,8 @@ public: _FORCE_INLINE_ int get_language_index() { return lang_idx; } void set_language_index(int p_idx); + _FORCE_INLINE_ const StringNameCache &get_string_names() { return string_names; } + _FORCE_INLINE_ static CSharpLanguage *get_singleton() { return singleton; } bool debug_break(const String &p_error, bool p_allow_continue = true); diff --git a/modules/mono/editor/bindings_generator.cpp b/modules/mono/editor/bindings_generator.cpp index 123f00ea100..5cb38c3301e 100644 --- a/modules/mono/editor/bindings_generator.cpp +++ b/modules/mono/editor/bindings_generator.cpp @@ -916,10 +916,6 @@ Error BindingsGenerator::_generate_cs_type(const TypeInterface &itype, const Str return ERR_BUG; } - cs_file.push_back(MEMBER_BEGIN "private void _AwaitedSignalCallback("); - cs_file.push_back(array_itype->get().cs_type); - cs_file.push_back(" args, SignalAwaiter awaiter)\n" OPEN_BLOCK_L2 "awaiter.SignalCallback(args);\n" CLOSE_BLOCK_L2); - Map::Element *object_itype = obj_types.find("Object"); if (!object_itype) { diff --git a/modules/mono/mono_gd/gd_mono_utils.cpp b/modules/mono/mono_gd/gd_mono_utils.cpp index 5deca8e64d8..d23fcf8a67e 100644 --- a/modules/mono/mono_gd/gd_mono_utils.cpp +++ b/modules/mono/mono_gd/gd_mono_utils.cpp @@ -111,7 +111,7 @@ void MonoCache::clear_members() { methodthunk_MarshalUtils_DictionaryToArrays = NULL; methodthunk_MarshalUtils_ArraysToDictionary = NULL; - methodthunk_GodotObject__AwaitedSignalCallback = NULL; + methodthunk_SignalAwaiter_SignalCallback = NULL; methodthunk_SignalAwaiter_FailureCallback = NULL; methodthunk_GodotTaskScheduler_Activate = NULL; @@ -178,7 +178,7 @@ void update_godot_api_cache() { CACHE_METHOD_THUNK_AND_CHECK(MarshalUtils, DictionaryToArrays, (MarshalUtils_DictToArrays)CACHED_CLASS(MarshalUtils)->get_method("DictionaryToArrays", 3)->get_thunk()); CACHE_METHOD_THUNK_AND_CHECK(MarshalUtils, ArraysToDictionary, (MarshalUtils_ArraysToDict)CACHED_CLASS(MarshalUtils)->get_method("ArraysToDictionary", 2)->get_thunk()); - CACHE_METHOD_THUNK_AND_CHECK(GodotObject, _AwaitedSignalCallback, (GodotObject__AwaitedSignalCallback)CACHED_CLASS(GodotObject)->get_method("_AwaitedSignalCallback", 2)->get_thunk()); + CACHE_METHOD_THUNK_AND_CHECK(SignalAwaiter, SignalCallback, (SignalAwaiter_SignalCallback)GODOT_API_CLASS(SignalAwaiter)->get_method("SignalCallback", 1)->get_thunk()); CACHE_METHOD_THUNK_AND_CHECK(SignalAwaiter, FailureCallback, (SignalAwaiter_FailureCallback)GODOT_API_CLASS(SignalAwaiter)->get_method("FailureCallback", 0)->get_thunk()); CACHE_METHOD_THUNK_AND_CHECK(GodotTaskScheduler, Activate, (GodotTaskScheduler_Activate)GODOT_API_CLASS(GodotTaskScheduler)->get_method("Activate", 0)->get_thunk()); diff --git a/modules/mono/mono_gd/gd_mono_utils.h b/modules/mono/mono_gd/gd_mono_utils.h index f97f048aa93..bbda3a01d41 100644 --- a/modules/mono/mono_gd/gd_mono_utils.h +++ b/modules/mono/mono_gd/gd_mono_utils.h @@ -42,7 +42,7 @@ namespace GDMonoUtils { typedef MonoObject *(*MarshalUtils_DictToArrays)(MonoObject *, MonoArray **, MonoArray **, MonoObject **); typedef MonoObject *(*MarshalUtils_ArraysToDict)(MonoArray *, MonoArray *, MonoObject **); -typedef MonoObject *(*GodotObject__AwaitedSignalCallback)(MonoObject *, MonoArray **, MonoObject *, MonoObject **); +typedef MonoObject *(*SignalAwaiter_SignalCallback)(MonoObject *, MonoArray **, MonoObject **); typedef MonoObject *(*SignalAwaiter_FailureCallback)(MonoObject *, MonoObject **); typedef MonoObject *(*GodotTaskScheduler_Activate)(MonoObject *, MonoObject **); @@ -113,7 +113,7 @@ struct MonoCache { MarshalUtils_DictToArrays methodthunk_MarshalUtils_DictionaryToArrays; MarshalUtils_ArraysToDict methodthunk_MarshalUtils_ArraysToDictionary; - GodotObject__AwaitedSignalCallback methodthunk_GodotObject__AwaitedSignalCallback; + SignalAwaiter_SignalCallback methodthunk_SignalAwaiter_SignalCallback; SignalAwaiter_FailureCallback methodthunk_SignalAwaiter_FailureCallback; GodotTaskScheduler_Activate methodthunk_GodotTaskScheduler_Activate; diff --git a/modules/mono/signal_awaiter_utils.cpp b/modules/mono/signal_awaiter_utils.cpp index 012dd119b1b..99bcc72b41c 100644 --- a/modules/mono/signal_awaiter_utils.cpp +++ b/modules/mono/signal_awaiter_utils.cpp @@ -29,6 +29,9 @@ /*************************************************************************/ #include "signal_awaiter_utils.h" +#include "csharp_script.h" +#include "mono_gd/gd_mono_class.h" +#include "mono_gd/gd_mono_marshal.h" #include "mono_gd/gd_mono_utils.h" namespace SignalAwaiterUtils { @@ -40,13 +43,20 @@ Error connect_signal_awaiter(Object *p_source, const String &p_signal, Object *p uint32_t awaiter_handle = MonoGCHandle::make_strong_handle(p_awaiter); Ref sa_con = memnew(SignalAwaiterHandle(awaiter_handle)); +#ifdef DEBUG_ENABLED + sa_con->set_connection_target(p_target); +#endif + Vector binds; binds.push_back(sa_con); - Error err = p_source->connect(p_signal, p_target, "_AwaitedSignalCallback", binds, Object::CONNECT_ONESHOT); + + Error err = p_source->connect(p_signal, sa_con.ptr(), + CSharpLanguage::get_singleton()->get_string_names()._signal_callback, + binds, Object::CONNECT_ONESHOT); if (err != OK) { - // set it as completed to prevent it from calling the failure callback when deleted - // the awaiter will be aware of the failure by checking the returned error + // Set it as completed to prevent it from calling the failure callback when released. + // The awaiter will be aware of the failure by checking the returned error. sa_con->set_completed(true); } @@ -54,11 +64,66 @@ Error connect_signal_awaiter(Object *p_source, const String &p_signal, Object *p } } -SignalAwaiterHandle::SignalAwaiterHandle(uint32_t p_handle) - : MonoGCHandle(p_handle) { +Variant SignalAwaiterHandle::_signal_callback(const Variant **p_args, int p_argcount, Variant::CallError &r_error) { + +#ifdef DEBUG_ENABLED + if (conn_target_id && !ObjectDB::get_instance(conn_target_id)) { + ERR_EXPLAIN("Resumed after await, but class instance is gone"); + ERR_FAIL_V(Variant()); + } +#endif + + if (p_argcount < 1) { + r_error.error = Variant::CallError::CALL_ERROR_TOO_FEW_ARGUMENTS; + r_error.argument = 1; + return Variant(); + } + + Ref self = *p_args[p_argcount - 1]; + + if (self.is_null()) { + r_error.error = Variant::CallError::CALL_ERROR_INVALID_ARGUMENT; + r_error.argument = p_argcount - 1; + r_error.expected = Variant::OBJECT; + return Variant(); + } + + set_completed(true); + + int signal_argc = p_argcount - 1; + MonoArray *signal_args = mono_array_new(SCRIPTS_DOMAIN, CACHED_CLASS_RAW(MonoObject), signal_argc); + + for (int i = 0; i < signal_argc; i++) { + MonoObject *boxed = GDMonoMarshal::variant_to_mono_object(*p_args[i]); + mono_array_set(signal_args, MonoObject *, i, boxed); + } + + GDMonoUtils::SignalAwaiter_SignalCallback thunk = CACHED_METHOD_THUNK(SignalAwaiter, SignalCallback); + + MonoObject *ex = NULL; + thunk(get_target(), &signal_args, &ex); + + if (ex) { + mono_print_unhandled_exception(ex); + ERR_FAIL_V(Variant()); + } + + return Variant(); +} + +void SignalAwaiterHandle::_bind_methods() { + + ClassDB::bind_vararg_method(METHOD_FLAGS_DEFAULT, "_signal_callback", &SignalAwaiterHandle::_signal_callback, MethodInfo("_signal_callback")); +} + +SignalAwaiterHandle::SignalAwaiterHandle(uint32_t p_managed_handle) + : MonoGCHandle(p_managed_handle) { + + conn_target_id = 0; } SignalAwaiterHandle::~SignalAwaiterHandle() { + if (!completed) { GDMonoUtils::SignalAwaiter_FailureCallback thunk = CACHED_METHOD_THUNK(SignalAwaiter, FailureCallback); diff --git a/modules/mono/signal_awaiter_utils.h b/modules/mono/signal_awaiter_utils.h index 422ed4754f9..0d615b58265 100644 --- a/modules/mono/signal_awaiter_utils.h +++ b/modules/mono/signal_awaiter_utils.h @@ -40,13 +40,30 @@ Error connect_signal_awaiter(Object *p_source, const String &p_signal, Object *p class SignalAwaiterHandle : public MonoGCHandle { + GDCLASS(SignalAwaiterHandle, MonoGCHandle) + bool completed; +#ifdef DEBUG_ENABLED + ObjectID conn_target_id; +#endif + + Variant _signal_callback(const Variant **p_args, int p_argcount, Variant::CallError &r_error); + +protected: + static void _bind_methods(); + public: _FORCE_INLINE_ bool is_completed() { return completed; } _FORCE_INLINE_ void set_completed(bool p_completed) { completed = p_completed; } - SignalAwaiterHandle(uint32_t p_handle); +#ifdef DEBUG_ENABLED + _FORCE_INLINE_ void set_connection_target(Object *p_target) { + conn_target_id = p_target->get_instance_id(); + } +#endif + + SignalAwaiterHandle(uint32_t p_managed_handle); ~SignalAwaiterHandle(); };