Discussion about impl AsObjectArg<T> for &Option<Gd<T>> (outer ref)

It's relatively common that Godot APIs return `Option<Gd<T>>` or pass
this type in virtual functions. To avoid excessive `as_ref()` calls, we
**could** directly support `&Option<Gd>` in addition to `Option<&Gd>`.

However, this is currently not done as it hides nullability, especially
in situations where a return type is directly propagated:

    api(create_obj().as_ref())
    api(&create_obj())

While the first is slightly longer, it looks different from a function
create_obj() that returns Gd<T> and thus can never be null. It's also
quite idiomatic to use as_ref() for inner-option transforms in Rust.

For now, the code is still added but inactive.

Author: Bromeon

godot-core/src/obj/object_arg.rs

@@ -97,6 +97,37 @@ where
     }
 }
 
+/*
+It's relatively common that Godot APIs return `Option<Gd<T>>` or pass this type in virtual functions. To avoid excessive `as_ref()` calls, we
+**could** directly support `&Option<Gd>` in addition to `Option<&Gd>`. However, this is currently not done as it hides nullability,
+especially in situations where a return type is directly propagated:
+    api(create_obj().as_ref())
+    api(&create_obj())
+While the first is slightly longer, it looks different from a function create_obj() that returns Gd<T> and thus can never be null.
+In some scenarios, it's better to immediately ensure non-null (e.g. through `unwrap()`) instead of propagating nulls to the engine.
+It's also quite idiomatic to use as_ref() for inner-option transforms in Rust.
+
+impl<T, U> AsObjectArg<T> for &Option<U>
+where
+    T: GodotClass + Bounds<Declarer = bounds::DeclEngine>,
+    for<'a> &'a U: AsObjectArg<T>,
+{
+    fn as_object_arg(&self) -> ObjectArg<T> {
+        match self {
+            Some(obj) => obj.as_object_arg(),
+            None => ObjectArg::null(),
+        }
+    }
+
+    fn consume_arg(self) -> ObjectCow<T> {
+        match self {
+            Some(obj) => obj.consume_arg(),
+            None => Gd::null_arg().consume_arg(),
+        }
+    }
+}
+*/
+
 impl<T> AsObjectArg<T> for ObjectNullArg<T>
 where
     T: GodotClass + Bounds<Declarer = bounds::DeclEngine>,

itest/rust/src/object_tests/object_arg_test.rs

@@ -71,6 +71,18 @@ fn object_arg_option_borrowed() {
     });
 }
 
+/*
+#[itest]
+fn object_arg_option_borrowed_outer() {
+    with_objects(|manual, refc| {
+        let db = ClassDb::singleton();
+        let a = db.class_set_property(&Some(manual), "name", &Variant::from("hello"));
+        let b = db.class_set_property(&Some(refc), "value", &Variant::from(-123));
+        (a, b)
+    });
+}
+*/
+
 #[itest]
 fn object_arg_option_borrowed_mut() {
     // If you have an Option<&mut Gd<T>>, you can use as_deref() to get Option<&Gd<T>>.
@@ -94,10 +106,10 @@ fn object_arg_option_none() {
 
     // Will emit errors but should not crash.
     let db = ClassDb::singleton();
-    let error = db.class_set_property(&manual, "name", &Variant::from("hello"));
+    let error = db.class_set_property(manual.as_ref(), "name", &Variant::from("hello"));
     assert_eq!(error, global::Error::ERR_UNAVAILABLE);
 
-    let error = db.class_set_property(&refc, "value", &Variant::from(-123));
+    let error = db.class_set_property(refc.as_ref(), "value", &Variant::from(-123));
     assert_eq!(error, global::Error::ERR_UNAVAILABLE);
 }
 

itest/rust/src/register_tests/rpc_test.rs

@@ -77,12 +77,13 @@ impl RpcTest {
 fn node_enters_tree() {
     let node = RpcTest::new_alloc();
 
-    // Registering is done in `UserClass::__before_ready()`, and it requires a multiplayer api to exist.
+    // Registering is done in `UserClass::__before_ready()`, and it requires a multiplayer API to exist.
     let mut scene_tree = Engine::singleton()
         .get_main_loop()
         .unwrap()
         .cast::<SceneTree>();
-    scene_tree.set_multiplayer(&MultiplayerApi::create_default_interface());
+    scene_tree.set_multiplayer(MultiplayerApi::create_default_interface().as_ref());
+
     let mut root = scene_tree.get_root().unwrap();
     root.add_child(&node);
     root.remove_child(&node);