Test AsArg when interacting with engine + custom GDScript

Author: Bromeon

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

@@ -228,7 +228,7 @@ impl<T> Deref for CowArg<'_, T> {
             CowArg::Owned(value) => value,
             CowArg::Borrowed(value) => value,
             CowArg::FfiObject(_) => {
-                todo!("Deref not implemented for FfiObject variant")
+                unreachable!("deref(): FfiObject path should only be used for FFI logic")
             }
         }
     }

itest/rust/src/object_tests/object_arg_test.rs

@@ -5,12 +5,14 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-use godot::builtin::Variant;
-use godot::classes::{ClassDb, Node, ResourceFormatLoader, ResourceLoader};
+use godot::builtin::{vslice, Variant};
+use godot::classes::{ClassDb, Node, RefCounted, ResourceFormatLoader, ResourceLoader};
 use godot::global;
+use godot::meta::ToGodot;
 use godot::obj::{Gd, NewAlloc, NewGd};
+use godot::register::{godot_api, GodotClass};
 
-use crate::framework::itest;
+use crate::framework::{create_gdscript, itest};
 use crate::object_tests::object_test::{user_refc_instance, RefcPayload};
 
 /*
@@ -142,6 +144,73 @@ fn object_arg_owned_default_params() {
     ResourceLoader::singleton().remove_resource_format_loader(&b);
 }
 
+// Gd<RefCounted> passed to GDScript should not create unnecessary clones.
+#[itest]
+fn refcount_asarg_gdscript_calls() {
+    let script = create_gdscript(
+        r#"
+extends RefCounted
+
+func observe_refcount_ptrcall(obj: RefCounted) -> int:
+    return obj.get_reference_count()
+
+func observe_refcount_varcall(obj) -> int:
+    if obj == null:
+        return 0
+    return obj.get_reference_count()
+"#,
+    );
+
+    let mut test_instance = RefCounted::new_gd();
+    test_instance.set_script(&script);
+
+    // Already pack into Variant, to have 1 less reference count increment.
+    let refc = RefCounted::new_gd().to_variant();
+    assert_eq!(refc.call("get_reference_count", &[]), 1.to_variant());
+
+    let refcount_typed: i32 = test_instance
+        .call("observe_refcount_ptrcall", &[refc])
+        .to::<i32>();
+
+    let refc = RefCounted::new_gd().to_variant();
+    let refcount_untyped = test_instance
+        .call("observe_refcount_varcall", &[refc])
+        .to::<i32>();
+
+    let refcount_none = test_instance
+        .call("observe_refcount_varcall", vslice![Variant::nil()])
+        .to::<i32>();
+
+    // Both should result in refcount 2: 1 variant in Rust + 1 reference created on GDScript side.
+    assert_eq!(refcount_typed, 2, "typed GDScript param (ptrcall)");
+    assert_eq!(refcount_untyped, 2, "untyped GDScript param (varcall)");
+    assert_eq!(refcount_none, 0, "None/null parameter should return 0");
+}
+
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+// Tests with engine APIs + AsArg below, in module.
+
+#[derive(GodotClass)]
+#[class(base = RefCounted, init)]
+pub struct RefCountAsArgTest;
+
+#[godot_api]
+impl RefCountAsArgTest {
+    #[func]
+    fn accept_option_refcounted(&self, obj: Option<Gd<RefCounted>>) -> i32 {
+        match obj {
+            Some(gd) => gd.get_reference_count(),
+            None => 0,
+        }
+    }
+
+    #[func]
+    fn accept_object(&self, obj: Gd<godot::classes::Object>) -> bool {
+        // Just verify we can receive an Object (for upcast testing).
+        !obj.get_class().is_empty()
+    }
+}
+
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Helpers
 
@@ -164,3 +233,139 @@ where
 
     manual2.free();
 }
+
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+// Tests requiring codegen-full feature
+
+#[cfg(feature = "codegen-full")]
+mod engine_api_tests {
+    use std::cell::Cell;
+
+    use godot::builtin::{Rid, Variant};
+    use godot::classes::{base_material_3d, ITexture2D, StandardMaterial3D, Texture2D};
+    use godot::meta::ToGodot;
+    use godot::obj::{Base, Gd, NewGd, WithBaseField};
+    use godot::register::{godot_api, GodotClass};
+
+    use crate::framework::itest;
+
+    const ALBEDO: base_material_3d::TextureParam = base_material_3d::TextureParam::ALBEDO;
+
+    /// Various internal references are created during `set_texture()`, thus 4. This also matches GDScript code doing the same.
+    /// Verified that before this optimization, the refcount was 5.
+    const EXPECTED_REFCOUNT: i32 = 4;
+
+    fn verify_refcount<F>(exp_refcount: i32, arg_desription: &str, operation: F)
+    where
+        F: FnOnce(&mut Gd<StandardMaterial3D>, &Gd<ArgTestTexture>),
+    {
+        let texture = ArgTestTexture::new_gd();
+        let mut material = StandardMaterial3D::new_gd();
+
+        operation(&mut material, &texture);
+
+        let captured = texture.bind().get_captured_refcount();
+        assert_eq!(captured, exp_refcount, "{}", arg_desription);
+    }
+
+    #[itest]
+    fn refcount_asarg_ref() {
+        verify_refcount(EXPECTED_REFCOUNT, "&object", |mat, tex| {
+            // Sanity check: refcount is 1 before call.
+            assert_eq!(tex.get_reference_count(), 1);
+
+            mat.set_texture(ALBEDO, tex);
+        });
+
+        // Derived -> base conversion. 1 extra due to clone().
+        verify_refcount(EXPECTED_REFCOUNT + 1, "&base_obj", |mat, tex| {
+            let base = tex.clone().upcast::<Texture2D>();
+            mat.set_texture(ALBEDO, &base);
+        });
+    }
+
+    #[itest]
+    fn refcount_asarg_option() {
+        verify_refcount(EXPECTED_REFCOUNT, "Some(&object)", |mat, tex| {
+            mat.set_texture(ALBEDO, Some(tex));
+        });
+
+        // Derived -> base conversion. 1 extra due to clone().
+        verify_refcount(EXPECTED_REFCOUNT + 1, "Some(&base_obj)", |mat, tex| {
+            let base = tex.clone().upcast::<Texture2D>();
+            mat.set_texture(ALBEDO, Some(&base));
+        });
+
+        verify_refcount(0, "None [derived]", |mat, _tex| {
+            mat.set_texture(ALBEDO, None::<&Gd<ArgTestTexture>>);
+        });
+
+        verify_refcount(0, "None [base]", |mat, _tex| {
+            mat.set_texture(ALBEDO, None::<&Gd<Texture2D>>);
+        });
+    }
+
+    #[itest]
+    fn refcount_asarg_null_arg() {
+        verify_refcount(0, "Gd::null_arg()", |mat, _tex| {
+            mat.set_texture(ALBEDO, Gd::null_arg());
+        });
+    }
+
+    #[itest]
+    fn refcount_asarg_variant() {
+        verify_refcount(EXPECTED_REFCOUNT, "&Variant(tex)", |mat, tex| {
+            mat.set("albedo_texture", &tex.to_variant());
+        });
+
+        verify_refcount(0, "&Variant(nil)", |mat, _tex| {
+            mat.set("albedo_texture", &Variant::nil());
+        });
+    }
+
+    // ------------------------------------------------------------------------------------------------------------------------------------------
+    // Test classes for AsArg testing
+
+    #[derive(GodotClass)]
+    #[class(base = Texture2D)]
+    pub struct ArgTestTexture {
+        base: Base<Texture2D>,
+        captured_refcount: Cell<i32>,
+        rid: Rid,
+    }
+
+    #[godot_api]
+    impl ArgTestTexture {
+        fn get_captured_refcount(&self) -> i32 {
+            self.captured_refcount.get()
+        }
+    }
+
+    #[godot_api]
+    impl ITexture2D for ArgTestTexture {
+        fn init(base: Base<Texture2D>) -> Self {
+            Self {
+                base,
+                captured_refcount: Cell::new(0),
+                rid: Rid::new(0),
+            }
+        }
+
+        // Override this method because it's called by StandardMaterial3D::set_texture().
+        // We use it as a hook to observe the reference count from the engine side (after passing through AsArg).
+        fn get_rid(&self) -> Rid {
+            self.captured_refcount
+                .set(self.base().get_reference_count());
+
+            self.rid
+        }
+
+        fn get_width(&self) -> i32 {
+            1
+        }
+
+        fn get_height(&self) -> i32 {
+            1
+        }
+    }
+}