From 229dd2cf4057b43161307d0c5d6609d2cc1a1884 Mon Sep 17 00:00:00 2001 From: Marko Galevski Date: Sun, 12 Oct 2025 22:04:49 +0200 Subject: [PATCH 1/2] Checked cpp (and left comments) detailing why all godot hashes are hash_u32. Renamed and added tests --- godot-core/src/builtin/callable.rs | 7 +++++-- godot-core/src/builtin/collections/array.rs | 7 +++++-- godot-core/src/builtin/collections/dictionary.rs | 7 +++++-- godot-core/src/builtin/macros.rs | 2 +- godot-core/src/builtin/string/gstring.rs | 2 +- godot-core/src/builtin/string/node_path.rs | 2 +- godot-core/src/builtin/string/string_name.rs | 2 +- godot-core/src/builtin/variant/mod.rs | 7 ++++++- .../src/builtin_tests/containers/array_test.rs | 2 +- .../builtin_tests/containers/callable_test.rs | 11 +++++++---- .../builtin_tests/containers/dictionary_test.rs | 16 ++++++++++++---- .../src/builtin_tests/containers/variant_test.rs | 15 ++++++++++++--- .../src/builtin_tests/string/gstring_test.rs | 2 ++ .../src/builtin_tests/string/node_path_test.rs | 2 ++ .../src/builtin_tests/string/string_name_test.rs | 2 ++ 15 files changed, 63 insertions(+), 23 deletions(-) diff --git a/godot-core/src/builtin/callable.rs b/godot-core/src/builtin/callable.rs index 272c79271..d9aa056d6 100644 --- a/godot-core/src/builtin/callable.rs +++ b/godot-core/src/builtin/callable.rs @@ -449,8 +449,11 @@ impl Callable { /// Returns the 32-bit hash value of this callable's object. /// /// _Godot equivalent: `hash`_ - pub fn hash(&self) -> u32 { - self.as_inner().hash().try_into().unwrap() + pub fn hash_u32(&self) -> u32 { + self.as_inner() + .hash() + .try_into() + .expect("Godot hashes are uin32_t") } /// Returns true if this callable is a custom callable. diff --git a/godot-core/src/builtin/collections/array.rs b/godot-core/src/builtin/collections/array.rs index cdbd5ba5d..07c174876 100644 --- a/godot-core/src/builtin/collections/array.rs +++ b/godot-core/src/builtin/collections/array.rs @@ -290,10 +290,13 @@ impl Array { /// Note: Arrays with equal content will always produce identical hash values. However, the /// reverse is not true. Returning identical hash values does not imply the arrays are equal, /// because different arrays can have identical hash values due to hash collisions. - pub fn hash(&self) -> u32 { + pub fn hash_u32(&self) -> u32 { // The GDExtension interface only deals in `i64`, but the engine's own `hash()` function // actually returns `uint32_t`. - self.as_inner().hash().try_into().unwrap() + self.as_inner() + .hash() + .try_into() + .expect("Godot hashes are uint32_t") } /// Returns the first element in the array, or `None` if the array is empty. diff --git a/godot-core/src/builtin/collections/dictionary.rs b/godot-core/src/builtin/collections/dictionary.rs index 830e096e5..60758f2d9 100644 --- a/godot-core/src/builtin/collections/dictionary.rs +++ b/godot-core/src/builtin/collections/dictionary.rs @@ -287,8 +287,11 @@ impl Dictionary { /// Returns a 32-bit integer hash value representing the dictionary and its contents. #[must_use] - pub fn hash(&self) -> u32 { - self.as_inner().hash().try_into().unwrap() + pub fn hash_u32(&self) -> u32 { + self.as_inner() + .hash() + .try_into() + .expect("Godot hashes are uint32_t") } /// Creates a new `Array` containing all the keys currently in the dictionary. diff --git a/godot-core/src/builtin/macros.rs b/godot-core/src/builtin/macros.rs index a55e7d9ff..a00899205 100644 --- a/godot-core/src/builtin/macros.rs +++ b/godot-core/src/builtin/macros.rs @@ -103,7 +103,7 @@ macro_rules! impl_builtin_traits_inner { ( Hash for $Type:ty ) => { impl std::hash::Hash for $Type { fn hash(&self, state: &mut H) { - self.hash().hash(state) + self.hash_u32().hash(state) } } }; diff --git a/godot-core/src/builtin/string/gstring.rs b/godot-core/src/builtin/string/gstring.rs index 9bc168716..4f6b731ce 100644 --- a/godot-core/src/builtin/string/gstring.rs +++ b/godot-core/src/builtin/string/gstring.rs @@ -157,7 +157,7 @@ impl GString { } /// Returns a 32-bit integer hash value representing the string. - pub fn hash(&self) -> u32 { + pub fn hash_u32(&self) -> u32 { self.as_inner() .hash() .try_into() diff --git a/godot-core/src/builtin/string/node_path.rs b/godot-core/src/builtin/string/node_path.rs index 890bdfe64..7faf76c92 100644 --- a/godot-core/src/builtin/string/node_path.rs +++ b/godot-core/src/builtin/string/node_path.rs @@ -118,7 +118,7 @@ impl NodePath { } /// Returns a 32-bit integer hash value representing the string. - pub fn hash(&self) -> u32 { + pub fn hash_u32(&self) -> u32 { self.as_inner() .hash() .try_into() diff --git a/godot-core/src/builtin/string/string_name.rs b/godot-core/src/builtin/string/string_name.rs index c15d082fe..3b905e578 100644 --- a/godot-core/src/builtin/string/string_name.rs +++ b/godot-core/src/builtin/string/string_name.rs @@ -140,7 +140,7 @@ impl StringName { } /// Returns a 32-bit integer hash value representing the string. - pub fn hash(&self) -> u32 { + pub fn hash_u32(&self) -> u32 { self.as_inner() .hash() .try_into() diff --git a/godot-core/src/builtin/variant/mod.rs b/godot-core/src/builtin/variant/mod.rs index 26500857b..21a135bd4 100644 --- a/godot-core/src/builtin/variant/mod.rs +++ b/godot-core/src/builtin/variant/mod.rs @@ -310,8 +310,13 @@ impl Variant { /// Return Godot's hash value for the variant. /// /// _Godot equivalent : `@GlobalScope.hash()`_ - pub fn hash(&self) -> i64 { + /// @GlobalScope.hash() actually calls the VariantUtilityFunctions::hash(&Variant) function (cpp). + /// This function calls the passed reference's `hash` method, which returns a uint32_t. + /// Therefore, casting this function to u32 is always safe + pub fn hash_u32(&self) -> u32 { unsafe { interface_fn!(variant_hash)(self.var_sys()) } + .try_into() + .expect("Godot hashes are uint32_t") } /// Interpret the `Variant` as `bool`. diff --git a/itest/rust/src/builtin_tests/containers/array_test.rs b/itest/rust/src/builtin_tests/containers/array_test.rs index ea8e3de98..5a5e4b8b4 100644 --- a/itest/rust/src/builtin_tests/containers/array_test.rs +++ b/itest/rust/src/builtin_tests/containers/array_test.rs @@ -102,7 +102,7 @@ fn array_iter_shared() { fn array_hash() { let array = array![1, 2]; // Just testing that it converts successfully from i64 to u32. - array.hash(); + array.hash_u32(); } #[itest] diff --git a/itest/rust/src/builtin_tests/containers/callable_test.rs b/itest/rust/src/builtin_tests/containers/callable_test.rs index 5d3ad1ef2..e25813201 100644 --- a/itest/rust/src/builtin_tests/containers/callable_test.rs +++ b/itest/rust/src/builtin_tests/containers/callable_test.rs @@ -72,15 +72,18 @@ fn callable_validity() { #[itest] fn callable_hash() { let obj = CallableTestObj::new_gd(); + assert_eq!(obj.callable("assign_int"), obj.callable("assign_int")); // sanity check assert_eq!( - obj.callable("assign_int").hash(), - obj.callable("assign_int").hash() + obj.callable("assign_int").hash_u32(), + obj.callable("assign_int").hash_u32() ); + assert_ne!(obj.callable("assign_int"), obj.callable("stringify_int")); // sanity check + // Not guaranteed, but unlikely. assert_ne!( - obj.callable("assign_int").hash(), - obj.callable("stringify_int").hash() + obj.callable("assign_int").hash_u32(), + obj.callable("stringify_int").hash_u32() ); } diff --git a/itest/rust/src/builtin_tests/containers/dictionary_test.rs b/itest/rust/src/builtin_tests/containers/dictionary_test.rs index dc0cb4c8c..b84e0fd1c 100644 --- a/itest/rust/src/builtin_tests/containers/dictionary_test.rs +++ b/itest/rust/src/builtin_tests/containers/dictionary_test.rs @@ -126,15 +126,23 @@ fn dictionary_hash() { "bar": true, }; - assert_eq!(a.hash(), b.hash(), "equal dictionaries have same hash"); + assert_eq!( + a.hash_u32(), + b.hash_u32(), + "equal dictionaries have same hash" + ); + assert_ne!( - a.hash(), - c.hash(), + a.hash_u32(), + c.hash_u32(), "dictionaries with reordered content have different hash" ); // NaNs are not equal (since Godot 4.2) but share same hash. - assert_eq!(vdict! {772: f32::NAN}.hash(), vdict! {772: f32::NAN}.hash()); + assert_eq!( + vdict! {772: f32::NAN}.hash_u32(), + vdict! {772: f32::NAN}.hash_u32() + ); } #[itest] diff --git a/itest/rust/src/builtin_tests/containers/variant_test.rs b/itest/rust/src/builtin_tests/containers/variant_test.rs index c73b97e43..34a187834 100644 --- a/itest/rust/src/builtin_tests/containers/variant_test.rs +++ b/itest/rust/src/builtin_tests/containers/variant_test.rs @@ -713,13 +713,22 @@ fn variant_hash() { ]; for variant in hash_is_not_0 { - assert_ne!(variant.hash(), 0) + assert_ne!(variant.hash_u32(), 0); } for variant in self_equal { - assert_eq!(variant.hash(), variant.hash()) + assert_eq!(variant.hash_u32(), variant.hash_u32()); } - assert_eq!(Variant::nil().hash(), 0); + let v_first = 1.to_variant(); + let v_second = 1.to_variant(); + let v_third = "hello".to_variant(); + + assert_eq!(v_first, v_second); + assert_eq!(v_first.hash_u32(), v_second.hash_u32()); + assert_ne!(v_first, v_third); + assert_ne!(v_first.hash_u32(), v_third.hash_u32()); + + assert_eq!(Variant::nil().hash_u32(), 0); // It's not guaranteed that different object will have different hash, but it is // extremely unlikely for a collision to happen. diff --git a/itest/rust/src/builtin_tests/string/gstring_test.rs b/itest/rust/src/builtin_tests/string/gstring_test.rs index 39c714059..c290b48af 100644 --- a/itest/rust/src/builtin_tests/string/gstring_test.rs +++ b/itest/rust/src/builtin_tests/string/gstring_test.rs @@ -37,7 +37,9 @@ fn string_equality() { let different = GString::from("some"); assert_eq!(string, second); + assert_eq!(string.hash_u32(), second.hash_u32()); assert_ne!(string, different); + assert_ne!(string.hash_u32(), different.hash_u32()); } #[itest] diff --git a/itest/rust/src/builtin_tests/string/node_path_test.rs b/itest/rust/src/builtin_tests/string/node_path_test.rs index 918d6348e..a55ff6c48 100644 --- a/itest/rust/src/builtin_tests/string/node_path_test.rs +++ b/itest/rust/src/builtin_tests/string/node_path_test.rs @@ -36,7 +36,9 @@ fn node_path_equality() { let different = NodePath::from("some"); assert_eq!(string, second); + assert_eq!(string.hash_u32(), second.hash_u32()); assert_ne!(string, different); + assert_ne!(string.hash_u32(), different.hash_u32()); } #[itest] diff --git a/itest/rust/src/builtin_tests/string/string_name_test.rs b/itest/rust/src/builtin_tests/string/string_name_test.rs index 8678ea627..d04fc847a 100644 --- a/itest/rust/src/builtin_tests/string/string_name_test.rs +++ b/itest/rust/src/builtin_tests/string/string_name_test.rs @@ -46,7 +46,9 @@ fn string_name_equality() { let different = StringName::from("some"); assert_eq!(string, second); + assert_eq!(string.hash_u32(), second.hash_u32()); assert_ne!(string, different); + assert_ne!(string.hash_u32(), different.hash_u32()); } #[itest] From 9e536c3893ecc8f4e885e4602904658cf89a1fef Mon Sep 17 00:00:00 2001 From: Marko Galevski Date: Mon, 13 Oct 2025 21:23:13 +0200 Subject: [PATCH 2/2] Moved comments out of public API. Re-added old hash methods as deprecated to prevent breaks in v0.4 --- godot-core/src/builtin/callable.rs | 5 +++++ godot-core/src/builtin/collections/array.rs | 7 +++++-- godot-core/src/builtin/collections/dictionary.rs | 6 ++++++ godot-core/src/builtin/macros.rs | 2 ++ godot-core/src/builtin/string/gstring.rs | 8 ++++++++ godot-core/src/builtin/string/node_path.rs | 8 ++++++++ godot-core/src/builtin/string/string_name.rs | 8 ++++++++ godot-core/src/builtin/variant/mod.rs | 11 ++++++++--- 8 files changed, 50 insertions(+), 5 deletions(-) diff --git a/godot-core/src/builtin/callable.rs b/godot-core/src/builtin/callable.rs index d9aa056d6..a8268522f 100644 --- a/godot-core/src/builtin/callable.rs +++ b/godot-core/src/builtin/callable.rs @@ -456,6 +456,11 @@ impl Callable { .expect("Godot hashes are uin32_t") } + #[deprecated = "renamed to hash_u32"] + pub fn hash(&self) -> u32 { + self.as_inner().hash().try_into().unwrap() + } + /// Returns true if this callable is a custom callable. /// /// Custom callables are mainly created from bind or unbind. In GDScript, lambda functions are also diff --git a/godot-core/src/builtin/collections/array.rs b/godot-core/src/builtin/collections/array.rs index 07c174876..0cb2e96aa 100644 --- a/godot-core/src/builtin/collections/array.rs +++ b/godot-core/src/builtin/collections/array.rs @@ -291,14 +291,17 @@ impl Array { /// reverse is not true. Returning identical hash values does not imply the arrays are equal, /// because different arrays can have identical hash values due to hash collisions. pub fn hash_u32(&self) -> u32 { - // The GDExtension interface only deals in `i64`, but the engine's own `hash()` function - // actually returns `uint32_t`. self.as_inner() .hash() .try_into() .expect("Godot hashes are uint32_t") } + #[deprecated = "renamed to hash_32"] + pub fn hash(&self) -> u32 { + self.as_inner().hash().try_into().unwrap() + } + /// Returns the first element in the array, or `None` if the array is empty. #[doc(alias = "first")] pub fn front(&self) -> Option { diff --git a/godot-core/src/builtin/collections/dictionary.rs b/godot-core/src/builtin/collections/dictionary.rs index 60758f2d9..f08fa4179 100644 --- a/godot-core/src/builtin/collections/dictionary.rs +++ b/godot-core/src/builtin/collections/dictionary.rs @@ -294,6 +294,12 @@ impl Dictionary { .expect("Godot hashes are uint32_t") } + #[deprecated = "renamed to hash_u32"] + #[must_use] + pub fn hash(&self) -> u32 { + self.as_inner().hash().try_into().unwrap() + } + /// Creates a new `Array` containing all the keys currently in the dictionary. /// /// _Godot equivalent: `keys`_ diff --git a/godot-core/src/builtin/macros.rs b/godot-core/src/builtin/macros.rs index a00899205..605292875 100644 --- a/godot-core/src/builtin/macros.rs +++ b/godot-core/src/builtin/macros.rs @@ -103,6 +103,8 @@ macro_rules! impl_builtin_traits_inner { ( Hash for $Type:ty ) => { impl std::hash::Hash for $Type { fn hash(&self, state: &mut H) { + // The GDExtension interface only deals in `i64`, but the engine's own `hash()` function + // actually returns `uint32_t`. self.hash_u32().hash(state) } } diff --git a/godot-core/src/builtin/string/gstring.rs b/godot-core/src/builtin/string/gstring.rs index 4f6b731ce..f3270aa96 100644 --- a/godot-core/src/builtin/string/gstring.rs +++ b/godot-core/src/builtin/string/gstring.rs @@ -164,6 +164,14 @@ impl GString { .expect("Godot hashes are uint32_t") } + #[deprecated = "renamed to hash_u32"] + pub fn hash(&self) -> u32 { + self.as_inner() + .hash() + .try_into() + .expect("Godot hashes are uint32_t") + } + /// Gets the UTF-32 character slice from a `GString`. pub fn chars(&self) -> &[char] { // SAFETY: Since 4.1, Godot ensures valid UTF-32, making interpreting as char slice safe. diff --git a/godot-core/src/builtin/string/node_path.rs b/godot-core/src/builtin/string/node_path.rs index 7faf76c92..542f13d1c 100644 --- a/godot-core/src/builtin/string/node_path.rs +++ b/godot-core/src/builtin/string/node_path.rs @@ -125,6 +125,14 @@ impl NodePath { .expect("Godot hashes are uint32_t") } + #[deprecated = "renamed to hash_u32"] + pub fn hash(&self) -> u32 { + self.as_inner() + .hash() + .try_into() + .expect("Godot hashes are uint32_t") + } + /// Returns the range `begin..exclusive_end` as a new `NodePath`. /// /// The absolute value of `begin` and `exclusive_end` will be clamped to [`get_total_count()`][Self::get_total_count]. diff --git a/godot-core/src/builtin/string/string_name.rs b/godot-core/src/builtin/string/string_name.rs index 3b905e578..3b9e87dd2 100644 --- a/godot-core/src/builtin/string/string_name.rs +++ b/godot-core/src/builtin/string/string_name.rs @@ -147,6 +147,14 @@ impl StringName { .expect("Godot hashes are uint32_t") } + #[deprecated = "renamed to hash_u32"] + pub fn hash(&self) -> u32 { + self.as_inner() + .hash() + .try_into() + .expect("Godot hashes are uint32_t") + } + meta::declare_arg_method! { /// Use as argument for an [`impl AsArg`][crate::meta::AsArg] parameter. /// diff --git a/godot-core/src/builtin/variant/mod.rs b/godot-core/src/builtin/variant/mod.rs index 21a135bd4..fda32e50c 100644 --- a/godot-core/src/builtin/variant/mod.rs +++ b/godot-core/src/builtin/variant/mod.rs @@ -310,15 +310,20 @@ impl Variant { /// Return Godot's hash value for the variant. /// /// _Godot equivalent : `@GlobalScope.hash()`_ - /// @GlobalScope.hash() actually calls the VariantUtilityFunctions::hash(&Variant) function (cpp). - /// This function calls the passed reference's `hash` method, which returns a uint32_t. - /// Therefore, casting this function to u32 is always safe pub fn hash_u32(&self) -> u32 { + // @GlobalScope.hash() actually calls the VariantUtilityFunctions::hash(&Variant) function (cpp). + // This function calls the passed reference's `hash` method, which returns a uint32_t. + // Therefore, casting this function to u32 is always fine. unsafe { interface_fn!(variant_hash)(self.var_sys()) } .try_into() .expect("Godot hashes are uint32_t") } + #[deprecated = "renamed to hash_32 and type changed to u32"] + pub fn hash(&self) -> i64 { + unsafe { interface_fn!(variant_hash)(self.var_sys()) } + } + /// Interpret the `Variant` as `bool`. /// /// Returns `false` only if the variant's current value is the default value for its type. For example: