From 3ad694018fe02447d9ad3051b9d2f6616bc52e74 Mon Sep 17 00:00:00 2001 From: PouleyKetchoupp Date: Fri, 1 May 2020 18:41:41 +0200 Subject: [PATCH] Fix Container sorting not working when overriding _sort_children in gdscript Remove _sort_children from script bindings: _sort_children is an internal method which shouldn't be exposed to scripts. Added support for non-bound methods in MessageQueue: So we can use deferred calls without exposing internal methods to scripts. Added debug checks in CallableCustomMethodPointer: Adding method pointer callables to the message queue was causing crashes in case an object was destroyed and the same memory was allocated for another one. The new object had a valid object id but the call was erroneous. Release will be fixed later, along with Variant which has the same problem and is also fixed for debug only. --- core/callable_method_pointer.h | 36 ++++++++++++++++++++++++++++++++-- core/message_queue.cpp | 15 ++++++++++++++ core/message_queue.h | 1 + scene/gui/container.cpp | 4 +--- 4 files changed, 51 insertions(+), 5 deletions(-) diff --git a/core/callable_method_pointer.h b/core/callable_method_pointer.h index 3b0503e2595..22db7d1c827 100644 --- a/core/callable_method_pointer.h +++ b/core/callable_method_pointer.h @@ -161,19 +161,35 @@ template class CallableCustomMethodPointer : public CallableCustomMethodPointerBase { struct Data { T *instance; +#ifdef DEBUG_ENABLED + uint64_t object_id; +#endif void (T::*method)(P...); } data; public: - virtual ObjectID get_object() const { return data.instance->get_instance_id(); } + virtual ObjectID get_object() const { +#ifdef DEBUG_ENABLED + if (ObjectDB::get_instance(ObjectID(data.object_id)) == nullptr) { + return ObjectID(); + } +#endif + return data.instance->get_instance_id(); + } virtual void call(const Variant **p_arguments, int p_argcount, Variant &r_return_value, Callable::CallError &r_call_error) const { +#ifdef DEBUG_ENABLED + ERR_FAIL_COND_MSG(ObjectDB::get_instance(ObjectID(data.object_id)) == nullptr, "Invalid Object id '" + uitos(data.object_id) + "', can't call method."); +#endif call_with_variant_args(data.instance, data.method, p_arguments, p_argcount, r_call_error); } CallableCustomMethodPointer(T *p_instance, void (T::*p_method)(P...)) { zeromem(&data, sizeof(Data)); // Clear beforehand, may have padding bytes. data.instance = p_instance; +#ifdef DEBUG_ENABLED + data.object_id = p_instance->get_instance_id(); +#endif data.method = p_method; _setup((uint32_t *)&data, sizeof(Data)); } @@ -242,20 +258,36 @@ template class CallableCustomMethodPointerRet : public CallableCustomMethodPointerBase { struct Data { T *instance; +#ifdef DEBUG_ENABLED + uint64_t object_id; +#endif R(T::*method) (P...); } data; public: - virtual ObjectID get_object() const { return data.instance->get_instance_id(); } + virtual ObjectID get_object() const { +#ifdef DEBUG_ENABLED + if (ObjectDB::get_instance(ObjectID(data.object_id)) == nullptr) { + return ObjectID(); + } +#endif + return data.instance->get_instance_id(); + } virtual void call(const Variant **p_arguments, int p_argcount, Variant &r_return_value, Callable::CallError &r_call_error) const { +#ifdef DEBUG_ENABLED + ERR_FAIL_COND_MSG(ObjectDB::get_instance(ObjectID(data.object_id)) == nullptr, "Invalid Object id '" + uitos(data.object_id) + "', can't call method."); +#endif call_with_variant_args_ret(data.instance, data.method, p_arguments, p_argcount, r_return_value, r_call_error); } CallableCustomMethodPointerRet(T *p_instance, R (T::*p_method)(P...)) { zeromem(&data, sizeof(Data)); // Clear beforehand, may have padding bytes. data.instance = p_instance; +#ifdef DEBUG_ENABLED + data.object_id = p_instance->get_instance_id(); +#endif data.method = p_method; _setup((uint32_t *)&data, sizeof(Data)); } diff --git a/core/message_queue.cpp b/core/message_queue.cpp index 8c71f760b28..6dcf24e7ede 100644 --- a/core/message_queue.cpp +++ b/core/message_queue.cpp @@ -155,6 +155,21 @@ Error MessageQueue::push_callable(const Callable &p_callable, const Variant **p_ return OK; } +Error MessageQueue::push_callable(const Callable &p_callable, VARIANT_ARG_DECLARE) { + VARIANT_ARGPTRS; + + int argc = 0; + + for (int i = 0; i < VARIANT_ARG_MAX; i++) { + if (argptr[i]->get_type() == Variant::NIL) { + break; + } + argc++; + } + + return push_callable(p_callable, argptr, argc); +} + void MessageQueue::statistics() { Map set_count; Map notify_count; diff --git a/core/message_queue.h b/core/message_queue.h index 8e50f1b2b7e..7d13e26208f 100644 --- a/core/message_queue.h +++ b/core/message_queue.h @@ -79,6 +79,7 @@ public: Error push_notification(ObjectID p_id, int p_notification); Error push_set(ObjectID p_id, const StringName &p_prop, const Variant &p_value); Error push_callable(const Callable &p_callable, const Variant **p_args, int p_argcount, bool p_show_error = false); + Error push_callable(const Callable &p_callable, VARIANT_ARG_LIST); Error push_call(Object *p_object, const StringName &p_method, VARIANT_ARG_LIST); Error push_notification(Object *p_object, int p_notification); diff --git a/scene/gui/container.cpp b/scene/gui/container.cpp index 18a84ce3484..a89eef62091 100644 --- a/scene/gui/container.cpp +++ b/scene/gui/container.cpp @@ -140,7 +140,7 @@ void Container::queue_sort() { return; } - MessageQueue::get_singleton()->push_call(this, "_sort_children"); + MessageQueue::get_singleton()->push_callable(callable_mp(this, &Container::_sort_children)); pending_sort = true; } @@ -177,8 +177,6 @@ String Container::get_configuration_warning() const { } void Container::_bind_methods() { - ClassDB::bind_method(D_METHOD("_sort_children"), &Container::_sort_children); - ClassDB::bind_method(D_METHOD("queue_sort"), &Container::queue_sort); ClassDB::bind_method(D_METHOD("fit_child_in_rect", "child", "rect"), &Container::fit_child_in_rect);