Use Global<T> instead of Mutex<Option<T>> in one place

Leave in 2 places: one actually needs Option for late-init, the other needs try_lock().
Also use ClassName by value whenever possible, since it's Copy.

Author: Bromeon

godot-core/src/builtin/meta/class_name.rs

@@ -8,23 +8,15 @@
 use godot_ffi as sys;
 use std::collections::HashMap;
 use std::ffi::CStr;
-use std::{fmt, sync};
-
+use std::fmt;
 use std::hash::{Hash, Hasher};
 
 use crate::builtin::*;
+use sys::Global;
 
-// Why is this so ugly?
-// - Mutex: needed for global access (Sync).
-// - Option: needed to initialize lazily, because HashMap::new() is not const.
-// - Box: needed for pointer stability (HashMap insertion may invalidate pointers -- with_capacity() would be an alternative,
-//   but we don't know how many classes).
-// In theory a static mut would do the job, however if we allow for manual class registration (at any time), we need to count with
-// later adjustments.
-// We may also consider OnceLock with a static per class, but that needs to be code-generated (for #[derive] and engine classes), and
-// any manually registered classes would need to replicate it later.
-static CACHED_STRING_NAMES: sync::Mutex<Option<HashMap<ClassName, Box<StringName>>>> =
-    sync::Mutex::new(None);
+// Box is needed for pointer stability (HashMap insertion may invalidate pointers -- with_capacity() would be an alternative,
+// but we don't know how many classes).
+static CACHED_STRING_NAMES: Global<HashMap<ClassName, Box<StringName>>> = Global::default();
 
 /// Name of a class registered with Godot.
 ///
@@ -80,8 +72,7 @@ impl ClassName {
 
     // Takes a closure because the mutex guard protects the reference; so the &StringName cannot leave the scope.
     fn with_string_name<R>(&self, func: impl FnOnce(&StringName) -> R) -> R {
-        let mut guard = CACHED_STRING_NAMES.lock().unwrap();
-        let map = guard.get_or_insert_with(HashMap::new);
+        let mut map = CACHED_STRING_NAMES.lock();
 
         let value = map
             .entry(*self)

godot-core/src/builtin/meta/registration/constant.rs

@@ -27,7 +27,7 @@ impl IntegerConstant {
         }
     }
 
-    fn register(&self, class_name: &ClassName, enum_name: &StringName, is_bitfield: bool) {
+    fn register(&self, class_name: ClassName, enum_name: &StringName, is_bitfield: bool) {
         unsafe {
             interface_fn!(classdb_register_extension_class_integer_constant)(
                 sys::get_library(),
@@ -56,7 +56,7 @@ pub enum ConstantKind {
 }
 
 impl ConstantKind {
-    fn register(&self, class_name: &ClassName) {
+    fn register(&self, class_name: ClassName) {
         match self {
             ConstantKind::Integer(integer) => {
                 integer.register(class_name, &StringName::default(), false)
@@ -87,6 +87,6 @@ impl ExportConstant {
     }
 
     pub fn register(&self) {
-        self.kind.register(&self.class_name)
+        self.kind.register(self.class_name)
     }
 }

godot-core/src/lib.rs

@@ -51,7 +51,7 @@ pub mod private {
     pub use crate::gen::classes::class_macros;
     pub use crate::registry::{callbacks, ClassPlugin, ErasedRegisterFn, PluginComponent};
     pub use crate::storage::as_storage;
-    pub use godot_ffi::out;
+    pub use sys::out;
 
     use crate::{log, sys};
 

godot-core/src/registry.rs

@@ -5,30 +5,28 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-#![allow(dead_code)] // FIXME
-
+use crate::builtin::meta::ClassName;
+use crate::builtin::StringName;
 use crate::init::InitLevel;
 use crate::log;
 use crate::obj::*;
+use crate::out;
 use crate::private::as_storage;
 use crate::storage::InstanceStorage;
 use godot_ffi as sys;
 
 use sys::interface_fn;
 
-use crate::builtin::meta::ClassName;
-use crate::builtin::StringName;
-use crate::out;
 use std::any::Any;
 use std::collections::HashMap;
 use std::sync::{Mutex, MutexGuard, TryLockError};
 use std::{fmt, ptr};
 
-// For now, that variable is needed for class unregistering. It's populated during class
-// registering. There is no actual concurrency here, because Godot call register/unregister in main
-// thread - Mutex is just casual way to ensure safety in this performance non-critical path.
-// Note that we panic on concurrent access instead of blocking - that's fail fast approach. If that
-// happen, most likely something changed on Godot side and analysis required to adopt these changes.
+// Needed for class unregistering. The variable is populated during class registering. There is no actual concurrency here, because Godot
+// calls register/unregister in the main thread. Mutex is just casual way to ensure safety in this non-performance-critical path.
+// Note that we panic on concurrent access instead of blocking (fail-fast approach). If that happens, most likely something changed on Godot
+// side and analysis required to adopt these changes.
+// For now, don't use Global<T> because we need try_lock(), which isn't yet provided.
 static LOADED_CLASSES: Mutex<Option<HashMap<InitLevel, Vec<ClassName>>>> = Mutex::new(None);
 
 // TODO(bromeon): some information coming from the proc-macro API is deferred through PluginComponent, while others is directly
@@ -182,6 +180,7 @@ struct ClassRegistrationInfo {
     godot_params: sys::GDExtensionClassCreationInfo,
     #[cfg(since_api = "4.2")]
     godot_params: sys::GDExtensionClassCreationInfo2,
+    #[allow(dead_code)] // Currently unused; may be useful for diagnostics in the future.
     init_level: InitLevel,
     is_editor_plugin: bool,
 }
@@ -305,7 +304,7 @@ pub fn unregister_classes(init_level: InitLevel) {
         .unwrap_or_default();
     out!("Unregistering classes of level {init_level:?}...");
     for class_name in loaded_classes_current_level.iter().rev() {
-        unregister_class_raw(class_name);
+        unregister_class_raw(*class_name);
     }
 }
 
@@ -315,9 +314,9 @@ fn get_loaded_classes_with_mutex() -> MutexGuard<'static, Option<HashMap<InitLev
         Ok(it) => it,
         Err(err) => match err {
             TryLockError::Poisoned(_err) => panic!(
-                "LOADED_CLASSES poisoned. seems like class registration or deregistration panicked."
+                "global lock for loaded classes poisoned; class registration or deregistration may have panicked"
             ),
-            TryLockError::WouldBlock => panic!("unexpected concurrent access detected to CLASSES"),
+            TryLockError::WouldBlock => panic!("unexpected concurrent access to global lock for loaded classes"),
         },
     }
 }
@@ -494,7 +493,7 @@ fn register_class_raw(mut info: ClassRegistrationInfo) {
     assert!(!info.is_editor_plugin);
 }
 
-fn unregister_class_raw(class_name: &ClassName) {
+fn unregister_class_raw(class_name: ClassName) {
     out!("Unregister class: {class_name}");
     unsafe {
         #[allow(clippy::let_unit_value)]