Merge #247

247: impl `GodotFfi` for `Option<T>` when `T` is pointer sized and nullable #240 r=Bromeon a=TitanNano

`ToVariant` and `FromVariant` are implemented for `Option<Gd<T>>` as well as they are a requirement of `#[godot_api]`.

Closes #240.

Co-authored-by: Jovan Gerodetti <jovan.gerodetti@titannano.de>

godot-core/src/obj/gd.rs

@@ -12,7 +12,9 @@ use std::ptr;
 use godot_ffi as sys;
 use godot_ffi::VariantType;
 use sys::types::OpaqueObject;
-use sys::{ffi_methods, interface_fn, static_assert_eq_size, GodotFfi, PtrcallType};
+use sys::{
+    ffi_methods, interface_fn, static_assert_eq_size, GodotFfi, GodotNullablePtr, PtrcallType,
+};
 
 use crate::builtin::meta::{ClassName, VariantMetadata};
 use crate::builtin::{
@@ -570,6 +572,11 @@ where
     }
 }
 
+// SAFETY:
+// `Gd<T: GodotClass>` will only contain types that inherit from `crate::engine::Object`.
+// Godots `Object` in turn is known to be nullable and always a pointer.
+unsafe impl<T: GodotClass> GodotNullablePtr for Gd<T> {}
+
 impl<T: GodotClass> Gd<T> {
     /// Runs `init_fn` on the address of a pointer (initialized to null). If that pointer is still null after the `init_fn` call,
     /// then `None` will be returned; otherwise `Gd::from_obj_sys(ptr)`.
@@ -754,6 +761,25 @@ impl<T: GodotClass> ToVariant for Gd<T> {
     }
 }
 
+impl<T: GodotClass> ToVariant for Option<Gd<T>> {
+    fn to_variant(&self) -> Variant {
+        match self {
+            Some(gd) => gd.to_variant(),
+            None => Variant::nil(),
+        }
+    }
+}
+
+impl<T: GodotClass> FromVariant for Option<Gd<T>> {
+    fn try_from_variant(variant: &Variant) -> Result<Self, VariantConversionError> {
+        if variant.is_nil() {
+            Ok(None)
+        } else {
+            Gd::try_from_variant(variant).map(Some)
+        }
+    }
+}
+
 impl<T: GodotClass> PartialEq for Gd<T> {
     /// ⚠️ Returns whether two `Gd` pointers point to the same object.
     ///

godot-ffi/src/godot_ffi.rs

@@ -4,8 +4,8 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-use crate as sys;
-use std::fmt::Debug;
+use crate::{self as sys, ptr_then};
+use std::{fmt::Debug, ptr};
 
 /// Adds methods to convert from and to Godot FFI pointers.
 /// See [crate::ffi_methods] for ergonomic implementation.
@@ -96,6 +96,53 @@ pub unsafe trait GodotFfi {
     unsafe fn move_return_ptr(self, dst: sys::GDExtensionTypePtr, call_type: PtrcallType);
 }
 
+/// Marks a type as having a nullable counterpart in Godot.
+///
+/// This trait primarily exists to implement GodotFfi for `Option<Gd<T>>`, which is not possible
+/// due to Rusts orphan rule. The rule also enforces better API design, though. `godot_ffi` should
+/// not concern itself with the details of how Godot types work and merely defines the FFI abstraction.
+/// By having a marker trait for nullable types, we can provide a generic implementation for
+/// compatible types, without knowing their definition.
+///
+/// # Safety
+///
+/// The type has to have a pointer-sized counterpart in Godot, which needs to be nullable.
+/// So far, this only applies to class types (Object hierarchy).
+pub unsafe trait GodotNullablePtr: GodotFfi {}
+
+unsafe impl<T> GodotFfi for Option<T>
+where
+    T: GodotNullablePtr,
+{
+    fn sys(&self) -> sys::GDExtensionTypePtr {
+        match self {
+            Some(value) => value.sys(),
+            None => ptr::null_mut() as sys::GDExtensionTypePtr,
+        }
+    }
+
+    unsafe fn from_sys(ptr: sys::GDExtensionTypePtr) -> Self {
+        ptr_then(ptr, |ptr| T::from_sys(ptr))
+    }
+
+    unsafe fn from_sys_init(init_fn: impl FnOnce(sys::GDExtensionUninitializedTypePtr)) -> Self {
+        let mut raw = std::mem::MaybeUninit::uninit();
+        init_fn(raw.as_mut_ptr() as sys::GDExtensionUninitializedTypePtr);
+
+        Self::from_sys(raw.assume_init())
+    }
+
+    unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, call_type: PtrcallType) -> Self {
+        ptr_then(ptr, |ptr| T::from_arg_ptr(ptr, call_type))
+    }
+
+    unsafe fn move_return_ptr(self, ptr: sys::GDExtensionTypePtr, call_type: PtrcallType) {
+        if let Some(value) = self {
+            value.move_return_ptr(ptr, call_type)
+        }
+    }
+}
+
 /// An indication of what type of pointer call is being made.
 #[derive(Default, Copy, Clone, Eq, PartialEq, Debug)]
 pub enum PtrcallType {

godot-ffi/src/lib.rs

@@ -32,7 +32,7 @@ use std::ffi::CStr;
 #[doc(hidden)]
 pub use paste;
 
-pub use crate::godot_ffi::{GodotFfi, GodotFuncMarshal, PtrcallType};
+pub use crate::godot_ffi::{GodotFfi, GodotFuncMarshal, GodotNullablePtr, PtrcallType};
 pub use gen::central::*;
 pub use gen::gdextension_interface::*;
 pub use gen::interface::*;

itest/godot/ManualFfiTests.gd

@@ -149,4 +149,88 @@ func test_refcounted_as_object_return_from_user_func_ptrcall():
 func test_custom_constructor():
 	var obj = CustomConstructor.construct_object(42)
 	assert_eq(obj.val, 42)
-	obj.free()
+	obj.free()
+
+func test_option_refcounted_none_varcall():
+	var ffi := OptionFfiTest.new()
+
+	var from_rust: Variant = ffi.return_option_refcounted_none()
+	assert_that(ffi.accept_option_refcounted_none(from_rust), "ffi.accept_option_refcounted_none(from_rust)")
+
+	var from_gdscript: Variant = null
+	var mirrored: Variant = ffi.mirror_option_refcounted(from_gdscript)
+	assert_eq(mirrored, from_gdscript, "mirrored == from_gdscript")
+
+func test_option_refcounted_none_ptrcall():
+	var ffi := OptionFfiTest.new()
+
+	var from_rust: Object = ffi.return_option_refcounted_none()
+	assert_that(ffi.accept_option_refcounted_none(from_rust), "ffi.accept_option_refcounted_none(from_rust)")
+
+	var from_gdscript: Object = null
+	var mirrored: Object = ffi.mirror_option_refcounted(from_gdscript)
+	assert_eq(mirrored, from_gdscript, "mirrored == from_gdscript")
+
+func test_option_refcounted_some_varcall():
+	var ffi := OptionFfiTest.new()
+
+	var from_rust: Variant = ffi.return_option_refcounted_some()
+	assert_that(ffi.accept_option_refcounted_some(from_rust), "ffi.accept_option_refcounted_some(from_rust)")
+
+	var from_gdscript: Variant = RefCounted.new()
+	var mirrored: Variant = ffi.mirror_option_refcounted(from_gdscript)
+	assert_eq(mirrored, from_gdscript, "mirrored == from_gdscript")
+
+func test_option_refcounted_some_ptrcall():
+	var ffi := OptionFfiTest.new()
+
+	var from_rust: Object = ffi.return_option_refcounted_some()
+	assert_that(ffi.accept_option_refcounted_some(from_rust), "ffi.accept_option_refcounted_some(from_rust)")
+
+	var from_gdscript: Object = RefCounted.new()
+	var mirrored: Object = ffi.mirror_option_refcounted(from_gdscript)
+	assert_eq(mirrored, from_gdscript, "mirrored == from_gdscript")
+
+func test_option_node_none_varcall():
+	var ffi := OptionFfiTest.new()
+
+	var from_rust: Variant = ffi.return_option_node_none()
+	assert_that(ffi.accept_option_node_none(from_rust), "ffi.accept_option_node_none(from_rust)")
+
+	var from_gdscript: Variant = null
+	var mirrored: Variant = ffi.mirror_option_node(from_gdscript)
+	assert_eq(mirrored, from_gdscript, "mirrored == from_gdscript")
+
+func test_option_node_none_ptrcall():
+	var ffi := OptionFfiTest.new()
+
+	var from_rust: Node = ffi.return_option_node_none()
+	assert_that(ffi.accept_option_node_none(from_rust), "ffi.accept_option_node_none(from_rust)")
+
+	var from_gdscript: Node = null
+	var mirrored: Node = ffi.mirror_option_node(from_gdscript)
+	assert_eq(mirrored, from_gdscript, "mirrored == from_gdscript")
+
+func test_option_node_some_varcall():
+	var ffi := OptionFfiTest.new()
+
+	var from_rust: Variant = ffi.return_option_node_some()
+	assert_that(ffi.accept_option_node_some(from_rust), "ffi.accept_option_node_some(from_rust)")
+
+	var from_gdscript: Variant = Node.new()
+	var mirrored: Variant = ffi.mirror_option_node(from_gdscript)
+	assert_eq(mirrored, from_gdscript, "mirrored == from_gdscript")
+	from_gdscript.free()
+	from_rust.free()
+
+func test_option_node_some_ptrcall():
+	var ffi := OptionFfiTest.new()
+
+	var from_rust: Node = ffi.return_option_node_some()
+	assert_that(ffi.accept_option_node_some(from_rust), "ffi.accept_option_node_some(from_rust)")
+
+	var from_gdscript: Node = Node.new()
+	var mirrored: Node = ffi.mirror_option_node(from_gdscript)
+	assert_eq(mirrored, from_gdscript, "mirrored == from_gdscript")
+	from_gdscript.free()
+	from_rust.free()

itest/rust/src/lib.rs

@@ -25,6 +25,7 @@ mod init_test;
 mod native_structures_test;
 mod node_test;
 mod object_test;
+mod option_ffi_test;
 mod packed_array_test;
 mod projection_test;
 mod quaternion_test;

itest/rust/src/option_ffi_test.rs

@@ -0,0 +1,87 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
+use godot::prelude::{godot_api, Gd, GodotClass, Node, Object, RefCounted};
+use godot::sys::GodotFfi;
+
+use crate::itest;
+
+#[itest]
+fn option_some_sys_conversion() {
+    let v = Some(Object::new_alloc());
+    let ptr = v.sys();
+
+    let v2 = unsafe { Option::<Gd<Object>>::from_sys(ptr) };
+    assert_eq!(v2, v);
+
+    v.unwrap().free();
+}
+
+#[itest]
+fn option_none_sys_conversion() {
+    let v = None;
+    let ptr = v.sys();
+
+    let v2 = unsafe { Option::<Gd<Object>>::from_sys(ptr) };
+    assert_eq!(v2, v);
+}
+
+#[derive(GodotClass, Debug)]
+#[class(base = RefCounted, init)]
+struct OptionFfiTest;
+
+#[godot_api]
+impl OptionFfiTest {
+    #[func]
+    fn return_option_refcounted_none(&self) -> Option<Gd<RefCounted>> {
+        None
+    }
+
+    #[func]
+    fn accept_option_refcounted_none(&self, value: Option<Gd<RefCounted>>) -> bool {
+        value.is_none()
+    }
+
+    #[func]
+    fn return_option_refcounted_some(&self) -> Option<Gd<RefCounted>> {
+        Some(RefCounted::new())
+    }
+
+    #[func]
+    fn accept_option_refcounted_some(&self, value: Option<Gd<RefCounted>>) -> bool {
+        value.is_some()
+    }
+
+    #[func]
+    fn mirror_option_refcounted(&self, value: Option<Gd<RefCounted>>) -> Option<Gd<RefCounted>> {
+        value
+    }
+
+    #[func]
+    fn return_option_node_none(&self) -> Option<Gd<Node>> {
+        None
+    }
+
+    #[func]
+    fn accept_option_node_none(&self, value: Option<Gd<Node>>) -> bool {
+        value.is_none()
+    }
+
+    #[func]
+    fn return_option_node_some(&self) -> Option<Gd<Node>> {
+        Some(Node::new_alloc())
+    }
+
+    #[func]
+    fn accept_option_node_some(&self, value: Option<Gd<Node>>) -> bool {
+        value.is_some()
+    }
+
+    #[func]
+    fn mirror_option_node(&self, value: Option<Gd<Node>>) -> Option<Gd<Node>> {
+        value
+    }
+}