Merge pull request #1323 from godot-rust/qol/option-dyn-gd

`AsArg` now supports `Option<DynGd>`

Author: Bromeon

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

@@ -143,6 +143,7 @@ where
     T: Inherits<Base>,
     Base: GodotClass,
 {
+    //noinspection RsConstantConditionIf - false positive in IDE for `T::IS_SAME_CLASS`.
     fn into_arg<'arg>(self) -> CowArg<'arg, Gd<Base>>
     where
         Self: 'arg,
@@ -199,7 +200,7 @@ where
     }
 }
 
-// Convert `DynGd` -> `Gd` (with upcast).
+/// Convert `DynGd` -> `Gd` (with upcast).
 impl<T, D, Base> AsArg<Gd<Base>> for &DynGd<T, D>
 where
     T: Inherits<Base>,
@@ -223,9 +224,68 @@ where
     }
 }
 
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+// Null arguments
+
+/// Private struct for passing null arguments to optional object parameters.
+///
+/// This struct implements `AsArg` for both `Option<Gd<T>>` and `Option<DynGd<T, D>>`, allowing [`Gd::null_arg()`] and [`DynGd::null_arg()`]
+/// to share implementation.
+///
+/// Not public, as `impl AsArg<...>` is used by `null_arg()` methods.
+pub(crate) struct NullArg<T>(pub std::marker::PhantomData<*mut T>);
+
+impl<T> AsArg<Option<Gd<T>>> for NullArg<T>
+where
+    T: GodotClass,
+{
+    fn into_arg<'arg>(self) -> CowArg<'arg, Option<Gd<T>>>
+    where
+        Self: 'arg,
+    {
+        CowArg::Owned(None)
+    }
+}
+
+impl<T, D> AsArg<Option<DynGd<T, D>>> for NullArg<T>
+where
+    T: GodotClass,
+    D: ?Sized + 'static,
+{
+    fn into_arg<'arg>(self) -> CowArg<'arg, Option<DynGd<T, D>>>
+    where
+        Self: 'arg,
+    {
+        CowArg::Owned(None)
+    }
+}
+
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Optional object (Gd + DynGd) impls
 
+/// Convert `&Gd` -> `Option<Gd>` (with upcast).
+impl<T, Base> AsArg<Option<Gd<Base>>> for &Gd<T>
+where
+    T: Inherits<Base>,
+    Base: GodotClass + Bounds<Declarer = bounds::DeclEngine>,
+{
+    fn into_arg<'arg>(self) -> CowArg<'arg, Option<Gd<Base>>>
+    where
+        Self: 'arg,
+    {
+        // Upcasting to an owned value Gd<Base> requires cloning. Optimized path in into_ffi_arg().
+        CowArg::Owned(Some(self.clone().upcast::<Base>()))
+    }
+
+    fn into_ffi_arg<'arg>(self) -> FfiArg<'arg, Option<Gd<Base>>>
+    where
+        Self: 'arg,
+    {
+        let arg = ObjectArg::from_gd(self);
+        FfiArg::FfiObject(arg)
+    }
+}
+
 /// Convert `Option<&Gd>` -> `Option<Gd>` (with upcast).
 impl<T, Base> AsArg<Option<Gd<Base>>> for Option<&Gd<T>>
 where
@@ -252,21 +312,22 @@ where
     }
 }
 
-/// Convert `&Gd` -> `Option<Gd>` (with upcast).
-impl<T, Base> AsArg<Option<Gd<Base>>> for &Gd<T>
+/// Convert `&DynGd` -> `Option<DynGd>` (with upcast).
+impl<T, D, Base> AsArg<Option<DynGd<Base, D>>> for &DynGd<T, D>
 where
     T: Inherits<Base>,
+    D: ?Sized,
     Base: GodotClass + Bounds<Declarer = bounds::DeclEngine>,
 {
-    fn into_arg<'arg>(self) -> CowArg<'arg, Option<Gd<Base>>>
+    fn into_arg<'arg>(self) -> CowArg<'arg, Option<DynGd<Base, D>>>
     where
         Self: 'arg,
     {
-        // Upcasting to an owned value Gd<Base> requires cloning. Optimized path in into_ffi_arg().
-        CowArg::Owned(Some(self.clone().upcast::<Base>()))
+        // Upcasting to an owned value DynGd<Base, D> requires cloning. Optimized path in into_ffi_arg().
+        CowArg::Owned(Some(self.clone().upcast()))
     }
 
-    fn into_ffi_arg<'arg>(self) -> FfiArg<'arg, Option<Gd<Base>>>
+    fn into_ffi_arg<'arg>(self) -> FfiArg<'arg, Option<DynGd<Base, D>>>
     where
         Self: 'arg,
     {
@@ -299,6 +360,34 @@ where
     }
 }
 
+/// Convert `Option<&DynGd>` -> `Option<DynGd>` (with upcast).
+impl<T, D, Base> AsArg<Option<DynGd<Base, D>>> for Option<&DynGd<T, D>>
+where
+    T: Inherits<Base>,
+    D: ?Sized,
+    Base: GodotClass + Bounds<Declarer = bounds::DeclEngine>,
+{
+    fn into_arg<'arg>(self) -> CowArg<'arg, Option<DynGd<Base, D>>>
+    where
+        Self: 'arg,
+    {
+        // Upcasting to an owned value Gd<Base> requires cloning. Optimized path in into_ffi_arg().
+        match self {
+            Some(gd_ref) => AsArg::into_arg(gd_ref),
+            None => CowArg::Owned(None),
+        }
+    }
+
+    fn into_ffi_arg<'arg>(self) -> FfiArg<'arg, Option<DynGd<Base, D>>>
+    where
+        Self: 'arg,
+    {
+        let option_gd: Option<&Gd<T>> = self.map(|v| &**v); // as_deref() not working.
+        let arg = ObjectArg::from_option_gd(option_gd);
+        FfiArg::FfiObject(arg)
+    }
+}
+
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Public helper functions (T|&T -> AsArg)
 

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

@@ -13,24 +13,19 @@ mod ref_arg;
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Public APIs
 
-pub use as_arg::{
-    owned_into_arg, ref_to_arg, ArgPassing, AsArg, ByObject, ByOption, ByRef, ByValue, ToArg,
-};
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Internal APIs
 
 // Solely public for itest/convert_test.rs.
+pub(crate) use as_arg::NullArg;
+pub use as_arg::{
+    owned_into_arg, ref_to_arg, ArgPassing, AsArg, ByObject, ByOption, ByRef, ByValue, ToArg,
+};
+#[cfg(not(feature = "trace"))]
+pub(crate) use cow_arg::{CowArg, FfiArg};
+// Integration test only.
 #[cfg(feature = "trace")]
 #[doc(hidden)]
 pub use cow_arg::{CowArg, FfiArg};
-#[cfg(not(feature = "trace"))]
-pub(crate) use cow_arg::{CowArg, FfiArg};
-#[allow(unused)] // TODO(v0.4): replace contents with newer changes
 pub use object_arg::ObjectArg;
-pub use ref_arg::RefArg;
-
-// #[doc(hidden)]
-// pub use cow_arg::*;
-//
-// #[doc(hidden)]
-// pub use ref_arg::*;
+pub(crate) use ref_arg::RefArg;

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

@@ -16,7 +16,10 @@ use crate::sys;
 
 /// Simple reference wrapper, used when passing arguments by-ref to Godot APIs.
 ///
-/// This type is often used as the result of [`ToGodot::to_godot()`], if `Self` is not a `Copy` type.
+/// This type is exclusively used at the FFI boundary, to avoid unnecessary cloning of values.
+///
+/// Private type. Cannot be `pub(crate)` because it's used in `#[doc(hidden)]` associate type `GodotType::ToFfi<'f>`, and `GodotType` is public.
+#[doc(hidden)]
 pub struct RefArg<'r, T> {
     /// Only `None` if `T: GodotNullableFfi` and `T::is_null()` is true.
     shared_ref: Option<&'r T>,

godot-core/src/meta/sealed.rs

@@ -62,6 +62,7 @@ impl<T: ArrayElement> Sealed for Array<T> {}
 impl<T: GodotClass> Sealed for Gd<T> {}
 impl<T: GodotClass> Sealed for RawGd<T> {}
 impl<T: GodotClass, D: ?Sized> Sealed for DynGd<T, D> {}
+impl<T: GodotClass, D: ?Sized + 'static> Sealed for Option<DynGd<T, D>> {}
 impl<T> Sealed for Option<T>
 where
     T: GodotType,

godot-core/src/obj/dyn_gd.rs

@@ -403,6 +403,13 @@ where
     pub fn into_gd(self) -> Gd<T> {
         self.obj
     }
+
+    /// Represents `null` when passing a dynamic object argument to Godot.
+    ///
+    /// See [`Gd::null_arg()`]
+    pub fn null_arg() -> impl meta::AsArg<Option<DynGd<T, D>>> {
+        meta::NullArg(std::marker::PhantomData)
+    }
 }
 
 impl<T, D> DynGd<T, D>
@@ -606,6 +613,16 @@ where
     }
 }
 
+impl<T, D> meta::ArrayElement for Option<DynGd<T, D>>
+where
+    T: GodotClass,
+    D: ?Sized + 'static,
+{
+    fn element_type_string() -> String {
+        DynGd::<T, D>::element_type_string()
+    }
+}
+
 impl<T, D> Var for DynGd<T, D>
 where
     T: GodotClass,

godot-core/src/obj/gd.rs

@@ -14,7 +14,7 @@ use sys::{static_assert_eq_size_align, SysPtr as _};
 use crate::builtin::{Callable, GString, NodePath, StringName, Variant};
 use crate::meta::error::{ConvertError, FromFfiError};
 use crate::meta::{
-    ArrayElement, AsArg, CallContext, ClassName, CowArg, FromGodot, GodotConvert, GodotType,
+    ArrayElement, AsArg, CallContext, ClassName, FromGodot, GodotConvert, GodotType,
     PropertyHintInfo, RefArg, ToGodot,
 };
 use crate::obj::{
@@ -816,22 +816,7 @@ where
     /// let mut shape: Gd<Node> = some_node();
     /// shape.set_owner(Gd::null_arg());
     pub fn null_arg() -> impl AsArg<Option<Gd<T>>> {
-        // Anonymous struct that creates None for optional object arguments.
-        struct NullGdArg<T>(std::marker::PhantomData<*mut T>);
-
-        impl<T> AsArg<Option<Gd<T>>> for NullGdArg<T>
-        where
-            T: GodotClass,
-        {
-            fn into_arg<'arg>(self) -> CowArg<'arg, Option<Gd<T>>>
-            where
-                Self: 'arg,
-            {
-                CowArg::Owned(None)
-            }
-        }
-
-        NullGdArg(std::marker::PhantomData)
+        meta::NullArg(std::marker::PhantomData)
     }
 }
 

itest/rust/src/object_tests/dyn_gd_test.rs

@@ -430,6 +430,47 @@ fn dyn_gd_store_in_godot_array() {
     */
 }
 
+#[itest]
+fn dyn_gd_as_arg() {
+    let refc_health = Gd::from_object(RefcHealth { hp: 42 }).into_dyn();
+    let node_health = foreign::NodeHealth::new_alloc().into_dyn();
+    let typed_none = None::<&DynGd<RefcHealth, dyn Health>>;
+
+    // Array<DynGd>.
+    let array: Array<DynGd<Object, dyn Health>> = array![&refc_health, &node_health];
+    assert_eq!(array.len(), 2);
+
+    let first = array.at(0);
+    assert_eq!(first.dyn_bind().get_hitpoints(), 42);
+
+    let second = array.at(1);
+    assert_eq!(second.dyn_bind().get_hitpoints(), 100);
+
+    // Array<Option<DynGd>>.
+    let opt_array: Array<Option<DynGd<Object, dyn Health>>> = array![
+        Some(&refc_health),
+        Some(&node_health),
+        typed_none,
+        DynGd::null_arg(),
+    ];
+    assert_eq!(opt_array.len(), 4);
+
+    let first = opt_array.at(0).expect("element 0 is Some");
+    assert_eq!(first.dyn_bind().get_hitpoints(), 42);
+
+    let second = opt_array.at(1).expect("element 1 is Some");
+    assert_eq!(second.dyn_bind().get_hitpoints(), 100);
+
+    let third = opt_array.at(2);
+    assert!(third.is_none(), "element 2 is None");
+
+    let fourth = opt_array.at(3);
+    assert!(fourth.is_none(), "element 3 is None (null_arg)");
+
+    // Clean up manually managed objects.
+    opt_array.at(1).unwrap().free();
+}
+
 #[itest]
 fn dyn_gd_error_unregistered_trait() {
     trait UnrelatedTrait {}