Merge #693 #700

693: Add the MapOwned trait and the `Once<T>` userdata type r=toasteater a=toasteater

This adds support for exporting methods that take ownership of `self`. `Once<T>` is a userdata wrapper that uses atomic compare-exchange to allow taking ownership safely once.

700: Add into_inner method to ArcData r=toasteater a=Waridley

As discussed in Discord, this method is especially useful for coercing the `script` of an `Instance` into a trait object, thus being able to benefit from more Rust conveniences within Godot scripts.

This *could* be written as an `impl<T> Into<Arc<T>> for ArcData<T>`, but I feel like this makes it more clear that it's not expected for the user to be able to freely treat `ArcData` as an `Arc` whenever they want to, but rather must be explicit about accessing the internals of this type -- even if it should theoretically be as memory-safe as using the `ArcData` anyway.

I decided to have this method take ownership of `self` because if you have  reference to an `ArcData`, you should still be able to call `clone()` on it to get an owned copy, but the opposite would not be as easy if the user didn't want to increment the ref count.

Co-authored-by: toasteater <48371905+toasteater@users.noreply.github.com>
Co-authored-by: Waridley <Waridley64@gmail.com>

Author: 3 people

gdnative-core/Cargo.toml

@@ -28,3 +28,4 @@ gdnative-impl-proc-macros = { path = "../impl/proc_macros", version = "=0.9.3" }
 
 bitflags = { version = "1.2", optional = true }
 parking_lot = { version = "0.11.0", optional = true }
+atomic-take = "1.0.0"

gdnative-core/src/nativescript/class.rs

@@ -4,9 +4,7 @@ use crate::core_types::{
     FromVariant, FromVariantError, GodotString, OwnedToVariant, ToVariant, Variant,
 };
 use crate::nativescript::init::ClassBuilder;
-use crate::nativescript::Map;
-use crate::nativescript::MapMut;
-use crate::nativescript::UserData;
+use crate::nativescript::{Map, MapMut, MapOwned, UserData};
 use crate::object::{
     AssumeSafeLifetime, LifetimeConstraint, QueueFree, RawObject, Ref, RefImplBound, SafeAsRaw,
     SafeDeref, TRef,
@@ -441,6 +439,18 @@ where
         self.script
             .map_mut(|script| op(script, self.owner.as_ref()))
     }
+
+    /// Calls a function with a NativeClass instance and its owner, and returns its return
+    /// value. Can be used on reference counted types for multiple times.
+    #[inline]
+    pub fn map_owned<F, U>(&self, op: F) -> Result<U, <T::UserData as MapOwned>::Err>
+    where
+        T::UserData: MapOwned,
+        F: FnOnce(T, TRef<'_, T::Base, Access>) -> U,
+    {
+        self.script
+            .map_owned(|script| op(script, self.owner.as_ref()))
+    }
 }
 
 /// Methods for instances with manually-managed base classes.
@@ -648,6 +658,17 @@ where
     {
         self.script.map_mut(|script| op(script, self.owner))
     }
+
+    /// Calls a function with a NativeClass instance and its owner, and returns its return
+    /// value.
+    #[inline]
+    pub fn map_owned<F, U>(&self, op: F) -> Result<U, <T::UserData as MapOwned>::Err>
+    where
+        T::UserData: MapOwned,
+        F: FnOnce(T, TRef<'_, T::Base, Access>) -> U,
+    {
+        self.script.map_owned(|script| op(script, self.owner))
+    }
 }
 
 impl<T, Access: ThreadAccess> Clone for Instance<T, Access>

gdnative-core/src/nativescript/macros.rs

@@ -188,6 +188,50 @@ macro_rules! godot_wrap_method {
             ) -> $retty
         )
     };
+    // owned
+    (
+        $type_name:ty,
+        fn $method_name:ident(
+            mut $self:ident,
+            $owner:ident : $owner_ty:ty
+            $(,$pname:ident : $pty:ty)*
+            $(,#[opt] $opt_pname:ident : $opt_pty:ty)*
+            $(,)?
+        ) -> $retty:ty
+    ) => {
+        $crate::godot_wrap_method_inner!(
+            $type_name,
+            map_owned,
+            fn $method_name(
+                $self,
+                $owner: $owner_ty
+                $(,$pname : $pty)*
+                $(,#[opt] $opt_pname : $opt_pty)*
+            ) -> $retty
+        )
+    };
+    // owned
+    (
+        $type_name:ty,
+        fn $method_name:ident(
+            $self:ident,
+            $owner:ident : $owner_ty:ty
+            $(,$pname:ident : $pty:ty)*
+            $(,#[opt] $opt_pname:ident : $opt_pty:ty)*
+            $(,)?
+        ) -> $retty:ty
+    ) => {
+        $crate::godot_wrap_method_inner!(
+            $type_name,
+            map_owned,
+            fn $method_name(
+                $self,
+                $owner: $owner_ty
+                $(,$pname : $pty)*
+                $(,#[opt] $opt_pname : $opt_pty)*
+            ) -> $retty
+        )
+    };
     // mutable without return type
     (
         $type_name:ty,
@@ -230,6 +274,48 @@ macro_rules! godot_wrap_method {
             ) -> ()
         )
     };
+    // owned without return type
+    (
+        $type_name:ty,
+        fn $method_name:ident(
+            mut $self:ident,
+            $owner:ident : $owner_ty:ty
+            $(,$pname:ident : $pty:ty)*
+            $(,#[opt] $opt_pname:ident : $opt_pty:ty)*
+            $(,)?
+        )
+    ) => {
+        $crate::godot_wrap_method!(
+            $type_name,
+            fn $method_name(
+                $self,
+                $owner: $owner_ty
+                $(,$pname : $pty)*
+                $(,#[opt] $opt_pname : $opt_pty)*
+            ) -> ()
+        )
+    };
+    // owned without return type
+    (
+        $type_name:ty,
+        fn $method_name:ident(
+            $self:ident,
+            $owner:ident : $owner_ty:ty
+            $(,$pname:ident : $pty:ty)*
+            $(,#[opt] $opt_pname:ident : $opt_pty:ty)*
+            $(,)?
+        )
+    ) => {
+        $crate::godot_wrap_method!(
+            $type_name,
+            fn $method_name(
+                $self,
+                $owner: $owner_ty
+                $(,$pname : $pty)*
+                $(,#[opt] $opt_pname : $opt_pty)*
+            ) -> ()
+        )
+    };
 }
 
 /// Convenience macro to create a profiling signature with a given tag.

gdnative-core/src/nativescript/mod.rs

@@ -11,4 +11,4 @@ pub mod user_data;
 
 pub use class::*;
 pub use init::*;
-pub use user_data::{Map, MapMut, UserData};
+pub use user_data::{Map, MapMut, MapOwned, UserData};

gdnative-core/src/nativescript/user_data.rs

@@ -63,7 +63,7 @@
 //! - You don't need to do anything special in `Drop`.
 
 use parking_lot::{Mutex, RwLock};
-use std::fmt::Debug;
+use std::fmt::{self, Debug, Display};
 use std::marker::PhantomData;
 use std::mem;
 use std::sync::Arc;
@@ -145,6 +145,20 @@ pub trait MapMut: UserData {
         F: FnOnce(&mut Self::Target) -> U;
 }
 
+/// Trait for wrappers that can be mapped once.
+pub trait MapOwned: UserData {
+    type Err: Debug;
+
+    /// Maps a `T` to `U`. Called for methods that take `self`. This method may fail with
+    /// an error if it is called more than once on the same object.
+    ///
+    /// Implementations of this method must not panic. Failures should be indicated by
+    /// returning `Err`.
+    fn map_owned<F, U>(&self, op: F) -> Result<U, Self::Err>
+    where
+        F: FnOnce(Self::Target) -> U;
+}
+
 /// The default user data wrapper used by derive macro, when no `user_data` attribute is present.
 /// This may change in the future.
 pub type DefaultUserData<T> = LocalCellData<T>;
@@ -440,6 +454,21 @@ impl<T, OPT> Clone for RwLockData<T, OPT> {
 #[derive(Debug)]
 pub struct ArcData<T>(Arc<T>);
 
+impl<T> ArcData<T> {
+    /// Returns the internal `Arc<T>`. Useful for API's that require an `Arc`
+    /// directly, or for coercing it into a trait object.
+    ///
+    /// Note that this removes
+    /// the restriction of only being able to access the `NativeClass` instance
+    /// temporarily through the `Map` trait; however, it should be exactly as safe
+    /// as permanently storing an owned `ArcData` and then calling `.map()` on
+    /// it later.
+    #[inline]
+    pub fn into_inner(self) -> Arc<T> {
+        self.0
+    }
+}
+
 unsafe impl<T> UserData for ArcData<T>
 where
     T: NativeClass + Send + Sync,
@@ -732,3 +761,76 @@ where
         Ok(op(&Default::default()))
     }
 }
+
+/// Special user-data wrapper intended for objects that can only be used once. Only
+/// implements `MapOwned`.
+pub struct Once<T>(Arc<atomic_take::AtomicTake<T>>);
+
+impl<T> Clone for Once<T> {
+    #[inline]
+    fn clone(&self) -> Self {
+        Once(Arc::clone(&self.0))
+    }
+}
+
+unsafe impl<T> UserData for Once<T>
+where
+    T: NativeClass + Send,
+{
+    type Target = T;
+
+    #[inline]
+    fn new(val: Self::Target) -> Self {
+        Once(Arc::new(atomic_take::AtomicTake::new(val)))
+    }
+
+    #[inline]
+    fn into_user_data(self) -> *const libc::c_void {
+        Arc::into_raw(self.0) as *const _
+    }
+
+    #[inline]
+    unsafe fn consume_user_data_unchecked(ptr: *const libc::c_void) -> Self {
+        Once(Arc::from_raw(ptr as *const _))
+    }
+
+    #[inline]
+    unsafe fn clone_from_user_data_unchecked(ptr: *const libc::c_void) -> Self {
+        let borrowed = Arc::from_raw(ptr as *const _);
+        let arc = Arc::clone(&borrowed);
+        mem::forget(borrowed);
+        Once(arc)
+    }
+}
+
+#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Default)]
+pub struct ValueTaken;
+
+impl std::error::Error for ValueTaken {}
+impl Display for ValueTaken {
+    #[inline]
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "this object has already been used once")
+    }
+}
+
+impl<T> MapOwned for Once<T>
+where
+    T: NativeClass + Send,
+{
+    type Err = ValueTaken;
+
+    /// Maps a `T` to `U`. Called for methods that take `self`. This method may fail with
+    /// an error if it is called more than once on the same object.
+    ///
+    /// Implementations of this method must not panic. Failures should be indicated by
+    /// returning `Err`.
+    #[inline]
+    fn map_owned<F, U>(&self, op: F) -> Result<U, Self::Err>
+    where
+        F: FnOnce(Self::Target) -> U,
+    {
+        let v = self.0.take().ok_or(ValueTaken)?;
+        Ok(op(v))
+    }
+}

test/src/lib.rs

@@ -5,6 +5,7 @@ use gdnative::prelude::*;
 mod test_constructor;
 mod test_derive;
 mod test_free_ub;
+mod test_map_owned;
 mod test_register;
 mod test_return_leak;
 mod test_vararray_return;
@@ -61,6 +62,7 @@ pub extern "C" fn run_tests(
     status &= test_derive::run_tests();
     status &= test_free_ub::run_tests();
     status &= test_constructor::run_tests();
+    status &= test_map_owned::run_tests();
     status &= test_register::run_tests();
     status &= test_return_leak::run_tests();
     status &= test_variant_call_args::run_tests();
@@ -255,6 +257,7 @@ fn init(handle: InitHandle) {
     test_derive::register(handle);
     test_free_ub::register(handle);
     test_constructor::register(handle);
+    test_map_owned::register(handle);
     test_register::register(handle);
     test_return_leak::register(handle);
     test_variant_call_args::register(handle);

test/src/test_map_owned.rs

@@ -0,0 +1,69 @@
+use gdnative::nativescript::user_data::Once;
+use gdnative::prelude::*;
+
+pub(crate) fn run_tests() -> bool {
+    let mut status = true;
+
+    status &= test_map_owned();
+
+    status
+}
+
+pub(crate) fn register(handle: InitHandle) {
+    handle.add_class::<VecBuilder>();
+}
+
+#[derive(NativeClass)]
+#[no_constructor]
+#[inherit(Reference)]
+#[user_data(Once<Self>)]
+struct VecBuilder {
+    v: Vec<i32>,
+}
+
+#[methods]
+impl VecBuilder {
+    #[export]
+    fn append(mut self, _owner: TRef<Reference>, mut numbers: Vec<i32>) -> Instance<Self, Shared> {
+        self.v.append(&mut numbers);
+        Instance::emplace(Self { v: self.v }).into_shared()
+    }
+}
+
+fn test_map_owned() -> bool {
+    println!(" -- test_map_owned");
+
+    let ok = std::panic::catch_unwind(|| {
+        let v1 = Instance::emplace(VecBuilder { v: Vec::new() }).into_shared();
+        let v1 = unsafe { v1.assume_safe() };
+
+        let v2 = v1
+            .map_owned(|s, owner| s.append(owner, vec![1, 2, 3]))
+            .unwrap();
+        let v2 = unsafe { v2.assume_safe() };
+        assert!(v1
+            .map_owned(|_, _| panic!("should never be called"))
+            .is_err());
+
+        let v3 = v2
+            .map_owned(|s, owner| s.append(owner, vec![4, 5, 6]))
+            .unwrap();
+        let v3 = unsafe { v3.assume_safe() };
+        assert!(v2
+            .map_owned(|_, _| panic!("should never be called"))
+            .is_err());
+
+        let v = v3.map_owned(|s, _| s.v).unwrap();
+        assert_eq!(&v, &[1, 2, 3, 4, 5, 6]);
+        assert!(v3
+            .map_owned(|_, _| panic!("should never be called"))
+            .is_err());
+    })
+    .is_ok();
+
+    if !ok {
+        gdnative::godot_error!("   !! Test test_map_owned failed");
+    }
+
+    ok
+}