From 5ed9f10c3a236250c9fa45c1a14034a6b20b8933 Mon Sep 17 00:00:00 2001 From: Chitose Yuuzaki Date: Sun, 9 Jan 2022 12:29:44 +0000 Subject: [PATCH 1/2] Separate static name from `NativeClass` into new trait This allows NativeScript types to be registered under names only determined at run time, enabling better ergonomics for libraries. Ref #601, #603 --- gdnative-async/src/rt.rs | 21 +++++- gdnative-async/src/rt/bridge.rs | 5 -- gdnative-async/src/rt/func_state.rs | 5 -- gdnative-core/src/core_types/variant.rs | 3 +- gdnative-core/src/export/class.rs | 21 +++--- gdnative-core/src/export/class_registry.rs | 44 ++++++++++--- gdnative-core/src/export/emplace.rs | 4 +- gdnative-core/src/export/method.rs | 6 +- gdnative-core/src/export/property/accessor.rs | 10 +-- .../src/export/property/invalid_accessor.rs | 6 +- gdnative-core/src/init/init_handle.rs | 64 ++++++++++++++----- gdnative-core/src/object/instance.rs | 15 +++-- gdnative-derive/src/lib.rs | 6 +- gdnative-derive/src/native_script.rs | 41 +++++++++--- test/src/test_register.rs | 16 +++-- test/src/test_return_leak.rs | 9 +-- test/src/test_variant_call_args.rs | 8 ++- 17 files changed, 192 insertions(+), 92 deletions(-) diff --git a/gdnative-async/src/rt.rs b/gdnative-async/src/rt.rs index 4d5eb5713..1178142aa 100644 --- a/gdnative-async/src/rt.rs +++ b/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::(); - handle.add_class::(); + 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(handle: &InitHandle, prefix: S) +where + S: Display, +{ + handle.add_class_as::(format!("{}SignalBridge", prefix)); + handle.add_class_as::(format!("{}FuncState", prefix)); } /// Releases all observers still in use. This should be called in the diff --git a/gdnative-async/src/rt/bridge.rs b/gdnative-async/src/rt/bridge.rs index 86fbb6e78..98eb9c302 100644 --- a/gdnative-async/src/rt/bridge.rs +++ b/gdnative-async/src/rt/bridge.rs @@ -52,11 +52,6 @@ impl NativeClass for SignalBridge { type Base = Reference; type UserData = ArcData; - 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) {} } diff --git a/gdnative-async/src/rt/func_state.rs b/gdnative-async/src/rt/func_state.rs index 32ac0a458..ba3e94f0b 100644 --- a/gdnative-async/src/rt/func_state.rs +++ b/gdnative-async/src/rt/func_state.rs @@ -25,11 +25,6 @@ impl NativeClass for FuncState { type Base = Reference; type UserData = LocalCellData; - 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) { builder .signal("completed") diff --git a/gdnative-core/src/core_types/variant.rs b/gdnative-core/src/core_types/variant.rs index 6fd95cede..eb2679cd0 100644 --- a/gdnative-core/src/core_types/variant.rs +++ b/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, diff --git a/gdnative-core/src/export/class.rs b/gdnative-core/src/export/class.rs index e15ee086c..1bbdff246 100644 --- a/gdnative-core/src/export/class.rs +++ b/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; - /// 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::() ) } @@ -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. diff --git a/gdnative-core/src/export/class_registry.rs b/gdnative-core/src/export/class_registry.rs index 4604b7b89..8c887d940 100644 --- a/gdnative-core/src/export/class_registry.rs +++ b/gdnative-core/src/export/class_registry.rs @@ -1,26 +1,50 @@ -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>> = Lazy::new(|| RwLock::new(HashSet::new())); +use crate::export::NativeClass; + +static CLASS_REGISTRY: Lazy>> = + 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()` /// Returns true if added otherwise false. #[inline] pub(crate) fn is_class_registered() -> bool { let type_id = TypeId::of::(); - CLASS_REGISTRY.read().contains(&type_id) + CLASS_REGISTRY.read().contains_key(&type_id) +} + +/// Access the [`ClassInfo`] of the class `C`. +#[inline] +pub(crate) fn with_class_info(f: F) -> Option +where + F: FnOnce(&ClassInfo) -> R, +{ + CLASS_REGISTRY.read().get(&TypeId::of::()).map(f) +} + +/// Returns the NativeScript name of the class `C` if it is registered. Returns a best-effort +/// description of the type for error reporting otherwise. +#[inline] +pub(crate) fn class_name_or_default() -> Cow<'static, str> { + with_class_info::(|i| i.name.clone()) + .unwrap_or_else(|| Cow::Borrowed(std::any::type_name::())) } -/// 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() -> bool { +pub(crate) fn register_class_as(name: Cow<'static, str>) -> Option { let type_id = TypeId::of::(); - CLASS_REGISTRY.write().insert(type_id) + CLASS_REGISTRY.write().insert(type_id, ClassInfo { name }) } /// Clears the registry diff --git a/gdnative-core/src/export/emplace.rs b/gdnative-core/src/export/emplace.rs index 52de62607..0f0dfe7f4 100644 --- a/gdnative-core/src/export/emplace.rs +++ b/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() -> Option { 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::(), any.type_id(), ), }) diff --git a/gdnative-core/src/export/method.rs b/gdnative-core/src/export/method.rs index 35bec90ab..756a11f9d 100644 --- a/gdnative-core/src/export/method.rs +++ b/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>( 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::(), ), ); return Variant::nil().leak(); @@ -560,7 +560,7 @@ unsafe extern "C" fn method_wrapper>( 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::(), ), ); return Variant::nil().leak(); diff --git a/gdnative-core/src/export/property/accessor.rs b/gdnative-core/src/export/property/accessor.rs index 0f5a0faca..5ca9937a5 100644 --- a/gdnative-core/src/export/property/accessor.rs +++ b/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::(), ); 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::(), ); 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::(), ); 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::(), ); return Variant::nil().leak(); } diff --git a/gdnative-core/src/export/property/invalid_accessor.rs b/gdnative-core/src/export/property/invalid_accessor.rs index c343f5f60..202a07744 100644 --- a/gdnative-core/src/export/property/invalid_accessor.rs +++ b/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 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::().into_owned(), property_name: self.property_name.to_string(), }); @@ -101,7 +101,7 @@ unsafe impl<'l, C: NativeClass, T: ToVariant> RawGetter 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::().into_owned(), property_name: self.property_name.to_string(), }); diff --git a/gdnative-core/src/init/init_handle.rs b/gdnative-core/src/init/init_handle.rs index db6e8820e..d12b88b28 100644 --- a/gdnative-core/src/init/init_handle.rs +++ b/gdnative-core/src/init/init_handle.rs @@ -1,7 +1,10 @@ use crate::export::user_data::UserData; -use crate::export::{class_registry, emplace, ClassBuilder, NativeClass, NativeClassMethods}; +use crate::export::{ + class_registry, emplace, ClassBuilder, NativeClass, NativeClassMethods, StaticallyNamed, +}; use crate::object::{GodotObject, RawObject, TRef}; use crate::private::get_api; +use std::borrow::Cow; use std::ffi::CString; use std::ptr; @@ -26,33 +29,60 @@ impl InitHandle { #[inline] pub fn add_class(self) where - C: NativeClassMethods, + C: NativeClassMethods + StaticallyNamed, { - self.add_maybe_tool_class::(false) + self.add_maybe_tool_class_as::(Cow::Borrowed(C::CLASS_NAME), false) } /// Registers a new tool class to the engine. #[inline] pub fn add_tool_class(self) + where + C: NativeClassMethods + StaticallyNamed, + { + self.add_maybe_tool_class_as::(Cow::Borrowed(C::CLASS_NAME), true) + } + + /// Registers a new class to the engine + /// + /// If the type implements [`StaticallyTyped`], that name is ignored in favor of the + /// name provided at registration. + #[inline] + pub fn add_class_as(self, name: String) + where + C: NativeClassMethods, + { + self.add_maybe_tool_class_as::(Cow::Owned(name), false) + } + + /// Registers a new tool class to the engine + /// + /// If the type implements [`StaticallyTyped`], that name is ignored in favor of the + /// name provided at registration. + #[inline] + pub fn add_tool_class_as(self, name: String) where C: NativeClassMethods, { - self.add_maybe_tool_class::(true) + self.add_maybe_tool_class_as::(Cow::Owned(name), true) } #[inline] - fn add_maybe_tool_class(self, is_tool: bool) + fn add_maybe_tool_class_as(self, name: Cow<'static, str>, is_tool: bool) where C: NativeClassMethods, { - if !class_registry::register_class::() { + let c_class_name = CString::new(&*name).unwrap(); + + if let Some(class_info) = class_registry::register_class_as::(name) { panic!( - "`{type_name}` has already been registered", - type_name = std::any::type_name::() + "`{type_name}` has already been registered as `{old_name}`", + type_name = std::any::type_name::(), + old_name = class_info.name, ); } + unsafe { - let class_name = CString::new(C::class_name()).unwrap(); let base_name = CString::new(C::Base::class_name()).unwrap(); let create = { @@ -67,7 +97,7 @@ impl InitHandle { None => { godot_error!( "gdnative-core: error constructing {}: owner pointer is null", - C::class_name(), + class_registry::class_name_or_default::(), ); return ptr::null_mut(); @@ -79,7 +109,7 @@ impl InitHandle { None => { godot_error!( "gdnative-core: error constructing {}: incompatible owner type, expecting {}", - C::class_name(), + class_registry::class_name_or_default::(), C::Base::class_name(), ); return ptr::null_mut(); @@ -94,7 +124,7 @@ impl InitHandle { Err(_) => { godot_error!( "gdnative-core: error constructing {}: constructor panicked", - C::class_name(), + class_registry::class_name_or_default::(), ); return ptr::null_mut(); } @@ -120,7 +150,7 @@ impl InitHandle { if user_data.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::(), ); return; } @@ -139,7 +169,7 @@ impl InitHandle { if is_tool { (get_api().godot_nativescript_register_tool_class)( self.handle as *mut _, - class_name.as_ptr() as *const _, + c_class_name.as_ptr() as *const _, base_name.as_ptr() as *const _, create, destroy, @@ -147,7 +177,7 @@ impl InitHandle { } else { (get_api().godot_nativescript_register_class)( self.handle as *mut _, - class_name.as_ptr() as *const _, + c_class_name.as_ptr() as *const _, base_name.as_ptr() as *const _, create, destroy, @@ -156,11 +186,11 @@ impl InitHandle { (get_api().godot_nativescript_set_type_tag)( self.handle as *mut _, - class_name.as_ptr() as *const _, + c_class_name.as_ptr() as *const _, crate::export::type_tag::create::(), ); - let builder = ClassBuilder::new(self.handle, class_name); + let builder = ClassBuilder::new(self.handle, c_class_name); C::register_properties(&builder); diff --git a/gdnative-core/src/object/instance.rs b/gdnative-core/src/object/instance.rs index 69f8610db..e59554c91 100644 --- a/gdnative-core/src/object/instance.rs +++ b/gdnative-core/src/object/instance.rs @@ -137,18 +137,21 @@ impl Instance { ); native_script.init_ref_count(); - let script_class_name = GodotString::from(T::class_name()); - let mut args: [*const libc::c_void; 1] = [script_class_name.sys() as *const _]; + // `set_library` should be called before `set_class_name` to trigger class registration + // before trying to fetch the class name, in case the first NativeScript instance of this + // library is being constructed from Rust. + let mut args: [*const libc::c_void; 1] = [crate::private::get_gdnative_library_sys()]; (gd_api.godot_method_bind_ptrcall)( - nativescript_methods.set_class_name, + nativescript_methods.set_library, native_script.sys().as_ptr(), args.as_mut_ptr(), std::ptr::null_mut(), ); - let mut args: [*const libc::c_void; 1] = [crate::private::get_gdnative_library_sys()]; + let script_class_name = GodotString::from(class_registry::class_name_or_default::()); + let mut args: [*const libc::c_void; 1] = [script_class_name.sys() as *const _]; (gd_api.godot_method_bind_ptrcall)( - nativescript_methods.set_library, + nativescript_methods.set_class_name, native_script.sys().as_ptr(), args.as_mut_ptr(), std::ptr::null_mut(), @@ -586,7 +589,7 @@ where fn from_variant(variant: &Variant) -> Result { let owner = Ref::::from_variant(variant)?; Self::from_base(owner).ok_or(FromVariantError::InvalidInstance { - expected: T::class_name(), + expected: class_registry::class_name_or_default::(), }) } } diff --git a/gdnative-derive/src/lib.rs b/gdnative-derive/src/lib.rs index 7a6ed0288..3996714fa 100644 --- a/gdnative-derive/src/lib.rs +++ b/gdnative-derive/src/lib.rs @@ -43,9 +43,9 @@ mod variant; /// impl NativeClass for Foo { /// type Base = gdnative::api::Reference; /// type UserData = gdnative::export::user_data::LocalCellData; -/// fn class_name() -> &'static str { -/// "Foo" -/// } +/// } +/// impl gdnative::export::StaticallyNamed for Foo { +/// const CLASS_NAME: &'static str = "Foo"; /// } /// impl gdnative::export::NativeClassMethods for Foo { /// fn register(builder: &ClassBuilder) { diff --git a/gdnative-derive/src/native_script.rs b/gdnative-derive/src/native_script.rs index a0da35852..733499954 100644 --- a/gdnative-derive/src/native_script.rs +++ b/gdnative-derive/src/native_script.rs @@ -9,6 +9,7 @@ use property_args::{PropertyAttrArgs, PropertyAttrArgsBuilder}; pub(crate) struct DeriveData { pub(crate) name: Ident, + pub(crate) godot_name: Option, pub(crate) base: Type, pub(crate) register_callback: Option, pub(crate) user_data: Type, @@ -20,19 +21,30 @@ pub(crate) fn impl_empty_nativeclass(derive_input: &DeriveInput) -> TokenStream2 let derived = crate::automatically_derived(); let name = &derive_input.ident; + let maybe_statically_named = if derive_input.generics.params.is_empty() { + let name_str = name.to_string(); + Some(quote! { + #derived + impl ::gdnative::export::StaticallyNamed for #name { + const CLASS_NAME: &'static str = #name_str; + } + }) + } else { + None + }; + quote! { #derived impl ::gdnative::export::NativeClass for #name { type Base = ::gdnative::api::Object; type UserData = ::gdnative::export::user_data::LocalCellData; - fn class_name() -> &'static str { - unimplemented!() - } fn init(owner: ::gdnative::object::TRef<'_, Self::Base, Shared>) -> Self { unimplemented!() } } + + #maybe_statically_named } } @@ -98,8 +110,14 @@ pub(crate) fn derive_native_class(derive_input: &DeriveInput) -> Result Result &'static str { - #name_str - } - #init fn register_properties(builder: &::gdnative::export::ClassBuilder) { @@ -128,6 +142,8 @@ pub(crate) fn derive_native_class(derive_input: &DeriveInput) -> Result Result { syn::parse2::(quote! { ::gdnative::api::Reference }).unwrap() }; + let godot_name = if input.generics.params.is_empty() { + Some(ident.to_string()) + } else { + None + }; + let register_callback = input .attrs .iter() @@ -235,6 +257,7 @@ fn parse_derive_input(input: &DeriveInput) -> Result { Ok(DeriveData { name: ident, + godot_name, base, register_callback, user_data, diff --git a/test/src/test_register.rs b/test/src/test_register.rs index 160e40747..680d07c7a 100644 --- a/test/src/test_register.rs +++ b/test/src/test_register.rs @@ -1,6 +1,6 @@ use std::ops::Add; -use gdnative::export::{StaticArgs, StaticArgsMethod}; +use gdnative::export::{StaticArgs, StaticArgsMethod, StaticallyNamed}; use gdnative::prelude::*; pub(crate) fn run_tests() -> bool { @@ -24,9 +24,6 @@ struct RegisterSignal; impl NativeClass for RegisterSignal { type Base = Reference; type UserData = user_data::Aether; - fn class_name() -> &'static str { - "RegisterSignal" - } fn init(_owner: TRef) -> RegisterSignal { RegisterSignal } @@ -38,6 +35,10 @@ impl NativeClass for RegisterSignal { } } +impl StaticallyNamed for RegisterSignal { + const CLASS_NAME: &'static str = "RegisterSignal"; +} + #[methods] impl RegisterSignal {} @@ -48,9 +49,6 @@ struct RegisterProperty { impl NativeClass for RegisterProperty { type Base = Reference; type UserData = user_data::MutexData; - fn class_name() -> &'static str { - "RegisterProperty" - } fn init(_owner: TRef) -> RegisterProperty { RegisterProperty { value: 42 } } @@ -64,6 +62,10 @@ impl NativeClass for RegisterProperty { } } +impl StaticallyNamed for RegisterProperty { + const CLASS_NAME: &'static str = "RegisterProperty"; +} + #[methods] impl RegisterProperty { #[export] diff --git a/test/src/test_return_leak.rs b/test/src/test_return_leak.rs index 9c8a80c70..ff4faef37 100644 --- a/test/src/test_return_leak.rs +++ b/test/src/test_return_leak.rs @@ -1,4 +1,5 @@ use gdnative::api; +use gdnative::export::StaticallyNamed; use gdnative::prelude::*; use std::sync::atomic::{AtomicUsize, Ordering as AtomicOrdering}; use std::sync::Arc; @@ -27,13 +28,13 @@ impl NativeClass for Probe { type Base = api::AnimationNodeAdd2; type UserData = user_data::RwLockData; - fn class_name() -> &'static str { - "ReturnLeakProbe" - } - fn register_properties(_builder: &ClassBuilder) {} } +impl StaticallyNamed for Probe { + const CLASS_NAME: &'static str = "ReturnLeakProbe"; +} + impl Drop for Probe { fn drop(&mut self) { self.drop_count.fetch_add(1, AtomicOrdering::AcqRel); diff --git a/test/src/test_variant_call_args.rs b/test/src/test_variant_call_args.rs index d4173ad35..22edf362e 100644 --- a/test/src/test_variant_call_args.rs +++ b/test/src/test_variant_call_args.rs @@ -1,3 +1,4 @@ +use gdnative::export::StaticallyNamed; use gdnative::prelude::*; pub(crate) fn run_tests() -> bool { @@ -17,15 +18,16 @@ struct VariantCallArgs; impl NativeClass for VariantCallArgs { type Base = Reference; type UserData = user_data::MutexData; - fn class_name() -> &'static str { - "VariantCallArgs" - } fn init(_owner: TRef) -> VariantCallArgs { VariantCallArgs } fn register_properties(_builder: &ClassBuilder) {} } +impl StaticallyNamed for VariantCallArgs { + const CLASS_NAME: &'static str = "VariantCallArgs"; +} + #[methods] impl VariantCallArgs { #[export] From fecf465907aad065c196df93e39ea0b75ff67f10 Mon Sep 17 00:00:00 2001 From: Chitose Yuuzaki Date: Sun, 9 Jan 2022 16:40:34 +0000 Subject: [PATCH 2/2] Make it clear the `class_name_or_default` is only used for diagnostics --- gdnative-core/src/export/class_registry.rs | 25 +++++++++++----------- gdnative-core/src/object/instance.rs | 12 ++++++++--- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/gdnative-core/src/export/class_registry.rs b/gdnative-core/src/export/class_registry.rs index 8c887d940..cd728765d 100644 --- a/gdnative-core/src/export/class_registry.rs +++ b/gdnative-core/src/export/class_registry.rs @@ -14,14 +14,6 @@ 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()` -/// Returns true if added otherwise false. -#[inline] -pub(crate) fn is_class_registered() -> bool { - let type_id = TypeId::of::(); - CLASS_REGISTRY.read().contains_key(&type_id) -} - /// Access the [`ClassInfo`] of the class `C`. #[inline] pub(crate) fn with_class_info(f: F) -> Option @@ -31,12 +23,21 @@ where CLASS_REGISTRY.read().get(&TypeId::of::()).map(f) } -/// Returns the NativeScript name of the class `C` if it is registered. Returns a best-effort -/// description of the type for error reporting otherwise. +/// 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()` #[inline] -pub(crate) fn class_name_or_default() -> Cow<'static, str> { +pub(crate) fn class_name() -> Option> { with_class_info::(|i| i.name.clone()) - .unwrap_or_else(|| Cow::Borrowed(std::any::type_name::())) +} + +/// 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() -> Cow<'static, str> { + class_name::().unwrap_or_else(|| Cow::Borrowed(std::any::type_name::())) } /// Registers the class `C` in the class registry, using a custom name. diff --git a/gdnative-core/src/object/instance.rs b/gdnative-core/src/object/instance.rs index e59554c91..8e66e22af 100644 --- a/gdnative-core/src/object/instance.rs +++ b/gdnative-core/src/object/instance.rs @@ -148,7 +148,15 @@ impl Instance { std::ptr::null_mut(), ); - let script_class_name = GodotString::from(class_registry::class_name_or_default::()); + let script_class_name = class_registry::class_name::() + .map(GodotString::from) + .unwrap_or_else(|| { + panic!( + "`{type_name}` must be registered before it can be used; call `handle.add_class::<{type_name}>()` in your `nativescript_init` callback", + type_name = std::any::type_name::(), + ); + }); + let mut args: [*const libc::c_void; 1] = [script_class_name.sys() as *const _]; (gd_api.godot_method_bind_ptrcall)( nativescript_methods.set_class_name, @@ -170,8 +178,6 @@ impl Instance { std::ptr::null_mut(), ); - debug_assert!(class_registry::is_class_registered::(), "`{type_name}` must be registered before it can be used; call `handle.add_class::<{type_name}>()` in your `nativescript_init` callback", type_name = std::any::type_name::()); - assert!( emplace::take::().is_none(), "emplacement value should be taken by the constructor wrapper (this is a bug in the bindings)",