ToGodot for Gd/DynGd is again pass-by-ref

Author: Bromeon

godot-core/src/meta/args/as_arg.rs

@@ -10,7 +10,7 @@ use std::ffi::CStr;
 use crate::builtin::{GString, NodePath, StringName, Variant};
 use crate::meta::sealed::Sealed;
 use crate::meta::traits::GodotFfiVariant;
-use crate::meta::{CowArg, GodotType, ToGodot};
+use crate::meta::{CowArg, GodotType, ObjectArg, ToGodot};
 use crate::obj::{bounds, Bounds, DynGd, Gd, GodotClass, Inherits};
 
 /// Implicit conversions for arguments passed to Godot APIs.
@@ -515,7 +515,39 @@ impl ArgPassing for ByRef {
         T::Via: GodotType,
     {
         // Use by-ref conversion if possible, avoiding unnecessary clones when passing to FFI.
-        value.to_godot().to_ffi()
+        GodotType::to_ffi(value.to_godot())
+    }
+}
+
+/// Pass arguments to Godot by object pointer (for objects only).
+///
+/// See [`ToGodot::Pass`].
+pub enum ByObject {}
+impl Sealed for ByObject {}
+impl ArgPassing for ByObject {
+    type Output<'r, T: 'r> = &'r T;
+
+    type FfiOutput<'f, T>
+        = ObjectArg
+    where
+        T: GodotType + 'f;
+
+    fn ref_to_owned_via<T>(value: &T) -> T::Via
+    where
+        T: ToGodot<Pass = Self>,
+        T::Via: Clone,
+    {
+        // For ByObject types, do like ByRef: clone the reference to get owned value.
+        value.to_godot().clone()
+    }
+
+    fn ref_to_ffi<T>(value: &T) -> ObjectArg
+    where
+        T: ToGodot<Pass = Self>,
+        T::Via: GodotType,
+    {
+        let obj_ref: &T::Via = value.to_godot(); // implements GodotType.
+        unsafe { obj_ref.as_object_arg() }
     }
 }
 

godot-core/src/meta/args/mod.rs

@@ -13,7 +13,7 @@ mod ref_arg;
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Public APIs
 
-pub use as_arg::{owned_into_arg, ref_to_arg, ArgPassing, AsArg, ByRef, ByValue, ToArg};
+pub use as_arg::{owned_into_arg, ref_to_arg, ArgPassing, AsArg, ByObject, ByRef, ByValue, ToArg};
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Internal APIs
 

godot-core/src/meta/args/object_arg.rs

@@ -9,7 +9,10 @@ use std::ptr;
 
 use godot_ffi::{ExtVariantType, GodotFfi, GodotNullableFfi, PtrcallType};
 
-use crate::obj::{Gd, GodotClass, Inherits, RawGd};
+use crate::builtin::Variant;
+use crate::meta::error::ConvertError;
+use crate::meta::traits::GodotFfiVariant;
+use crate::obj::{Gd, GodotClass};
 use crate::{obj, sys};
 
 /// View for object arguments passed to the Godot engine. Never owning; must be null or backed by `Gd<T>`.
@@ -34,29 +37,13 @@ impl ObjectArg {
         }
     }
 
-    /// Creates `ObjectArg` from a `RawGd`.
+    /// Creates `ObjectArg` from an `Option<Gd>`.
     ///
     /// # Safety
-    /// The referenced `RawGd` must remain valid for the lifetime of this `ObjectArg`.
-    pub unsafe fn from_raw_gd<T: GodotClass>(obj: &RawGd<T>) -> Self {
-        // Runtime check is necessary, to ensure that object is still alive and has correct runtime type.
-        if !obj.is_null() {
-            obj.check_rtti("from_raw_gd");
-        }
-
-        Self {
-            object_ptr: obj.obj_sys(),
-        }
-    }
-
-    /// Creates `ObjectArg` from `Option<&Gd<U>>`, handling upcast to target type `T`.
-    pub fn from_option_gd_ref<T, U>(opt: Option<&Gd<U>>) -> Self
-    where
-        T: GodotClass,
-        U: GodotClass + Inherits<T>,
-    {
-        match opt {
-            Some(gd) => unsafe { Self::from_gd(gd) },
+    /// The referenced `Gd`, if not `None`, must remain valid for the lifetime of this `ObjectArg`.
+    pub unsafe fn from_option_gd<T: GodotClass>(obj: Option<&Gd<T>>) -> Self {
+        match obj {
+            Some(gd) => Self::from_gd(gd),
             None => Self::null(),
         }
     }
@@ -72,11 +59,6 @@ impl ObjectArg {
     pub fn is_null(&self) -> bool {
         self.object_ptr.is_null()
     }
-
-    /// Returns the raw object pointer
-    pub fn raw_ptr(&self) -> sys::GDExtensionObjectPtr {
-        self.object_ptr
-    }
 }
 
 // #[derive(Clone)] doesn't seem to get bounds right.
@@ -130,6 +112,16 @@ unsafe impl GodotFfi for ObjectArg {
     }
 }
 
+impl GodotFfiVariant for ObjectArg {
+    fn ffi_to_variant(&self) -> Variant {
+        obj::object_ffi_to_variant(self)
+    }
+
+    fn ffi_from_variant(_variant: &Variant) -> Result<Self, ConvertError> {
+        unreachable!("ObjectArg should only be passed *to* Godot, not *from*.")
+    }
+}
+
 impl GodotNullableFfi for ObjectArg {
     fn null() -> Self {
         Self::null()

godot-core/src/meta/godot_convert/impls.rs

@@ -83,6 +83,14 @@ where
     fn godot_type_name() -> String {
         T::godot_type_name()
     }
+
+    // Only relevant for object types T.
+    unsafe fn as_object_arg(&self) -> crate::meta::ObjectArg {
+        match self {
+            Some(inner) => unsafe { inner.as_object_arg() },
+            None => crate::meta::ObjectArg::null(),
+        }
+    }
 }
 
 impl<T: GodotConvert> GodotConvert for Option<T>

godot-core/src/meta/traits.rs

@@ -140,6 +140,25 @@ pub trait GodotType: GodotConvert<Via = Self> + sealed::Sealed + Sized + 'static
     fn qualifies_as_special_none(_from_variant: &Variant) -> bool {
         false
     }
+
+    /// Convert to `ObjectArg` for efficient object argument passing.
+    ///
+    /// Implemented in `GodotType` because Rust has no specialization, and there's no good way to have trait bounds in `ByObject`, but not in
+    /// other arg-passing strategies `ByValue`/`ByRef`.
+    ///
+    /// # Panics
+    /// If `Self` is not an object type (`Gd<T>`, `Option<Gd<T>>`). Note that `DynGd<T>` isn't directly implemented here, but uses `Gd<T>`'s
+    /// impl on the FFI layer.
+    ///
+    /// # Safety
+    /// `self` must be kept alive while return value is in use. Might be addressed with lifetime in the future.
+    #[doc(hidden)]
+    unsafe fn as_object_arg(&self) -> crate::meta::ObjectArg {
+        panic!(
+            "as_object_arg() called for non-object type: {}",
+            std::any::type_name::<Self>()
+        )
+    }
 }
 
 // ----------------------------------------------------------------------------------------------------------------------------------------------

godot-core/src/obj/dyn_gd.rs

@@ -553,9 +553,10 @@ where
     T: GodotClass,
     D: ?Sized,
 {
+    // Delegate to Gd<T> passing strategy.
     type Pass = <Gd<T> as ToGodot>::Pass;
 
-    fn to_godot(&self) -> Self::Via {
+    fn to_godot(&self) -> &Self::Via {
         self.obj.to_godot()
     }
 

godot-core/src/obj/gd.rs

@@ -895,15 +895,12 @@ impl<T: GodotClass> GodotConvert for Gd<T> {
 }
 
 impl<T: GodotClass> ToGodot for Gd<T> {
-    // FIXME(v0.4): ByRef/ByObject.
-    type Pass = meta::ByValue;
+    type Pass = meta::ByObject;
 
-    fn to_godot(&self) -> Self::Via {
-        // For null objects created for FFI parameter passing, skip RTTI check.
-        if !self.raw.is_null() {
-            self.raw.check_rtti("to_godot");
-        }
-        self.clone()
+    fn to_godot(&self) -> &Self {
+        // Note: Gd<T> never null, so no need to check raw.is_null().
+        self.raw.check_rtti("to_godot");
+        self
     }
 }
 
@@ -963,6 +960,10 @@ impl<T: GodotClass> GodotType for Gd<T> {
 
         false
     }
+
+    unsafe fn as_object_arg(&self) -> meta::ObjectArg {
+        meta::ObjectArg::from_gd(self)
+    }
 }
 
 impl<T: GodotClass> ArrayElement for Gd<T> {