Allow parsing arguments lists into custom generic structs

This adds the `FromVarargs` trait and derive macro, which supports using
generic structs as argument lists, with the same `#[opt]` syntax for
optional arguments. This makes it a lot easier to actually make use of
the `Method` interface outside `#[methods]`.

This is the final step in simplifying `godot_wrap_method_inner`, which is
now a lot easier to follow, and no longer outputs unsafe code into the user
crate!

Author: toasteater

gdnative-core/Cargo.toml

@@ -23,7 +23,6 @@ approx = "0.3.2"
 euclid = "0.22.1"
 indexmap = "1.6.0"
 ahash = "0.4.5"
-thiserror = "1.0"
 
 gdnative-impl-proc-macros = { path = "../impl/proc_macros", version = "=0.9.1" }
 

gdnative-core/src/nativescript/init.rs

@@ -31,6 +31,9 @@
 //! For full examples, see [`examples`](https://github.com/godot-rust/godot-rust/tree/master/examples)
 //! in the godot-rust repository.
 
+// Temporary for unsafe method registration
+#![allow(deprecated)]
+
 use crate::*;
 
 use std::ffi::CString;
@@ -48,7 +51,9 @@ use super::emplace;
 pub mod method;
 pub mod property;
 
-pub use self::method::{Method, Varargs, RpcMode, ScriptMethod, MethodBuilder, ScriptMethodAttributes, ScriptMethodFn};
+pub use self::method::{
+    Method, MethodBuilder, RpcMode, ScriptMethod, ScriptMethodAttributes, ScriptMethodFn, Varargs,
+};
 pub use self::property::{Export, ExportInfo, PropertyBuilder, Usage as PropertyUsage};
 
 /// A handle that can register new classes to the engine during initialization.
@@ -223,6 +228,7 @@ pub struct ClassBuilder<C> {
 
 impl<C: NativeClass> ClassBuilder<C> {
     #[inline]
+    #[deprecated(note = "Unsafe registration is deprecated. Use `build_method` instead.")]
     pub fn add_method_advanced(&self, method: ScriptMethod) {
         let method_name = CString::new(method.name).unwrap();
 
@@ -256,6 +262,7 @@ impl<C: NativeClass> ClassBuilder<C> {
     }
 
     #[inline]
+    #[deprecated(note = "Unsafe registration is deprecated. Use `build_method` instead.")]
     pub fn add_method_with_rpc_mode(&self, name: &str, method: ScriptMethodFn, rpc_mode: RpcMode) {
         self.add_method_advanced(ScriptMethod {
             name,
@@ -267,12 +274,32 @@ impl<C: NativeClass> ClassBuilder<C> {
     }
 
     #[inline]
+    #[deprecated(note = "Unsafe registration is deprecated. Use `build_method` instead.")]
     pub fn add_method(&self, name: &str, method: ScriptMethodFn) {
         self.add_method_with_rpc_mode(name, method, RpcMode::Disabled);
     }
 
+    /// Returns a `MethodBuilder` which can be used to add a method to the class being
+    /// registered.
+    ///
+    /// # Examples
+    ///
+    /// Basic usage:
+    ///
+    /// ```ignore
+    /// // `Bar` is a stateful method implementing `Method<C>`
+    ///
+    /// builder
+    ///     .build_method("foo", Bar { baz: 42 })
+    ///     .with_rpc_mode(RpcMode::RemoteSync)
+    ///     .done();
+    /// ```
     #[inline]
-    pub fn build_method<'a, F: Method<C>>(&'a self, name: &'a str, method: F) -> MethodBuilder<'a, C, F> {
+    pub fn build_method<'a, F: Method<C>>(
+        &'a self,
+        name: &'a str,
+        method: F,
+    ) -> MethodBuilder<'a, C, F> {
         MethodBuilder::new(self, name, method)
     }
 

gdnative-core/src/nativescript/init/method.rs

@@ -1,13 +1,19 @@
+//! Method registration
+
+// Temporary for unsafe method registration
+#![allow(deprecated)]
+
 use std::fmt;
 use std::marker::PhantomData;
 
-use crate::thread_access::Shared;
-use crate::nativescript::class::{NativeClass, RefInstance};
 use crate::core_types::{FromVariant, FromVariantError, Variant};
+use crate::nativescript::class::{NativeClass, RefInstance};
 use crate::object::{Ref, TRef};
+use crate::thread_access::Shared;
 
 use super::ClassBuilder;
 
+/// Builder type used to register a method on a `NativeClass`.
 pub struct MethodBuilder<'a, C, F> {
     class_builder: &'a super::ClassBuilder<C>,
     name: &'a str,
@@ -80,6 +86,9 @@ where
     }
 }
 
+#[deprecated(
+    note = "Unsafe registration is deprecated. Use the safe, higher-level `MethodBuilder` API instead."
+)]
 pub type ScriptMethodFn = unsafe extern "C" fn(
     *mut sys::godot_object,
     *mut libc::c_void,
@@ -105,10 +114,16 @@ impl Default for RpcMode {
     }
 }
 
+#[deprecated(
+    note = "Unsafe registration is deprecated. Use the safe, higher-level `MethodBuilder` API instead."
+)]
 pub struct ScriptMethodAttributes {
     pub rpc_mode: RpcMode,
 }
 
+#[deprecated(
+    note = "Unsafe registration is deprecated. Use the safe, higher-level `MethodBuilder` API instead."
+)]
 pub struct ScriptMethod<'l> {
     pub name: &'l str,
     pub method_ptr: Option<ScriptMethodFn>,
@@ -118,7 +133,7 @@ pub struct ScriptMethod<'l> {
     pub free_func: Option<unsafe extern "C" fn(*mut libc::c_void) -> ()>,
 }
 
-/// Low-level trait for stateful, variadic methods that can be called on a native script type.
+/// Safe low-level trait for stateful, variadic methods that can be called on a native script type.
 pub trait Method<C: NativeClass>: Send + Sync + 'static {
     fn call(&self, this: RefInstance<'_, C, Shared>, args: Varargs<'_>) -> Variant;
 }
@@ -135,7 +150,50 @@ impl<C: NativeClass, F: Method<C> + Copy + Default> Method<C> for Stateless<F> {
     }
 }
 
-/// Interface to a list of borrowed method arguments with a convenient interface
+/// Adapter for methods whose arguments are statically determined. If the arguments would fail to
+/// type check, the method will print the errors to Godot's debug console and return `null`.
+#[derive(Clone, Copy, Default, Debug)]
+pub struct StaticArgs<F> {
+    f: F,
+}
+
+impl<F> StaticArgs<F> {
+    /// Wrap `f` in an adapter that implements `Method`.
+    #[inline]
+    pub fn new(f: F) -> Self {
+        StaticArgs { f }
+    }
+}
+
+/// Trait for methods whose argument lists are known at compile time. Not to be confused with a
+/// "static method".
+pub trait StaticArgsMethod<C: NativeClass>: Send + Sync + 'static {
+    type Args: FromVarargs;
+    fn call(&self, this: RefInstance<'_, C, Shared>, args: Self::Args) -> Variant;
+}
+
+impl<C: NativeClass, F: StaticArgsMethod<C>> Method<C> for StaticArgs<F> {
+    #[inline]
+    fn call(&self, this: RefInstance<'_, C, Shared>, mut args: Varargs<'_>) -> Variant {
+        match args.read_many::<F::Args>() {
+            Ok(parsed) => {
+                if let Err(err) = args.done() {
+                    godot_error!("{}", err);
+                    return Variant::new();
+                }
+                F::call(&self.f, this, parsed)
+            }
+            Err(errors) => {
+                for err in errors {
+                    godot_error!("{}", err);
+                }
+                Variant::new()
+            }
+        }
+    }
+}
+
+/// Safe interface to a list of borrowed method arguments with a convenient API
 /// for common operations with them. Can also be used as an iterator.
 pub struct Varargs<'a> {
     idx: usize,
@@ -166,6 +224,13 @@ impl<'a> Varargs<'a> {
         }
     }
 
+    /// Parses a structure that implements `FromVarargs` incrementally from the
+    /// remaining arguments.
+    #[inline]
+    pub fn read_many<T: FromVarargs>(&mut self) -> Result<T, Vec<ArgumentError<'a>>> {
+        T::read(self)
+    }
+
     /// Returns the remaining arguments as a slice of `Variant`s.
     #[inline]
     pub fn as_slice(&self) -> &'a [&'a Variant] {
@@ -213,19 +278,36 @@ impl<'a> Iterator for Varargs<'a> {
     }
 }
 
+/// Trait for structures that can be parsed from `Varargs`.
+///
+/// This trait can be derived for structure types where each type implements `FromVariant`.
+/// The order of fields matter for this purpose:
+///
+/// ```ignore
+/// #[derive(FromVarargs)]
+/// struct MyArgs {
+///     foo: i32,
+///     bar: String,
+///     #[opt] baz: Option<Ref<Node>>,
+/// }
+/// ```
+pub trait FromVarargs: Sized {
+    fn read<'a>(args: &mut Varargs<'a>) -> Result<Self, Vec<ArgumentError<'a>>>;
+}
+
 /// Builder for providing additional argument information for error reporting.
 pub struct ArgBuilder<'r, 'a, T> {
     args: &'r mut Varargs<'a>,
-    name: Option<&'r str>,
-    ty: Option<&'r str>,
+    name: Option<&'a str>,
+    ty: Option<&'a str>,
     _marker: PhantomData<T>,
 }
 
 impl<'r, 'a, T> ArgBuilder<'r, 'a, T> {
     /// Provides a name for this argument. If an old name is already set, it is
     /// silently replaced.
     #[inline]
-    pub fn with_name(mut self, name: &'r str) -> Self {
+    pub fn with_name(mut self, name: &'a str) -> Self {
         self.name = Some(name);
         self
     }
@@ -234,7 +316,7 @@ impl<'r, 'a, T> ArgBuilder<'r, 'a, T> {
     /// already set, it is silently replaced. If no type name is given, a value
     /// from `std::any::type_name` is used.
     #[inline]
-    pub fn with_type_name(mut self, ty: &'r str) -> Self {
+    pub fn with_type_name(mut self, ty: &'a str) -> Self {
         self.ty = Some(ty);
         self
     }
@@ -247,7 +329,7 @@ impl<'r, 'a, T: FromVariant> ArgBuilder<'r, 'a, T> {
     ///
     /// If the argument is missing, or cannot be converted to the desired type.
     #[inline]
-    pub fn get(self) -> Result<T, ArgumentError<'r>> {
+    pub fn get(self) -> Result<T, ArgumentError<'a>> {
         let name = self.name;
         let idx = self.args.idx;
 
@@ -261,7 +343,7 @@ impl<'r, 'a, T: FromVariant> ArgBuilder<'r, 'a, T> {
     ///
     /// If the argument is present, but cannot be converted to the desired type.
     #[inline]
-    pub fn get_optional(self) -> Result<Option<T>, ArgumentError<'r>> {
+    pub fn get_optional(self) -> Result<Option<T>, ArgumentError<'a>> {
         let Self { args, name, ty, .. } = self;
         let idx = args.idx;
 
@@ -282,6 +364,7 @@ impl<'r, 'a, T: FromVariant> ArgBuilder<'r, 'a, T> {
     }
 }
 
+/// Error during argument parsing.
 #[derive(Debug)]
 pub enum ArgumentError<'a> {
     Missing {
@@ -380,7 +463,7 @@ unsafe extern "C" fn method_wrapper<C: NativeClass, F: Method<C>>(
                 C::class_name(),
             );
             return Variant::new().forget();
-        },
+        }
     };
 
     let result = std::panic::catch_unwind(move || {
@@ -389,7 +472,7 @@ unsafe extern "C" fn method_wrapper<C: NativeClass, F: Method<C>>(
         let this: Ref<C::Base, Shared> = Ref::from_sys(this);
         let this: TRef<'_, C::Base, _> = this.assume_safe_unchecked();
         let this: RefInstance<'_, C, _> = RefInstance::from_raw_unchecked(this, user_data);
-    
+
         let args = Varargs::from_sys(num_args, args);
 
         F::call(method, this, args)
@@ -405,4 +488,4 @@ unsafe extern "C" fn method_wrapper<C: NativeClass, F: Method<C>>(
 
 unsafe extern "C" fn free_func<F>(method_data: *mut libc::c_void) {
     drop(Box::from_raw(method_data as *mut F))
-}
+}