Merge #622

622: Implement emplacement construction of instances r=toasteater a=toasteater

This adds the `Instance::emplace` constructor, that allows users to construct `Instance`s from existing values. The `NativeClass` trait and its derive macro are modifed to allow script types without zero-argument constructors:

- A default implementation that panics is added for `NativeClass::init`
- A `#[no_constructor]` attribute is added to the derive macro.
- Additionally, doc-comments are added to the `NativeClass` macro, with explanations and examples for the available attributes.

Close #621

Co-authored-by: toasteater <[email protected]>

Author: bors[bot]

gdnative-core/src/nativescript/class.rs

@@ -16,12 +16,17 @@ use crate::private::{get_api, ReferenceCountedClassPlaceholder};
 use crate::ref_kind::{ManuallyManaged, RefCounted};
 use crate::thread_access::{NonUniqueThreadAccess, Shared, ThreadAccess, ThreadLocal, Unique};
 
+use super::emplace;
+
 /// Trait used for describing and initializing a Godot script class.
 ///
 /// This trait is used to provide data and functionality to the
 /// "data-part" of the class, such as name, initialization and information
 /// about exported properties.
 ///
+/// A derive macro is available for this trait. See documentation on the
+/// `NativeClass` macro for detailed usage and examples.
+///
 /// For exported methods, see the [`NativeClassMethods`] trait.
 ///
 /// [`NativeClassMethods`]: ./trait.NativeClassMethods.html
@@ -59,11 +64,22 @@ pub trait NativeClass: Sized + 'static {
     /// To identify which class has to be used, a library-unique name has to be given.
     fn class_name() -> &'static str;
 
-    /// Function that creates a value of `Self`, used for the script-instance.
+    /// Function that creates a value of `Self`, used for the script-instance. The default
+    /// implementation simply panics.
     ///
     /// This function has a reference to the owner object as a parameter, which can be used to
     /// set state on the owner upon creation or to query values
-    fn init(owner: TRef<'_, Self::Base, Shared>) -> Self;
+    ///
+    /// It is possible to declare script classes without zero-argument constructors. Instances
+    /// of such scripts can only be created from Rust using `Instance::emplace`. See
+    /// documentation on `Instance::emplace` for an example.
+    #[inline]
+    fn init(_owner: TRef<'_, Self::Base, Shared>) -> Self {
+        panic!(
+            "{} does not have a zero-argument constructor",
+            Self::class_name()
+        )
+    }
 
     /// Register any exported properties to Godot.
     #[inline]
@@ -84,6 +100,22 @@ pub trait NativeClass: Sized + 'static {
     {
         Instance::new()
     }
+
+    /// Convenience method to emplace `self` into an `Instance<Self, Unique>`. This is a new
+    /// `Self::Base` with the script attached.
+    ///
+    /// If `Self::Base` is manually-managed, then the resulting `Instance` must be passed to
+    /// the engine or manually freed with `Instance::free`. Otherwise, the base object will be
+    /// leaked.
+    ///
+    /// Must be called after the library is initialized.
+    #[inline]
+    fn emplace(self) -> Instance<Self, Unique>
+    where
+        Self::Base: Instanciable,
+    {
+        Instance::emplace(self)
+    }
 }
 
 /// Trait used to provide information of Godot-exposed methods of a script class.
@@ -171,6 +203,61 @@ impl<T: NativeClass> Instance<T, Unique> {
     #[inline]
     #[allow(clippy::new_without_default)]
     pub fn new() -> Self
+    where
+        T::Base: Instanciable,
+    {
+        Self::maybe_emplace(None)
+    }
+
+    /// Creates a `T::Base` with a given instance of the script `T` attached. `T::Base` must
+    /// have a zero-argument constructor.
+    ///
+    /// This may be used to create instances of scripts that do not have zero-argument
+    /// constructors:
+    ///
+    /// ```ignore
+    /// // This type does not have a zero-argument constructor. As a result, `Instance::new`
+    /// // will panic and `Foo.new` from GDScript will result in errors when the object is used.
+    /// #[derive(NativeScript)]
+    /// #[inherit(Reference)]
+    /// #[no_constructor]
+    /// struct MyIntVector(i64, i64);
+    ///
+    /// #[methods]
+    /// impl MyIntVector {
+    ///     // - snip -
+    /// }
+    ///
+    /// // With `Instance::emplace`, however, we can expose "constructors" from a factory
+    /// // auto-load for our script type.
+    /// #[derive(NativeScript)]
+    /// #[inherit(Node)]
+    /// #[user_data(Aether<Self>)]
+    /// struct MyIntVectorFactory;
+    ///
+    /// #[methods]
+    /// impl MyIntVectorFactory {
+    ///     #[export]
+    ///     fn make(&self, _owner: &Node, x: i64, y: i64) -> Instance<MyIntVector, Unique> {
+    ///         Instance::emplace(MyIntVector(x, y))
+    ///     }
+    /// }
+    /// ```
+    ///
+    /// If `T::Base` is manually-managed, then the resulting `Instance` must be passed to
+    /// the engine or manually freed with `Instance::free`. Otherwise, the base object will be
+    /// leaked.
+    ///
+    /// Must be called after the library is initialized.
+    #[inline]
+    pub fn emplace(script: T) -> Self
+    where
+        T::Base: Instanciable,
+    {
+        Self::maybe_emplace(Some(script))
+    }
+
+    fn maybe_emplace(script: Option<T>) -> Self
     where
         T::Base: Instanciable,
     {
@@ -223,6 +310,10 @@ impl<T: NativeClass> Instance<T, Unique> {
                 std::ptr::null_mut(),
             );
 
+            if let Some(script) = script {
+                emplace::place(script);
+            }
+
             let mut args: [*const sys::godot_variant; 0] = [];
             let variant = (gd_api.godot_method_bind_call)(
                 nativescript_methods.new,
@@ -232,6 +323,11 @@ impl<T: NativeClass> Instance<T, Unique> {
                 std::ptr::null_mut(),
             );
 
+            assert!(
+                emplace::take::<T>().is_none(),
+                "emplacement value should be taken by the constructor wrapper (this is a bug in the bindings)",
+            );
+
             let variant = Variant::from_sys(variant);
 
             let owner = variant

gdnative-core/src/nativescript/emplace.rs

@@ -0,0 +1,46 @@
+//! Support code for script emplacement.
+
+use std::any::Any;
+use std::cell::RefCell;
+
+use super::NativeClass;
+
+thread_local! {
+    static CELL: RefCell<Option<Box<dyn Any>>> = RefCell::default();
+}
+
+/// Place a script to be taken by the emplacement constructor. Must be called
+/// directly before `NativeScript::_new` for intended behavior.
+///
+/// # Panics
+///
+/// If there is already a value placed for this thread, or if the thread is
+/// exiting. This is always a bug in the bindings.
+pub fn place<T: NativeClass>(script: T) {
+    CELL.with(|f| {
+        if f.replace(Some(Box::new(script))).is_some() {
+            panic!(
+                "there is already a value in the emplacement cell (this is a bug in the bindings)"
+            );
+        }
+    });
+}
+
+/// Take the script stored for emplacement and return it. Returns `None` if
+/// there is no value in store.
+///
+/// # Panics
+///
+/// If there is a value in store but it is of the incorrect type. This is always
+/// a bug in the bindings.
+pub fn take<T: NativeClass>() -> Option<T> {
+    CELL.with(|f| f.borrow_mut().take())
+        .map(|script| match script.downcast() {
+            Ok(script) => *script,
+            Err(any) => panic!(
+                "expecting {} in the emplacement cell, got {:?} (this is a bug in the bindings)",
+                T::class_name(),
+                any.type_id(),
+            ),
+        })
+}

gdnative-core/src/nativescript/init.rs

@@ -43,6 +43,8 @@ use crate::nativescript::NativeClassMethods;
 use crate::nativescript::UserData;
 use crate::private::get_api;
 
+use super::emplace;
+
 pub mod property;
 
 pub use self::property::{Export, ExportInfo, PropertyBuilder, Usage as PropertyUsage};
@@ -123,7 +125,8 @@ impl InitHandle {
                     };
 
                     let val = match panic::catch_unwind(AssertUnwindSafe(|| {
-                        C::init(TRef::new(C::Base::cast_ref(owner)))
+                        emplace::take()
+                            .unwrap_or_else(|| C::init(TRef::new(C::Base::cast_ref(owner))))
                     })) {
                         Ok(val) => val,
                         Err(_) => {

gdnative-core/src/nativescript/mod.rs

@@ -1,5 +1,6 @@
 //! Types and functions related to the NativeScript extension of GDNative.
 
+mod emplace;
 mod macros;
 
 pub mod class;

gdnative-derive/src/lib.rs

@@ -52,9 +52,91 @@ pub fn profiled(meta: TokenStream, input: TokenStream) -> TokenStream {
     profiled::derive_profiled(meta, input)
 }
 
+/// Makes it possible to use a type as a NativeScript.
+///
+/// ## Required attributes
+///
+/// The following attributes are required on the type deriving `NativeClass`:
+///
+/// ### `#[inherit(gdnative::api::BaseClass)]`
+///
+/// Sets `gdnative::api::BaseClass` as the base class for the script. This *must* be
+/// a type from the generated Godot API (that implements `GodotObject`). All `owner`
+/// arguments of exported methods must be references (`TRef`, `Ref`, or `&`) to this
+/// type.
+///
+/// Inheritance from other scripts, either in Rust or other languages, is
+/// not supported.
+///
+/// ## Optional type attributes
+///
+/// Behavior of the derive macro can be customized using attribute on the type:
+///
+/// ### `#[user_data(gdnative::user_data::SomeWrapper<Self>)]`
+///
+/// Use the given type as the user-data wrapper. See the module-level docs on
+/// `gdnative::user_data` for more information.
+///
+/// ### `#[register_with(path::to::function)]`
+///
+/// Use a custom function to register signals, properties or methods, in addition
+/// to the one generated by `#[methods]`:
+///
+/// ```ignore
+/// #[derive(NativeClass)]
+/// #[inherit(Reference)]
+/// #[register_with(my_register_function)]
+/// struct Foo;
+///
+/// fn my_register_function(builder: &ClassBuilder<Foo>) {
+///     builder.add_signal(Signal { name: "foo", args: &[] });
+///     builder.add_property::<f32>("bar")
+///         .with_getter(|_, _| 42.0)
+///         .with_hint(FloatHint::Range(RangeHint::new(0.0, 100.0)))
+///         .done();
+/// }
+/// ```
+///
+/// ### `#[no_constructor]`
+///
+/// Indicates that this type has no zero-argument constructor. Instances of such
+/// scripts can only be created from Rust using `Instance::emplace`. `Instance::new`
+/// or `ScriptName.new` from GDScript will result in panics at runtime.
+///
+/// See documentation on `Instance::emplace` for an example on how this can be used.
+///
+/// ## Optional field attributes
+///
+/// ### `#[property]`
+///
+/// Convenience attribute to register a field as a property. Possible arguments for
+/// the attribute are:
+///
+/// - `path = "my_category/my_property_name"`
+///
+/// Puts the property under the `my_category` category and renames it to
+/// `my_property_name` in the inspector and for GDScript.
+///
+/// - `default = 42.0`
+///
+/// Sets the default value *in the inspector* for this property. The setter is *not*
+/// guaranteed to be called by the engine with the value.
+///
+/// - `before_get` / `after_get` / `before_set` / `after_set` `= "Self::hook_method"`
+///
+/// Call hook methods with `self` and `owner` before and/or after the generated property
+/// accessors.
 #[proc_macro_derive(
     NativeClass,
-    attributes(inherit, export, opt, user_data, property, register_with)
+    attributes(
+        inherit,
+        export,
+        opt,
+        user_data,
+        property,
+        register_with,
+        no_constructor
+    )
 )]
 pub fn derive_native_class(input: TokenStream) -> TokenStream {
     native_script::derive_native_class(input)

gdnative-derive/src/native_script.rs

@@ -12,6 +12,7 @@ pub(crate) struct DeriveData {
     pub(crate) register_callback: Option<Path>,
     pub(crate) user_data: Type,
     pub(crate) properties: HashMap<Ident, PropertyAttrArgs>,
+    pub(crate) no_constructor: bool,
 }
 
 pub(crate) fn derive_native_class(input: TokenStream) -> TokenStream {
@@ -74,6 +75,16 @@ pub(crate) fn derive_native_class(input: TokenStream) -> TokenStream {
         // string variant needed for the `class_name` function.
         let name_str = quote!(#name).to_string();
 
+        let init = if data.no_constructor {
+            None
+        } else {
+            Some(quote! {
+                fn init(owner: ::gdnative::TRef<Self::Base>) -> Self {
+                    Self::new(::gdnative::nativescript::OwnerArg::from_safe_ref(owner))
+                }
+            })
+        };
+
         quote!(
             impl ::gdnative::nativescript::NativeClass for #name {
                 type Base = #base;
@@ -83,9 +94,7 @@ pub(crate) fn derive_native_class(input: TokenStream) -> TokenStream {
                     #name_str
                 }
 
-                fn init(owner: ::gdnative::TRef<Self::Base>) -> Self {
-                    Self::new(::gdnative::nativescript::OwnerArg::from_safe_ref(owner))
-                }
+                #init
 
                 fn register_properties(builder: &::gdnative::nativescript::init::ClassBuilder<Self>) {
                     #(#properties)*;
@@ -146,6 +155,11 @@ fn parse_derive_input(input: TokenStream) -> Result<DeriveData, TokenStream> {
             .expect("quoted tokens for default userdata should be a valid type"))
         })?;
 
+    let no_constructor = input
+        .attrs
+        .iter()
+        .any(|a| a.path.is_ident("no_constructor"));
+
     // make sure it's a struct
     let struct_data = if let Data::Struct(data) = input.data {
         data
@@ -212,5 +226,6 @@ fn parse_derive_input(input: TokenStream) -> Result<DeriveData, TokenStream> {
         register_callback,
         user_data,
         properties,
+        no_constructor,
     })
 }