Add StringName::transient_ord()

Also document some other traits for builtin types.

Author: Bromeon

godot-core/src/builtin/callable.rs

@@ -271,15 +271,16 @@ impl Callable {
 
 impl_builtin_traits! {
     for Callable {
-        // Currently no Default::default() to encourage explicit valid initialization.
-        //Default => callable_construct_default;
+        // Default is absent by design, to encourage explicit valid initialization.
+
+        Clone => callable_construct_copy;
+        Drop => callable_destroy;
 
         // Equality for custom callables depend on the equality implementation of that custom callable.
         // So we cannot implement `Eq` here and be confident equality will be total for all future custom callables.
-        // Godot does not define a less-than operator, so there is no `PartialOrd`.
         PartialEq => callable_operator_equal;
-        Clone => callable_construct_copy;
-        Drop => callable_destroy;
+        // Godot does not define a less-than operator, so there is no `PartialOrd`.
+        // Hash could be added, but without Eq it's not that useful; wait for actual use cases.
     }
 }
 

godot-core/src/builtin/dictionary.rs

@@ -297,6 +297,8 @@ impl_builtin_traits! {
         Default => dictionary_construct_default;
         Drop => dictionary_destroy;
         PartialEq => dictionary_operator_equal;
+        // No < operator for dictionaries.
+        // Hash could be added, but without Eq it's not that useful.
     }
 }
 
@@ -305,6 +307,7 @@ impl fmt::Debug for Dictionary {
         write!(f, "{:?}", self.to_variant().stringify())
     }
 }
+
 impl fmt::Display for Dictionary {
     /// Formats `Dictionary` to match Godot's string representation.
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {

godot-core/src/builtin/mod.rs

@@ -51,7 +51,7 @@ pub use rect2::*;
 pub use rect2i::*;
 pub use rid::*;
 pub use signal::*;
-pub use string::*;
+pub use string::{GString, NodePath, StringName};
 pub use transform2d::*;
 pub use transform3d::*;
 pub use variant::*;
@@ -73,6 +73,11 @@ pub mod dictionary {
     pub use super::dictionary_inner::{Iter, Keys, TypedIter, TypedKeys};
 }
 
+/// Specialized types related to Godot's various string implementations.
+pub mod strings {
+    pub use super::string::TransientStringNameOrd;
+}
+
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Implementation
 
@@ -145,6 +150,7 @@ pub enum RectSide {
 }
 
 // ----------------------------------------------------------------------------------------------------------------------------------------------
+// #[test] utils for serde
 
 #[cfg(all(test, feature = "serde"))]
 pub(crate) mod test_utils {

godot-core/src/builtin/plane.rs

@@ -299,6 +299,9 @@ impl std::fmt::Display for Plane {
     }
 }
 
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+// Tests
+
 #[cfg(test)]
 mod test {
     use crate::assert_eq_approx;

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

@@ -14,8 +14,8 @@ mod string_chars;
 mod string_name;
 
 pub use gstring::*;
-pub use node_path::*;
-pub use string_name::*;
+pub use node_path::NodePath;
+pub use string_name::{StringName, TransientStringNameOrd};
 
 use super::meta::{ConvertError, FromGodot, GodotConvert, ToGodot};
 

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

@@ -82,6 +82,7 @@ impl_builtin_traits! {
         Clone => node_path_construct_copy;
         Drop => node_path_destroy;
         Eq => node_path_operator_equal;
+        // NodePath provides no < operator.
         Hash;
     }
 }

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

@@ -18,6 +18,14 @@ use crate::builtin::{GString, NodePath};
 ///
 /// StringNames are immutable strings designed for representing unique names. StringName ensures that only
 /// one instance of a given name exists.
+///
+/// # Ordering
+///
+/// In Godot, `StringName`s are **not** ordered lexicographically, and the ordering relation is **not** stable across multiple runs of your
+/// application. Therefore, this type does not implement `PartialOrd` and `Ord`, as it would be very easy to introduce bugs by accidentally
+/// relying on lexicographical ordering.
+///
+/// Instead, we provide [`transient_ord()`][Self::transient_ord] for ordering relations.
 #[repr(C)]
 pub struct StringName {
     opaque: sys::types::OpaqueStringName,
@@ -90,6 +98,19 @@ impl StringName {
             .expect("Godot hashes are uint32_t")
     }
 
+    /// O(1), non-lexicographic, non-stable ordering relation.
+    ///
+    /// The result of the comparison is **not** lexicographic and **not** stable across multiple runs of your application.
+    ///
+    /// However, it is very fast. It doesn't depend on the length of the strings, but on the memory location of string names.
+    /// This can still be useful if you need to establish an ordering relation, but are not interested in the actual order of the strings
+    /// (example: binary search).
+    ///
+    /// For lexicographical ordering, convert to `GString` (significantly slower).
+    pub fn transient_ord(&self) -> TransientStringNameOrd<'_> {
+        TransientStringNameOrd(self)
+    }
+
     ffi_methods! {
         type sys::GDExtensionStringNamePtr = *mut Opaque;
 
@@ -152,8 +173,8 @@ impl_builtin_traits! {
         Clone => string_name_construct_copy;
         Drop => string_name_destroy;
         Eq => string_name_operator_equal;
-        // currently broken: https://github.com/godotengine/godot/issues/76218
-        // Ord => string_name_operator_less;
+        // Do not provide PartialOrd or Ord. Even though Godot provides a `operator <`, it is non-lexicographic and non-deterministic
+        // (based on pointers). See transient_ord() method.
         Hash;
     }
 }
@@ -248,6 +269,61 @@ impl From<NodePath> for StringName {
     }
 }
 
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+// Ordering
+
+/// Type that implements `Ord` for `StringNames`.
+///
+/// See [`StringName::transient_ord()`].
+pub struct TransientStringNameOrd<'a>(&'a StringName);
+
+impl<'a> PartialEq for TransientStringNameOrd<'a> {
+    fn eq(&self, other: &Self) -> bool {
+        self.0 == other.0
+    }
+}
+
+impl<'a> Eq for TransientStringNameOrd<'a> {}
+
+impl<'a> PartialOrd for TransientStringNameOrd<'a> {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+// implement Ord like above
+impl<'a> Ord for TransientStringNameOrd<'a> {
+    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+        // SAFETY: builtin operator provided by Godot.
+        let op_less = |lhs, rhs| unsafe {
+            let mut result = false;
+            sys::builtin_call! {
+                string_name_operator_less(lhs, rhs, result.sys_mut())
+            }
+            result
+        };
+
+        let self_ptr = self.0.sys();
+        let other_ptr = other.0.sys();
+
+        if op_less(self_ptr, other_ptr) {
+            std::cmp::Ordering::Less
+        } else if op_less(other_ptr, self_ptr) {
+            std::cmp::Ordering::Greater
+        } else if self.eq(other) {
+            std::cmp::Ordering::Equal
+        } else {
+            panic!(
+                "Godot provides inconsistent StringName ordering for \"{}\" and \"{}\"",
+                self.0, other.0
+            );
+        }
+    }
+}
+
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+// serde support
+
 #[cfg(feature = "serde")]
 mod serialize {
     use super::*;

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

@@ -306,6 +306,16 @@ unsafe impl GodotFfi for Variant {
 
 impl_godot_as_self!(Variant);
 
+impl Default for Variant {
+    fn default() -> Self {
+        unsafe {
+            Self::from_var_sys_init(|variant_ptr| {
+                interface_fn!(variant_new_nil)(variant_ptr);
+            })
+        }
+    }
+}
+
 impl Clone for Variant {
     fn clone(&self) -> Self {
         unsafe {
@@ -324,16 +334,6 @@ impl Drop for Variant {
     }
 }
 
-impl Default for Variant {
-    fn default() -> Self {
-        unsafe {
-            Self::from_var_sys_init(|variant_ptr| {
-                interface_fn!(variant_new_nil)(variant_ptr);
-            })
-        }
-    }
-}
-
 // Variant is not Eq because it can contain floats and other types composed of floats.
 impl PartialEq for Variant {
     fn eq(&self, other: &Self) -> bool {

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

@@ -58,17 +58,28 @@ fn string_name_equality() {
     assert_ne!(string, different);
 }
 
-// TODO: add back in when ordering StringNames is fixed
-#[itest(skip)]
-fn string_name_ordering() {
-    let _low = StringName::from("Alpha");
-    let _high = StringName::from("Beta");
-    /*
+#[itest]
+fn string_name_transient_ord() {
+    // We can't deterministically know the ordering, so this test only ensures consistency between different operators.
+    let low = StringName::from("Alpha");
+    let high = StringName::from("Beta");
+
+    let mut low = low.transient_ord();
+    let mut high = high.transient_ord();
+
+    if low > high {
+        std::mem::swap(&mut low, &mut high);
+    }
+
     assert!(low < high);
     assert!(low <= high);
-    assert!(high > low);
+    assert!(high > low); // implied.
     assert!(high >= low);
-     */
+
+    // Check PartialEq/Eq relation.
+    assert_eq!(low, low);
+    assert_ne!(low, high);
+    assert_eq!(high, high);
 }
 
 #[itest]