-
-
Notifications
You must be signed in to change notification settings - Fork 586
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed pointer indirection in the PtrToArg template for Object arguments. #677
Conversation
This patch fixes the similar case #655 for --- a/include/godot_cpp/classes/ref.hpp
+++ b/include/godot_cpp/classes/ref.hpp
@@ -240,7 +240,7 @@ public:
template <class T>
struct PtrToArg<Ref<T>> {
_FORCE_INLINE_ static Ref<T> convert(const void *p_ptr) {
- return Ref<T>(godot::internal::gdn_interface->object_get_instance_binding((void *)p_ptr, godot::internal::token, &T::___binding_callbacks));
+ return Ref<T>(reinterpret_cast<T*>(godot::internal::gdn_interface->object_get_instance_binding(*(const GDNativeObjectPtr *)p_ptr, godot::internal::token, &T::___binding_callbacks)));
}
typedef Ref<T> EncodeT;
@@ -255,7 +255,7 @@ struct PtrToArg<const Ref<T> &> {
typedef Ref<T> EncodeT;
_FORCE_INLINE_ static Ref<T> convert(const void *p_ptr) {
- return Ref<T>(godot::internal::gdn_interface->object_get_instance_binding((void *)p_ptr, godot::internal::token, &T::___binding_callbacks));
+ return Ref<T>(reinterpret_cast<T*>(godot::internal::gdn_interface->object_get_instance_binding(*(const GDNativeObjectPtr *)p_ptr, godot::internal::token, &T::___binding_callbacks)));
}
}; @lukas-toenne Could you include this in your PR? |
Patch by Nana Sakisaka (saki7).
Thanks! |
I can also confirm that these two commits fix #655 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It took me a bit to fully understand what was going on, but this is correct.
Really excellent work both 🏅 🎆 !
And sorry for the very late review :/ it's been some crazy times, but we are finally moving into stabilization :).
Note to self, we should probably add a test for this, I had one, but it's not great for CI, maybe an |
Object pointer arguments for virtual function callbacks are not correctly converted by the
PtrToArg
template in godot-cpp. It is missing one level of pointer indirection, passing the void* wrapper directly to thegdnative_object_get_instance_binding
implementation, which expects aObject*
.This patch adds the additional pointer dereference, which brings it in line with the core Godot
PtrToArg
template.This fixes #651