Merge #233

233: Refactor `GodotString`, `NodePath`, and `StringName` r=lilizoey a=lilizoey

Creates a new module in `builtin` that contains all the string-related modules.

Make all the string-files look more similar where it made sense. And add things that only existed in one of the three but not the others, such as: `Hash` impl (now using godot's hashing), new-constructor, conversions.

Added pass-by-value From impls in addition to pass-by-reference From impls, so we can do just
```rs
let node_path: NodePath = string.into();
```
instead of needing to do 
```rs
let node_path: NodePath = NodePath::from(&string);
```

Moves `String`-specific stuff into `builtin/string/mod.rs` and renamed `string.rs` to `godot_string.rs`. And adds a `VariantMetadata` impl for `String`.

Adds some more tests to test all the types a bit more extensively.

Since this is gonna conflict with #231 (as i added some stuff to StringName there) i wanna wait with merging this until that is merged. but otherwise the PR is ready to be merged.

Co-authored-by: Lili Zoey <[email protected]>

Author: bors[bot]

godot-core/src/builtin/macros.rs

@@ -107,17 +107,27 @@ macro_rules! impl_builtin_traits_inner {
             }
         }
     };
+
+
+    // Requires a `hash` function.
+    ( Hash for $Type:ty ) => {
+        impl std::hash::Hash for $Type {
+            fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+                self.hash().hash(state)
+            }
+        }
+    };
 }
 
 macro_rules! impl_builtin_traits {
     (
         for $Type:ty {
-            $( $Trait:ident => $gd_method:ident; )*
+            $( $Trait:ident $(=> $gd_method:ident)?; )*
         }
     ) => (
         $(
             impl_builtin_traits_inner! {
-                $Trait for $Type => $gd_method
+                $Trait for $Type $(=> $gd_method)?
             }
         )*
     )

godot-core/src/builtin/mod.rs

@@ -42,7 +42,6 @@ pub use callable::*;
 pub use color::*;
 pub use dictionary_inner::Dictionary;
 pub use math::*;
-pub use node_path::*;
 pub use others::*;
 pub use packed_array::*;
 pub use plane::*;
@@ -52,7 +51,6 @@ pub use rect2::*;
 pub use rect2i::*;
 pub use rid::*;
 pub use string::*;
-pub use string_name::*;
 pub use transform2d::*;
 pub use transform3d::*;
 pub use variant::*;
@@ -95,7 +93,6 @@ mod callable;
 mod color;
 mod glam_helpers;
 mod math;
-mod node_path;
 mod others;
 mod packed_array;
 mod plane;
@@ -105,8 +102,6 @@ mod rect2;
 mod rect2i;
 mod rid;
 mod string;
-mod string_chars;
-mod string_name;
 mod transform2d;
 mod transform3d;
 mod variant;

godot-core/src/builtin/string.rs renamed to godot-core/src/builtin/string/godot_string.rs

@@ -10,17 +10,19 @@ use godot_ffi as sys;
 use sys::types::OpaqueString;
 use sys::{ffi_methods, interface_fn, GodotFfi};
 
-use super::{
-    string_chars::validate_unicode_scalar_sequence, FromVariant, ToVariant, Variant,
-    VariantConversionError,
-};
+use crate::builtin::inner;
 
+use super::string_chars::validate_unicode_scalar_sequence;
+use super::{NodePath, StringName};
+
+/// Godot's reference counted string type.
 #[repr(C, align(8))]
 pub struct GodotString {
     opaque: OpaqueString,
 }
 
 impl GodotString {
+    /// Construct a new empty GodotString.
     pub fn new() -> Self {
         Self::default()
     }
@@ -29,12 +31,12 @@ impl GodotString {
         Self { opaque }
     }
 
-    ffi_methods! {
-        type sys::GDExtensionStringPtr = *mut Opaque;
-
-        fn from_string_sys = from_sys;
-        fn from_string_sys_init = from_sys_init;
-        fn string_sys = sys;
+    /// Returns a 32-bit integer hash value representing the string.
+    pub fn hash(&self) -> u32 {
+        self.as_inner()
+            .hash()
+            .try_into()
+            .expect("Godot hashes are uint32_t")
     }
 
     /// Move `self` into a system pointer. This transfers ownership and thus does not call the destructor.
@@ -83,6 +85,19 @@ impl GodotString {
         }
         std::slice::from_raw_parts(ptr as *const char, len as usize)
     }
+
+    ffi_methods! {
+        type sys::GDExtensionStringPtr = *mut Opaque;
+
+        fn from_string_sys = from_sys;
+        fn from_string_sys_init = from_sys_init;
+        fn string_sys = sys;
+    }
+
+    #[doc(hidden)]
+    pub fn as_inner(&self) -> inner::InnerString {
+        inner::InnerString::from_outer(self)
+    }
 }
 
 // SAFETY:
@@ -122,9 +137,28 @@ impl_builtin_traits! {
         Drop => string_destroy;
         Eq => string_operator_equal;
         Ord => string_operator_less;
+        Hash;
+    }
+}
+
+impl fmt::Display for GodotString {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let s: String = self.chars_checked().iter().collect();
+        f.write_str(s.as_str())
+    }
+}
+
+/// Uses literal syntax from GDScript: `"string"`
+impl fmt::Debug for GodotString {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let s = String::from(self);
+        write!(f, "\"{s}\"")
     }
 }
 
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+// Conversion from/into rust string-types
+
 impl<S> From<S> for GodotString
 where
     S: AsRef<str>,
@@ -162,7 +196,14 @@ impl From<&GodotString> for String {
     }
 }
 
-// TODO From<&NodePath> + test
+impl From<GodotString> for String {
+    /// Converts this `GodotString` to a `String`.
+    ///
+    /// This is identical to `String::from(&string)`, and as such there is no performance benefit.
+    fn from(string: GodotString) -> Self {
+        Self::from(&string)
+    }
+}
 
 impl FromStr for GodotString {
     type Err = Infallible;
@@ -172,49 +213,47 @@ impl FromStr for GodotString {
     }
 }
 
-impl fmt::Display for GodotString {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let s = String::from(self);
-        f.write_str(s.as_str())
-    }
-}
-
-/// Uses literal syntax from GDScript: `"string"`
-impl fmt::Debug for GodotString {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let s = String::from(self);
-        write!(f, "\"{s}\"")
-    }
-}
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+// Conversion from other Godot string-types
 
-impl ToVariant for &str {
-    fn to_variant(&self) -> Variant {
-        GodotString::from(*self).to_variant()
+impl From<&StringName> for GodotString {
+    fn from(string: &StringName) -> Self {
+        unsafe {
+            Self::from_sys_init_default(|self_ptr| {
+                let ctor = sys::builtin_fn!(string_from_string_name);
+                let args = [string.sys_const()];
+                ctor(self_ptr, args.as_ptr());
+            })
+        }
     }
 }
 
-impl ToVariant for String {
-    fn to_variant(&self) -> Variant {
-        GodotString::from(self).to_variant()
+impl From<StringName> for GodotString {
+    /// Converts this `StringName` to a `GodotString`.
+    ///
+    /// This is identical to `GodotString::from(&string_name)`, and as such there is no performance benefit.
+    fn from(string_name: StringName) -> Self {
+        Self::from(&string_name)
     }
 }
 
-impl FromVariant for String {
-    fn try_from_variant(variant: &Variant) -> Result<Self, VariantConversionError> {
-        Ok(GodotString::try_from_variant(variant)?.to_string())
+impl From<&NodePath> for GodotString {
+    fn from(path: &NodePath) -> Self {
+        unsafe {
+            Self::from_sys_init_default(|self_ptr| {
+                let ctor = sys::builtin_fn!(string_from_node_path);
+                let args = [path.sys_const()];
+                ctor(self_ptr, args.as_ptr());
+            })
+        }
     }
 }
 
-// While this is a nice optimisation for ptrcalls, it's not easily possible
-// to pass in &GodotString when doing varcalls.
-/*
-impl PtrCall for &GodotString {
-    unsafe fn from_ptr_call_arg(arg: *const godot_ffi::GDExtensionTypePtr) -> Self {
-        &*(*arg as *const GodotString)
-    }
-
-    unsafe fn to_ptr_call_arg(self, arg: godot_ffi::GDExtensionTypePtr) {
-        std::ptr::write(arg as *mut GodotString, self.clone());
+impl From<NodePath> for GodotString {
+    /// Converts this `NodePath` to a `GodotString`.
+    ///
+    /// This is identical to `GodotString::from(&path)`, and as such there is no performance benefit.
+    fn from(path: NodePath) -> Self {
+        Self::from(&path)
     }
 }
-*/

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

@@ -0,0 +1,42 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
+#![macro_use]
+
+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);
+                Self::from(&intermediate)
+            }
+        }
+
+        impl From<$Ty> for String {
+            fn from(string: $Ty) -> Self {
+                Self::from(&string)
+            }
+        }
+
+        impl std::str::FromStr for $Ty {
+            type Err = std::convert::Infallible;
+
+            fn from_str(string: &str) -> Result<Self, Self::Err> {
+                Ok(Self::from(string))
+            }
+        }
+    };
+}

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

@@ -0,0 +1,44 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
+//! Godot-types that are Strings.
+
+mod godot_string;
+mod macros;
+mod node_path;
+mod string_chars;
+mod string_name;
+
+use godot_ffi::VariantType;
+pub use godot_string::*;
+pub use node_path::*;
+pub use string_name::*;
+
+use super::{meta::VariantMetadata, FromVariant, ToVariant, Variant, VariantConversionError};
+
+impl ToVariant for &str {
+    fn to_variant(&self) -> Variant {
+        GodotString::from(*self).to_variant()
+    }
+}
+
+impl ToVariant for String {
+    fn to_variant(&self) -> Variant {
+        GodotString::from(self).to_variant()
+    }
+}
+
+impl FromVariant for String {
+    fn try_from_variant(variant: &Variant) -> Result<Self, VariantConversionError> {
+        Ok(GodotString::try_from_variant(variant)?.to_string())
+    }
+}
+
+impl VariantMetadata for String {
+    fn variant_type() -> VariantType {
+        VariantType::String
+    }
+}