Skip to content

Commit 1cf9c34

Browse files
ecobiubiuBromeon
authored andcommitted
Implement deterministic ordering for exported HashMap/HashSet
1 parent b51093b commit 1cf9c34

File tree

2 files changed

+35
-2
lines changed

2 files changed

+35
-2
lines changed

examples/property-export/GDScriptPrinter.gd

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ func _ready():
44
var rust = get_node("../PropertyExport")
55

66
print("\n-----------------------------------------------------------------")
7-
print("Print from GDScript (note the deterministically ordered map/set):")
7+
print("Print from GDScript (note the lexicographically ordered map/set):")
88
print(" Vec (name):");
99
for name in rust.name_vec:
1010
print(" * %s" % name)

gdnative-core/src/core_types/variant.rs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1605,17 +1605,37 @@ impl<T: FromVariant> FromVariant for Vec<T> {
16051605
}
16061606
}
16071607

1608+
/// Converts the hash map to a `Dictionary`, wrapped in a `Variant`.
1609+
///
1610+
/// Note that Rust's `HashMap` is non-deterministically ordered for security reasons, meaning that
1611+
/// the order of the same elements will differ between two program invocations. To provide a
1612+
/// deterministic output in Godot (e.g. UI elements for properties), the elements are sorted by key.
16081613
impl<K: ToVariant + Hash + ToVariantEq, V: ToVariant> ToVariant for HashMap<K, V> {
16091614
#[inline]
16101615
fn to_variant(&self) -> Variant {
1616+
// Note: dictionary currently provides neither a sort() function nor random access (or at least bidirectional)
1617+
// iterators, making it difficult to sort in-place. Workaround: copy to vector
1618+
1619+
let mut intermediate: Vec<(Variant, Variant)> = self
1620+
.iter()
1621+
.map(|(k, v)| (k.to_variant(), v.to_variant()))
1622+
.collect();
1623+
1624+
intermediate.sort();
1625+
16111626
let dict = Dictionary::new();
1612-
for (key, value) in self {
1627+
for (key, value) in intermediate.into_iter() {
16131628
dict.insert(key, value);
16141629
}
1630+
16151631
dict.owned_to_variant()
16161632
}
16171633
}
16181634

1635+
/// Expects a `Variant` populated with a `Dictionary` and tries to convert it into a `HashMap`.
1636+
///
1637+
/// Since Rust's `HashMap` is unordered, there is no guarantee about the resulting element order.
1638+
/// In fact it is possible that two program invocations cause a different output.
16191639
impl<K: FromVariant + Hash + Eq, V: FromVariant> FromVariant for HashMap<K, V> {
16201640
#[inline]
16211641
fn from_variant(variant: &Variant) -> Result<Self, FromVariantError> {
@@ -1624,6 +1644,7 @@ impl<K: FromVariant + Hash + Eq, V: FromVariant> FromVariant for HashMap<K, V> {
16241644
.len()
16251645
.try_into()
16261646
.expect("Dictionary length should fit in usize");
1647+
16271648
let mut hash_map = HashMap::with_capacity(len);
16281649
for (key, value) in dictionary.iter() {
16291650
hash_map.insert(K::from_variant(&key)?, V::from_variant(&value)?);
@@ -1632,17 +1653,28 @@ impl<K: FromVariant + Hash + Eq, V: FromVariant> FromVariant for HashMap<K, V> {
16321653
}
16331654
}
16341655

1656+
/// Converts the hash set to a `VariantArray`, wrapped in a `Variant`.
1657+
///
1658+
/// Note that Rust's `HashSet` is non-deterministically ordered for security reasons, meaning that
1659+
/// the order of the same elements will differ between two program invocations. To provide a
1660+
/// deterministic output in Godot (e.g. UI elements for properties), the elements are sorted by key.
16351661
impl<T: ToVariant> ToVariant for HashSet<T> {
16361662
#[inline]
16371663
fn to_variant(&self) -> Variant {
16381664
let array = VariantArray::new();
16391665
for value in self {
16401666
array.push(value.to_variant());
16411667
}
1668+
1669+
array.sort(); // deterministic order in Godot
16421670
array.owned_to_variant()
16431671
}
16441672
}
16451673

1674+
/// Expects a `Variant` populated with a `VariantArray` and tries to convert it into a `HashSet`.
1675+
///
1676+
/// Since Rust's `HashSet` is unordered, there is no guarantee about the resulting element order.
1677+
/// In fact it is possible that two program invocations cause a different output.
16461678
impl<T: FromVariant + Eq + Hash> FromVariant for HashSet<T> {
16471679
#[inline]
16481680
fn from_variant(variant: &Variant) -> Result<Self, FromVariantError> {
@@ -1651,6 +1683,7 @@ impl<T: FromVariant + Eq + Hash> FromVariant for HashSet<T> {
16511683
.len()
16521684
.try_into()
16531685
.expect("VariantArray length should fit in usize");
1686+
16541687
let mut set = HashSet::with_capacity(len);
16551688
for idx in 0..len as i32 {
16561689
let item =

0 commit comments

Comments
 (0)