NewAlloc + NewGd traits to unify construction across engine and user classes

Deprecate <EngineT>::new(), <UserT>::alloc_gd().
Remove <EngineT>::new_alloc(), <UserT>::new_gd() -- same functions in different trait.

Author: Bromeon

examples/dodge-the-creeps/rust/src/main_scene.rs

@@ -119,7 +119,7 @@ impl Main {
 impl INode for Main {
     fn init(base: Base<Node>) -> Self {
         Main {
-            mob_scene: PackedScene::new(),
+            mob_scene: PackedScene::new_gd(),
             score: 0,
             base,
             music: None,

godot-codegen/src/class_generator.rs

@@ -452,7 +452,7 @@ fn make_constructor_and_default(
     // Note: this could use class_name() but is not yet done due to upcoming lazy-load refactoring.
     //let class_name_obj = quote! { <Self as crate::obj::GodotClass>::class_name() };
 
-    let (constructor, godot_default_impl);
+    let (constructor, has_godot_default_impl);
     if ctx.is_singleton(godot_class_name) {
         // Note: we cannot return &'static mut Self, as this would be very easy to mutably alias.
         // &'static Self would be possible, but we would lose the whole mutability information (even if that is best-effort and
@@ -468,43 +468,38 @@ fn make_constructor_and_default(
                 }
             }
         };
-        godot_default_impl = TokenStream::new();
+        has_godot_default_impl = false;
     } else if !class.is_instantiable {
         // Abstract base classes or non-singleton classes without constructor
         constructor = TokenStream::new();
-        godot_default_impl = TokenStream::new();
+        has_godot_default_impl = false;
     } else if class.is_refcounted {
         // RefCounted, Resource, etc
         constructor = quote! {
+            #[deprecated = "Replaced with `new_gd` in extension trait `NewGd`."]
             pub fn new() -> Gd<Self> {
-                unsafe {
-                    let class_name = #godot_class_stringname;
-                    let object_ptr = sys::interface_fn!(classdb_construct_object)(class_name.string_sys());
-                    Gd::from_obj_sys(object_ptr)
-                }
+                // <Self as crate::obj::NewGd>::new_gd()
+                crate::obj::Gd::default()
             }
         };
-        godot_default_impl = quote! {
+        has_godot_default_impl = true;
+    } else {
+        // Manually managed classes: Object, Node etc
+        constructor = quote! {};
+        has_godot_default_impl = true;
+    }
+
+    let godot_default_impl = if has_godot_default_impl {
+        quote! {
             impl crate::obj::cap::GodotDefault for #class_name {
                 fn __godot_default() -> crate::obj::Gd<Self> {
-                    Self::new()
+                    crate::engine::construct_engine_object::<Self>()
                 }
             }
-        };
+        }
     } else {
-        // Manually managed classes: Object, Node etc
-        constructor = quote! {
-            #[must_use]
-            pub fn new_alloc() -> Gd<Self> {
-                unsafe {
-                    let class_name = #godot_class_stringname;
-                    let object_ptr = sys::interface_fn!(classdb_construct_object)(class_name.string_sys());
-                    Gd::from_obj_sys(object_ptr)
-                }
-            }
-        };
-        godot_default_impl = TokenStream::new();
-    }
+        TokenStream::new()
+    };
 
     (constructor, godot_default_impl)
 }

godot-core/src/engine/mod.rs

@@ -161,8 +161,16 @@ pub(crate) fn object_ptr_from_id(instance_id: InstanceId) -> sys::GDExtensionObj
     unsafe { sys::interface_fn!(object_get_instance_from_id)(instance_id.to_u64()) }
 }
 
-// ----------------------------------------------------------------------------------------------------------------------------------------------
-// Implementation of this file
+pub(crate) fn construct_engine_object<T>() -> Gd<T>
+where
+    T: GodotClass<Declarer = EngineDomain>,
+{
+    // SAFETY: adhere to Godot API; valid class name and returned pointer is an object.
+    unsafe {
+        let object_ptr = sys::interface_fn!(classdb_construct_object)(T::class_name().string_sys());
+        Gd::from_obj_sys(object_ptr)
+    }
+}
 
 pub(crate) fn ensure_object_alive(
     instance_id: InstanceId,
@@ -203,6 +211,9 @@ pub(crate) fn ensure_object_inherits(
     )
 }
 
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+// Implementation of this file
+
 /// Checks if `derived` inherits from `base`, using a cache for _successful_ queries.
 #[cfg(debug_assertions)]
 fn is_derived_base_cached(derived: ClassName, base: ClassName) -> bool {

godot-core/src/obj/gd.rs

@@ -56,10 +56,9 @@ use super::RawGd;
 ///
 /// To construct default instances of various `Gd<T>` types, there are extension methods on the type `T` itself:
 ///
-/// | Type \ Memory Strategy | Ref-counted          | Manually managed      | Singleton               |
-/// |------------------------|----------------------|-----------------------|-------------------------|
-/// | **Engine type**        | `Resource::new()`    | `Node::new_alloc()`   | `Os::singleton()`       |
-/// | **User type**          | `MyClass::new_gd()`  | `MyClass::alloc_gd()` | _(not yet implemented)_ |
+/// - Manually managed: [`NewAlloc::new_alloc()`][crate::obj::NewAlloc::new_alloc]
+/// - Reference-counted: [`NewGd::new_gd()`][crate::obj::NewGd::new_gd]
+/// - Singletons: `T::singleton()` (inherent)
 ///
 /// In addition, the smart pointer can be constructed in multiple ways:
 ///
@@ -150,7 +149,7 @@ where
         Self::from_object(user_object)
     }
 
-    #[deprecated = "Use `Gd::default()` or the short-hands `T::new_gd()` and `T::alloc_gd()` instead."]
+    #[deprecated = "Use `Gd::default()` or the short-hands `T::new_gd()` and `T::new_alloc()` instead."]
     pub fn new_default() -> Self
     where
         T: cap::GodotDefault,
@@ -351,7 +350,7 @@ impl<T: GodotClass> Gd<T> {
         T: cap::GodotDefault,
     {
         unsafe {
-            let object_ptr = crate::callbacks::create::<T>(std::ptr::null_mut());
+            let object_ptr = callbacks::create::<T>(std::ptr::null_mut());
             Gd::from_obj_sys(object_ptr)
         }
     }

godot-core/src/obj/traits.rs

@@ -131,20 +131,12 @@ impl<T: GodotClass> Inherits<T> for T {}
 pub trait ExportableObject: GodotClass {}
 
 /// Implemented for all user-defined classes, providing extensions on the raw object to interact with `Gd`.
+// #[deprecated = "Use `NewGd` and `NewAlloc` traits instead."]
 pub trait UserClass: GodotClass<Declarer = dom::UserDomain> {
     /// Return a new Gd which contains a default-constructed instance.
     ///
     /// `MyClass::new_gd()` is equivalent to `Gd::<MyClass>::default()`.
-    fn new_gd() -> Gd<Self>
-    where
-        Self: cap::GodotDefault + GodotClass<Mem = mem::StaticRefCount>,
-    {
-        Gd::default()
-    }
-
-    /// Return a new Gd which contains a default-constructed instance.
-    ///
-    /// `MyClass::new_gd()` is equivalent to `Gd::<MyClass>::default()`.
+    #[deprecated = "Use `NewAlloc::new_alloc()` instead."]
     #[must_use]
     fn alloc_gd<U>() -> Gd<Self>
     where
@@ -389,6 +381,45 @@ pub trait WithBaseField: GodotClass<Declarer = dom::UserDomain> {
     }
 }
 
+/// Extension trait for all reference-counted classes.
+pub trait NewGd: GodotClass {
+    /// Return a new Gd which contains a default-constructed instance.
+    ///
+    /// `MyClass::new_gd()` is equivalent to `Gd::<MyClass>::default()`.
+    fn new_gd() -> Gd<Self>;
+}
+
+impl<T> NewGd for T
+where
+    T: cap::GodotDefault + GodotClass<Mem = mem::StaticRefCount>,
+{
+    fn new_gd() -> Gd<Self> {
+        Gd::default()
+    }
+}
+
+/// Extension trait for all manually managed classes.
+pub trait NewAlloc: GodotClass {
+    /// Return a new Gd which contains a default-constructed instance.
+    ///
+    /// The result must be manually managed, e.g. by attaching it to the scene tree or calling `free()` after usage.
+    /// Failure to do so will result in memory leaks.
+    #[must_use]
+    fn new_alloc() -> Gd<Self>;
+}
+
+impl<T, U> NewAlloc for T
+where
+    T: cap::GodotDefault + GodotClass<Mem = U>,
+    U: mem::PossiblyManual,
+{
+    fn new_alloc() -> Gd<Self> {
+        use crate::obj::dom::Domain as _;
+
+        <Self as GodotClass>::Declarer::create_gd()
+    }
+}
+
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 
 /// Capability traits, providing dedicated functionalities for Godot classes
@@ -425,7 +456,7 @@ pub mod cap {
             debug_assert_eq!(
                 std::any::TypeId::of::<<Self as GodotClass>::Declarer>(),
                 std::any::TypeId::of::<dom::UserDomain>(),
-                "__godot_default() called on engine class; must be overridden for user classes"
+                "__godot_default() called on engine class; must be overridden for engine classes"
             );
 
             Gd::default_instance()
@@ -490,10 +521,10 @@ mod private {
 
 pub mod dom {
     use super::private::Sealed;
-    use crate::{
-        obj::{GodotClass, RawGd},
-        storage::Storage,
-    };
+    use crate::obj::cap::GodotDefault;
+    use crate::obj::{Gd, GodotClass, RawGd};
+    use crate::{callbacks, sys};
+    use crate::storage::Storage;
 
     /// Trait that specifies who declares a given `GodotClass`.
     pub trait Domain: Sealed {
@@ -513,6 +544,11 @@ pub mod dom {
         unsafe fn is_currently_bound<T>(obj: &RawGd<T>) -> bool
         where
             T: GodotClass<Declarer = Self>;
+
+        #[doc(hidden)]
+        fn create_gd<T>() -> Gd<T>
+        where
+            T: GodotDefault + GodotClass<Declarer = Self>;
     }
 
     /// Expresses that a class is declared by the Godot engine.
@@ -538,6 +574,17 @@ pub mod dom {
         {
             false
         }
+
+        fn create_gd<T>() -> Gd<T>
+        where
+            T: GodotDefault + GodotClass<Declarer = Self>,
+        {
+            unsafe {
+                let object_ptr =
+                    sys::interface_fn!(classdb_construct_object)(T::class_name().string_sys());
+                Gd::from_obj_sys(object_ptr)
+            }
+        }
     }
 
     /// Expresses that a class is declared by the user.
@@ -561,6 +608,16 @@ pub mod dom {
         {
             obj.storage().unwrap_unchecked().is_bound()
         }
+
+        fn create_gd<T>() -> Gd<T>
+        where
+            T: GodotDefault + GodotClass<Declarer = Self>,
+        {
+            unsafe {
+                let object_ptr = callbacks::create::<T>(std::ptr::null_mut());
+                Gd::from_obj_sys(object_ptr)
+            }
+        }
     }
 }
 

godot/src/lib.rs

@@ -235,14 +235,14 @@ pub mod prelude {
     };
     pub use super::init::{gdextension, ExtensionLibrary, InitLevel};
     pub use super::log::*;
-    pub use super::obj::{
-        Base, Gd, GdMut, GdRef, GodotClass, Inherits, InstanceId, OnReady,
-    };
+    pub use super::obj::{Base, Gd, GdMut, GdRef, GodotClass, Inherits, InstanceId, OnReady};
 
     // Make trait methods available
     pub use super::engine::NodeExt as _;
     pub use super::obj::EngineBitfield as _;
     pub use super::obj::EngineEnum as _;
-    pub use super::obj::UserClass as _; // new_gd(), alloc_gd()
+    pub use super::obj::NewAlloc as _;
+    pub use super::obj::NewGd as _;
+    pub use super::obj::UserClass as _; // new_gd(), alloc_gd() -- TODO: remove (exposed functions are deprecated)
     pub use super::obj::WithBaseField as _; // to_gd()
 }

itest/rust/src/benchmarks/mod.rs

@@ -13,7 +13,7 @@ use godot::bind::GodotClass;
 use godot::builtin::inner::InnerRect2i;
 use godot::builtin::{GString, Rect2i, StringName, Vector2i};
 use godot::engine::{Node3D, Os, RefCounted};
-use godot::obj::{Gd, InstanceId};
+use godot::obj::{Gd, InstanceId, NewAlloc, NewGd};
 
 use crate::framework::bench;
 
@@ -57,7 +57,7 @@ fn class_node_life() -> InstanceId {
 
 #[bench(repeat = 25)]
 fn class_refcounted_life() -> Gd<RefCounted> {
-    RefCounted::new()
+    RefCounted::new_gd()
 }
 
 #[bench(repeat = 25)]

itest/rust/src/builtin_tests/containers/array_test.rs

@@ -323,7 +323,7 @@ fn array_mixed_values() {
     let typed_array = array![1, 2];
     let object = Object::new_alloc();
     let node = Node::new_alloc();
-    let ref_counted = RefCounted::new();
+    let ref_counted = RefCounted::new_gd();
 
     let array = varray![
         int,
@@ -398,7 +398,7 @@ fn typed_array_pass_to_godot_func() {
     use godot::engine::image::Format;
     use godot::engine::{Image, Texture2DArray};
 
-    let mut image = Image::new();
+    let mut image = Image::new_gd();
     image.set_data(
         2,
         4,
@@ -407,7 +407,7 @@ fn typed_array_pass_to_godot_func() {
         PackedByteArray::from(&[255, 0, 255, 0, 0, 255, 0, 255]),
     );
     let images = array![image];
-    let mut texture = Texture2DArray::new();
+    let mut texture = Texture2DArray::new_gd();
     let error = texture.create_from_images(images);
 
     assert_eq!(error, Error::OK);

itest/rust/src/builtin_tests/containers/callable_test.rs

@@ -10,7 +10,7 @@ use godot::builtin::inner::InnerCallable;
 use godot::builtin::meta::ToGodot;
 use godot::builtin::{varray, Callable, GString, StringName, Variant};
 use godot::engine::{Node2D, Object};
-use godot::obj::UserClass;
+use godot::obj::{NewAlloc, NewGd};
 
 use crate::framework::itest;
 

itest/rust/src/builtin_tests/containers/signal_test.rs

@@ -11,7 +11,7 @@ use godot::bind::{godot_api, GodotClass};
 use godot::builtin::{GString, Variant};
 
 use godot::engine::Object;
-use godot::obj::{Base, Gd, UserClass, WithBaseField};
+use godot::obj::{Base, Gd, NewAlloc, WithBaseField};
 use godot::sys;
 
 use crate::framework::itest;
@@ -63,8 +63,8 @@ const SIGNAL_ARG_STRING: &str = "Signal string arg";
 #[itest]
 /// Test that godot can call a method that is connect with a signal
 fn signals() {
-    let mut emitter = Emitter::alloc_gd();
-    let receiver = Receiver::alloc_gd();
+    let mut emitter = Emitter::new_alloc();
+    let receiver = Receiver::new_alloc();
 
     let args = [
         vec![],

itest/rust/src/builtin_tests/containers/variant_test.rs

@@ -12,7 +12,7 @@ use godot::builtin::meta::{FromGodot, ToGodot};
 use godot::builtin::{dict, varray, GString, NodePath, StringName, Variant, Vector2, Vector3};
 use godot::builtin::{Basis, Dictionary, VariantArray, VariantOperator, VariantType};
 use godot::engine::Node2D;
-use godot::obj::InstanceId;
+use godot::obj::{InstanceId, NewAlloc};
 use godot::sys::GodotFfi;
 
 use crate::common::roundtrip;

itest/rust/src/builtin_tests/convert_test.rs

@@ -10,7 +10,7 @@ use godot::builtin::{
     dict, Array, Dictionary, GString, Variant, VariantArray, Vector2, Vector2Axis,
 };
 use godot::engine::{Node, Resource};
-use godot::obj::Gd;
+use godot::obj::{Gd, NewAlloc};
 
 use crate::framework::itest;