Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions godot-codegen/src/generator/classes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ fn make_class(class: &Class, ctx: &mut Context, view: &ApiView) -> GeneratedClas

// Strings
let godot_class_str = &class_name.godot_ty;
let class_name_cstr = util::c_str(godot_class_str);

// Idents and tokens
let (base_ty, base_ident_opt) = match class.base_class.as_ref() {
Expand Down Expand Up @@ -248,7 +247,7 @@ fn make_class(class: &Class, ctx: &mut Context, view: &ApiView) -> GeneratedClas
// Optimization note: instead of lazy init, could use separate static which is manually initialized during registration.
static CLASS_ID: std::sync::OnceLock<ClassId> = std::sync::OnceLock::new();

let name: &'static ClassId = CLASS_ID.get_or_init(|| ClassId::__alloc_next_ascii(#class_name_cstr));
let name: &'static ClassId = CLASS_ID.get_or_init(|| ClassId::__alloc_next_unicode(#godot_class_str));
*name
}

Expand Down
89 changes: 18 additions & 71 deletions godot-core/src/meta/class_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use std::any::TypeId;
use std::borrow::Cow;
use std::cell::OnceCell;
use std::collections::HashMap;
use std::ffi::CStr;
use std::fmt;
use std::hash::Hash;

Expand Down Expand Up @@ -95,7 +94,7 @@ impl ClassId {
"In Godot < 4.4, class name must be ASCII: '{name}'"
);

cache.insert_class_id(ClassIdSource::Owned(name), Some(type_id), false)
cache.insert_class_id(Cow::Owned(name), Some(type_id), false)
}

/// Create a `ClassId` from a runtime string (for dynamic class names).
Expand All @@ -105,7 +104,7 @@ impl ClassId {
pub(crate) fn new_dynamic(class_name: String) -> Self {
let mut cache = CLASS_ID_CACHE.lock();

cache.insert_class_id(ClassIdSource::Owned(class_name), None, false)
cache.insert_class_id(Cow::Owned(class_name), None, false)
}

// Test-only APIs.
Expand All @@ -127,37 +126,16 @@ impl ClassId {
Self { global_index: 0 }
}

/// Create a new ASCII; expect to be unique. Internal, reserved for macros.
#[doc(hidden)]
pub fn __alloc_next_ascii(class_name_cstr: &'static CStr) -> Self {
let utf8 = class_name_cstr
.to_str()
.expect("class name is invalid UTF-8");

assert!(
utf8.is_ascii(),
"ClassId::alloc_next_ascii() with non-ASCII Unicode string '{utf8}'"
);

let source = ClassIdSource::Borrowed(class_name_cstr);
let mut cache = CLASS_ID_CACHE.lock();
cache.insert_class_id(source, None, true)
}

/// Create a new Unicode entry; expect to be unique. Internal, reserved for macros.
#[doc(hidden)]
pub fn __alloc_next_unicode(class_name_str: &'static str) -> Self {
#[cfg(before_api = "4.4")]
assert!(
cfg!(since_api = "4.4"),
class_name_str.is_ascii(),
"Before Godot 4.4, class names must be ASCII, but '{class_name_str}' is not.\nSee https://github.com/godotengine/godot/pull/96501."
);

assert!(
!class_name_str.is_ascii(),
"ClassId::__alloc_next_unicode() with ASCII string '{class_name_str}'"
);

let source = ClassIdSource::Owned(class_name_str.to_owned());
let source = Cow::Borrowed(class_name_str);
let mut cache = CLASS_ID_CACHE.lock();
cache.insert_class_id(source, None, true)
}
Expand All @@ -181,7 +159,7 @@ impl ClassId {
pub fn to_cow_str(&self) -> Cow<'static, str> {
let cache = CLASS_ID_CACHE.lock();
let entry = cache.get_entry(self.global_index as usize);
entry.rust_str.as_cow_str()
entry.rust_str.clone()
}

/// The returned pointer is valid indefinitely, as entries are never deleted from the cache.
Expand All @@ -198,23 +176,21 @@ impl ClassId {

let string_name = entry
.godot_str
.get_or_init(|| entry.rust_str.to_string_name());
.get_or_init(|| StringName::from(entry.rust_str.as_ref()));

func(string_name)
}
}

impl fmt::Display for ClassId {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.with_string_name(|s| s.fmt(f))
self.to_cow_str().fmt(f)
}
}

impl fmt::Debug for ClassId {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let cache = CLASS_ID_CACHE.lock();
let entry = cache.get_entry(self.global_index as usize);
let name = entry.rust_str.as_cow_str();
let name = self.to_cow_str();

if name.is_empty() {
write!(f, "ClassId(none)")
Expand All @@ -224,58 +200,31 @@ impl fmt::Debug for ClassId {
}
}

fn ascii_cstr_to_str(cstr: &CStr) -> &str {
cstr.to_str().expect("should be validated ASCII")
}

// ----------------------------------------------------------------------------------------------------------------------------------------------

/// Entry in the class name cache.
///
/// `StringName` needs to be lazy-initialized because the Godot binding may not be initialized yet.
struct ClassIdEntry {
rust_str: ClassIdSource,
rust_str: Cow<'static, str>,
godot_str: OnceCell<StringName>,
}

impl ClassIdEntry {
fn new(rust_str: ClassIdSource) -> Self {
fn new(rust_str: Cow<'static, str>) -> Self {
Self {
rust_str,
godot_str: OnceCell::new(),
}
}

fn none() -> Self {
Self::new(ClassIdSource::Borrowed(c""))
Self::new(Cow::Borrowed(""))
}
}

// ----------------------------------------------------------------------------------------------------------------------------------------------

/// `Cow`-like enum for class names, but with C strings as the borrowed variant.
enum ClassIdSource {
Owned(String),
Borrowed(&'static CStr),
}

impl ClassIdSource {
fn to_string_name(&self) -> StringName {
match self {
ClassIdSource::Owned(s) => StringName::from(s),
ClassIdSource::Borrowed(cstr) => StringName::__cstr(cstr),
}
}

fn as_cow_str(&self) -> Cow<'static, str> {
match self {
ClassIdSource::Owned(s) => Cow::Owned(s.clone()),
ClassIdSource::Borrowed(cstr) => Cow::Borrowed(ascii_cstr_to_str(cstr)),
}
}
}
// ----------------------------------------------------------------------------------------------------------------------------------------------

/// Unified cache for all class name data.
struct ClassIdCache {
/// All class name entries, with index representing [`ClassId::global_index`].
Expand Down Expand Up @@ -308,23 +257,21 @@ impl ClassIdCache {
/// If `expect_first` is true and the string is already present in the cache.
fn insert_class_id(
&mut self,
source: ClassIdSource,
source: Cow<'static, str>,
type_id: Option<TypeId>,
expect_first: bool,
) -> ClassId {
let name_str = source.as_cow_str();

if expect_first {
// Debug verification that we're indeed the first to register this string.
#[cfg(debug_assertions)]
assert!(
!self.string_to_index.contains_key(name_str.as_ref()),
!self.string_to_index.contains_key(source.as_ref()),
"insert_class_name() called for already-existing string: {}",
name_str
source
);
} else {
// Check string cache first (dynamic path may reuse existing entries).
if let Some(&existing_index) = self.string_to_index.get(name_str.as_ref()) {
if let Some(&existing_index) = self.string_to_index.get(source.as_ref()) {
// Update type cache if we have a TypeId and it's not already cached (dynamic-then-static case).
// Note: if type_id is Some, we know it came from new_cached_inner after a failed TypeId lookup.
if let Some(type_id) = type_id {
Expand All @@ -342,9 +289,9 @@ impl ClassIdCache {
panic!("ClassId cache exceeded maximum capacity of 65536 entries")
});

self.entries.push(ClassIdEntry::new(source));
self.entries.push(ClassIdEntry::new(source.clone()));
self.string_to_index
.insert(name_str.into_owned(), global_index);
.insert(source.into_owned(), global_index);

if let Some(type_id) = type_id {
self.type_to_index.insert(type_id, global_index);
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/meta/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ impl<'a> CallContext<'a> {
}

/// Call from Godot into a custom Callable.
pub fn custom_callable(function_name: &'a str) -> Self {
pub const fn custom_callable(function_name: &'a str) -> Self {
Self {
class_name: Cow::Borrowed("<Callable>"),
function_name,
Expand Down
10 changes: 1 addition & 9 deletions godot-macros/src/class/derive_godot_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,7 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult<TokenStream> {
.map_or_else(|| class.name.clone(), |rename| rename)
.to_string();

// Determine if we can use ASCII for the class name (in most cases).
// Crate godot-macros does not have knowledge of `api-*` features (and neither does user crate where macros are generated),
// so we can't cause a compile error if Unicode is used before Godot 4.4. However, this causes a runtime error at startup.
let class_name_allocation = if class_name_str.is_ascii() {
let c_str = util::c_str(&class_name_str);
quote! { ClassId::__alloc_next_ascii(#c_str) }
} else {
quote! { ClassId::__alloc_next_unicode(#class_name_str) }
};
let class_name_allocation = quote! { ClassId::__alloc_next_unicode(#class_name_str) };

if struct_cfg.is_internal {
modifiers.push(quote! { with_internal })
Expand Down
4 changes: 2 additions & 2 deletions itest/rust/src/object_tests/class_name_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,10 @@ fn class_name_debug() {
fn class_name_alloc_panic() {
// ASCII.
{
let _1st = ClassId::__alloc_next_ascii(c"DuplicateTestClass");
let _1st = ClassId::__alloc_next_unicode("DuplicateTestClass");

expect_panic("2nd allocation with same ASCII string fails", || {
let _2nd = ClassId::__alloc_next_ascii(c"DuplicateTestClass");
let _2nd = ClassId::__alloc_next_unicode("DuplicateTestClass");
});
}

Expand Down
Loading