feat: Better HashMaps - hashmap iteration and read-access in map_get()

[aggyomfg]

Hey! This PR improves how we work with HashMaps in the scripting layer. Two main things here:

What changed?

HashMap iteration support (commit b632625)

Added proper iteration over maps! Now you can do for key, value in pairs(map) in Lua and use .iter() in Rhai. This was trickier than it looks because Bevy's reflection path system doesn't play nice with maps - it rejects ListIndex access on map types. So I added a dedicated ReflectMapRefIter that bypasses the path system and uses Map::get_at() directly.

The iterator returns (key, value) tuples that get properly unpacked in both languages - Lua's pairs() just works, and Rhai gets a wrapped iterator with a next() method.

Fixed map_get() to use read-only access (commit fc7d746)

Changed map_get() from using from_reflect() to to_dynamic(). This might seem like a small thing but it's actually pretty important:

• More reliable: Works with ALL reflected types now, not just ones that have ReflectFromReflect registered. No more mysterious failures when accessing certain map value types.
• Correct semantics: Reading from a map doesn't need mutable access! Changed from with_reflect_mut to with_reflect. This is also groundwork for an upcoming PR that will fix Bevy's change detection by properly checking ReflectReference access modes.
• Better perf: Eliminates the registry lookup and type reconstruction that from_reflect() does. to_dynamic() is simpler.

Testing

Added test scripts for both Lua and Rhai that verify:

• Iterating over map entries
• Getting correct key-value pairs
• Proper entry counts

Existing map operations should all still work as before.

[semanticdiff-com]

Review changes with  

Changed Files

File	Status
assets/tests/hashmap/can_get_hashmap_value.lua	89% smaller
assets/tests/insert/vec.lua	81% smaller
crates/languages/bevy_mod_scripting_rhai/src/lib.rs	35% smaller
crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs	17% smaller
crates/bevy_mod_scripting_bindings/src/function/magic_functions.rs	16% smaller
crates/bevy_mod_scripting_functions/src/core.rs	11% smaller
crates/bevy_mod_scripting_bindings/src/reference.rs	8% smaller
crates/testing_crates/test_utils/src/test_data.rs	5% smaller
assets/tests/hashmap/can_get_hashmap_value.rhai	Unsupported file format
assets/tests/hashmap/can_set_hashmap_value.lua	0% smaller
assets/tests/hashmap/can_set_hashmap_value.rhai	Unsupported file format
assets/tests/hashset/can_check_hashset_contains.lua	0% smaller
assets/tests/hashset/can_check_hashset_contains.rhai	Unsupported file format
assets/tests/hashset/can_insert_into_hashset.lua	0% smaller
assets/tests/hashset/can_insert_into_hashset.rhai	Unsupported file format
assets/tests/insert/vec.rhai	Unsupported file format
assets/tests/iter/hashmap.lua	0% smaller
assets/tests/iter/hashmap.rhai	Unsupported file format
assets/tests/iter/hashmap_pairs.lua	0% smaller
assets/tests/iter/hashset.lua	0% smaller
assets/tests/iter/hashset.rhai	Unsupported file format
assets/tests/iter/hashset_pairs.lua	0% smaller
assets/tests/iter/vec_pairs.lua	0% smaller
crates/bevy_mod_scripting_bindings/src/reflection_extensions.rs	0% smaller
crates/languages/bevy_mod_scripting_rhai/src/bindings/reference.rs	0% smaller

[aggyomfg]

Added old map_get back as map_get_clone() - some times we still need to get fully Reflected values (for example can't constuct or dowcast asset handle from PartialReflect)

[aggyomfg]

Adding map_get_clone() back pushed me to idea, that iters may also need fully reflected values, added this also.

[github-actions]

Bencher Report

Branch	hashmap_iter
Testbed	linux-gha

Click to view all benchmark results

Benchmark	Latency	Benchmark Result nanoseconds (ns) (Result Δ%)	Upper Boundary nanoseconds (ns) (Limit %)
component/access Lua	📈 view plot 🚷 view threshold	3,925.20 ns (-3.42%) Baseline: 4,064.19 ns	4,351.91 ns (90.19%)
component/access Rhai	📈 view plot 🚷 view threshold	5,758.80 ns (-3.45%) Baseline: 5,964.64 ns	6,213.86 ns (92.68%)
component/get Lua	📈 view plot 🚷 view threshold	2,198.70 ns (-9.32%) Baseline: 2,424.66 ns	2,709.27 ns (81.15%)
component/get Rhai	📈 view plot 🚷 view threshold	4,172.50 ns (-6.52%) Baseline: 4,463.40 ns	4,868.07 ns (85.71%)
conversions/Mut::from	📈 view plot 🚷 view threshold	80.41 ns (-7.89%) Baseline: 87.29 ns	95.42 ns (84.27%)
conversions/Ref::from	📈 view plot 🚷 view threshold	76.90 ns (-10.16%) Baseline: 85.60 ns	97.65 ns (78.75%)
conversions/ScriptValue::List	📈 view plot 🚷 view threshold	372.30 ns (+10.74%) Baseline: 336.19 ns	458.64 ns (81.17%)
conversions/ScriptValue::Map	📈 view plot 🚷 view threshold	993.46 ns (-7.60%) Baseline: 1,075.15 ns	1,211.20 ns (82.02%)
conversions/ScriptValue::Reference::from_into	📈 view plot 🚷 view threshold	24.63 ns (-6.53%) Baseline: 26.35 ns	30.56 ns (80.60%)
conversions/Val::from_into	📈 view plot 🚷 view threshold	302.52 ns (+0.51%) Baseline: 300.98 ns	338.54 ns (89.36%)
function/call 4 args Lua	📈 view plot 🚷 view threshold	1,626.90 ns (-12.60%) Baseline: 1,861.39 ns	2,117.11 ns (76.85%)
function/call 4 args Rhai	📈 view plot 🚷 view threshold	1,363.30 ns (-9.52%) Baseline: 1,506.82 ns	1,684.22 ns (80.95%)
function/call Lua	📈 view plot 🚷 view threshold	232.00 ns (-5.64%) Baseline: 245.86 ns	274.59 ns (84.49%)
function/call Rhai	📈 view plot 🚷 view threshold	430.12 ns (-4.72%) Baseline: 451.43 ns	531.83 ns (80.88%)
loading/empty Lua	📈 view plot 🚷 view threshold	940,640.00 ns (+137.96%) Baseline: 395,296.30 ns	1,540,126.46 ns (61.08%)
loading/empty Rhai	📈 view plot 🚷 view threshold	1,210,700.00 ns (+82.32%) Baseline: 664,035.00 ns	1,613,159.49 ns (75.05%)
math/vec mat ops Lua	📈 view plot 🚷 view threshold	6,958.80 ns (-11.36%) Baseline: 7,850.65 ns	9,236.54 ns (75.34%)
math/vec mat ops Rhai	📈 view plot 🚷 view threshold	6,333.50 ns (-6.67%) Baseline: 6,786.43 ns	7,591.35 ns (83.43%)
query/10 entities Lua	📈 view plot 🚷 view threshold	19,333.00 ns (-7.13%) Baseline: 20,816.90 ns	22,713.91 ns (85.12%)
query/10 entities Rhai	📈 view plot 🚷 view threshold	18,786.00 ns (-9.93%) Baseline: 20,857.20 ns	22,939.21 ns (81.89%)
query/100 entities Lua	📈 view plot 🚷 view threshold	40,201.00 ns (-6.47%) Baseline: 42,982.00 ns	46,436.65 ns (86.57%)
query/100 entities Rhai	📈 view plot 🚷 view threshold	30,763.00 ns (-10.37%) Baseline: 34,320.60 ns	37,945.60 ns (81.07%)
query/1000 entities Lua	📈 view plot 🚷 view threshold	263,870.00 ns (-6.39%) Baseline: 281,868.00 ns	314,696.40 ns (83.85%)
query/1000 entities Rhai	📈 view plot 🚷 view threshold	165,250.00 ns (-3.28%) Baseline: 170,852.00 ns	183,154.52 ns (90.22%)
reflection/10 Lua	📈 view plot 🚷 view threshold	5,770.30 ns (-8.79%) Baseline: 6,326.50 ns	6,917.99 ns (83.41%)
reflection/10 Rhai	📈 view plot 🚷 view threshold	15,724.00 ns (-1.48%) Baseline: 15,960.50 ns	16,320.16 ns (96.35%)
reflection/100 Lua	📈 view plot 🚷 view threshold	47,843.00 ns (-9.70%) Baseline: 52,982.10 ns	58,628.60 ns (81.60%)
reflection/100 Rhai	📈 view plot 🚷 view threshold	729,860.00 ns (-6.79%) Baseline: 782,993.00 ns	844,904.51 ns (86.38%)
resource/access Lua	📈 view plot 🚷 view threshold	3,552.80 ns (-2.01%) Baseline: 3,625.76 ns	3,883.66 ns (91.48%)
resource/access Rhai	📈 view plot 🚷 view threshold	5,247.80 ns (-3.56%) Baseline: 5,441.74 ns	5,695.72 ns (92.14%)
resource/get Lua	📈 view plot 🚷 view threshold	1,820.80 ns (-11.27%) Baseline: 2,052.03 ns	2,318.04 ns (78.55%)
resource/get Rhai	📈 view plot 🚷 view threshold	3,666.30 ns (-7.59%) Baseline: 3,967.30 ns	4,339.33 ns (84.49%)

🐰 View full continuous benchmarking report in Bencher

[makspll]

I am not convinced about using to_dynamic, all of of the current API surface avoids non Reflect values for a reason, any downcasts (i.e. into_script_ref calls) will fail on dynamic structs etc, this will work for primitives but more complex opaque values which don't have a reflect_clone (I think, i.e. opaque values without an explicit reflect(clone), or even anything with a reflect(ignore) field) will cause issues.

So while this might work individually, but will introduce problems with some bindings.

For example Ref<T> uses try_downcast_ref::<T>, which will fail if the value came from to_dynamic, as DynamicStruct etc are not concrete Reflect types. This is why need calls to FromReflect for getting owned variants of values.

Imo the decision to get a dynamic vs non dynamic value should not be made by the script user, as it's a bit of a leaky abstraction. Ideally the scripter should not need to know anything about the internals here

[aggyomfg]

Okay, I agree — I just made a few APIs to discuss the idea. So, just to clarify, we want to keep only the variant that I currently named with the _clone postfix, right?

[aggyomfg]

also tried to make access to map via default_get/default set instead of separate map_get/insert, it works, but maybe you know some limitations of this approach? but tests runs fine.