Merge pull request #427 from godot-rust/feature/direct-stringname-ctor

Add `StringName::from_latin1_with_nul()`, direct construction in `String::from(&str)`

Author: Bromeon

godot-codegen/src/util.rs

@@ -181,13 +181,20 @@ pub(crate) fn make_utility_function_ptr_name(function: &UtilityFunction) -> Iden
     safe_ident(&function.name)
 }
 
-// TODO should eventually be removed, as all StringNames are cached
+#[cfg(since_api = "4.2")]
 pub fn make_string_name(identifier: &str) -> TokenStream {
+    let lit = proc_macro2::Literal::byte_string(format!("{identifier}\0").as_bytes());
     quote! {
-        StringName::from(#identifier)
+        StringName::from_latin1_with_nul(#lit)
     }
 }
 
+#[cfg(before_api = "4.2")]
+pub fn make_string_name(identifier: &str) -> TokenStream {
+    quote! {
+        StringName::from(#identifier)
+    }
+}
 pub fn make_sname_ptr(identifier: &str) -> TokenStream {
     quote! {
         string_names.fetch(#identifier)

godot-core/src/builtin/string/macros.rs

@@ -8,16 +8,6 @@
 
 macro_rules! impl_rust_string_conv {
     ($Ty:ty) => {
-        impl<S> From<S> for $Ty
-        where
-            S: AsRef<str>,
-        {
-            fn from(string: S) -> Self {
-                let intermediate = GodotString::from(string.as_ref());
-                Self::from(&intermediate)
-            }
-        }
-
         impl From<&$Ty> for String {
             fn from(string: &$Ty) -> Self {
                 let intermediate = GodotString::from(string);

godot-core/src/builtin/string/node_path.rs

@@ -98,6 +98,16 @@ impl fmt::Debug for NodePath {
 
 impl_rust_string_conv!(NodePath);
 
+impl<S> From<S> for NodePath
+where
+    S: AsRef<str>,
+{
+    fn from(string: S) -> Self {
+        let intermediate = GodotString::from(string.as_ref());
+        Self::from(&intermediate)
+    }
+}
+
 impl From<&GodotString> for NodePath {
     fn from(string: &GodotString) -> Self {
         unsafe {

godot-core/src/builtin/string/string_name.rs

@@ -7,11 +7,10 @@
 use godot_ffi as sys;
 use sys::{ffi_methods, GodotFfi};
 
-use std::fmt;
-
 use crate::builtin::inner;
+use crate::builtin::{GodotString, NodePath};
 
-use super::{GodotString, NodePath};
+use std::fmt;
 
 /// A string optimized for unique names.
 ///
@@ -27,6 +26,43 @@ impl StringName {
         Self { opaque }
     }
 
+    /// Creates a `StringName` from a static, nul-terminated ASCII/Latin-1 `b"string"` literal.
+    ///
+    /// Avoids unnecessary copies and allocations and directly uses the backing buffer. Useful for literals.
+    ///
+    /// # Example
+    /// ```no_run
+    /// use godot::builtin::StringName;
+    ///
+    /// let sname = StringName::from_latin1_with_nul(b"± Latin-1 string\0");
+    /// ```
+    ///
+    /// # Panics
+    /// When the string is not nul-terminated or contains interior nul bytes.
+    ///
+    /// Note that every byte is valid in Latin-1, so there is no encoding validation being performed.
+    #[cfg(since_api = "4.2")]
+    pub fn from_latin1_with_nul(latin1_c_str: &'static [u8]) -> Self {
+        let c_str = std::ffi::CStr::from_bytes_with_nul(latin1_c_str)
+            .unwrap_or_else(|_| panic!("invalid or not nul-terminated CStr: '{latin1_c_str:?}'"));
+
+        let result = unsafe {
+            Self::from_string_sys_init(|ptr| {
+                sys::interface_fn!(string_name_new_with_latin1_chars)(
+                    ptr,
+                    c_str.as_ptr(),
+                    true as sys::GDExtensionBool, // p_is_static
+                )
+            })
+        };
+
+        // StringName expects that the destructor is not invoked on static instances (or only at global exit; see SNAME(..) macro in Godot).
+        // According to testing with godot4 --verbose, there is no mention of "Orphan StringName" at shutdown when incrementing the ref-count,
+        // so this should not leak memory.
+        result.inc_ref();
+        result
+    }
+
     /// Returns the number of characters in the string.
     ///
     /// _Godot equivalent: `length`_
@@ -63,6 +99,11 @@ impl StringName {
     pub fn as_inner(&self) -> inner::InnerStringName {
         inner::InnerStringName::from_outer(self)
     }
+
+    /// Increment ref-count. This may leak memory if used wrongly.
+    fn inc_ref(&self) {
+        std::mem::forget(self.clone());
+    }
 }
 
 // SAFETY:
@@ -84,7 +125,7 @@ unsafe impl GodotFfi for StringName {
 
     unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self {
         let string_name = Self::from_sys(ptr);
-        std::mem::forget(string_name.clone());
+        string_name.inc_ref();
         string_name
     }
 
@@ -134,6 +175,33 @@ unsafe impl Send for StringName {}
 
 impl_rust_string_conv!(StringName);
 
+impl<S> From<S> for StringName
+where
+    S: AsRef<str>,
+{
+    #[cfg(before_api = "4.2")]
+    fn from(string: S) -> Self {
+        let intermediate = GodotString::from(string.as_ref());
+        Self::from(&intermediate)
+    }
+
+    #[cfg(since_api = "4.2")]
+    fn from(string: S) -> Self {
+        let utf8 = string.as_ref().as_bytes();
+
+        // SAFETY: Rust guarantees validity and range of string.
+        unsafe {
+            Self::from_string_sys_init(|ptr| {
+                sys::interface_fn!(string_name_new_with_utf8_chars_and_len)(
+                    ptr,
+                    utf8.as_ptr() as *const std::ffi::c_char,
+                    utf8.len() as i64,
+                );
+            })
+        }
+    }
+}
+
 impl From<&GodotString> for StringName {
     fn from(string: &GodotString) -> Self {
         unsafe {

godot-core/src/log.rs

@@ -12,7 +12,7 @@ macro_rules! inner_godot_msg {
     //($($args:tt),* $(,)?) => {
         unsafe {
             let msg = format!("{}\0", format_args!($fmt $(, $args)*));
-            assert!(msg.is_ascii(), "godot_error: message must be ASCII");
+            // assert!(msg.is_ascii(), "godot_error: message must be ASCII");
 
             // Check whether engine is loaded, otherwise fall back to stderr.
             if $crate::sys::is_initialized() {

godot-ffi/src/toolbox.rs

@@ -123,10 +123,9 @@ pub fn c_str(s: &[u8]) -> *const std::ffi::c_char {
     s.as_ptr() as *const std::ffi::c_char
 }
 
+/// Returns a C `const char*` for a null-terminated string slice. UTF-8 encoded.
 #[inline]
 pub fn c_str_from_str(s: &str) -> *const std::ffi::c_char {
-    debug_assert!(s.is_ascii());
-
     c_str(s.as_bytes())
 }
 

itest/rust/src/builtin_tests/string/string_name_test.rs

@@ -19,6 +19,8 @@ fn string_name_default() {
 
 #[itest]
 fn string_name_conversion() {
+    // Note: StringName::from(&str) uses direct FFI constructor from Godot 4.2 onwards.
+
     let string = GodotString::from("some string");
     let name = StringName::from(&string);
     let back = GodotString::from(&name);
@@ -57,7 +59,7 @@ fn string_name_equality() {
 
 // TODO: add back in when ordering StringNames is fixed
 #[itest(skip)]
-fn string_ordering() {
+fn string_name_ordering() {
     let _low = StringName::from("Alpha");
     let _high = StringName::from("Beta");
     /*
@@ -109,3 +111,28 @@ fn string_name_is_empty() {
     let empty = StringName::default();
     assert!(empty.is_empty());
 }
+
+#[itest]
+#[cfg(since_api = "4.2")]
+fn string_name_from_latin1_with_nul() {
+    let cases: [(&[u8], &str); 3] = [
+        (b"pure ASCII\t[~]\0", "pure ASCII\t[~]\0"),
+        (b"\xB1\0", "±"),
+        (b"Latin-1 \xA3 \xB1 text \xBE\0", "Latin-1 £ ± text ¾"),
+    ];
+
+    for (bytes, string) in cases.into_iter() {
+        let a = StringName::from_latin1_with_nul(bytes);
+        let b = StringName::from(string);
+
+        println!();
+        println!(
+            "Arrays: a={:?}, b={:?}",
+            a.to_string().as_bytes(),
+            b.to_string().as_bytes()
+        );
+        println!("Hashes: a={:?}, b={:?}", a.hash(), b.hash());
+        println!("Lengths: a={}, b={}", a.len(), b.len());
+        assert_eq!(a, b);
+    }
+}