Re-enable Variant conversions for ObjectArg; extract code shared with RawGd

Author: Bromeon

godot-core/src/obj/object_arg.rs

@@ -8,7 +8,7 @@
 use crate::builtin::Variant;
 use crate::meta::error::ConvertError;
 use crate::meta::{ClassName, FromGodot, GodotConvert, GodotFfiVariant, GodotType, ToGodot};
-use crate::obj::{bounds, Bounds, Gd, GodotClass, Inherits, RawGd};
+use crate::obj::{bounds, raw_gd, Bounds, Gd, GodotClass, Inherits, RawGd};
 use crate::sys;
 use godot_ffi::{GodotFfi, GodotNullableFfi, PtrcallType};
 use std::ptr;
@@ -172,16 +172,7 @@ where
     // https://github.com/godotengine/godot-cpp/issues/954
 
     fn as_arg_ptr(&self) -> sys::GDExtensionConstTypePtr {
-        // See RawGd::as_arg_ptr().
-        #[cfg(before_api = "4.1")]
-        {
-            self.sys()
-        }
-
-        #[cfg(since_api = "4.1")]
-        {
-            ptr::addr_of!(self.object_ptr) as sys::GDExtensionConstTypePtr
-        }
+        raw_gd::object_as_arg_ptr(self, &self.object_ptr)
     }
 
     unsafe fn from_arg_ptr(_ptr: sys::GDExtensionTypePtr, _call_type: PtrcallType) -> Self {
@@ -240,7 +231,8 @@ impl<T: GodotClass> GodotType for ObjectArg<T> {
 
 impl<T: GodotClass> GodotFfiVariant for ObjectArg<T> {
     fn ffi_to_variant(&self) -> Variant {
-        unreachable!("ObjectArg::ffi_to_variant() is not expected to be called.")
+        // Note: currently likely not invoked since there are no known varcall APIs taking Object parameters; however this might change.
+        raw_gd::object_ffi_to_variant(self)
     }
 
     fn ffi_from_variant(_variant: &Variant) -> Result<Self, ConvertError> {

godot-core/src/obj/raw_gd.rs

@@ -480,27 +480,9 @@ where
     // https://github.com/godotengine/godot-cpp/issues/954
 
     fn as_arg_ptr(&self) -> sys::GDExtensionConstTypePtr {
-        // Do not extract this code. Address of field pointer matters, copying it into a local variable will create use-after-free.
+        // Even though ObjectArg exists, this function is still relevant, e.g. in Callable.
 
-        // No need to call self.check_rtti("as_arg_ptr") here, since this is already done in ToGodot impl.
-
-        // We pass an object to a Godot API. If the reference count needs to be incremented, then the callee (Godot C++ function) will do so.
-        // We do not need to prematurely do so. In Rust terms, if `T` is ref-counted, then we are effectively passing a `&Arc<T>`, and the
-        // callee would need to invoke `.clone()` if desired.
-
-        // In 4.0, argument pointers are passed to godot as `T*`, except for in virtual method calls. We can't perform virtual method calls
-        // currently, so they are always `T*`.
-        //
-        // In 4.1, argument pointers were standardized to always be `T**`.
-        #[cfg(before_api = "4.1")]
-        {
-            self.sys()
-        }
-
-        #[cfg(since_api = "4.1")]
-        {
-            ptr::addr_of!(self.obj) as sys::GDExtensionConstTypePtr
-        }
+        object_as_arg_ptr(self, &self.obj)
     }
 
     unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, call_type: PtrcallType) -> Self {
@@ -581,23 +563,7 @@ impl<T: GodotClass> GodotType for RawGd<T> {
 
 impl<T: GodotClass> GodotFfiVariant for RawGd<T> {
     fn ffi_to_variant(&self) -> Variant {
-        // The conversion method `object_to_variant` DOES increment the reference-count of the object; so nothing to do here.
-        // (This behaves differently in the opposite direction `variant_to_object`.)
-
-        unsafe {
-            Variant::new_with_var_uninit(|variant_ptr| {
-                let converter = sys::builtin_fn!(object_to_variant);
-
-                // Note: this is a special case because of an inconsistency in Godot, where sometimes the equivalency is
-                // GDExtensionTypePtr == Object** and sometimes GDExtensionTypePtr == Object*. Here, it is the former, thus extra pointer.
-                // Reported at https://github.com/godotengine/godot/issues/61967
-                let type_ptr = self.sys();
-                converter(
-                    variant_ptr,
-                    ptr::addr_of!(type_ptr) as sys::GDExtensionTypePtr,
-                );
-            })
-        }
+        object_ffi_to_variant(self)
     }
 
     fn ffi_from_variant(variant: &Variant) -> Result<Self, ConvertError> {
@@ -681,6 +647,9 @@ impl<T: GodotClass> fmt::Debug for RawGd<T> {
     }
 }
 
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+// Reusable functions, also shared with Gd, Variant, ObjectArg.
+
 /// Runs `init_fn` on the address of a pointer (initialized to null), then returns that pointer, possibly still null.
 ///
 /// # Safety
@@ -699,3 +668,50 @@ pub unsafe fn raw_object_init(
     // We don't need to know if Object** is null, but if Object* is null; return_ptr has the address of a local (never null).
     object_ptr
 }
+
+pub(super) fn object_ffi_to_variant<T: GodotFfi>(self_: &T) -> Variant {
+    // The conversion method `object_to_variant` DOES increment the reference-count of the object; so nothing to do here.
+    // (This behaves differently in the opposite direction `variant_to_object`.)
+
+    unsafe {
+        Variant::new_with_var_uninit(|variant_ptr| {
+            let converter = sys::builtin_fn!(object_to_variant);
+
+            // Note: this is a special case because of an inconsistency in Godot, where sometimes the equivalency is
+            // GDExtensionTypePtr == Object** and sometimes GDExtensionTypePtr == Object*. Here, it is the former, thus extra pointer.
+            // Reported at https://github.com/godotengine/godot/issues/61967
+            let type_ptr = self_.sys();
+            converter(
+                variant_ptr,
+                ptr::addr_of!(type_ptr) as sys::GDExtensionTypePtr,
+            );
+        })
+    }
+}
+
+pub(super) fn object_as_arg_ptr<T: GodotFfi, F>(
+    _self: &T,
+    _object_ptr_field: &*mut F,
+) -> sys::GDExtensionConstTypePtr {
+    // Be careful when refactoring this code. Address of field pointer matters, copying it into a local variable will create use-after-free.
+
+    // No need to call self.check_rtti("as_arg_ptr") here, since this is already done in ToGodot impl.
+
+    // We pass an object to a Godot API. If the reference count needs to be incremented, then the callee (Godot C++ function) will do so.
+    // We do not need to prematurely do so. In Rust terms, if `T` is ref-counted, then we are effectively passing a `&Arc<T>`, and the
+    // callee would need to invoke `.clone()` if desired.
+
+    // In 4.0, argument pointers are passed to godot as `T*`, except for in virtual method calls. We can't perform virtual method calls
+    // currently, so they are always `T*`.
+    //
+    // In 4.1, argument pointers were standardized to always be `T**`.
+    #[cfg(before_api = "4.1")]
+    {
+        _self.sys()
+    }
+
+    #[cfg(since_api = "4.1")]
+    {
+        ptr::addr_of!(*_object_ptr_field) as sys::GDExtensionConstTypePtr
+    }
+}