Merge #847

847: Separate static name from `NativeClass` into new trait r=Bromeon a=chitoyuu

This allows NativeScript types to be registered under names only determined
at run time, enabling better ergonomics for libraries.

Ref #601, #603

## Unresolved questions

This does not implement the `#[name = "MyCustomName"]` static renaming syntax as suggested in #601 (comment), because it's very uncommon (if at all possible) for a key-value pair to be the top-level meta in an attribute. It should also be noted that at this moment, the `NativeClass` macro already utilizes a large number of type-level attributes:

- `inherit`
- `user_data`
- `register_with`
- `no_constructor`

...so it might not be wise to add another into the mix, especially under such a non-specific identifier as `name`. We might want to consider bundling (some of) these into a single top-level attribute (like `variant` for the `From`/`ToVariant` macros) instead. See #848.

Co-authored-by: Chitose Yuuzaki <[email protected]>

Author: bors[bot]

gdnative-async/src/rt.rs

@@ -1,3 +1,4 @@
+use std::fmt::Display;
 use std::marker::PhantomData;
 
 use func_state::FuncState;
@@ -89,9 +90,25 @@ impl Context {
 
 /// Adds required supporting NativeScript classes to `handle`. This must be called once and
 /// only once per initialization.
+///
+/// This registers the internal types under an unspecified prefix, with the intention to avoid
+/// collision with user types. Users may provide a custom prefix using
+/// [`register_runtime_with_prefix`], should it be necessary to name these types.
 pub fn register_runtime(handle: &InitHandle) {
-    handle.add_class::<bridge::SignalBridge>();
-    handle.add_class::<func_state::FuncState>();
+    register_runtime_with_prefix(handle, "__GDNATIVE_ASYNC_INTERNAL__")
+}
+
+/// Adds required supporting NativeScript classes to `handle`. This must be called once and
+/// only once per initialization.
+///
+/// The user should ensure that no other NativeScript types is registered under the
+/// provided prefix.
+pub fn register_runtime_with_prefix<S>(handle: &InitHandle, prefix: S)
+where
+    S: Display,
+{
+    handle.add_class_as::<bridge::SignalBridge>(format!("{}SignalBridge", prefix));
+    handle.add_class_as::<func_state::FuncState>(format!("{}FuncState", prefix));
 }
 
 /// Releases all observers still in use. This should be called in the

gdnative-async/src/rt/bridge.rs

@@ -52,11 +52,6 @@ impl NativeClass for SignalBridge {
     type Base = Reference;
     type UserData = ArcData<SignalBridge>;
 
-    fn class_name() -> &'static str {
-        // Sort of just praying that there will be no duplicates of this.
-        "__GDNATIVE_ASYNC_INTERNAL__SignalBridge"
-    }
-
     fn register_properties(_builder: &ClassBuilder<Self>) {}
 }
 

gdnative-async/src/rt/func_state.rs

@@ -25,11 +25,6 @@ impl NativeClass for FuncState {
     type Base = Reference;
     type UserData = LocalCellData<FuncState>;
 
-    fn class_name() -> &'static str {
-        // Sort of just praying that there will be no duplicates of this.
-        "__GDNATIVE_ASYNC_INTERNAL__FuncState"
-    }
-
     fn register_properties(builder: &ClassBuilder<Self>) {
         builder
             .signal("completed")

gdnative-core/src/core_types/variant.rs

@@ -1,4 +1,5 @@
 use crate::*;
+use std::borrow::Cow;
 use std::default::Default;
 use std::fmt;
 use std::mem::{forget, transmute};
@@ -884,7 +885,7 @@ pub enum FromVariantError {
     },
 
     /// Given object is not an instance of the expected NativeClass.
-    InvalidInstance { expected: &'static str },
+    InvalidInstance { expected: Cow<'static, str> },
     /// Collection contains an invalid field.
     InvalidField {
         field_name: &'static str,

gdnative-core/src/export/class.rs

@@ -1,5 +1,5 @@
 use crate::export::user_data::UserData;
-use crate::export::ClassBuilder;
+use crate::export::{class_registry, ClassBuilder};
 use crate::object::ownership::{Ownership, Shared, Unique};
 use crate::object::{GodotObject, Instance, Instanciable, TRef};
 
@@ -43,12 +43,6 @@ pub trait NativeClass: Sized + 'static {
     /// See module-level documentation on `user_data` for more info.
     type UserData: UserData<Target = Self>;
 
-    /// The name of the class.
-    ///
-    /// In GDNative+NativeScript many classes can be defined in one dynamic library.
-    /// 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. The default
     /// implementation simply panics.
     ///
@@ -62,7 +56,7 @@ pub trait NativeClass: Sized + 'static {
     fn init(_owner: TRef<'_, Self::Base, Shared>) -> Self {
         panic!(
             "{} does not have a zero-argument constructor",
-            Self::class_name()
+            class_registry::class_name_or_default::<Self>()
         )
     }
 
@@ -103,6 +97,17 @@ pub trait NativeClass: Sized + 'static {
     }
 }
 
+/// A NativeScript "class" that is statically named. [`NativeClass`] types that implement this
+/// trait can be registered using  [`InitHandle::add_class`].
+pub trait StaticallyNamed: NativeClass {
+    /// The name of the class.
+    ///
+    /// This name must be unique for the dynamic library. For generic or library code where this
+    /// is hard to satisfy, consider using [`InitHandle::add_class_as`] to provide a name
+    /// at registration time instead.
+    const CLASS_NAME: &'static str;
+}
+
 /// Trait used to provide information of Godot-exposed methods of a script class.
 pub trait NativeClassMethods: NativeClass {
     /// Function that registers all exposed methods to Godot.

gdnative-core/src/export/class_registry.rs

@@ -1,26 +1,51 @@
-use crate::export::NativeClass;
+use std::any::TypeId;
+use std::borrow::Cow;
+use std::collections::HashMap;
+
 use once_cell::sync::Lazy;
 use parking_lot::RwLock;
-use std::any::TypeId;
-use std::collections::HashSet;
 
-static CLASS_REGISTRY: Lazy<RwLock<HashSet<TypeId>>> = Lazy::new(|| RwLock::new(HashSet::new()));
+use crate::export::NativeClass;
+
+static CLASS_REGISTRY: Lazy<RwLock<HashMap<TypeId, ClassInfo>>> =
+    Lazy::new(|| RwLock::new(HashMap::new()));
+
+pub(crate) struct ClassInfo {
+    pub name: Cow<'static, str>,
+}
 
-/// Can be used to validate whether or not `C` has been added using `InitHandle::add_class<C>()`
-/// Returns true if added otherwise false.
+/// Access the [`ClassInfo`] of the class `C`.
 #[inline]
-pub(crate) fn is_class_registered<C: NativeClass>() -> bool {
-    let type_id = TypeId::of::<C>();
-    CLASS_REGISTRY.read().contains(&type_id)
+pub(crate) fn with_class_info<C: NativeClass, F, R>(f: F) -> Option<R>
+where
+    F: FnOnce(&ClassInfo) -> R,
+{
+    CLASS_REGISTRY.read().get(&TypeId::of::<C>()).map(f)
+}
+
+/// Returns the NativeScript name of the class `C` if it is registered.
+/// Can also be used to validate whether or not `C` has been added using `InitHandle::add_class<C>()`
+#[inline]
+pub(crate) fn class_name<C: NativeClass>() -> Option<Cow<'static, str>> {
+    with_class_info::<C, _, _>(|i| i.name.clone())
+}
+
+/// Returns the NativeScript name of the class `C` if it is registered, or a best-effort description
+/// of the type otherwise.
+///
+/// The returned string should only be used for diagnostic purposes, not for types that the user
+/// has to name explicitly. The format is not guaranteed.
+#[inline]
+pub(crate) fn class_name_or_default<C: NativeClass>() -> Cow<'static, str> {
+    class_name::<C>().unwrap_or_else(|| Cow::Borrowed(std::any::type_name::<C>()))
 }
 
-/// Registers the class `C` in the class registry.
-/// Returns `true` if `C` was not already present in the list.
-/// Returns `false` if `C` was already added.
+/// Registers the class `C` in the class registry, using a custom name.
+/// Returns the old `ClassInfo` if `C` was already added.
 #[inline]
-pub(crate) fn register_class<C: NativeClass>() -> bool {
+pub(crate) fn register_class_as<C: NativeClass>(name: Cow<'static, str>) -> Option<ClassInfo> {
     let type_id = TypeId::of::<C>();
-    CLASS_REGISTRY.write().insert(type_id)
+    CLASS_REGISTRY.write().insert(type_id, ClassInfo { name })
 }
 
 /// Clears the registry

gdnative-core/src/export/emplace.rs

@@ -3,6 +3,8 @@
 use std::any::Any;
 use std::cell::RefCell;
 
+use crate::export::class_registry;
+
 use super::NativeClass;
 
 thread_local! {
@@ -39,7 +41,7 @@ pub fn take<T: NativeClass>() -> Option<T> {
             Ok(script) => *script,
             Err(any) => panic!(
                 "expecting {} in the emplacement cell, got {:?} (this is a bug in the bindings)",
-                T::class_name(),
+                class_registry::class_name_or_default::<T>(),
                 any.type_id(),
             ),
         })

gdnative-core/src/export/method.rs

@@ -6,7 +6,7 @@ use std::marker::PhantomData;
 
 use crate::core_types::{FromVariant, FromVariantError, Variant};
 use crate::export::class::NativeClass;
-use crate::export::ClassBuilder;
+use crate::export::{class_registry, ClassBuilder};
 use crate::log::Site;
 use crate::object::ownership::Shared;
 use crate::object::{Ref, TInstance, TRef};
@@ -547,7 +547,7 @@ unsafe extern "C" fn method_wrapper<C: NativeClass, F: Method<C>>(
             F::site().unwrap_or_default(),
             format_args!(
                 "gdnative-core: user data pointer for {} is null (did the constructor fail?)",
-                C::class_name(),
+                class_registry::class_name_or_default::<C>(),
             ),
         );
         return Variant::nil().leak();
@@ -560,7 +560,7 @@ unsafe extern "C" fn method_wrapper<C: NativeClass, F: Method<C>>(
                 F::site().unwrap_or_default(),
                 format_args!(
                     "gdnative-core: base object pointer for {} is null (probably a bug in Godot)",
-                    C::class_name(),
+                    class_registry::class_name_or_default::<C>(),
                 ),
             );
             return Variant::nil().leak();

gdnative-core/src/export/property/accessor.rs

@@ -5,7 +5,7 @@ use std::ptr::NonNull;
 
 use crate::core_types::{FromVariant, ToVariant, Variant};
 use crate::export::user_data::{Map, MapMut, UserData};
-use crate::export::NativeClass;
+use crate::export::{class_registry, NativeClass};
 use crate::object::{GodotObject, RawObject, TRef};
 
 /// Trait for raw property setters.
@@ -219,7 +219,7 @@ where
             if class.is_null() {
                 godot_error!(
                     "gdnative-core: user data pointer for {} is null (did the constructor fail?)",
-                    C::class_name(),
+                    class_registry::class_name_or_default::<C>(),
                 );
                 return;
             }
@@ -229,7 +229,7 @@ where
                 None => {
                     godot_error!(
                         "gdnative-core: owner pointer for {} is null",
-                        C::class_name(),
+                        class_registry::class_name_or_default::<C>(),
                     );
                     return;
                 }
@@ -294,7 +294,7 @@ where
             if class.is_null() {
                 godot_error!(
                     "gdnative-core: user data pointer for {} is null (did the constructor fail?)",
-                    C::class_name(),
+                    class_registry::class_name_or_default::<C>(),
                 );
                 return Variant::nil().leak();
             }
@@ -304,7 +304,7 @@ where
                 None => {
                     godot_error!(
                         "gdnative-core: owner pointer for {} is null",
-                        C::class_name(),
+                        class_registry::class_name_or_default::<C>(),
                     );
                     return Variant::nil().leak();
                 }

gdnative-core/src/export/property/invalid_accessor.rs

@@ -3,7 +3,7 @@
 use std::mem;
 
 use crate::core_types::{FromVariant, ToVariant, Variant};
-use crate::export::NativeClass;
+use crate::export::{class_registry, NativeClass};
 
 use super::accessor::{RawGetter, RawSetter};
 
@@ -84,7 +84,7 @@ unsafe impl<'l, C: NativeClass, T: FromVariant> RawSetter<C, T> for InvalidSette
         let mut set = sys::godot_property_set_func::default();
 
         let data = Box::new(InvalidAccessorData {
-            class_name: C::class_name().to_string(),
+            class_name: class_registry::class_name_or_default::<C>().into_owned(),
             property_name: self.property_name.to_string(),
         });
 
@@ -101,7 +101,7 @@ unsafe impl<'l, C: NativeClass, T: ToVariant> RawGetter<C, T> for InvalidGetter<
         let mut get = sys::godot_property_get_func::default();
 
         let data = Box::new(InvalidAccessorData {
-            class_name: C::class_name().to_string(),
+            class_name: class_registry::class_name_or_default::<C>().into_owned(),
             property_name: self.property_name.to_string(),
         });