From f426b12b5bc16689c4d06717fe276fafa9741280 Mon Sep 17 00:00:00 2001 From: David Snopek Date: Thu, 2 Nov 2023 15:17:23 -0500 Subject: [PATCH] Fix comparison of `Callable`s from callable_mp() of the same method --- .../variant/callable_method_pointer.hpp | 90 +++++++++++++------ src/variant/callable_method_pointer.cpp | 60 ++++++++++++- test/project/main.gd | 21 +++++ 3 files changed, 140 insertions(+), 31 deletions(-) diff --git a/include/godot_cpp/variant/callable_method_pointer.hpp b/include/godot_cpp/variant/callable_method_pointer.hpp index 8559253d..159f976d 100644 --- a/include/godot_cpp/variant/callable_method_pointer.hpp +++ b/include/godot_cpp/variant/callable_method_pointer.hpp @@ -37,6 +37,17 @@ namespace godot { class CallableCustomMethodPointerBase : public CallableCustomBase { + uint32_t *comp_ptr = nullptr; + uint32_t comp_size; + uint32_t h; + +protected: + void _setup(uint32_t *p_base_ptr, uint32_t p_ptr_size); + +public: + _FORCE_INLINE_ const uint32_t *get_comp_ptr() const { return comp_ptr; } + _FORCE_INLINE_ uint32_t get_comp_size() const { return comp_size; } + _FORCE_INLINE_ uint32_t get_hash() const { return h; } }; namespace internal { @@ -51,21 +62,26 @@ Callable create_callable_from_ccmp(CallableCustomMethodPointerBase *p_callable_m template class CallableCustomMethodPointer : public CallableCustomMethodPointerBase { - T *instance; - void (T::*method)(P...); + struct Data { + T *instance; + void (T::*method)(P...); + } data; + static_assert(sizeof(Data) % 4 == 0); public: virtual ObjectID get_object() const override { - return ObjectID(instance->get_instance_id()); + return ObjectID(data.instance->get_instance_id()); } virtual void call(const Variant **p_arguments, int p_argcount, Variant &r_return_value, GDExtensionCallError &r_call_error) const override { - call_with_variant_args(instance, method, p_arguments, p_argcount, r_call_error); + call_with_variant_args(data.instance, data.method, p_arguments, p_argcount, r_call_error); } CallableCustomMethodPointer(T *p_instance, void (T::*p_method)(P...)) { - instance = p_instance; - method = p_method; + memset(&data, 0, sizeof(Data)); + data.instance = p_instance; + data.method = p_method; + _setup((uint32_t *)&data, sizeof(Data)); } }; @@ -82,22 +98,27 @@ Callable create_custom_callable_function_pointer(T *p_instance, void (T::*p_meth template class CallableCustomMethodPointerRet : public CallableCustomMethodPointerBase { - T *instance; - R(T::*method) - (P...); + struct Data { + T *instance; + R(T::*method) + (P...); + } data; + static_assert(sizeof(Data) % 4 == 0); public: virtual ObjectID get_object() const override { - return ObjectID(instance->get_instance_id()); + return ObjectID(data.instance->get_instance_id()); } virtual void call(const Variant **p_arguments, int p_argcount, Variant &r_return_value, GDExtensionCallError &r_call_error) const override { - call_with_variant_args_ret(instance, method, p_arguments, p_argcount, r_return_value, r_call_error); + 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...)) { - instance = p_instance; - method = p_method; + memset(&data, 0, sizeof(Data)); + data.instance = p_instance; + data.method = p_method; + _setup((uint32_t *)&data, sizeof(Data)); } }; @@ -114,22 +135,27 @@ Callable create_custom_callable_function_pointer(T *p_instance, R (T::*p_method) template class CallableCustomMethodPointerRetC : public CallableCustomMethodPointerBase { - T *instance; - R(T::*method) - (P...) const; + struct Data { + T *instance; + R(T::*method) + (P...) const; + } data; + static_assert(sizeof(Data) % 4 == 0); public: virtual ObjectID get_object() const override { - return ObjectID(instance->get_instance_id()); + return ObjectID(data.instance->get_instance_id()); } virtual void call(const Variant **p_arguments, int p_argcount, Variant &r_return_value, GDExtensionCallError &r_call_error) const override { - call_with_variant_args_retc(instance, method, p_arguments, p_argcount, r_return_value, r_call_error); + call_with_variant_args_retc(data.instance, data.method, p_arguments, p_argcount, r_return_value, r_call_error); } CallableCustomMethodPointerRetC(const T *p_instance, R (T::*p_method)(P...) const) { - instance = const_cast(p_instance); - method = p_method; + memset(&data, 0, sizeof(Data)); + data.instance = const_cast(p_instance); + data.method = p_method; + _setup((uint32_t *)&data, sizeof(Data)); } }; @@ -146,7 +172,10 @@ Callable create_custom_callable_function_pointer(const T *p_instance, R (T::*p_m template class CallableCustomStaticMethodPointer : public CallableCustomMethodPointerBase { - void (*method)(P...); + struct Data { + void (*method)(P...); + } data; + static_assert(sizeof(Data) % 4 == 0); public: virtual ObjectID get_object() const override { @@ -154,12 +183,14 @@ public: } virtual void call(const Variant **p_arguments, int p_argcount, Variant &r_return_value, GDExtensionCallError &r_call_error) const override { - call_with_variant_args_static_ret(method, p_arguments, p_argcount, r_return_value, r_call_error); + call_with_variant_args_static_ret(data.method, p_arguments, p_argcount, r_return_value, r_call_error); r_return_value = Variant(); } CallableCustomStaticMethodPointer(void (*p_method)(P...)) { - method = p_method; + memset(&data, 0, sizeof(Data)); + data.method = p_method; + _setup((uint32_t *)&data, sizeof(Data)); } }; @@ -176,8 +207,11 @@ Callable create_custom_callable_static_function_pointer(void (*p_method)(P...)) template class CallableCustomStaticMethodPointerRet : public CallableCustomMethodPointerBase { - R(*method) - (P...); + struct Data { + R(*method) + (P...); + } data; + static_assert(sizeof(Data) % 4 == 0); public: virtual ObjectID get_object() const override { @@ -185,11 +219,13 @@ public: } virtual void call(const Variant **p_arguments, int p_argcount, Variant &r_return_value, GDExtensionCallError &r_call_error) const override { - call_with_variant_args_static_ret(method, p_arguments, p_argcount, r_return_value, r_call_error); + call_with_variant_args_static_ret(data.method, p_arguments, p_argcount, r_return_value, r_call_error); } CallableCustomStaticMethodPointerRet(R (*p_method)(P...)) { - method = p_method; + memset(&data, 0, sizeof(Data)); + data.method = p_method; + _setup((uint32_t *)&data, sizeof(Data)); } }; diff --git a/src/variant/callable_method_pointer.cpp b/src/variant/callable_method_pointer.cpp index 10f29205..520ed05a 100644 --- a/src/variant/callable_method_pointer.cpp +++ b/src/variant/callable_method_pointer.cpp @@ -30,18 +30,66 @@ #include +#include + namespace godot { -static void custom_callable_mp_call(void *userdata, const GDExtensionConstVariantPtr *p_args, GDExtensionInt p_argument_count, GDExtensionVariantPtr r_return, GDExtensionCallError *r_error) { - CallableCustomMethodPointerBase *callable_method_pointer = (CallableCustomMethodPointerBase *)userdata; +static void custom_callable_mp_call(void *p_userdata, const GDExtensionConstVariantPtr *p_args, GDExtensionInt p_argument_count, GDExtensionVariantPtr r_return, GDExtensionCallError *r_error) { + CallableCustomMethodPointerBase *callable_method_pointer = (CallableCustomMethodPointerBase *)p_userdata; callable_method_pointer->call((const Variant **)p_args, p_argument_count, *(Variant *)r_return, *r_error); } -static void custom_callable_mp_free(void *userdata) { - CallableCustomMethodPointerBase *callable_method_pointer = (CallableCustomMethodPointerBase *)userdata; +static GDExtensionBool custom_callable_mp_is_valid(void *p_userdata) { + CallableCustomMethodPointerBase *callable_method_pointer = (CallableCustomMethodPointerBase *)p_userdata; + ObjectID object = callable_method_pointer->get_object(); + return object == ObjectID() || ObjectDB::get_instance(object); +} + +static void custom_callable_mp_free(void *p_userdata) { + CallableCustomMethodPointerBase *callable_method_pointer = (CallableCustomMethodPointerBase *)p_userdata; memdelete(callable_method_pointer); } +static uint32_t custom_callable_mp_hash(void *p_userdata) { + CallableCustomMethodPointerBase *callable_method_pointer = (CallableCustomMethodPointerBase *)p_userdata; + return callable_method_pointer->get_hash(); +} + +static GDExtensionBool custom_callable_mp_equal_func(void *p_a, void *p_b) { + CallableCustomMethodPointerBase *a = (CallableCustomMethodPointerBase *)p_a; + CallableCustomMethodPointerBase *b = (CallableCustomMethodPointerBase *)p_b; + + if (a->get_comp_size() != b->get_comp_size()) { + return false; + } + + return memcmp(a->get_comp_ptr(), b->get_comp_ptr(), a->get_comp_size() * 4) == 0; +} + +static GDExtensionBool custom_callable_mp_less_than_func(void *p_a, void *p_b) { + CallableCustomMethodPointerBase *a = (CallableCustomMethodPointerBase *)p_a; + CallableCustomMethodPointerBase *b = (CallableCustomMethodPointerBase *)p_b; + + if (a->get_comp_size() != b->get_comp_size()) { + return a->get_comp_size() < b->get_comp_size(); + } + + return memcmp(a->get_comp_ptr(), b->get_comp_ptr(), a->get_comp_size() * 4) < 0; +} + +void CallableCustomMethodPointerBase::_setup(uint32_t *p_base_ptr, uint32_t p_ptr_size) { + comp_ptr = p_base_ptr; + comp_size = p_ptr_size / 4; + + for (uint32_t i = 0; i < comp_size; i++) { + if (i == 0) { + h = hash_murmur3_one_32(comp_ptr[i]); + } else { + h = hash_murmur3_one_32(comp_ptr[i], h); + } + } +} + namespace internal { Callable create_callable_from_ccmp(CallableCustomMethodPointerBase *p_callable_method_pointer) { @@ -50,7 +98,11 @@ Callable create_callable_from_ccmp(CallableCustomMethodPointerBase *p_callable_m info.token = internal::token; info.object_id = p_callable_method_pointer->get_object(); info.call_func = &custom_callable_mp_call; + info.is_valid_func = &custom_callable_mp_is_valid; info.free_func = &custom_callable_mp_free; + info.hash_func = &custom_callable_mp_hash; + info.equal_func = &custom_callable_mp_equal_func; + info.less_than_func = &custom_callable_mp_less_than_func; Callable callable; ::godot::internal::gdextension_interface_callable_custom_create(callable._native_ptr(), &info); diff --git a/test/project/main.gd b/test/project/main.gd index 74a11683..233ea41d 100644 --- a/test/project/main.gd +++ b/test/project/main.gd @@ -101,9 +101,20 @@ func _ready(): # mp_callable() with void method. var mp_callable: Callable = example.test_callable_mp() + assert_equal(mp_callable.is_valid(), true) mp_callable.call(example, "void", 36) assert_equal(custom_signal_emitted, ["unbound_method1: Example - void", 36]) + # Check that it works with is_connected(). + assert_equal(example.renamed.is_connected(mp_callable), false) + example.renamed.connect(mp_callable) + assert_equal(example.renamed.is_connected(mp_callable), true) + # Make sure a new object is still treated as equivalent. + assert_equal(example.renamed.is_connected(example.test_callable_mp()), true) + assert_equal(mp_callable.hash(), example.test_callable_mp().hash()) + example.renamed.disconnect(mp_callable) + assert_equal(example.renamed.is_connected(mp_callable), false) + # mp_callable() with return value. var mp_callable_ret: Callable = example.test_callable_mp_ret() assert_equal(mp_callable_ret.call(example, "test", 77), "unbound_method2: Example - test - 77") @@ -117,6 +128,16 @@ func _ready(): mp_callable_static.call(example, "static", 83) assert_equal(custom_signal_emitted, ["unbound_static_method1: Example - static", 83]) + # Check that it works with is_connected(). + assert_equal(example.renamed.is_connected(mp_callable_static), false) + example.renamed.connect(mp_callable_static) + assert_equal(example.renamed.is_connected(mp_callable_static), true) + # Make sure a new object is still treated as equivalent. + assert_equal(example.renamed.is_connected(example.test_callable_mp_static()), true) + assert_equal(mp_callable_static.hash(), example.test_callable_mp_static().hash()) + example.renamed.disconnect(mp_callable_static) + assert_equal(example.renamed.is_connected(mp_callable_static), false) + # mp_callable_static() with return value. var mp_callable_static_ret: Callable = example.test_callable_mp_static_ret() assert_equal(mp_callable_static_ret.call(example, "static-ret", 84), "unbound_static_method2: Example - static-ret - 84")