Skip to content
Open
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
7 changes: 5 additions & 2 deletions godot-core/src/builtin/callable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Comment on lines +452 to +456
Copy link
Contributor

@Yarwin Yarwin Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I wondered if it would be worth it to put it into same macro (or even covering it directly in builtin), such as

macro_rules! impl_hash_32 {
    (
        $( #[$maybe_docs:meta] )*
        $Type:ty
    ) => {
        impl $Type {
            #[doc=concat!("Returns a 32-bit integer hash value representing the ", stringify!($Type), " and its contents.")]
            $( #[$maybe_docs] )*
            pub fn hash_u32(&self) -> u32 {
                self.as_inner()
                    .hash()
                    .try_into()
                    .expect("Godot hashes are uint32_t")
            }
        }
    }
}

and called as

impl_hash_32!(
    ///
    /// 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.
    Array<T>
);

(...)
impl_hash_32!(StringName);

Since having one source for multiple instances of same code would be nice – but IMO it is not worth it getting into consideration all the differences (impl<T:ArrayElement> Array<T> vs. concrete Type). Ctrl+c ctrl+v ftw

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll give that a crack too.

Copy link
Contributor

@Yarwin Yarwin Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't sweat too much though (unless you'll find/figure out something nice); In this context repetition is fine.

}

/// Returns true if this callable is a custom callable.
Expand Down
7 changes: 5 additions & 2 deletions godot-core/src/builtin/collections/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,10 +290,13 @@ impl<T: ArrayElement> Array<T> {
/// 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`.
Comment on lines 294 to 295
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would cut out this comment and move it to builtin Hash macro.

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.
Expand Down
7 changes: 5 additions & 2 deletions godot-core/src/builtin/collections/dictionary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ macro_rules! impl_builtin_traits_inner {
( Hash for $Type:ty ) => {
impl std::hash::Hash for $Type {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.hash().hash(state)
self.hash_u32().hash(state)
}
}
};
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/string/gstring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/string/node_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/string/string_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
7 changes: 6 additions & 1 deletion godot-core/src/builtin/variant/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Comment on lines +313 to +319
Copy link
Contributor

@Yarwin Yarwin Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is public thus included in docs and IDE tips. IMO user doesn't need to know about safety guarantees – especially if they are always held. I would just use the same docs as for everything else:

Suggested change
/// @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")
/// Returns a 32-bit unsigned integer hash value representing the Variant.
///
/// _Godot equivalent : `@GlobalScope.hash()`_
pub fn hash_u32(&self) -> u32 {
// Note: Calls the passed reference's underlying `hash` method which returns an uint32_t.
// SAFETY: we pass in the valid pointer.
unsafe { interface_fn!(variant_hash)(self.var_sys()) }
.try_into()
.expect("Godot hashes are uint32_t")

idk if we should keep info about VariantUtilityFunctions::hash(&Variant), your call 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree that this should be in comment rather than RustDoc.

Note that the int conversion isn't safety relevant, so shouldn't be part of // SAFETY:. Making a mistake here will result in a worse hash function, not UB.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll move it into a non-safety dev comment.

Copy link
Contributor

@Yarwin Yarwin Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true true, I updated my suggestion

to elaborate – the only way to cause an UB here is by passing invalid GDExtensionConstVariantPtr.

static GDExtensionInt gdextension_variant_hash(GDExtensionConstVariantPtr p_self) {
	const Variant *self = (const Variant *)p_self;
	return self->hash();
}

}

/// Interpret the `Variant` as `bool`.
Expand Down
2 changes: 1 addition & 1 deletion itest/rust/src/builtin_tests/containers/array_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
11 changes: 7 additions & 4 deletions itest/rust/src/builtin_tests/containers/callable_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
);
}

Expand Down
16 changes: 12 additions & 4 deletions itest/rust/src/builtin_tests/containers/dictionary_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
15 changes: 12 additions & 3 deletions itest/rust/src/builtin_tests/containers/variant_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions itest/rust/src/builtin_tests/string/gstring_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
2 changes: 2 additions & 0 deletions itest/rust/src/builtin_tests/string/node_path_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
2 changes: 2 additions & 0 deletions itest/rust/src/builtin_tests/string/string_name_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Loading