From b632625a07b0a003aca1fd5dd4c2e9b0543ff0df Mon Sep 17 00:00:00 2001 From: aggyomfg Date: Sat, 4 Oct 2025 14:34:12 +0400 Subject: [PATCH 01/16] hashmap iteration --- assets/tests/iter/hashmap.lua | 15 +++++ assets/tests/iter/hashmap.rhai | 31 ++++++++++ .../src/reference.rs | 60 +++++++++++++++++++ .../bevy_mod_scripting_functions/src/core.rs | 56 +++++++++++------ .../src/bindings/reference.rs | 39 +++++++++--- .../src/bindings/reference.rs | 44 ++++++++++++++ .../bevy_mod_scripting_rhai/src/lib.rs | 3 +- 7 files changed, 222 insertions(+), 26 deletions(-) create mode 100644 assets/tests/iter/hashmap.lua create mode 100644 assets/tests/iter/hashmap.rhai diff --git a/assets/tests/iter/hashmap.lua b/assets/tests/iter/hashmap.lua new file mode 100644 index 0000000000..b74f9f4382 --- /dev/null +++ b/assets/tests/iter/hashmap.lua @@ -0,0 +1,15 @@ +local res_type = world.get_type_by_name("TestResourceWithVariousFields") +local res = world.get_resource(res_type) + +local map = res.string_map + +local count = 0 +local found_keys = {} +for key, value in pairs(map) do + count = count + 1 + found_keys[key] = value +end + +assert(count == 2, "Expected 2 entries, got " .. count) +assert(found_keys["foo"] == "bar", "Expected foo=>bar") +assert(found_keys["zoo"] == "zed", "Expected zoo=>zed") diff --git a/assets/tests/iter/hashmap.rhai b/assets/tests/iter/hashmap.rhai new file mode 100644 index 0000000000..90a3bda0f0 --- /dev/null +++ b/assets/tests/iter/hashmap.rhai @@ -0,0 +1,31 @@ +let res_type = world.get_type_by_name.call("TestResourceWithVariousFields"); +let res = world.get_resource.call(res_type); + +let map = res.string_map; + +let iterator = map.iter(); +let count = 0; +let found_keys = #{}; + +loop { + let result = iterator.next(); + + if result == () { + break; + } + + let key = result[0]; + let value = result[1]; + count += 1; + found_keys[key] = value; +} + +if count != 2 { + throw `Expected 2 entries, got ${count}`; +} +if found_keys["foo"] != "bar" { + throw "Expected foo=>bar"; +} +if found_keys["zoo"] != "zed" { + throw "Expected zoo=>zed"; +} diff --git a/crates/bevy_mod_scripting_bindings/src/reference.rs b/crates/bevy_mod_scripting_bindings/src/reference.rs index a9f4a08f80..d7d63a6d29 100644 --- a/crates/bevy_mod_scripting_bindings/src/reference.rs +++ b/crates/bevy_mod_scripting_bindings/src/reference.rs @@ -133,6 +133,15 @@ impl ReflectReference { ReflectRefIter::new_indexed(self) } + /// Creates a new iterator for Maps specifically. + /// Unlike `into_iter_infinite`, this iterator is finite and will return None when exhausted. + pub fn into_map_iter(self) -> ReflectMapRefIter { + ReflectMapRefIter { + base: self, + index: 0, + } + } + /// If this is a reference to something with a length accessible via reflection, returns that length. pub fn len(&self, world: WorldGuard) -> Result, InteropError> { self.with_reflect(world, |r| match r.reflect_ref() { @@ -784,6 +793,57 @@ impl ReflectRefIter { const fn list_index_access(index: usize) -> Access<'static> { Access::ListIndex(index) } + +/// Iterator specifically for Maps that doesn't use the path system. +/// This bypasses Bevy's path resolution which rejects ListIndex on Maps, +/// and instead directly uses Map::get_at() to iterate over map entries. +pub struct ReflectMapRefIter { + pub(crate) base: ReflectReference, + pub(crate) index: usize, +} + +#[profiling::all_functions] +impl ReflectMapRefIter { + /// Returns the next map entry as a (key, value) tuple. + /// Returns Ok(None) when there are no more entries. + pub fn next_ref(&mut self, world: WorldGuard) -> Result, InteropError> { + let idx = self.index; + self.index += 1; + + // Access the map and get the entry at index + self.base.with_reflect(world.clone(), |reflect| { + match reflect.reflect_ref() { + ReflectRef::Map(map) => { + if let Some((key, value)) = map.get_at(idx) { + let allocator = world.allocator(); + let mut allocator_guard = allocator.write(); + + let key_ref = ReflectReference::new_allocated_boxed_parial_reflect( + key.to_dynamic(), + &mut *allocator_guard + )?; + + let value_ref = ReflectReference::new_allocated_boxed_parial_reflect( + value.to_dynamic(), + &mut *allocator_guard + )?; + + drop(allocator_guard); + Ok(Some((key_ref, value_ref))) + } else { + Ok(None) + } + } + _ => Err(InteropError::unsupported_operation( + reflect.get_represented_type_info().map(|ti| ti.type_id()), + None, + "map iteration on non-map type".to_owned(), + )) + } + })? + } +} + #[profiling::all_functions] impl Iterator for ReflectRefIter { type Item = Result; diff --git a/crates/bevy_mod_scripting_functions/src/core.rs b/crates/bevy_mod_scripting_functions/src/core.rs index 8fc5c8bdaa..03371e5dbd 100644 --- a/crates/bevy_mod_scripting_functions/src/core.rs +++ b/crates/bevy_mod_scripting_functions/src/core.rs @@ -795,25 +795,45 @@ impl ReflectReference { profiling::function_scope!("iter"); let world = ctxt.world()?; let mut len = reference.len(world.clone())?.unwrap_or_default(); - let mut infinite_iter = reference.into_iter_infinite(); - let iter_function = move || { - // world is not thread safe, we can't capture it in the closure - // or it will also be non-thread safe - let world = ThreadWorldContainer.try_get_world()?; - if len == 0 { - return Ok(ScriptValue::Unit); - } - - let (next_ref, _) = infinite_iter.next_ref(); - - let converted = ReflectReference::into_script_ref(next_ref, world); - // println!("idx: {idx:?}, converted: {converted:?}"); - len -= 1; - // we stop once the reflection path is invalid - converted - }; + // Check if this is a Map type + let is_map = reference.with_reflect(world.clone(), |r| { + matches!(r.reflect_ref(), ReflectRef::Map(_)) + })?; - Ok(iter_function.into_dynamic_script_function_mut()) + if is_map { + // Use special map iterator that doesn't rely on path resolution + let mut map_iter = reference.into_map_iter(); + let iter_function = move || { + if len == 0 { + return Ok(ScriptValue::Unit); + } + len -= 1; + let world = ThreadWorldContainer.try_get_world()?; + match map_iter.next_ref(world.clone())? { + Some((key_ref, value_ref)) => { + // Return both key and value as a List for Lua's pairs() to unpack + let key_value = ReflectReference::into_script_ref(key_ref, world.clone())?; + let value_value = ReflectReference::into_script_ref(value_ref, world)?; + Ok(ScriptValue::List(vec![key_value, value_value])) + } + None => Ok(ScriptValue::Unit), + } + }; + Ok(iter_function.into_dynamic_script_function_mut()) + } else { + let mut infinite_iter = reference.into_iter_infinite(); + let iter_function = move || { + if len == 0 { + return Ok(ScriptValue::Unit); + } + len -= 1; + let world = ThreadWorldContainer.try_get_world()?; + let (next_ref, _) = infinite_iter.next_ref(); + // we stop once the reflection path is invalid + ReflectReference::into_script_ref(next_ref, world) + }; + Ok(iter_function.into_dynamic_script_function_mut()) + } } /// Lists the functions available on the reference. diff --git a/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs b/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs index 23f1a85643..d4f3fd2dc4 100644 --- a/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs +++ b/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs @@ -5,7 +5,7 @@ use bevy_mod_scripting_bindings::{ script_value::ScriptValue, }; use bevy_mod_scripting_display::OrFakeId; -use mlua::{ExternalError, MetaMethod, UserData, UserDataMethods}; +use mlua::{ExternalError, IntoLua, MetaMethod, UserData, UserDataMethods}; use crate::IntoMluaError; @@ -302,7 +302,7 @@ impl UserData for LuaReflectReference { feature = "lua52", feature = "luajit52", ))] - m.add_meta_function(MetaMethod::Pairs, |_, s: LuaReflectReference| { + m.add_meta_function(MetaMethod::Pairs, |lua, s: LuaReflectReference| { profiling::function_scope!("MetaMethod::Pairs"); // let mut iter_func = lookup_dynamic_function_typed::(l, "iter") // .expect("No iter function registered"); @@ -317,11 +317,36 @@ impl UserData for LuaReflectReference { }) .map_err(IntoMluaError::to_lua_error)?; - Ok(LuaScriptValue::from( - iter_func - .call(vec![ScriptValue::Reference(s.into())], LUA_CALLER_CONTEXT) - .map_err(IntoMluaError::to_lua_error)?, - )) + let result = iter_func + .call(vec![ScriptValue::Reference(s.into())], LUA_CALLER_CONTEXT) + .map_err(IntoMluaError::to_lua_error)?; + + match result { + ScriptValue::FunctionMut(func) => { + // Create a Lua function that wraps our iterator and unpacks List results + lua.create_function_mut(move |lua, _args: ()| { + let result = func + .call(vec![], LUA_CALLER_CONTEXT) + .map_err(IntoMluaError::to_lua_error)?; + + // If the result is a List with 2 elements, unpack it into multiple return values + match result { + ScriptValue::List(ref items) if items.len() == 2 => { + // Return as tuple (key, value) which Lua unpacks automatically + let key = LuaScriptValue(items[0].clone()).into_lua(lua)?; + let value = LuaScriptValue(items[1].clone()).into_lua(lua)?; + Ok((key, value)) + } + _ => { + // Single value or Unit - return as-is + let val = LuaScriptValue(result).into_lua(lua)?; + Ok((val, mlua::Value::Nil)) + } + } + }) + } + _ => Err(mlua::Error::RuntimeError("iter function did not return a FunctionMut".to_string())) + } }); m.add_meta_function(MetaMethod::ToString, |_, self_: LuaReflectReference| { diff --git a/crates/languages/bevy_mod_scripting_rhai/src/bindings/reference.rs b/crates/languages/bevy_mod_scripting_rhai/src/bindings/reference.rs index a5799c3427..ad463099a7 100644 --- a/crates/languages/bevy_mod_scripting_rhai/src/bindings/reference.rs +++ b/crates/languages/bevy_mod_scripting_rhai/src/bindings/reference.rs @@ -598,6 +598,29 @@ impl CustomType for RhaiReflectReference { Ok(str_) => str_.into(), Err(error) => error.to_string(), } + }) + .with_fn("iter", |self_: Self| { + let world = ThreadWorldContainer + .try_get_world() + .map_err(IntoRhaiError::into_rhai_error)?; + + let iter_func = world + .lookup_function([TypeId::of::()], "iter") + .map_err(|f| { + InteropError::missing_function(f, TypeId::of::().into()) + }) + .map_err(IntoRhaiError::into_rhai_error)?; + + let result = iter_func + .call(vec![ScriptValue::Reference(self_.0)], RHAI_CALLER_CONTEXT) + .map_err(IntoRhaiError::into_rhai_error)?; + + match result { + ScriptValue::FunctionMut(iter_fn) => { + Ok(Dynamic::from(RhaiIterator { iter_fn })) + } + _ => Err(InteropError::invariant("iter function did not return a FunctionMut").into_rhai_error()) + } }); } } @@ -632,3 +655,24 @@ impl CustomType for RhaiStaticReflectReference { }); } } + +/// Wrapper for map iterator that unpacks [key, value] pairs for Rhai +#[derive(Clone)] +pub struct RhaiIterator { + iter_fn: DynamicScriptFunctionMut, +} + +impl CustomType for RhaiIterator { + fn build(mut builder: rhai::TypeBuilder) { + builder + .with_name("RhaiIterator") + .with_fn("next", |self_: &mut Self| { + let result = self_ + .iter_fn + .call(vec![], RHAI_CALLER_CONTEXT) + .map_err(IntoRhaiError::into_rhai_error)?; + result.into_dynamic() + }); + } +} + diff --git a/crates/languages/bevy_mod_scripting_rhai/src/lib.rs b/crates/languages/bevy_mod_scripting_rhai/src/lib.rs index e6de10e1ff..51a8ba32fb 100644 --- a/crates/languages/bevy_mod_scripting_rhai/src/lib.rs +++ b/crates/languages/bevy_mod_scripting_rhai/src/lib.rs @@ -25,7 +25,7 @@ use bevy_mod_scripting_core::{ make_plugin_config_static, script::{ContextPolicy, DisplayProxy, ScriptAttachment}, }; -use bindings::reference::{ReservedKeyword, RhaiReflectReference, RhaiStaticReflectReference}; +use bindings::reference::{ReservedKeyword, RhaiReflectReference, RhaiStaticReflectReference, RhaiIterator}; use parking_lot::RwLock; pub use rhai; @@ -142,6 +142,7 @@ impl Default for RhaiScriptingPlugin { engine.set_max_expr_depths(999, 999); engine.build_type::(); engine.build_type::(); + engine.build_type::(); engine.register_iterator_result::(); Ok(()) }], From fc7d74621d8b8d04a7f6ee67312b3ef344288af1 Mon Sep 17 00:00:00 2001 From: aggyomfg Date: Sat, 4 Oct 2025 14:34:40 +0400 Subject: [PATCH 02/16] map_get via to_dynamic partial reflect --- .../bevy_mod_scripting_functions/src/core.rs | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/crates/bevy_mod_scripting_functions/src/core.rs b/crates/bevy_mod_scripting_functions/src/core.rs index 03371e5dbd..1f294d670a 100644 --- a/crates/bevy_mod_scripting_functions/src/core.rs +++ b/crates/bevy_mod_scripting_functions/src/core.rs @@ -7,6 +7,7 @@ use std::ops::Deref; use bevy_app::App; use bevy_asset::{AssetServer, Handle}; use bevy_ecs::{entity::Entity, prelude::AppTypeRegistry, schedule::Schedules, world::World}; +use bevy_reflect::ReflectRef; use bevy_mod_scripting_bindings::{ DynamicScriptFunctionMut, FunctionInfo, GlobalNamespace, InteropError, PartialReflectExt, ReflectReference, ScriptComponentRegistration, ScriptQueryBuilder, ScriptQueryResult, @@ -571,7 +572,7 @@ impl ReflectReference { Ok(format!("{reference:#?}")) } - /// Gets and clones the value under the specified key if the underlying type is a map type. + /// Gets the value reference under the specified key if the underlying type is a map type. /// /// Arguments: /// * `ctxt`: The function call context. @@ -597,17 +598,22 @@ impl ReflectReference { key, world.clone(), )?; - reference.with_reflect_mut(world.clone(), |s| match s.try_map_get(key.as_ref())? { - Some(value) => { - let reference = { - let allocator = world.allocator(); - let mut allocator = allocator.write(); - let owned_value = ::from_reflect(value, world.clone())?; - ReflectReference::new_allocated_boxed(owned_value, &mut allocator) - }; - Ok(Some(ReflectReference::into_script_ref(reference, world)?)) - } - None => Ok(None), + reference.with_reflect(world.clone(), |s| { + s.try_map_get(key.as_ref()).and_then(|maybe_value| { + maybe_value + .map(|value| { + let reference = { + let allocator = world.allocator(); + let mut allocator_guard = allocator.write(); + ReflectReference::new_allocated_boxed_parial_reflect( + value.to_dynamic(), + &mut *allocator_guard, + )? + }; + ReflectReference::into_script_ref(reference, world) + }) + .transpose() + }) })? } From e7e1f37aec214ec04a729a7aa55c93a6fc88d178 Mon Sep 17 00:00:00 2001 From: aggyomfg Date: Sat, 4 Oct 2025 22:28:11 +0400 Subject: [PATCH 03/16] get_map_clone - some times needs full reflected value (assets, construct), not partial --- .../bevy_mod_scripting_functions/src/core.rs | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/crates/bevy_mod_scripting_functions/src/core.rs b/crates/bevy_mod_scripting_functions/src/core.rs index 1f294d670a..c716811e39 100644 --- a/crates/bevy_mod_scripting_functions/src/core.rs +++ b/crates/bevy_mod_scripting_functions/src/core.rs @@ -572,7 +572,7 @@ impl ReflectReference { Ok(format!("{reference:#?}")) } - /// Gets the value reference under the specified key if the underlying type is a map type. + /// Gets and clones the value under the specified key if the underlying type is a map type. /// /// Arguments: /// * `ctxt`: The function call context. @@ -580,7 +580,7 @@ impl ReflectReference { /// * `key`: The key to index with. /// Returns: /// * `value`: The value at the key, if the reference is a map. - fn map_get( + fn map_get_clone( ctxt: FunctionCallContext, reference: ReflectReference, key: ScriptValue, @@ -598,6 +598,48 @@ impl ReflectReference { key, world.clone(), )?; + reference.with_reflect_mut(world.clone(), |s| match s.try_map_get(key.as_ref())? { + Some(value) => { + let reference = { + let allocator = world.allocator(); + let mut allocator = allocator.write(); + let owned_value = ::from_reflect(value, world.clone())?; + ReflectReference::new_allocated_boxed(owned_value, &mut allocator) + }; + Ok(Some(ReflectReference::into_script_ref(reference, world)?)) + } + None => Ok(None), + })? + } + + /// Gets the value reference under the specified key using dynamic reflection if the underlying type is a map type. + /// This method uses `to_dynamic()` which creates a dynamic representation that only implements `PartialReflect`. + /// Note: Values returned by this method cannot be used with asset operations that require full `Reflect` trait. + /// + /// Arguments: + /// * `ctxt`: The function call context. + /// * `reference`: The reference to index into. + /// * `key`: The key to index with. + /// Returns: + /// * `value`: The dynamic value at the key, if the reference is a map. + fn map_get( + ctxt: FunctionCallContext, + reference: ReflectReference, + key: ScriptValue, + ) -> Result, InteropError> { + profiling::function_scope!("map_get_dynamic"); + let world = ctxt.world()?; + let key = >::from_script_ref( + reference.key_type_id(world.clone())?.ok_or_else(|| { + InteropError::unsupported_operation( + reference.tail_type_id(world.clone()).unwrap_or_default(), + Some(Box::new(key.clone())), + "Could not get key type id. Are you trying to index into a type that's not a map?".to_owned(), + ) + })?, + key, + world.clone(), + )?; reference.with_reflect(world.clone(), |s| { s.try_map_get(key.as_ref()).and_then(|maybe_value| { maybe_value From 9d99dc395977866dbc94332d6f445ac6cfd9bb8a Mon Sep 17 00:00:00 2001 From: aggyomfg Date: Sat, 4 Oct 2025 22:48:31 +0400 Subject: [PATCH 04/16] map_get_clone don't need mutable acces --- crates/bevy_mod_scripting_functions/src/core.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_mod_scripting_functions/src/core.rs b/crates/bevy_mod_scripting_functions/src/core.rs index c716811e39..31d165d775 100644 --- a/crates/bevy_mod_scripting_functions/src/core.rs +++ b/crates/bevy_mod_scripting_functions/src/core.rs @@ -598,7 +598,7 @@ impl ReflectReference { key, world.clone(), )?; - reference.with_reflect_mut(world.clone(), |s| match s.try_map_get(key.as_ref())? { + reference.with_reflect(world.clone(), |s| match s.try_map_get(key.as_ref())? { Some(value) => { let reference = { let allocator = world.allocator(); From 55724f1effa1b45b1f809f130f9ed99cde520360 Mon Sep 17 00:00:00 2001 From: aggyomfg Date: Sun, 5 Oct 2025 11:58:59 +0400 Subject: [PATCH 05/16] improve iters to iter partial reflect or _clone (full Reflect) --- assets/tests/iter/hashmap.lua | 2 + assets/tests/iter_clone/hashmap.lua | 21 +++++ assets/tests/iter_clone/hashmap.rhai | 31 +++++++ assets/tests/iter_clone/hashmap_pairs.lua | 23 +++++ assets/tests/iter_clone/vec.lua | 17 ++++ assets/tests/iter_clone/vec.rhai | 22 +++++ assets/tests/iter_clone/vec_pairs.lua | 14 ++++ .../src/reference.rs | 83 +++++++++++++++++++ .../bevy_mod_scripting_functions/src/core.rs | 65 +++++++++++++++ .../src/bindings/reference.rs | 45 +++++++++- .../src/bindings/reference.rs | 23 +++++ 11 files changed, 345 insertions(+), 1 deletion(-) create mode 100644 assets/tests/iter_clone/hashmap.lua create mode 100644 assets/tests/iter_clone/hashmap.rhai create mode 100644 assets/tests/iter_clone/hashmap_pairs.lua create mode 100644 assets/tests/iter_clone/vec.lua create mode 100644 assets/tests/iter_clone/vec.rhai create mode 100644 assets/tests/iter_clone/vec_pairs.lua diff --git a/assets/tests/iter/hashmap.lua b/assets/tests/iter/hashmap.lua index b74f9f4382..a8a104799e 100644 --- a/assets/tests/iter/hashmap.lua +++ b/assets/tests/iter/hashmap.lua @@ -5,6 +5,8 @@ local map = res.string_map local count = 0 local found_keys = {} + +--- Iterate over PartialReflect refs using pairs for key, value in pairs(map) do count = count + 1 found_keys[key] = value diff --git a/assets/tests/iter_clone/hashmap.lua b/assets/tests/iter_clone/hashmap.lua new file mode 100644 index 0000000000..39eee0bdf4 --- /dev/null +++ b/assets/tests/iter_clone/hashmap.lua @@ -0,0 +1,21 @@ +local res_type = world.get_type_by_name("TestResourceWithVariousFields") +local res = world.get_resource(res_type) + +local map = res.string_map + +local iterator = map:iter_clone() +local count = 0 +local found_keys = {} + +local result = iterator() +while result ~= nil do + local key = result[1] + local value = result[2] + count = count + 1 + found_keys[key] = value + result = iterator() +end + +assert(count == 2, "Expected 2 entries, got " .. count) +assert(found_keys["foo"] == "bar", "Expected foo=>bar") +assert(found_keys["zoo"] == "zed", "Expected zoo=>zed") diff --git a/assets/tests/iter_clone/hashmap.rhai b/assets/tests/iter_clone/hashmap.rhai new file mode 100644 index 0000000000..5575cde9b9 --- /dev/null +++ b/assets/tests/iter_clone/hashmap.rhai @@ -0,0 +1,31 @@ +let res_type = world.get_type_by_name.call("TestResourceWithVariousFields"); +let res = world.get_resource.call(res_type); + +let map = res.string_map; + +let iterator = map.iter_clone(); +let count = 0; +let found_keys = #{}; + +loop { + let result = iterator.next(); + + if result == () { + break; + } + + let key = result[0]; + let value = result[1]; + count += 1; + found_keys[key] = value; +} + +if count != 2 { + throw `Expected 2 entries, got ${count}`; +} +if found_keys["foo"] != "bar" { + throw "Expected foo=>bar"; +} +if found_keys["zoo"] != "zed" { + throw "Expected zoo=>zed"; +} diff --git a/assets/tests/iter_clone/hashmap_pairs.lua b/assets/tests/iter_clone/hashmap_pairs.lua new file mode 100644 index 0000000000..12afaa06c4 --- /dev/null +++ b/assets/tests/iter_clone/hashmap_pairs.lua @@ -0,0 +1,23 @@ +local res_type = world.get_type_by_name("TestResourceWithVariousFields") +local res = world.get_resource(res_type) + +local map = res.string_map + +local count = 0 +local found_keys = {} + +-- Use pairs_clone to loop over Reflect values +for key, value in map:pairs_clone() do + count = count + 1 + found_keys[key] = value +end + +if count ~= 2 then + error(string.format("Expected 2 entries, got %d", count)) +end +if found_keys["foo"] ~= "bar" then + error("Expected foo=>bar") +end +if found_keys["zoo"] ~= "zed" then + error("Expected zoo=>zed") +end diff --git a/assets/tests/iter_clone/vec.lua b/assets/tests/iter_clone/vec.lua new file mode 100644 index 0000000000..9d24a7a9e3 --- /dev/null +++ b/assets/tests/iter_clone/vec.lua @@ -0,0 +1,17 @@ +local res_type = world.get_type_by_name("TestResourceWithVariousFields") +local res = world.get_resource(res_type) + +local iterated_vals = {} +local iterator = res.vec_usize:iter_clone() +local result = iterator() +while result ~= nil do + iterated_vals[#iterated_vals + 1] = result + result = iterator() +end + +assert(#iterated_vals == 5, "Length is not 5") +assert(iterated_vals[1] == 1, "First value is not 1") +assert(iterated_vals[2] == 2, "Second value is not 2") +assert(iterated_vals[3] == 3, "Third value is not 3") +assert(iterated_vals[4] == 4, "Fourth value is not 4") +assert(iterated_vals[5] == 5, "Fifth value is not 5") diff --git a/assets/tests/iter_clone/vec.rhai b/assets/tests/iter_clone/vec.rhai new file mode 100644 index 0000000000..d634a3a6bf --- /dev/null +++ b/assets/tests/iter_clone/vec.rhai @@ -0,0 +1,22 @@ +let res_type = world.get_type_by_name.call("TestResourceWithVariousFields"); +let res = world.get_resource.call(res_type); + +let iterated_vals = []; +let iterator = res.vec_usize.iter_clone(); + +loop { + let result = iterator.next(); + + if result == () { + break; + } + + iterated_vals.push(result); +} + +assert(iterated_vals.len == 5, "Length is not 5"); +assert(iterated_vals[0] == 1, "First value is not 1"); +assert(iterated_vals[1] == 2, "Second value is not 2"); +assert(iterated_vals[2] == 3, "Third value is not 3"); +assert(iterated_vals[3] == 4, "Fourth value is not 4"); +assert(iterated_vals[4] == 5, "Fifth value is not 5"); diff --git a/assets/tests/iter_clone/vec_pairs.lua b/assets/tests/iter_clone/vec_pairs.lua new file mode 100644 index 0000000000..7c9ad594aa --- /dev/null +++ b/assets/tests/iter_clone/vec_pairs.lua @@ -0,0 +1,14 @@ +local res_type = world.get_type_by_name("TestResourceWithVariousFields") +local res = world.get_resource(res_type) + +local iterated_vals = {} +for v in res.vec_usize:pairs_clone() do + iterated_vals[#iterated_vals + 1] = v +end + +assert(#iterated_vals == 5, "Length is not 5") +assert(iterated_vals[1] == 1, "First value is not 1") +assert(iterated_vals[2] == 2, "Second value is not 2") +assert(iterated_vals[3] == 3, "Third value is not 3") +assert(iterated_vals[4] == 4, "Fourth value is not 4") +assert(iterated_vals[5] == 5, "Fifth value is not 5") diff --git a/crates/bevy_mod_scripting_bindings/src/reference.rs b/crates/bevy_mod_scripting_bindings/src/reference.rs index d7d63a6d29..9a4b978870 100644 --- a/crates/bevy_mod_scripting_bindings/src/reference.rs +++ b/crates/bevy_mod_scripting_bindings/src/reference.rs @@ -788,6 +788,46 @@ impl ReflectRefIter { }; (next, index) } + + /// Returns the next element as a cloned value using `from_reflect`. + /// Returns a fully reflected value (Box) instead of a reference. + /// Returns Ok(None) when the path is invalid (end of iteration). + pub fn next_cloned(&mut self, world: WorldGuard) -> Result, InteropError> { + let index = match &mut self.index { + IterationKey::Index(i) => { + let idx = *i; + *i += 1; + idx + } + }; + + let element = self.base.with_reflect(world.clone(), |base_reflect| { + match base_reflect.reflect_ref() { + bevy_reflect::ReflectRef::List(list) => { + list.get(index).map(|item| { + ::from_reflect(item, world.clone()) + }) + } + bevy_reflect::ReflectRef::Array(array) => { + array.get(index).map(|item| { + ::from_reflect(item, world.clone()) + }) + } + _ => None, + } + })?; + + match element { + Some(result) => { + let owned_value = result?; + let allocator = world.allocator(); + let mut allocator_guard = allocator.write(); + let value_ref = ReflectReference::new_allocated_boxed(owned_value, &mut *allocator_guard); + Ok(Some(value_ref)) + } + None => Ok(None), + } + } } const fn list_index_access(index: usize) -> Access<'static> { @@ -842,6 +882,49 @@ impl ReflectMapRefIter { } })? } + + /// Returns the next map entry as a (key, value) tuple, cloning the values. + /// Returns Ok(None) when there are no more entries. + /// This uses `from_reflect` to clone the actual values instead of creating dynamic references. + /// Returns fully reflected values (Box) like `map_get_clone` does. + pub fn next_cloned(&mut self, world: WorldGuard) -> Result, InteropError> { + let idx = self.index; + self.index += 1; + + // Access the map and get the entry at index + self.base.with_reflect(world.clone(), |reflect| { + match reflect.reflect_ref() { + ReflectRef::Map(map) => { + if let Some((key, value)) = map.get_at(idx) { + let allocator = world.allocator(); + let mut allocator_guard = allocator.write(); + + let owned_key = ::from_reflect(key, world.clone())?; + let key_ref = ReflectReference::new_allocated_boxed( + owned_key, + &mut *allocator_guard + ); + + let owned_value = ::from_reflect(value, world.clone())?; + let value_ref = ReflectReference::new_allocated_boxed( + owned_value, + &mut *allocator_guard + ); + + drop(allocator_guard); + Ok(Some((key_ref, value_ref))) + } else { + Ok(None) + } + } + _ => Err(InteropError::unsupported_operation( + reflect.get_represented_type_info().map(|ti| ti.type_id()), + None, + "map iteration on non-map type".to_owned(), + )) + } + })? + } } #[profiling::all_functions] diff --git a/crates/bevy_mod_scripting_functions/src/core.rs b/crates/bevy_mod_scripting_functions/src/core.rs index 31d165d775..afb9044656 100644 --- a/crates/bevy_mod_scripting_functions/src/core.rs +++ b/crates/bevy_mod_scripting_functions/src/core.rs @@ -884,6 +884,71 @@ impl ReflectReference { } } + /// Iterates over the reference with cloned values, if the reference is an appropriate container type. + /// + /// This method clones the actual values using `from_reflect` instead of creating dynamic references. + /// Returns an "next" iterator function that returns fully reflected values (Box). + /// + /// The iterator function should be called until it returns `nil` to signal the end of the iteration. + /// + /// Arguments: + /// * `ctxt`: The function call context. + /// * `reference`: The reference to iterate over. + /// Returns: + /// * `iter`: The iterator function that returns cloned values. + fn iter_clone( + ctxt: FunctionCallContext, + reference: ReflectReference, + ) -> Result { + profiling::function_scope!("iter_clone"); + let world = ctxt.world()?; + let mut len = reference.len(world.clone())?.unwrap_or_default(); + // Check if this is a Map type + let is_map = reference.with_reflect(world.clone(), |r| { + matches!(r.reflect_ref(), ReflectRef::Map(_)) + })?; + + if is_map { + // Use special map iterator that clones values + let mut map_iter = reference.into_map_iter(); + let iter_function = move || { + if len == 0 { + return Ok(ScriptValue::Unit); + } + len -= 1; + let world = ThreadWorldContainer.try_get_world()?; + match map_iter.next_cloned(world.clone())? { + Some((key_ref, value_ref)) => { + // Return both key and value as a List for Lua's pairs() to unpack + let key_value = ReflectReference::into_script_ref(key_ref, world.clone())?; + let value_value = ReflectReference::into_script_ref(value_ref, world)?; + Ok(ScriptValue::List(vec![key_value, value_value])) + } + None => Ok(ScriptValue::Unit), + } + }; + Ok(iter_function.into_dynamic_script_function_mut()) + } else { + // Use cloning iterator for lists/arrays + let mut infinite_iter = reference.into_iter_infinite(); + let iter_function = move || { + if len == 0 { + return Ok(ScriptValue::Unit); + } + len -= 1; + let world = ThreadWorldContainer.try_get_world()?; + match infinite_iter.next_cloned(world.clone())? { + Some(value_ref) => { + // Convert the cloned value to a script value + ReflectReference::into_script_ref(value_ref, world) + } + None => Ok(ScriptValue::Unit), + } + }; + Ok(iter_function.into_dynamic_script_function_mut()) + } + } + /// Lists the functions available on the reference. /// /// Arguments: diff --git a/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs b/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs index d4f3fd2dc4..4171015bd1 100644 --- a/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs +++ b/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs @@ -323,7 +323,6 @@ impl UserData for LuaReflectReference { match result { ScriptValue::FunctionMut(func) => { - // Create a Lua function that wraps our iterator and unpacks List results lua.create_function_mut(move |lua, _args: ()| { let result = func .call(vec![], LUA_CALLER_CONTEXT) @@ -349,6 +348,50 @@ impl UserData for LuaReflectReference { } }); + m.add_method("pairs_clone", |lua, s: &LuaReflectReference, _args: ()| { + profiling::function_scope!("pairs_clone"); + let world = ThreadWorldContainer + .try_get_world() + .map_err(IntoMluaError::to_lua_error)?; + + let iter_func = world + .lookup_function([TypeId::of::()], "iter_clone") + .map_err(|f| { + InteropError::missing_function(f, TypeId::of::().into()) + }) + .map_err(IntoMluaError::to_lua_error)?; + + let result = iter_func + .call(vec![ScriptValue::Reference(s.clone().into())], LUA_CALLER_CONTEXT) + .map_err(IntoMluaError::to_lua_error)?; + + match result { + ScriptValue::FunctionMut(func) => { + lua.create_function_mut(move |lua, _args: ()| { + let result = func + .call(vec![], LUA_CALLER_CONTEXT) + .map_err(IntoMluaError::to_lua_error)?; + + // If the result is a List with 2 elements, unpack it into multiple return values + match result { + ScriptValue::List(ref items) if items.len() == 2 => { + // Return as tuple (key, value) which Lua unpacks automatically + let key = LuaScriptValue(items[0].clone()).into_lua(lua)?; + let value = LuaScriptValue(items[1].clone()).into_lua(lua)?; + Ok((key, value)) + } + _ => { + // Single value or Unit - return as-is + let val = LuaScriptValue(result).into_lua(lua)?; + Ok((val, mlua::Value::Nil)) + } + } + }) + } + _ => Err(mlua::Error::RuntimeError("iter_clone function did not return a FunctionMut".to_string())) + } + }); + m.add_meta_function(MetaMethod::ToString, |_, self_: LuaReflectReference| { profiling::function_scope!("MetaMethod::ToString"); let world = ThreadWorldContainer diff --git a/crates/languages/bevy_mod_scripting_rhai/src/bindings/reference.rs b/crates/languages/bevy_mod_scripting_rhai/src/bindings/reference.rs index ad463099a7..3e9816e863 100644 --- a/crates/languages/bevy_mod_scripting_rhai/src/bindings/reference.rs +++ b/crates/languages/bevy_mod_scripting_rhai/src/bindings/reference.rs @@ -621,6 +621,29 @@ impl CustomType for RhaiReflectReference { } _ => Err(InteropError::invariant("iter function did not return a FunctionMut").into_rhai_error()) } + }) + .with_fn("iter_clone", |self_: Self| { + let world = ThreadWorldContainer + .try_get_world() + .map_err(IntoRhaiError::into_rhai_error)?; + + let iter_func = world + .lookup_function([TypeId::of::()], "iter_clone") + .map_err(|f| { + InteropError::missing_function(f, TypeId::of::().into()) + }) + .map_err(IntoRhaiError::into_rhai_error)?; + + let result = iter_func + .call(vec![ScriptValue::Reference(self_.0)], RHAI_CALLER_CONTEXT) + .map_err(IntoRhaiError::into_rhai_error)?; + + match result { + ScriptValue::FunctionMut(iter_fn) => { + Ok(Dynamic::from(RhaiIterator { iter_fn })) + } + _ => Err(InteropError::invariant("iter_clone function did not return a FunctionMut").into_rhai_error()) + } }); } } From 52e1e146e40c258b4d5d635f5ab01b5f41c31772 Mon Sep 17 00:00:00 2001 From: aggyomfg Date: Sun, 5 Oct 2025 12:08:27 +0400 Subject: [PATCH 06/16] ipairs_clone --- assets/tests/iter_clone/vec_ipairs.lua | 15 +++++++ .../src/bindings/reference.rs | 44 +++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 assets/tests/iter_clone/vec_ipairs.lua diff --git a/assets/tests/iter_clone/vec_ipairs.lua b/assets/tests/iter_clone/vec_ipairs.lua new file mode 100644 index 0000000000..012c481233 --- /dev/null +++ b/assets/tests/iter_clone/vec_ipairs.lua @@ -0,0 +1,15 @@ +local res_type = world.get_type_by_name("TestResourceWithVariousFields") +local res = world.get_resource(res_type) + +local iterated_vals = {} +for i, v in res.vec_usize:ipairs_clone() do + assert(i == #iterated_vals + 1, "Index mismatch: expected " .. (#iterated_vals + 1) .. ", got " .. i) + iterated_vals[#iterated_vals + 1] = v +end + +assert(#iterated_vals == 5, "Length is not 5") +assert(iterated_vals[1] == 1, "First value is not 1") +assert(iterated_vals[2] == 2, "Second value is not 2") +assert(iterated_vals[3] == 3, "Third value is not 3") +assert(iterated_vals[4] == 4, "Fourth value is not 4") +assert(iterated_vals[5] == 5, "Fifth value is not 5") diff --git a/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs b/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs index 4171015bd1..7292fb197e 100644 --- a/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs +++ b/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs @@ -392,6 +392,50 @@ impl UserData for LuaReflectReference { } }); + m.add_method("ipairs_clone", |lua, s: &LuaReflectReference, _args: ()| { + profiling::function_scope!("ipairs_clone"); + let world = ThreadWorldContainer + .try_get_world() + .map_err(IntoMluaError::to_lua_error)?; + + let iter_func = world + .lookup_function([TypeId::of::()], "iter_clone") + .map_err(|f| { + InteropError::missing_function(f, TypeId::of::().into()) + }) + .map_err(IntoMluaError::to_lua_error)?; + + let result = iter_func + .call(vec![ScriptValue::Reference(s.clone().into())], LUA_CALLER_CONTEXT) + .map_err(IntoMluaError::to_lua_error)?; + + match result { + ScriptValue::FunctionMut(func) => { + let mut index = 0i64; + lua.create_function_mut(move |lua, _args: ()| { + let result = func + .call(vec![], LUA_CALLER_CONTEXT) + .map_err(IntoMluaError::to_lua_error)?; + + match result { + ScriptValue::Unit => { + // End of iteration + Ok((mlua::Value::Nil, mlua::Value::Nil)) + } + _ => { + // Return (index, value) tuple for ipairs + index += 1; + let idx = mlua::Value::Integer(index); + let val = LuaScriptValue(result).into_lua(lua)?; + Ok((idx, val)) + } + } + }) + } + _ => Err(mlua::Error::RuntimeError("iter_clone function did not return a FunctionMut".to_string())) + } + }); + m.add_meta_function(MetaMethod::ToString, |_, self_: LuaReflectReference| { profiling::function_scope!("MetaMethod::ToString"); let world = ThreadWorldContainer From 12865a8834e5d7901c63678fe6a9cd353d30a139 Mon Sep 17 00:00:00 2001 From: aggyomfg Date: Sun, 5 Oct 2025 12:18:22 +0400 Subject: [PATCH 07/16] ipairs_clone can be used with map (default lua ipairs can't) --- assets/tests/iter_clone/hashmap_ipairs.lua | 27 ++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 assets/tests/iter_clone/hashmap_ipairs.lua diff --git a/assets/tests/iter_clone/hashmap_ipairs.lua b/assets/tests/iter_clone/hashmap_ipairs.lua new file mode 100644 index 0000000000..4d9074132b --- /dev/null +++ b/assets/tests/iter_clone/hashmap_ipairs.lua @@ -0,0 +1,27 @@ +local res_type = world.get_type_by_name("TestResourceWithVariousFields") +local res = world.get_resource(res_type) + +local map = res.string_map + +local count = 0 +local found_keys = {} + +-- ipairs_clone on a map returns (index, [key, value]) where value is a list +for i, entry in map:ipairs_clone() do + assert(i == count + 1, "Index should be sequential: expected " .. (count + 1) .. ", got " .. i) + + -- entry should be a list with [key, value] + assert(entry ~= nil, "Entry should not be nil") + assert(entry[1] ~= nil, "Key should not be nil") + assert(entry[2] ~= nil, "Value should not be nil") + + local key = entry[1] + local value = entry[2] + + count = count + 1 + found_keys[key] = value +end + +assert(count == 2, "Expected 2 entries, got " .. count) +assert(found_keys["foo"] == "bar", "Expected foo=>bar, got " .. tostring(found_keys["foo"])) +assert(found_keys["zoo"] == "zed", "Expected zoo=>zed, got " .. tostring(found_keys["zoo"])) From 96c9156bf371dcc8367ae96a3cf79ded9b0c6848 Mon Sep 17 00:00:00 2001 From: aggyomfg Date: Sun, 5 Oct 2025 22:50:10 +0400 Subject: [PATCH 08/16] get rid of leaked abstraction --- assets/tests/iter/hashmap.lua | 8 +- .../{iter_clone => iter}/hashmap_ipairs.lua | 4 +- .../{iter_clone => iter}/hashmap_pairs.lua | 3 +- assets/tests/iter/vec.lua | 10 +- assets/tests/iter/vec.rhai | 11 +- .../tests/{iter_clone => iter}/vec_ipairs.lua | 2 +- .../tests/{iter_clone => iter}/vec_pairs.lua | 2 +- assets/tests/iter_clone/hashmap.lua | 21 ---- assets/tests/iter_clone/hashmap.rhai | 31 ----- assets/tests/iter_clone/vec.lua | 17 --- assets/tests/iter_clone/vec.rhai | 22 ---- .../bevy_mod_scripting_functions/src/core.rs | 115 +----------------- .../src/bindings/reference.rs | 16 +-- 13 files changed, 40 insertions(+), 222 deletions(-) rename assets/tests/{iter_clone => iter}/hashmap_ipairs.lua (87%) rename assets/tests/{iter_clone => iter}/hashmap_pairs.lua (84%) rename assets/tests/{iter_clone => iter}/vec_ipairs.lua (93%) rename assets/tests/{iter_clone => iter}/vec_pairs.lua (92%) delete mode 100644 assets/tests/iter_clone/hashmap.lua delete mode 100644 assets/tests/iter_clone/hashmap.rhai delete mode 100644 assets/tests/iter_clone/vec.lua delete mode 100644 assets/tests/iter_clone/vec.rhai diff --git a/assets/tests/iter/hashmap.lua b/assets/tests/iter/hashmap.lua index a8a104799e..0d27ae93b0 100644 --- a/assets/tests/iter/hashmap.lua +++ b/assets/tests/iter/hashmap.lua @@ -3,13 +3,17 @@ local res = world.get_resource(res_type) local map = res.string_map +local iterator = map:iter() local count = 0 local found_keys = {} ---- Iterate over PartialReflect refs using pairs -for key, value in pairs(map) do +local result = iterator() +while result ~= nil do + local key = result[1] + local value = result[2] count = count + 1 found_keys[key] = value + result = iterator() end assert(count == 2, "Expected 2 entries, got " .. count) diff --git a/assets/tests/iter_clone/hashmap_ipairs.lua b/assets/tests/iter/hashmap_ipairs.lua similarity index 87% rename from assets/tests/iter_clone/hashmap_ipairs.lua rename to assets/tests/iter/hashmap_ipairs.lua index 4d9074132b..acc41dcb68 100644 --- a/assets/tests/iter_clone/hashmap_ipairs.lua +++ b/assets/tests/iter/hashmap_ipairs.lua @@ -6,8 +6,8 @@ local map = res.string_map local count = 0 local found_keys = {} --- ipairs_clone on a map returns (index, [key, value]) where value is a list -for i, entry in map:ipairs_clone() do +-- ipairs on a map returns (index, [key, value]) where value is a list +for i, entry in map:ipairs() do assert(i == count + 1, "Index should be sequential: expected " .. (count + 1) .. ", got " .. i) -- entry should be a list with [key, value] diff --git a/assets/tests/iter_clone/hashmap_pairs.lua b/assets/tests/iter/hashmap_pairs.lua similarity index 84% rename from assets/tests/iter_clone/hashmap_pairs.lua rename to assets/tests/iter/hashmap_pairs.lua index 12afaa06c4..8735ac75f6 100644 --- a/assets/tests/iter_clone/hashmap_pairs.lua +++ b/assets/tests/iter/hashmap_pairs.lua @@ -6,8 +6,7 @@ local map = res.string_map local count = 0 local found_keys = {} --- Use pairs_clone to loop over Reflect values -for key, value in map:pairs_clone() do +for key, value in map:pairs() do count = count + 1 found_keys[key] = value end diff --git a/assets/tests/iter/vec.lua b/assets/tests/iter/vec.lua index e1da679d5a..be031a7eb2 100644 --- a/assets/tests/iter/vec.lua +++ b/assets/tests/iter/vec.lua @@ -1,10 +1,14 @@ local res_type = world.get_type_by_name("TestResourceWithVariousFields") local res = world.get_resource(res_type) -iterated_vals = {} -for v in pairs(res.vec_usize) do - iterated_vals[#iterated_vals + 1] = v +local iterated_vals = {} +local iterator = res.vec_usize:iter() +local result = iterator() +while result ~= nil do + iterated_vals[#iterated_vals + 1] = result + result = iterator() end + assert(#iterated_vals == 5, "Length is not 5") assert(iterated_vals[1] == 1, "First value is not 1") assert(iterated_vals[2] == 2, "Second value is not 2") diff --git a/assets/tests/iter/vec.rhai b/assets/tests/iter/vec.rhai index accbcd587a..f5d9233059 100644 --- a/assets/tests/iter/vec.rhai +++ b/assets/tests/iter/vec.rhai @@ -2,9 +2,16 @@ let res_type = world.get_type_by_name.call("TestResourceWithVariousFields"); let res = world.get_resource.call(res_type); let iterated_vals = []; +let iterator = res.vec_usize.iter(); -for v in res.vec_usize { - iterated_vals.push(v); +loop { + let result = iterator.next(); + + if result == () { + break; + } + + iterated_vals.push(result); } assert(iterated_vals.len == 5, "Length is not 5"); diff --git a/assets/tests/iter_clone/vec_ipairs.lua b/assets/tests/iter/vec_ipairs.lua similarity index 93% rename from assets/tests/iter_clone/vec_ipairs.lua rename to assets/tests/iter/vec_ipairs.lua index 012c481233..9c48877499 100644 --- a/assets/tests/iter_clone/vec_ipairs.lua +++ b/assets/tests/iter/vec_ipairs.lua @@ -2,7 +2,7 @@ local res_type = world.get_type_by_name("TestResourceWithVariousFields") local res = world.get_resource(res_type) local iterated_vals = {} -for i, v in res.vec_usize:ipairs_clone() do +for i, v in res.vec_usize:ipairs() do assert(i == #iterated_vals + 1, "Index mismatch: expected " .. (#iterated_vals + 1) .. ", got " .. i) iterated_vals[#iterated_vals + 1] = v end diff --git a/assets/tests/iter_clone/vec_pairs.lua b/assets/tests/iter/vec_pairs.lua similarity index 92% rename from assets/tests/iter_clone/vec_pairs.lua rename to assets/tests/iter/vec_pairs.lua index 7c9ad594aa..9bf6f7088a 100644 --- a/assets/tests/iter_clone/vec_pairs.lua +++ b/assets/tests/iter/vec_pairs.lua @@ -2,7 +2,7 @@ local res_type = world.get_type_by_name("TestResourceWithVariousFields") local res = world.get_resource(res_type) local iterated_vals = {} -for v in res.vec_usize:pairs_clone() do +for v in res.vec_usize:pairs() do iterated_vals[#iterated_vals + 1] = v end diff --git a/assets/tests/iter_clone/hashmap.lua b/assets/tests/iter_clone/hashmap.lua deleted file mode 100644 index 39eee0bdf4..0000000000 --- a/assets/tests/iter_clone/hashmap.lua +++ /dev/null @@ -1,21 +0,0 @@ -local res_type = world.get_type_by_name("TestResourceWithVariousFields") -local res = world.get_resource(res_type) - -local map = res.string_map - -local iterator = map:iter_clone() -local count = 0 -local found_keys = {} - -local result = iterator() -while result ~= nil do - local key = result[1] - local value = result[2] - count = count + 1 - found_keys[key] = value - result = iterator() -end - -assert(count == 2, "Expected 2 entries, got " .. count) -assert(found_keys["foo"] == "bar", "Expected foo=>bar") -assert(found_keys["zoo"] == "zed", "Expected zoo=>zed") diff --git a/assets/tests/iter_clone/hashmap.rhai b/assets/tests/iter_clone/hashmap.rhai deleted file mode 100644 index 5575cde9b9..0000000000 --- a/assets/tests/iter_clone/hashmap.rhai +++ /dev/null @@ -1,31 +0,0 @@ -let res_type = world.get_type_by_name.call("TestResourceWithVariousFields"); -let res = world.get_resource.call(res_type); - -let map = res.string_map; - -let iterator = map.iter_clone(); -let count = 0; -let found_keys = #{}; - -loop { - let result = iterator.next(); - - if result == () { - break; - } - - let key = result[0]; - let value = result[1]; - count += 1; - found_keys[key] = value; -} - -if count != 2 { - throw `Expected 2 entries, got ${count}`; -} -if found_keys["foo"] != "bar" { - throw "Expected foo=>bar"; -} -if found_keys["zoo"] != "zed" { - throw "Expected zoo=>zed"; -} diff --git a/assets/tests/iter_clone/vec.lua b/assets/tests/iter_clone/vec.lua deleted file mode 100644 index 9d24a7a9e3..0000000000 --- a/assets/tests/iter_clone/vec.lua +++ /dev/null @@ -1,17 +0,0 @@ -local res_type = world.get_type_by_name("TestResourceWithVariousFields") -local res = world.get_resource(res_type) - -local iterated_vals = {} -local iterator = res.vec_usize:iter_clone() -local result = iterator() -while result ~= nil do - iterated_vals[#iterated_vals + 1] = result - result = iterator() -end - -assert(#iterated_vals == 5, "Length is not 5") -assert(iterated_vals[1] == 1, "First value is not 1") -assert(iterated_vals[2] == 2, "Second value is not 2") -assert(iterated_vals[3] == 3, "Third value is not 3") -assert(iterated_vals[4] == 4, "Fourth value is not 4") -assert(iterated_vals[5] == 5, "Fifth value is not 5") diff --git a/assets/tests/iter_clone/vec.rhai b/assets/tests/iter_clone/vec.rhai deleted file mode 100644 index d634a3a6bf..0000000000 --- a/assets/tests/iter_clone/vec.rhai +++ /dev/null @@ -1,22 +0,0 @@ -let res_type = world.get_type_by_name.call("TestResourceWithVariousFields"); -let res = world.get_resource.call(res_type); - -let iterated_vals = []; -let iterator = res.vec_usize.iter_clone(); - -loop { - let result = iterator.next(); - - if result == () { - break; - } - - iterated_vals.push(result); -} - -assert(iterated_vals.len == 5, "Length is not 5"); -assert(iterated_vals[0] == 1, "First value is not 1"); -assert(iterated_vals[1] == 2, "Second value is not 2"); -assert(iterated_vals[2] == 3, "Third value is not 3"); -assert(iterated_vals[3] == 4, "Fourth value is not 4"); -assert(iterated_vals[4] == 5, "Fifth value is not 5"); diff --git a/crates/bevy_mod_scripting_functions/src/core.rs b/crates/bevy_mod_scripting_functions/src/core.rs index 2777fe9169..cc82bb8ab1 100644 --- a/crates/bevy_mod_scripting_functions/src/core.rs +++ b/crates/bevy_mod_scripting_functions/src/core.rs @@ -606,6 +606,7 @@ impl ReflectReference { } /// Gets and clones the value under the specified key if the underlying type is a map type. + /// Uses `from_reflect` to create a fully reflected clone that can be used with all operations. /// /// Arguments: /// * `ctxt`: The function call context. @@ -613,7 +614,7 @@ impl ReflectReference { /// * `key`: The key to index with. /// Returns: /// * `value`: The value at the key, if the reference is a map. - fn map_get_clone( + fn map_get( ctxt: FunctionCallContext, reference: ReflectReference, key: ScriptValue, @@ -645,53 +646,6 @@ impl ReflectReference { })? } - /// Gets the value reference under the specified key using dynamic reflection if the underlying type is a map type. - /// This method uses `to_dynamic()` which creates a dynamic representation that only implements `PartialReflect`. - /// Note: Values returned by this method cannot be used with asset operations that require full `Reflect` trait. - /// - /// Arguments: - /// * `ctxt`: The function call context. - /// * `reference`: The reference to index into. - /// * `key`: The key to index with. - /// Returns: - /// * `value`: The dynamic value at the key, if the reference is a map. - fn map_get( - ctxt: FunctionCallContext, - reference: ReflectReference, - key: ScriptValue, - ) -> Result, InteropError> { - profiling::function_scope!("map_get_dynamic"); - let world = ctxt.world()?; - let key = >::from_script_ref( - reference.key_type_id(world.clone())?.ok_or_else(|| { - InteropError::unsupported_operation( - reference.tail_type_id(world.clone()).unwrap_or_default(), - Some(Box::new(key.clone())), - "Could not get key type id. Are you trying to index into a type that's not a map?".to_owned(), - ) - })?, - key, - world.clone(), - )?; - reference.with_reflect(world.clone(), |s| { - s.try_map_get(key.as_ref()).and_then(|maybe_value| { - maybe_value - .map(|value| { - let reference = { - let allocator = world.allocator(); - let mut allocator_guard = allocator.write(); - ReflectReference::new_allocated_boxed_parial_reflect( - value.to_dynamic(), - &mut *allocator_guard, - )? - }; - ReflectReference::into_script_ref(reference, world) - }) - .transpose() - }) - })? - } - /// Pushes the value into the reference, if the reference is an appropriate container type. /// /// Arguments: @@ -859,67 +813,8 @@ impl ReflectReference { } /// Iterates over the reference, if the reference is an appropriate container type. + /// Uses `from_reflect` to create fully reflected clones that can be used with all operations. /// - /// Returns an "next" iterator function. - /// - /// The iterator function should be called until it returns `nil` to signal the end of the iteration. - /// - /// Arguments: - /// * `ctxt`: The function call context. - /// * `reference`: The reference to iterate over. - /// Returns: - /// * `iter`: The iterator function. - fn iter( - ctxt: FunctionCallContext, - reference: ReflectReference, - ) -> Result { - profiling::function_scope!("iter"); - let world = ctxt.world()?; - let mut len = reference.len(world.clone())?.unwrap_or_default(); - // Check if this is a Map type - let is_map = reference.with_reflect(world.clone(), |r| { - matches!(r.reflect_ref(), ReflectRef::Map(_)) - })?; - - if is_map { - // Use special map iterator that doesn't rely on path resolution - let mut map_iter = reference.into_map_iter(); - let iter_function = move || { - if len == 0 { - return Ok(ScriptValue::Unit); - } - len -= 1; - let world = ThreadWorldContainer.try_get_world()?; - match map_iter.next_ref(world.clone())? { - Some((key_ref, value_ref)) => { - // Return both key and value as a List for Lua's pairs() to unpack - let key_value = ReflectReference::into_script_ref(key_ref, world.clone())?; - let value_value = ReflectReference::into_script_ref(value_ref, world)?; - Ok(ScriptValue::List(vec![key_value, value_value])) - } - None => Ok(ScriptValue::Unit), - } - }; - Ok(iter_function.into_dynamic_script_function_mut()) - } else { - let mut infinite_iter = reference.into_iter_infinite(); - let iter_function = move || { - if len == 0 { - return Ok(ScriptValue::Unit); - } - len -= 1; - let world = ThreadWorldContainer.try_get_world()?; - let (next_ref, _) = infinite_iter.next_ref(); - // we stop once the reflection path is invalid - ReflectReference::into_script_ref(next_ref, world) - }; - Ok(iter_function.into_dynamic_script_function_mut()) - } - } - - /// Iterates over the reference with cloned values, if the reference is an appropriate container type. - /// - /// This method clones the actual values using `from_reflect` instead of creating dynamic references. /// Returns an "next" iterator function that returns fully reflected values (Box). /// /// The iterator function should be called until it returns `nil` to signal the end of the iteration. @@ -929,11 +824,11 @@ impl ReflectReference { /// * `reference`: The reference to iterate over. /// Returns: /// * `iter`: The iterator function that returns cloned values. - fn iter_clone( + fn iter( ctxt: FunctionCallContext, reference: ReflectReference, ) -> Result { - profiling::function_scope!("iter_clone"); + profiling::function_scope!("iter"); let world = ctxt.world()?; let mut len = reference.len(world.clone())?.unwrap_or_default(); // Check if this is a Map type diff --git a/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs b/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs index 7292fb197e..0b3161425f 100644 --- a/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs +++ b/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs @@ -348,14 +348,14 @@ impl UserData for LuaReflectReference { } }); - m.add_method("pairs_clone", |lua, s: &LuaReflectReference, _args: ()| { - profiling::function_scope!("pairs_clone"); + m.add_method("pairs", |lua, s: &LuaReflectReference, _args: ()| { + profiling::function_scope!("pairs"); let world = ThreadWorldContainer .try_get_world() .map_err(IntoMluaError::to_lua_error)?; let iter_func = world - .lookup_function([TypeId::of::()], "iter_clone") + .lookup_function([TypeId::of::()], "iter") .map_err(|f| { InteropError::missing_function(f, TypeId::of::().into()) }) @@ -388,18 +388,18 @@ impl UserData for LuaReflectReference { } }) } - _ => Err(mlua::Error::RuntimeError("iter_clone function did not return a FunctionMut".to_string())) + _ => Err(mlua::Error::RuntimeError("iter function did not return a FunctionMut".to_string())) } }); - m.add_method("ipairs_clone", |lua, s: &LuaReflectReference, _args: ()| { - profiling::function_scope!("ipairs_clone"); + m.add_method("ipairs", |lua, s: &LuaReflectReference, _args: ()| { + profiling::function_scope!("ipairs"); let world = ThreadWorldContainer .try_get_world() .map_err(IntoMluaError::to_lua_error)?; let iter_func = world - .lookup_function([TypeId::of::()], "iter_clone") + .lookup_function([TypeId::of::()], "iter") .map_err(|f| { InteropError::missing_function(f, TypeId::of::().into()) }) @@ -432,7 +432,7 @@ impl UserData for LuaReflectReference { } }) } - _ => Err(mlua::Error::RuntimeError("iter_clone function did not return a FunctionMut".to_string())) + _ => Err(mlua::Error::RuntimeError("iter function did not return a FunctionMut".to_string())) } }); From da7a771da7e7696dd6274e9fed6450f563e8a1c2 Mon Sep 17 00:00:00 2001 From: aggyomfg Date: Sun, 5 Oct 2025 23:02:59 +0400 Subject: [PATCH 09/16] get rid of leaked abstraction 2 --- assets/tests/iter/vec.lua | 10 ++--- assets/tests/iter/vec.rhai | 11 +---- .../src/reference.rs | 42 +----------------- .../src/bindings/reference.rs | 44 ------------------- .../src/bindings/reference.rs | 23 ---------- 5 files changed, 6 insertions(+), 124 deletions(-) diff --git a/assets/tests/iter/vec.lua b/assets/tests/iter/vec.lua index be031a7eb2..e1da679d5a 100644 --- a/assets/tests/iter/vec.lua +++ b/assets/tests/iter/vec.lua @@ -1,14 +1,10 @@ local res_type = world.get_type_by_name("TestResourceWithVariousFields") local res = world.get_resource(res_type) -local iterated_vals = {} -local iterator = res.vec_usize:iter() -local result = iterator() -while result ~= nil do - iterated_vals[#iterated_vals + 1] = result - result = iterator() +iterated_vals = {} +for v in pairs(res.vec_usize) do + iterated_vals[#iterated_vals + 1] = v end - assert(#iterated_vals == 5, "Length is not 5") assert(iterated_vals[1] == 1, "First value is not 1") assert(iterated_vals[2] == 2, "Second value is not 2") diff --git a/assets/tests/iter/vec.rhai b/assets/tests/iter/vec.rhai index f5d9233059..accbcd587a 100644 --- a/assets/tests/iter/vec.rhai +++ b/assets/tests/iter/vec.rhai @@ -2,16 +2,9 @@ let res_type = world.get_type_by_name.call("TestResourceWithVariousFields"); let res = world.get_resource.call(res_type); let iterated_vals = []; -let iterator = res.vec_usize.iter(); -loop { - let result = iterator.next(); - - if result == () { - break; - } - - iterated_vals.push(result); +for v in res.vec_usize { + iterated_vals.push(v); } assert(iterated_vals.len == 5, "Length is not 5"); diff --git a/crates/bevy_mod_scripting_bindings/src/reference.rs b/crates/bevy_mod_scripting_bindings/src/reference.rs index cf4444e3b2..bddf1f1896 100644 --- a/crates/bevy_mod_scripting_bindings/src/reference.rs +++ b/crates/bevy_mod_scripting_bindings/src/reference.rs @@ -989,49 +989,9 @@ pub struct ReflectMapRefIter { #[profiling::all_functions] impl ReflectMapRefIter { - /// Returns the next map entry as a (key, value) tuple. - /// Returns Ok(None) when there are no more entries. - pub fn next_ref(&mut self, world: WorldGuard) -> Result, InteropError> { - let idx = self.index; - self.index += 1; - - // Access the map and get the entry at index - self.base.with_reflect(world.clone(), |reflect| { - match reflect.reflect_ref() { - ReflectRef::Map(map) => { - if let Some((key, value)) = map.get_at(idx) { - let allocator = world.allocator(); - let mut allocator_guard = allocator.write(); - - let key_ref = ReflectReference::new_allocated_boxed_parial_reflect( - key.to_dynamic(), - &mut *allocator_guard - )?; - - let value_ref = ReflectReference::new_allocated_boxed_parial_reflect( - value.to_dynamic(), - &mut *allocator_guard - )?; - - drop(allocator_guard); - Ok(Some((key_ref, value_ref))) - } else { - Ok(None) - } - } - _ => Err(InteropError::unsupported_operation( - reflect.get_represented_type_info().map(|ti| ti.type_id()), - None, - "map iteration on non-map type".to_owned(), - )) - } - })? - } - /// Returns the next map entry as a (key, value) tuple, cloning the values. + /// Uses `from_reflect` to create fully reflected clones that can be used with all operations. /// Returns Ok(None) when there are no more entries. - /// This uses `from_reflect` to clone the actual values instead of creating dynamic references. - /// Returns fully reflected values (Box) like `map_get_clone` does. pub fn next_cloned(&mut self, world: WorldGuard) -> Result, InteropError> { let idx = self.index; self.index += 1; diff --git a/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs b/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs index 0b3161425f..6183d38bd2 100644 --- a/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs +++ b/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs @@ -348,50 +348,6 @@ impl UserData for LuaReflectReference { } }); - m.add_method("pairs", |lua, s: &LuaReflectReference, _args: ()| { - profiling::function_scope!("pairs"); - let world = ThreadWorldContainer - .try_get_world() - .map_err(IntoMluaError::to_lua_error)?; - - let iter_func = world - .lookup_function([TypeId::of::()], "iter") - .map_err(|f| { - InteropError::missing_function(f, TypeId::of::().into()) - }) - .map_err(IntoMluaError::to_lua_error)?; - - let result = iter_func - .call(vec![ScriptValue::Reference(s.clone().into())], LUA_CALLER_CONTEXT) - .map_err(IntoMluaError::to_lua_error)?; - - match result { - ScriptValue::FunctionMut(func) => { - lua.create_function_mut(move |lua, _args: ()| { - let result = func - .call(vec![], LUA_CALLER_CONTEXT) - .map_err(IntoMluaError::to_lua_error)?; - - // If the result is a List with 2 elements, unpack it into multiple return values - match result { - ScriptValue::List(ref items) if items.len() == 2 => { - // Return as tuple (key, value) which Lua unpacks automatically - let key = LuaScriptValue(items[0].clone()).into_lua(lua)?; - let value = LuaScriptValue(items[1].clone()).into_lua(lua)?; - Ok((key, value)) - } - _ => { - // Single value or Unit - return as-is - let val = LuaScriptValue(result).into_lua(lua)?; - Ok((val, mlua::Value::Nil)) - } - } - }) - } - _ => Err(mlua::Error::RuntimeError("iter function did not return a FunctionMut".to_string())) - } - }); - m.add_method("ipairs", |lua, s: &LuaReflectReference, _args: ()| { profiling::function_scope!("ipairs"); let world = ThreadWorldContainer diff --git a/crates/languages/bevy_mod_scripting_rhai/src/bindings/reference.rs b/crates/languages/bevy_mod_scripting_rhai/src/bindings/reference.rs index 3e9816e863..ad463099a7 100644 --- a/crates/languages/bevy_mod_scripting_rhai/src/bindings/reference.rs +++ b/crates/languages/bevy_mod_scripting_rhai/src/bindings/reference.rs @@ -621,29 +621,6 @@ impl CustomType for RhaiReflectReference { } _ => Err(InteropError::invariant("iter function did not return a FunctionMut").into_rhai_error()) } - }) - .with_fn("iter_clone", |self_: Self| { - let world = ThreadWorldContainer - .try_get_world() - .map_err(IntoRhaiError::into_rhai_error)?; - - let iter_func = world - .lookup_function([TypeId::of::()], "iter_clone") - .map_err(|f| { - InteropError::missing_function(f, TypeId::of::().into()) - }) - .map_err(IntoRhaiError::into_rhai_error)?; - - let result = iter_func - .call(vec![ScriptValue::Reference(self_.0)], RHAI_CALLER_CONTEXT) - .map_err(IntoRhaiError::into_rhai_error)?; - - match result { - ScriptValue::FunctionMut(iter_fn) => { - Ok(Dynamic::from(RhaiIterator { iter_fn })) - } - _ => Err(InteropError::invariant("iter_clone function did not return a FunctionMut").into_rhai_error()) - } }); } } From bb789b951688dd6c932bf45593e464aa094241e4 Mon Sep 17 00:00:00 2001 From: aggyomfg Date: Tue, 7 Oct 2025 20:56:52 +0400 Subject: [PATCH 10/16] use default_get instead of map_get --- assets/tests/iter/hashmap_ipairs.lua | 27 -------- assets/tests/iter/hashmap_pairs.lua | 2 +- assets/tests/iter/vec_ipairs.lua | 15 ----- assets/tests/iter/vec_pairs.lua | 2 +- .../tests/map_get/can_get_hashmap_value.lua | 2 +- ...e copy.rhai => can_get_hashmap_value.rhai} | 2 +- .../src/function/magic_functions.rs | 62 ++++++++++++++++--- .../bevy_mod_scripting_functions/src/core.rs | 41 ------------ .../src/bindings/reference.rs | 44 ------------- 9 files changed, 56 insertions(+), 141 deletions(-) delete mode 100644 assets/tests/iter/hashmap_ipairs.lua delete mode 100644 assets/tests/iter/vec_ipairs.lua rename assets/tests/map_get/{can_get_hashmap_value copy.rhai => can_get_hashmap_value.rhai} (51%) diff --git a/assets/tests/iter/hashmap_ipairs.lua b/assets/tests/iter/hashmap_ipairs.lua deleted file mode 100644 index acc41dcb68..0000000000 --- a/assets/tests/iter/hashmap_ipairs.lua +++ /dev/null @@ -1,27 +0,0 @@ -local res_type = world.get_type_by_name("TestResourceWithVariousFields") -local res = world.get_resource(res_type) - -local map = res.string_map - -local count = 0 -local found_keys = {} - --- ipairs on a map returns (index, [key, value]) where value is a list -for i, entry in map:ipairs() do - assert(i == count + 1, "Index should be sequential: expected " .. (count + 1) .. ", got " .. i) - - -- entry should be a list with [key, value] - assert(entry ~= nil, "Entry should not be nil") - assert(entry[1] ~= nil, "Key should not be nil") - assert(entry[2] ~= nil, "Value should not be nil") - - local key = entry[1] - local value = entry[2] - - count = count + 1 - found_keys[key] = value -end - -assert(count == 2, "Expected 2 entries, got " .. count) -assert(found_keys["foo"] == "bar", "Expected foo=>bar, got " .. tostring(found_keys["foo"])) -assert(found_keys["zoo"] == "zed", "Expected zoo=>zed, got " .. tostring(found_keys["zoo"])) diff --git a/assets/tests/iter/hashmap_pairs.lua b/assets/tests/iter/hashmap_pairs.lua index 8735ac75f6..c27cd113c5 100644 --- a/assets/tests/iter/hashmap_pairs.lua +++ b/assets/tests/iter/hashmap_pairs.lua @@ -6,7 +6,7 @@ local map = res.string_map local count = 0 local found_keys = {} -for key, value in map:pairs() do +for key, value in pairs(map) do count = count + 1 found_keys[key] = value end diff --git a/assets/tests/iter/vec_ipairs.lua b/assets/tests/iter/vec_ipairs.lua deleted file mode 100644 index 9c48877499..0000000000 --- a/assets/tests/iter/vec_ipairs.lua +++ /dev/null @@ -1,15 +0,0 @@ -local res_type = world.get_type_by_name("TestResourceWithVariousFields") -local res = world.get_resource(res_type) - -local iterated_vals = {} -for i, v in res.vec_usize:ipairs() do - assert(i == #iterated_vals + 1, "Index mismatch: expected " .. (#iterated_vals + 1) .. ", got " .. i) - iterated_vals[#iterated_vals + 1] = v -end - -assert(#iterated_vals == 5, "Length is not 5") -assert(iterated_vals[1] == 1, "First value is not 1") -assert(iterated_vals[2] == 2, "Second value is not 2") -assert(iterated_vals[3] == 3, "Third value is not 3") -assert(iterated_vals[4] == 4, "Fourth value is not 4") -assert(iterated_vals[5] == 5, "Fifth value is not 5") diff --git a/assets/tests/iter/vec_pairs.lua b/assets/tests/iter/vec_pairs.lua index 9bf6f7088a..2ec25ec144 100644 --- a/assets/tests/iter/vec_pairs.lua +++ b/assets/tests/iter/vec_pairs.lua @@ -2,7 +2,7 @@ local res_type = world.get_type_by_name("TestResourceWithVariousFields") local res = world.get_resource(res_type) local iterated_vals = {} -for v in res.vec_usize:pairs() do +for v in pairs(res.vec_usize) do iterated_vals[#iterated_vals + 1] = v end diff --git a/assets/tests/map_get/can_get_hashmap_value.lua b/assets/tests/map_get/can_get_hashmap_value.lua index 2bd4a4459e..972cff850b 100644 --- a/assets/tests/map_get/can_get_hashmap_value.lua +++ b/assets/tests/map_get/can_get_hashmap_value.lua @@ -1,4 +1,4 @@ local Resource = world.get_type_by_name("TestResourceWithVariousFields") local resource = world.get_resource(Resource) -assert(resource.string_map:map_get("foo") == "bar", "Expected bar, got " .. resource.string_map:map_get("foo")) \ No newline at end of file +assert(resource.string_map["foo"] == "bar", "Expected bar, got " .. resource.string_map["foo"]) diff --git a/assets/tests/map_get/can_get_hashmap_value copy.rhai b/assets/tests/map_get/can_get_hashmap_value.rhai similarity index 51% rename from assets/tests/map_get/can_get_hashmap_value copy.rhai rename to assets/tests/map_get/can_get_hashmap_value.rhai index 33f2dc1525..4057ecf0a6 100644 --- a/assets/tests/map_get/can_get_hashmap_value copy.rhai +++ b/assets/tests/map_get/can_get_hashmap_value.rhai @@ -1,4 +1,4 @@ let Resource = world.get_type_by_name.call("TestResourceWithVariousFields"); let resource = world.get_resource.call(Resource); -assert(resource.string_map.map_get.call("foo") == "bar", "Expected bar, got " + resource.string_map.map_get.call("foo")); \ No newline at end of file +assert(resource.string_map["foo"] == "bar", "Expected bar, got " + resource.string_map["foo"]); \ No newline at end of file diff --git a/crates/bevy_mod_scripting_bindings/src/function/magic_functions.rs b/crates/bevy_mod_scripting_bindings/src/function/magic_functions.rs index 54387fe8e0..722b97ccf0 100644 --- a/crates/bevy_mod_scripting_bindings/src/function/magic_functions.rs +++ b/crates/bevy_mod_scripting_bindings/src/function/magic_functions.rs @@ -1,9 +1,9 @@ //! All the switchable special functions used by language implementors use super::{FromScriptRef, FunctionCallContext, IntoScriptRef}; -use crate::{ReflectReference, ReflectionPathExt, ScriptValue, error::InteropError}; +use crate::{error::InteropError, PartialReflectExt, ReflectReference, ReflectionPathExt, ScriptValue}; use bevy_mod_scripting_derive::DebugWithTypeInfo; use bevy_mod_scripting_display::OrFakeId; -use bevy_reflect::{ParsedPath, PartialReflect}; +use bevy_reflect::{ParsedPath, PartialReflect, ReflectRef}; /// A list of magic methods, these only have one replacable implementation, and apply to all `ReflectReferences`. /// It's up to the language implementer to call these in the right order (after any type specific overrides). @@ -51,8 +51,6 @@ impl MagicFunctions { /// Indexes into the given reference and if the nested type is a reference type, returns a deeper reference, otherwise /// returns the concrete value. /// - /// Does not support map types at the moment, for maps see `map_get` - /// /// Arguments: /// * `ctxt`: The function call context. /// * `reference`: The reference to index into. @@ -65,13 +63,57 @@ impl MagicFunctions { mut reference: ReflectReference, key: ScriptValue, ) -> Result { - let mut path: ParsedPath = key.try_into()?; - if ctxt.convert_to_0_indexed() { - path.convert_to_0_indexed(); - } - reference.index_path(path); let world = ctxt.world()?; - ReflectReference::into_script_ref(reference, world) + + // Check if the reference is a map type + let is_map = reference.with_reflect(world.clone(), |r| { + matches!(r.reflect_ref(), ReflectRef::Map(_)) + })?; + + if is_map { + // Handle map indexing specially - need to get the key type and convert the script value + let key = >::from_script_ref( + reference.key_type_id(world.clone())?.ok_or_else(|| { + InteropError::unsupported_operation( + reference.tail_type_id(world.clone()).unwrap_or_default(), + Some(Box::new(key.clone())), + "Could not get key type id. Are you trying to index into a type that's not a map?".to_owned(), + ) + })?, + key, + world.clone(), + )?; + + reference.with_reflect(world.clone(), |s| match s.try_map_get(key.as_ref())? { + Some(value) => { + let reference = { + let allocator = world.allocator(); + let mut allocator = allocator.write(); + let owned_value = ::from_reflect(value, world.clone())?; + ReflectReference::new_allocated_boxed(owned_value, &mut allocator) + }; + ReflectReference::into_script_ref(reference, world) + } + None => { + // Return None option if key doesn't exist + let none_option: Option<()> = None; + let reference = { + let allocator = world.allocator(); + let mut allocator = allocator.write(); + ReflectReference::new_allocated_boxed(Box::new(none_option), &mut allocator) + }; + ReflectReference::into_script_ref(reference, world) + } + })? + } else { + // Handle path-based indexing for non-map types + let mut path: ParsedPath = key.try_into()?; + if ctxt.convert_to_0_indexed() { + path.convert_to_0_indexed(); + } + reference.index_path(path); + ReflectReference::into_script_ref(reference, world) + } } /// Sets the value under the specified path on the underlying value. diff --git a/crates/bevy_mod_scripting_functions/src/core.rs b/crates/bevy_mod_scripting_functions/src/core.rs index cc82bb8ab1..8650b510db 100644 --- a/crates/bevy_mod_scripting_functions/src/core.rs +++ b/crates/bevy_mod_scripting_functions/src/core.rs @@ -605,47 +605,6 @@ impl ReflectReference { Ok(format!("{reference:#?}")) } - /// Gets and clones the value under the specified key if the underlying type is a map type. - /// Uses `from_reflect` to create a fully reflected clone that can be used with all operations. - /// - /// Arguments: - /// * `ctxt`: The function call context. - /// * `reference`: The reference to index into. - /// * `key`: The key to index with. - /// Returns: - /// * `value`: The value at the key, if the reference is a map. - fn map_get( - ctxt: FunctionCallContext, - reference: ReflectReference, - key: ScriptValue, - ) -> Result, InteropError> { - profiling::function_scope!("map_get"); - let world = ctxt.world()?; - let key = >::from_script_ref( - reference.key_type_id(world.clone())?.ok_or_else(|| { - InteropError::unsupported_operation( - reference.tail_type_id(world.clone()).unwrap_or_default(), - Some(Box::new(key.clone())), - "Could not get key type id. Are you trying to index into a type that's not a map?".to_owned(), - ) - })?, - key, - world.clone(), - )?; - reference.with_reflect(world.clone(), |s| match s.try_map_get(key.as_ref())? { - Some(value) => { - let reference = { - let allocator = world.allocator(); - let mut allocator = allocator.write(); - let owned_value = ::from_reflect(value, world.clone())?; - ReflectReference::new_allocated_boxed(owned_value, &mut allocator) - }; - Ok(Some(ReflectReference::into_script_ref(reference, world)?)) - } - None => Ok(None), - })? - } - /// Pushes the value into the reference, if the reference is an appropriate container type. /// /// Arguments: diff --git a/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs b/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs index 6183d38bd2..b8fc06e43a 100644 --- a/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs +++ b/crates/languages/bevy_mod_scripting_lua/src/bindings/reference.rs @@ -348,50 +348,6 @@ impl UserData for LuaReflectReference { } }); - m.add_method("ipairs", |lua, s: &LuaReflectReference, _args: ()| { - profiling::function_scope!("ipairs"); - let world = ThreadWorldContainer - .try_get_world() - .map_err(IntoMluaError::to_lua_error)?; - - let iter_func = world - .lookup_function([TypeId::of::()], "iter") - .map_err(|f| { - InteropError::missing_function(f, TypeId::of::().into()) - }) - .map_err(IntoMluaError::to_lua_error)?; - - let result = iter_func - .call(vec![ScriptValue::Reference(s.clone().into())], LUA_CALLER_CONTEXT) - .map_err(IntoMluaError::to_lua_error)?; - - match result { - ScriptValue::FunctionMut(func) => { - let mut index = 0i64; - lua.create_function_mut(move |lua, _args: ()| { - let result = func - .call(vec![], LUA_CALLER_CONTEXT) - .map_err(IntoMluaError::to_lua_error)?; - - match result { - ScriptValue::Unit => { - // End of iteration - Ok((mlua::Value::Nil, mlua::Value::Nil)) - } - _ => { - // Return (index, value) tuple for ipairs - index += 1; - let idx = mlua::Value::Integer(index); - let val = LuaScriptValue(result).into_lua(lua)?; - Ok((idx, val)) - } - } - }) - } - _ => Err(mlua::Error::RuntimeError("iter function did not return a FunctionMut".to_string())) - } - }); - m.add_meta_function(MetaMethod::ToString, |_, self_: LuaReflectReference| { profiling::function_scope!("MetaMethod::ToString"); let world = ThreadWorldContainer From 9a8a94316d9bd6abcd6d343147c9447da3076d00 Mon Sep 17 00:00:00 2001 From: aggyomfg Date: Tue, 7 Oct 2025 22:06:16 +0400 Subject: [PATCH 11/16] set maps via default_set --- .../can_get_hashmap_value.lua | 0 .../can_get_hashmap_value.rhai | 0 .../tests/hashmap/can_set_hashmap_value.lua | 10 +++ .../tests/hashmap/can_set_hashmap_value.rhai | 10 +++ .../src/function/magic_functions.rs | 74 +++++++++++++++---- 5 files changed, 79 insertions(+), 15 deletions(-) rename assets/tests/{map_get => hashmap}/can_get_hashmap_value.lua (100%) rename assets/tests/{map_get => hashmap}/can_get_hashmap_value.rhai (100%) create mode 100644 assets/tests/hashmap/can_set_hashmap_value.lua create mode 100644 assets/tests/hashmap/can_set_hashmap_value.rhai diff --git a/assets/tests/map_get/can_get_hashmap_value.lua b/assets/tests/hashmap/can_get_hashmap_value.lua similarity index 100% rename from assets/tests/map_get/can_get_hashmap_value.lua rename to assets/tests/hashmap/can_get_hashmap_value.lua diff --git a/assets/tests/map_get/can_get_hashmap_value.rhai b/assets/tests/hashmap/can_get_hashmap_value.rhai similarity index 100% rename from assets/tests/map_get/can_get_hashmap_value.rhai rename to assets/tests/hashmap/can_get_hashmap_value.rhai diff --git a/assets/tests/hashmap/can_set_hashmap_value.lua b/assets/tests/hashmap/can_set_hashmap_value.lua new file mode 100644 index 0000000000..f0d0943d64 --- /dev/null +++ b/assets/tests/hashmap/can_set_hashmap_value.lua @@ -0,0 +1,10 @@ +local Resource = world.get_type_by_name("TestResourceWithVariousFields") +local resource = world.get_resource(Resource) + +resource.string_map["foo"] = "updated" +resource.string_map["new_key"] = "new_value" + +local resource_changed = world.get_resource(Resource) +assert(resource_changed.string_map["foo"] == "updated", "Expected updated, got " .. resource_changed.string_map["foo"]) +assert(resource_changed.string_map["new_key"] == "new_value", + "Expected new_value, got " .. resource_changed.string_map["new_key"]) diff --git a/assets/tests/hashmap/can_set_hashmap_value.rhai b/assets/tests/hashmap/can_set_hashmap_value.rhai new file mode 100644 index 0000000000..56381c6149 --- /dev/null +++ b/assets/tests/hashmap/can_set_hashmap_value.rhai @@ -0,0 +1,10 @@ +let Resource = world.get_type_by_name.call("TestResourceWithVariousFields"); +let resource = world.get_resource.call(Resource); + + +resource.string_map["foo"] = "updated"; +resource.string_map["new_key"] = "new_value"; + +let resource_changed = world.get_resource.call(Resource); +assert(resource_changed.string_map["foo"] == "updated", "Expected updated, got " + resource_changed.string_map["foo"]); +assert(resource_changed.string_map["new_key"] == "new_value", "Expected new_value, got " + resource_changed.string_map["new_key"]); diff --git a/crates/bevy_mod_scripting_bindings/src/function/magic_functions.rs b/crates/bevy_mod_scripting_bindings/src/function/magic_functions.rs index 722b97ccf0..64dec69eca 100644 --- a/crates/bevy_mod_scripting_bindings/src/function/magic_functions.rs +++ b/crates/bevy_mod_scripting_bindings/src/function/magic_functions.rs @@ -133,22 +133,66 @@ impl MagicFunctions { value: ScriptValue, ) -> Result<(), InteropError> { let world = ctxt.world()?; - let mut path: ParsedPath = key.try_into()?; - if ctxt.convert_to_0_indexed() { - path.convert_to_0_indexed(); + + // Check if the reference is a map type + let is_map = reference.with_reflect(world.clone(), |r| { + matches!(r.reflect_ref(), ReflectRef::Map(_)) + })?; + + if is_map { + // Handle map setting specially - need to get the key type and convert the script value + let key = >::from_script_ref( + reference.key_type_id(world.clone())?.ok_or_else(|| { + InteropError::unsupported_operation( + reference.tail_type_id(world.clone()).unwrap_or_default(), + Some(Box::new(key.clone())), + "Could not get key type id. Are you trying to index into a type that's not a map?".to_owned(), + ) + })?, + key, + world.clone(), + )?; + + // Get the value type for the map and convert the script value + let value_type_id = reference.element_type_id(world.clone())?.ok_or_else(|| { + InteropError::unsupported_operation( + reference.tail_type_id(world.clone()).unwrap_or_default(), + Some(Box::new(value.clone())), + "Could not get value type id. Are you trying to set a value in a type that's not a map?".to_owned(), + ) + })?; + + let value = >::from_script_ref( + value_type_id, + value, + world.clone(), + )?; + + reference.with_reflect_mut(world, |s| { + s.try_insert_boxed(key, value) + })??; + } else { + let mut path: ParsedPath = key.try_into()?; + if ctxt.convert_to_0_indexed() { + path.convert_to_0_indexed(); + } + reference.index_path(path); + + let target_type_id = reference.with_reflect(world.clone(), |r| { + r.get_represented_type_info() + .map(|i| i.type_id()) + .or_fake_id() + })?; + + let other = >::from_script_ref(target_type_id, value, world.clone())?; + + reference.with_reflect_mut(world, |r| { + r.try_apply(other.as_partial_reflect()) + .map_err(InteropError::reflect_apply_error) + })??; } - reference.index_path(path); - reference.with_reflect_mut(world.clone(), |r| { - let target_type_id = r - .get_represented_type_info() - .map(|i| i.type_id()) - .or_fake_id(); - let other = - >::from_script_ref(target_type_id, value, world.clone())?; - r.try_apply(other.as_partial_reflect()) - .map_err(InteropError::reflect_apply_error)?; - Ok::<_, InteropError>(()) - })? + + Ok(()) } } From af9eff7967e37e0d54e96aa06e5d1e1dcfd0d63e Mon Sep 17 00:00:00 2001 From: aggyomfg Date: Tue, 7 Oct 2025 22:26:37 +0400 Subject: [PATCH 12/16] rm insert binding --- assets/tests/insert/vec.lua | 4 +- .../bevy_mod_scripting_functions/src/core.rs | 45 ------------------- 2 files changed, 2 insertions(+), 47 deletions(-) diff --git a/assets/tests/insert/vec.lua b/assets/tests/insert/vec.lua index abd159b9dc..e901ea69a8 100644 --- a/assets/tests/insert/vec.lua +++ b/assets/tests/insert/vec.lua @@ -1,6 +1,6 @@ local res_type = world.get_type_by_name("TestResourceWithVariousFields") local res = world.get_resource(res_type) -res.vec_usize:insert(2, 42) +res.vec_usize[2] = 42 -assert(res.vec_usize[2] == 42, "insert did not work") \ No newline at end of file +assert(res.vec_usize[2] == 42, "insert did not work") diff --git a/crates/bevy_mod_scripting_functions/src/core.rs b/crates/bevy_mod_scripting_functions/src/core.rs index 8650b510db..ba2ee35e75 100644 --- a/crates/bevy_mod_scripting_functions/src/core.rs +++ b/crates/bevy_mod_scripting_functions/src/core.rs @@ -655,51 +655,6 @@ impl ReflectReference { ReflectReference::into_script_ref(reference, world) } - /// Inserts the value into the reference at the specified index, if the reference is an appropriate container type. - /// - /// Arguments: - /// * `ctxt`: The function call context. - /// * `reference`: The reference to insert the value into. - /// * `key`: The index to insert the value at. - /// * `value`: The value to insert. - /// Returns: - /// * `result`: Nothing if the value was inserted successfully. - fn insert( - ctxt: FunctionCallContext, - reference: ReflectReference, - key: ScriptValue, - value: ScriptValue, - ) -> Result<(), InteropError> { - profiling::function_scope!("insert"); - let world = ctxt.world()?; - let key_type_id = reference.key_type_id(world.clone())?.ok_or_else(|| { - InteropError::unsupported_operation( - reference.tail_type_id(world.clone()).unwrap_or_default(), - Some(Box::new(key.clone())), - "Could not get key type id. Are you trying to insert elements into a type that's not a map?".to_owned(), - ) - })?; - - let mut key = >::from_script_ref(key_type_id, key, world.clone())?; - - if ctxt.convert_to_0_indexed() { - key.convert_to_0_indexed_key(); - } - - let value_type_id = reference.element_type_id(world.clone())?.ok_or_else(|| { - InteropError::unsupported_operation( - reference.tail_type_id(world.clone()).unwrap_or_default(), - Some(Box::new(value.clone())), - "Could not get element type id. Are you trying to insert elements into a type that's not a map?".to_owned(), - ) - })?; - - let value = - >::from_script_ref(value_type_id, value, world.clone())?; - - reference.with_reflect_mut(world, |s| s.try_insert_boxed(key, value))? - } - /// Clears the container, if the reference is an appropriate container type. /// Arguments: /// * `ctxt`: The function call context. From 64c0221d8d5508258c11d42a5d8d7ac9c634dd09 Mon Sep 17 00:00:00 2001 From: aggyomfg Date: Tue, 7 Oct 2025 22:29:16 +0400 Subject: [PATCH 13/16] fix rhai insert test --- assets/tests/insert/vec.rhai | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/tests/insert/vec.rhai b/assets/tests/insert/vec.rhai index d5f250418a..4413d1dbb6 100644 --- a/assets/tests/insert/vec.rhai +++ b/assets/tests/insert/vec.rhai @@ -1,6 +1,6 @@ let res_type = world.get_type_by_name.call("TestResourceWithVariousFields"); let res = world.get_resource.call(res_type); -res.vec_usize.insert.call(2, 42); +res.vec_usize[2] = 42; assert(res.vec_usize[2] == 42, "insert did not work"); \ No newline at end of file From 75bded4842ad761eb388e92d408727612e22baee Mon Sep 17 00:00:00 2001 From: aggyomfg Date: Wed, 8 Oct 2025 18:52:00 +0400 Subject: [PATCH 14/16] hashset support and better magic functions --- .../hashset/can_check_hashset_contains.lua | 14 + .../hashset/can_check_hashset_contains.rhai | 14 + .../tests/hashset/can_insert_into_hashset.lua | 19 ++ .../hashset/can_insert_into_hashset.rhai | 19 ++ .../src/function/magic_functions.rs | 118 +------- .../src/reference.rs | 260 ++++++++++++++---- .../src/reflection_extensions.rs | 33 +++ .../test_utils/src/test_data.rs | 7 +- 8 files changed, 321 insertions(+), 163 deletions(-) create mode 100644 assets/tests/hashset/can_check_hashset_contains.lua create mode 100644 assets/tests/hashset/can_check_hashset_contains.rhai create mode 100644 assets/tests/hashset/can_insert_into_hashset.lua create mode 100644 assets/tests/hashset/can_insert_into_hashset.rhai diff --git a/assets/tests/hashset/can_check_hashset_contains.lua b/assets/tests/hashset/can_check_hashset_contains.lua new file mode 100644 index 0000000000..683bde92cc --- /dev/null +++ b/assets/tests/hashset/can_check_hashset_contains.lua @@ -0,0 +1,14 @@ +local Resource = world.get_type_by_name("TestResourceWithVariousFields") +local resource = world.get_resource(Resource) + +-- Check if "apple" exists in the set (should return Some(apple)) +local result = resource.string_set["apple"] +assert(result ~= nil, "Expected to find 'apple' in set") + +-- Check if "banana" exists in the set (should return Some(banana)) +local result2 = resource.string_set["banana"] +assert(result2 ~= nil, "Expected to find 'banana' in set") + +-- Check if "nonexistent" doesn't exist in the set (should return None) +local result3 = resource.string_set["nonexistent"] +assert(result3 == nil, "Expected not to find 'nonexistent' in set") diff --git a/assets/tests/hashset/can_check_hashset_contains.rhai b/assets/tests/hashset/can_check_hashset_contains.rhai new file mode 100644 index 0000000000..cd3559836b --- /dev/null +++ b/assets/tests/hashset/can_check_hashset_contains.rhai @@ -0,0 +1,14 @@ +let Resource = world.get_type_by_name.call("TestResourceWithVariousFields"); +let resource = world.get_resource.call(Resource); + +// Check if "apple" exists in the set (should return Some(apple)) +let result = resource.string_set["apple"]; +assert(result != (), "Expected to find 'apple' in set"); + +// Check if "banana" exists in the set (should return Some(banana)) +let result2 = resource.string_set["banana"]; +assert(result2 != (), "Expected to find 'banana' in set"); + +// Check if "nonexistent" doesn't exist in the set (should return None) +let result3 = resource.string_set["nonexistent"]; +assert(result3 == (), "Expected not to find 'nonexistent' in set"); diff --git a/assets/tests/hashset/can_insert_into_hashset.lua b/assets/tests/hashset/can_insert_into_hashset.lua new file mode 100644 index 0000000000..7a370eb91a --- /dev/null +++ b/assets/tests/hashset/can_insert_into_hashset.lua @@ -0,0 +1,19 @@ +local Resource = world.get_type_by_name("TestResourceWithVariousFields") +local resource = world.get_resource(Resource) + +-- Insert new values into the set +resource.string_set["orange"] = "orange" +resource.string_set["grape"] = "grape" + +local resource_changed = world.get_resource(Resource) + +-- Verify the new values were added +local orange = resource_changed.string_set["orange"] +assert(orange ~= nil, "Expected to find 'orange' in set after insertion") + +local grape = resource_changed.string_set["grape"] +assert(grape ~= nil, "Expected to find 'grape' in set after insertion") + +-- Verify original values are still there +local apple = resource_changed.string_set["apple"] +assert(apple ~= nil, "Expected 'apple' to still be in set") diff --git a/assets/tests/hashset/can_insert_into_hashset.rhai b/assets/tests/hashset/can_insert_into_hashset.rhai new file mode 100644 index 0000000000..d590fae4dd --- /dev/null +++ b/assets/tests/hashset/can_insert_into_hashset.rhai @@ -0,0 +1,19 @@ +let Resource = world.get_type_by_name.call("TestResourceWithVariousFields"); +let resource = world.get_resource.call(Resource); + +// Insert new values into the set +resource.string_set["orange"] = "orange"; +resource.string_set["grape"] = "grape"; + +let resource_changed = world.get_resource.call(Resource); + +// Verify the new values were added +let orange = resource_changed.string_set["orange"]; +assert(orange != (), "Expected to find 'orange' in set after insertion"); + +let grape = resource_changed.string_set["grape"]; +assert(grape != (), "Expected to find 'grape' in set after insertion"); + +// Verify original values are still there +let apple = resource_changed.string_set["apple"]; +assert(apple != (), "Expected 'apple' to still be in set"); diff --git a/crates/bevy_mod_scripting_bindings/src/function/magic_functions.rs b/crates/bevy_mod_scripting_bindings/src/function/magic_functions.rs index 64dec69eca..48afce8290 100644 --- a/crates/bevy_mod_scripting_bindings/src/function/magic_functions.rs +++ b/crates/bevy_mod_scripting_bindings/src/function/magic_functions.rs @@ -1,9 +1,7 @@ //! All the switchable special functions used by language implementors -use super::{FromScriptRef, FunctionCallContext, IntoScriptRef}; -use crate::{error::InteropError, PartialReflectExt, ReflectReference, ReflectionPathExt, ScriptValue}; +use super::FunctionCallContext; +use crate::{error::InteropError, ReflectReference, ScriptValue}; use bevy_mod_scripting_derive::DebugWithTypeInfo; -use bevy_mod_scripting_display::OrFakeId; -use bevy_reflect::{ParsedPath, PartialReflect, ReflectRef}; /// A list of magic methods, these only have one replacable implementation, and apply to all `ReflectReferences`. /// It's up to the language implementer to call these in the right order (after any type specific overrides). @@ -64,56 +62,7 @@ impl MagicFunctions { key: ScriptValue, ) -> Result { let world = ctxt.world()?; - - // Check if the reference is a map type - let is_map = reference.with_reflect(world.clone(), |r| { - matches!(r.reflect_ref(), ReflectRef::Map(_)) - })?; - - if is_map { - // Handle map indexing specially - need to get the key type and convert the script value - let key = >::from_script_ref( - reference.key_type_id(world.clone())?.ok_or_else(|| { - InteropError::unsupported_operation( - reference.tail_type_id(world.clone()).unwrap_or_default(), - Some(Box::new(key.clone())), - "Could not get key type id. Are you trying to index into a type that's not a map?".to_owned(), - ) - })?, - key, - world.clone(), - )?; - - reference.with_reflect(world.clone(), |s| match s.try_map_get(key.as_ref())? { - Some(value) => { - let reference = { - let allocator = world.allocator(); - let mut allocator = allocator.write(); - let owned_value = ::from_reflect(value, world.clone())?; - ReflectReference::new_allocated_boxed(owned_value, &mut allocator) - }; - ReflectReference::into_script_ref(reference, world) - } - None => { - // Return None option if key doesn't exist - let none_option: Option<()> = None; - let reference = { - let allocator = world.allocator(); - let mut allocator = allocator.write(); - ReflectReference::new_allocated_boxed(Box::new(none_option), &mut allocator) - }; - ReflectReference::into_script_ref(reference, world) - } - })? - } else { - // Handle path-based indexing for non-map types - let mut path: ParsedPath = key.try_into()?; - if ctxt.convert_to_0_indexed() { - path.convert_to_0_indexed(); - } - reference.index_path(path); - ReflectReference::into_script_ref(reference, world) - } + reference.get_indexed(key, world, ctxt.convert_to_0_indexed()) } /// Sets the value under the specified path on the underlying value. @@ -133,66 +82,7 @@ impl MagicFunctions { value: ScriptValue, ) -> Result<(), InteropError> { let world = ctxt.world()?; - - // Check if the reference is a map type - let is_map = reference.with_reflect(world.clone(), |r| { - matches!(r.reflect_ref(), ReflectRef::Map(_)) - })?; - - if is_map { - // Handle map setting specially - need to get the key type and convert the script value - let key = >::from_script_ref( - reference.key_type_id(world.clone())?.ok_or_else(|| { - InteropError::unsupported_operation( - reference.tail_type_id(world.clone()).unwrap_or_default(), - Some(Box::new(key.clone())), - "Could not get key type id. Are you trying to index into a type that's not a map?".to_owned(), - ) - })?, - key, - world.clone(), - )?; - - // Get the value type for the map and convert the script value - let value_type_id = reference.element_type_id(world.clone())?.ok_or_else(|| { - InteropError::unsupported_operation( - reference.tail_type_id(world.clone()).unwrap_or_default(), - Some(Box::new(value.clone())), - "Could not get value type id. Are you trying to set a value in a type that's not a map?".to_owned(), - ) - })?; - - let value = >::from_script_ref( - value_type_id, - value, - world.clone(), - )?; - - reference.with_reflect_mut(world, |s| { - s.try_insert_boxed(key, value) - })??; - } else { - let mut path: ParsedPath = key.try_into()?; - if ctxt.convert_to_0_indexed() { - path.convert_to_0_indexed(); - } - reference.index_path(path); - - let target_type_id = reference.with_reflect(world.clone(), |r| { - r.get_represented_type_info() - .map(|i| i.type_id()) - .or_fake_id() - })?; - - let other = >::from_script_ref(target_type_id, value, world.clone())?; - - reference.with_reflect_mut(world, |r| { - r.try_apply(other.as_partial_reflect()) - .map_err(InteropError::reflect_apply_error) - })??; - } - - Ok(()) + reference.set_indexed(key, value, world, ctxt.convert_to_0_indexed()) } } diff --git a/crates/bevy_mod_scripting_bindings/src/reference.rs b/crates/bevy_mod_scripting_bindings/src/reference.rs index bddf1f1896..577745060e 100644 --- a/crates/bevy_mod_scripting_bindings/src/reference.rs +++ b/crates/bevy_mod_scripting_bindings/src/reference.rs @@ -4,32 +4,35 @@ //! reflection gives us access to `dyn PartialReflect` objects via their type name, //! Scripting languages only really support `Clone` objects so if we want to support references, //! we need wrapper types which have owned and ref variants. -use super::{WorldGuard, access_map::ReflectAccessId}; -use crate::{ - ReflectAllocationId, ReflectAllocator, ThreadWorldContainer, WorldContainer, - error::InteropError, reflection_extensions::PartialReflectExt, with_access_read, - with_access_write, +use core::alloc; +use std::{ + any::{Any, TypeId}, + fmt::Debug, }; + use bevy_asset::{ReflectAsset, UntypedHandle}; -use bevy_ecs::{component::Component, ptr::Ptr, resource::Resource}; +use bevy_ecs::{ + change_detection::MutUntyped, + component::{Component, ComponentId}, + entity::Entity, + ptr::Ptr, + resource::Resource, + world::unsafe_world_cell::UnsafeWorldCell, +}; use bevy_mod_scripting_derive::DebugWithTypeInfo; use bevy_mod_scripting_display::{ DebugWithTypeInfo, DisplayWithTypeInfo, OrFakeId, PrintReflectAsDebug, WithTypeInfo, }; -use bevy_reflect::{Access, OffsetAccess, ReflectRef}; -use core::alloc; -use std::{ - any::{Any, TypeId}, - fmt::Debug, +use bevy_reflect::{ + Access, OffsetAccess, ParsedPath, PartialReflect, Reflect, ReflectFromPtr, ReflectPath, + ReflectRef, prelude::ReflectDefault, }; -use { - bevy_ecs::{ - change_detection::MutUntyped, component::ComponentId, entity::Entity, - world::unsafe_world_cell::UnsafeWorldCell, - }, - bevy_reflect::{ - ParsedPath, PartialReflect, Reflect, ReflectFromPtr, ReflectPath, prelude::ReflectDefault, - }, + +use super::{WorldGuard, access_map::ReflectAccessId}; +use crate::{ + FromScriptRef, IntoScriptRef, ReflectAllocationId, ReflectAllocator, ThreadWorldContainer, + WorldContainer, error::InteropError, reflection_extensions::PartialReflectExt, + with_access_read, with_access_write, }; /// A reference to an arbitrary reflected instance. @@ -352,6 +355,123 @@ impl ReflectReference { self.reflect_path.0.extend(index.into().0); } + #[inline] + /// Checks if the reference points to a map type. + pub fn is_map(&self, world: WorldGuard) -> Result { + self.with_reflect(world, |r| matches!(r.reflect_ref(), ReflectRef::Map(_))) + } + + #[inline] + /// Checks if the reference points to a set type. + pub fn is_set(&self, world: WorldGuard) -> Result { + self.with_reflect(world, |r| matches!(r.reflect_ref(), ReflectRef::Set(_))) + } + + /// Gets a value from the reference using the provided key. + /// If the reference is a map, uses map indexing with proper key type conversion. + /// If the reference is a set, checks if the value exists and returns it or None. + /// Otherwise, uses path-based indexing. + /// + /// Returns a new allocated reference to the retrieved value, or None if not found (for maps/sets). + pub fn get_indexed( + &mut self, + key: crate::ScriptValue, + world: WorldGuard, + convert_to_0_indexed: bool, + ) -> Result { + if self.is_map(world.clone())? { + // Handle map indexing specially - need to get the key type and convert the script value + let key_type_id = self.get_key_type_id_or_error(&key, world.clone())?; + let key = >::from_script_ref(key_type_id, key, world.clone())?; + + self.with_reflect(world.clone(), |s| match s.try_map_get(key.as_ref())? { + Some(value) => { + let owned_value = ::from_reflect(value, world.clone())?; + Self::allocate_reflect_as_script_value(owned_value, world) + } + None => Self::allocate_reflect_as_script_value(Box::new(None::<()>), world), + })? + } else if self.is_set(world.clone())? { + // Set containment check - need to get the value type and convert the script value + let element_type_id = self.get_element_type_id_or_error(&key, world.clone())?; + let value = + >::from_script_ref(element_type_id, key, world.clone())?; + + self.with_reflect(world.clone(), |s| match s.try_set_get(value.as_ref())? { + Some(contained_value) => { + let owned_value = ::from_reflect( + contained_value.as_ref(), + world.clone(), + )?; + Self::allocate_reflect_as_script_value(owned_value, world) + } + None => Self::allocate_reflect_as_script_value(Box::new(None::<()>), world), + })? + } else { + let mut path: ParsedPath = key.try_into()?; + if convert_to_0_indexed { + path.convert_to_0_indexed(); + } + self.index_path(path); + ReflectReference::into_script_ref(self.clone(), world) + } + } + + /// Sets a value in the reference using the provided key. + /// If the reference is a map, uses map insertion with proper key/value type conversion. + /// If the reference is a set, inserts the key (ignoring the value parameter). + /// Otherwise, uses path-based indexing and applies the value. + pub fn set_indexed( + &mut self, + key: crate::ScriptValue, + value: crate::ScriptValue, + world: WorldGuard, + convert_to_0_indexed: bool, + ) -> Result<(), InteropError> { + if self.is_map(world.clone())? { + // Handle map setting specially - need to get the key type and convert the script value + let key_type_id = self.get_key_type_id_or_error(&key, world.clone())?; + let key = >::from_script_ref(key_type_id, key, world.clone())?; + + // Get the value type for the map and convert the script value + let value_type_id = self.get_element_type_id_or_error(&value, world.clone())?; + let value = + >::from_script_ref(value_type_id, value, world.clone())?; + + self.with_reflect_mut(world, |s| s.try_insert_boxed(key, value))??; + } else if self.is_set(world.clone())? { + // Handle set insertion - need to get the element type and convert the key (which is the value to insert) + let element_type_id = self.get_element_type_id_or_error(&key, world.clone())?; + let insert_value = + >::from_script_ref(element_type_id, key, world.clone())?; + + // For sets, try_insert_boxed takes (key, value) but only uses value, so we pass a dummy key + self.with_reflect_mut(world, |s| s.try_insert_boxed(Box::new(()), insert_value))??; + } else { + let mut path: ParsedPath = key.try_into()?; + if convert_to_0_indexed { + path.convert_to_0_indexed(); + } + self.index_path(path); + + let target_type_id = self.with_reflect(world.clone(), |r| { + r.get_represented_type_info() + .map(|i| i.type_id()) + .or_fake_id() + })?; + + let other = + >::from_script_ref(target_type_id, value, world.clone())?; + + self.with_reflect_mut(world, |r| { + r.try_apply(other.as_partial_reflect()) + .map_err(InteropError::reflect_apply_error) + })??; + } + + Ok(()) + } + /// Tries to downcast to the specified type and cloning the value if successful. pub fn downcast( &self, @@ -607,6 +727,51 @@ impl ReflectReference { .reflect_element_mut(root) .map_err(|e| InteropError::reflection_path_error(e, Some(self.clone()))) } + + /// Allocates a reflected value and returns it as a script value. + /// Handles the allocator locking and reference wrapping. + fn allocate_reflect_as_script_value( + value: Box, + world: WorldGuard, + ) -> Result { + let reference = { + let allocator = world.allocator(); + let mut allocator = allocator.write(); + ReflectReference::new_allocated_boxed(value, &mut allocator) + }; + ReflectReference::into_script_ref(reference, world) + } + + /// Retrieves the key type id with a descriptive error if not available. + fn get_key_type_id_or_error( + &self, + key_value: &crate::ScriptValue, + world: WorldGuard, + ) -> Result { + self.key_type_id(world.clone())?.ok_or_else(|| { + InteropError::unsupported_operation( + self.tail_type_id(world).unwrap_or_default(), + Some(Box::new(key_value.clone())), + "Could not get key type id. Are you trying to index into a type that's not a map?" + .to_owned(), + ) + }) + } + + /// Retrieves the element type id with a descriptive error if not available. + fn get_element_type_id_or_error( + &self, + value: &crate::ScriptValue, + world: WorldGuard, + ) -> Result { + self.element_type_id(world.clone())?.ok_or_else(|| { + InteropError::unsupported_operation( + self.tail_type_id(world).unwrap_or_default(), + Some(Box::new(value.clone())), + "Could not get element type id. Are you trying to index/insert into a type that's not a set/map?".to_owned(), + ) + }) + } } /// The type id and base id of the value we want to access @@ -937,7 +1102,10 @@ impl ReflectRefIter { /// Returns the next element as a cloned value using `from_reflect`. /// Returns a fully reflected value (Box) instead of a reference. /// Returns Ok(None) when the path is invalid (end of iteration). - pub fn next_cloned(&mut self, world: WorldGuard) -> Result, InteropError> { + pub fn next_cloned( + &mut self, + world: WorldGuard, + ) -> Result, InteropError> { let index = match &mut self.index { IterationKey::Index(i) => { let idx = *i; @@ -948,16 +1116,12 @@ impl ReflectRefIter { let element = self.base.with_reflect(world.clone(), |base_reflect| { match base_reflect.reflect_ref() { - bevy_reflect::ReflectRef::List(list) => { - list.get(index).map(|item| { - ::from_reflect(item, world.clone()) - }) - } - bevy_reflect::ReflectRef::Array(array) => { - array.get(index).map(|item| { - ::from_reflect(item, world.clone()) - }) - } + bevy_reflect::ReflectRef::List(list) => list + .get(index) + .map(|item| ::from_reflect(item, world.clone())), + bevy_reflect::ReflectRef::Array(array) => array + .get(index) + .map(|item| ::from_reflect(item, world.clone())), _ => None, } })?; @@ -967,7 +1131,8 @@ impl ReflectRefIter { let owned_value = result?; let allocator = world.allocator(); let mut allocator_guard = allocator.write(); - let value_ref = ReflectReference::new_allocated_boxed(owned_value, &mut *allocator_guard); + let value_ref = + ReflectReference::new_allocated_boxed(owned_value, &mut *allocator_guard); Ok(Some(value_ref)) } None => Ok(None), @@ -992,30 +1157,31 @@ impl ReflectMapRefIter { /// Returns the next map entry as a (key, value) tuple, cloning the values. /// Uses `from_reflect` to create fully reflected clones that can be used with all operations. /// Returns Ok(None) when there are no more entries. - pub fn next_cloned(&mut self, world: WorldGuard) -> Result, InteropError> { + pub fn next_cloned( + &mut self, + world: WorldGuard, + ) -> Result, InteropError> { let idx = self.index; self.index += 1; - + // Access the map and get the entry at index - self.base.with_reflect(world.clone(), |reflect| { - match reflect.reflect_ref() { + self.base + .with_reflect(world.clone(), |reflect| match reflect.reflect_ref() { ReflectRef::Map(map) => { if let Some((key, value)) = map.get_at(idx) { let allocator = world.allocator(); let mut allocator_guard = allocator.write(); - + let owned_key = ::from_reflect(key, world.clone())?; - let key_ref = ReflectReference::new_allocated_boxed( - owned_key, - &mut *allocator_guard - ); - + let key_ref = + ReflectReference::new_allocated_boxed(owned_key, &mut *allocator_guard); + let owned_value = ::from_reflect(value, world.clone())?; let value_ref = ReflectReference::new_allocated_boxed( owned_value, - &mut *allocator_guard + &mut *allocator_guard, ); - + drop(allocator_guard); Ok(Some((key_ref, value_ref))) } else { @@ -1026,9 +1192,8 @@ impl ReflectMapRefIter { reflect.get_represented_type_info().map(|ti| ti.type_id()), None, "map iteration on non-map type".to_owned(), - )) - } - })? + )), + })? } } @@ -1059,9 +1224,8 @@ mod test { component::Component, reflect::AppTypeRegistry, resource::Resource, world::World, }; - use crate::{AppReflectAllocator, function::script_function::AppScriptFunctionRegistry}; - use super::*; + use crate::{AppReflectAllocator, function::script_function::AppScriptFunctionRegistry}; #[derive(Reflect, Component, Debug, Clone, PartialEq)] struct TestComponent(Vec); diff --git a/crates/bevy_mod_scripting_bindings/src/reflection_extensions.rs b/crates/bevy_mod_scripting_bindings/src/reflection_extensions.rs index cc7e7b4165..90b33892ab 100644 --- a/crates/bevy_mod_scripting_bindings/src/reflection_extensions.rs +++ b/crates/bevy_mod_scripting_bindings/src/reflection_extensions.rs @@ -17,6 +17,14 @@ pub trait PartialReflectExt { key: &dyn PartialReflect, ) -> Result, InteropError>; + /// Try to get the value from the set if it exists. + /// For sets, this checks if the value exists and returns it as a boxed value. + /// Returns Some(value) if the value exists in the set, None if it doesn't. + fn try_set_get( + &self, + value: &dyn PartialReflect, + ) -> Result>, InteropError>; + /// Try to remove the value at the given key, if the type supports removing with the given key. fn try_remove_boxed( &mut self, @@ -317,6 +325,31 @@ impl PartialReflectExt for T { } } + fn try_set_get( + &self, + value: &dyn PartialReflect, + ) -> Result>, InteropError> { + match self.reflect_ref() { + ReflectRef::Set(s) => { + // Check if the set contains the value + if s.contains(value) { + // Clone and return the value if it exists in the set + let cloned = value.reflect_clone() + .map_err(|e| InteropError::string(format!("Failed to clone set value: {}", e)))?; + // Downcast to PartialReflect since reflect_clone returns Box + Ok(Some(cloned)) + } else { + Ok(None) + } + } + _ => Err(InteropError::unsupported_operation( + self.get_represented_type_info().map(|ti| ti.type_id()), + None, + "set_get".to_owned(), + )), + } + } + fn try_pop_boxed(&mut self) -> Result, InteropError> { match self.reflect_mut() { ReflectMut::List(l) => l.pop().ok_or_else(|| { diff --git a/crates/testing_crates/test_utils/src/test_data.rs b/crates/testing_crates/test_utils/src/test_data.rs index e25ce138be..5ccb2fa2a7 100644 --- a/crates/testing_crates/test_utils/src/test_data.rs +++ b/crates/testing_crates/test_utils/src/test_data.rs @@ -1,4 +1,4 @@ -use std::{alloc::Layout, collections::HashMap}; +use std::{alloc::Layout, collections::{HashMap, HashSet}}; use bevy_app::{App, ScheduleRunnerPlugin, TaskPoolPlugin}; use bevy_diagnostic::FrameCountPlugin; @@ -158,6 +158,7 @@ pub struct TestResourceWithVariousFields { pub bool: bool, pub vec_usize: Vec, pub string_map: HashMap, + pub string_set: HashSet, } impl TestResourceWithVariousFields { @@ -173,6 +174,10 @@ impl TestResourceWithVariousFields { .into_iter() .map(|(a, b)| (a.to_owned(), b.to_owned())) .collect(), + string_set: vec!["apple", "banana"] + .into_iter() + .map(|s| s.to_owned()) + .collect(), } } } From f476eef6d90881fcf84e00805b8de3d0ce33407a Mon Sep 17 00:00:00 2001 From: aggyomfg Date: Wed, 8 Oct 2025 20:45:26 +0400 Subject: [PATCH 15/16] hashset iter --- assets/tests/iter/hashset.lua | 19 ++++++ assets/tests/iter/hashset.rhai | 29 +++++++++ assets/tests/iter/hashset_pairs.lua | 22 +++++++ .../src/reference.rs | 65 +++++++++++++++++++ .../bevy_mod_scripting_functions/src/core.rs | 31 ++++++--- 5 files changed, 157 insertions(+), 9 deletions(-) create mode 100644 assets/tests/iter/hashset.lua create mode 100644 assets/tests/iter/hashset.rhai create mode 100644 assets/tests/iter/hashset_pairs.lua diff --git a/assets/tests/iter/hashset.lua b/assets/tests/iter/hashset.lua new file mode 100644 index 0000000000..91876a298b --- /dev/null +++ b/assets/tests/iter/hashset.lua @@ -0,0 +1,19 @@ +local res_type = world.get_type_by_name("TestResourceWithVariousFields") +local res = world.get_resource(res_type) + +local set = res.string_set + +local iterator = set:iter() +local count = 0 +local found_values = {} + +local result = iterator() +while result ~= nil do + count = count + 1 + found_values[result] = true + result = iterator() +end + +assert(count == 2, "Expected 2 entries, got " .. count) +assert(found_values["apple"] == true, "Expected to find 'apple'") +assert(found_values["banana"] == true, "Expected to find 'banana'") diff --git a/assets/tests/iter/hashset.rhai b/assets/tests/iter/hashset.rhai new file mode 100644 index 0000000000..dbbb19fa5e --- /dev/null +++ b/assets/tests/iter/hashset.rhai @@ -0,0 +1,29 @@ +let res_type = world.get_type_by_name.call("TestResourceWithVariousFields"); +let res = world.get_resource.call(res_type); + +let set = res.string_set; + +let iterator = set.iter(); +let count = 0; +let found_values = #{}; + +loop { + let result = iterator.next(); + + if result == () { + break; + } + + count += 1; + found_values[result] = true; +} + +if count != 2 { + throw `Expected 2 entries, got ${count}`; +} +if found_values["apple"] != true { + throw "Expected to find 'apple'"; +} +if found_values["banana"] != true { + throw "Expected to find 'banana'"; +} diff --git a/assets/tests/iter/hashset_pairs.lua b/assets/tests/iter/hashset_pairs.lua new file mode 100644 index 0000000000..f2bde89e81 --- /dev/null +++ b/assets/tests/iter/hashset_pairs.lua @@ -0,0 +1,22 @@ +local res_type = world.get_type_by_name("TestResourceWithVariousFields") +local res = world.get_resource(res_type) + +local set = res.string_set + +local count = 0 +local found_values = {} + +for value in pairs(set) do + count = count + 1 + found_values[value] = true +end + +if count ~= 2 then + error(string.format("Expected 2 entries, got %d", count)) +end +if found_values["apple"] ~= true then + error("Expected to find 'apple'") +end +if found_values["banana"] ~= true then + error("Expected to find 'banana'") +end diff --git a/crates/bevy_mod_scripting_bindings/src/reference.rs b/crates/bevy_mod_scripting_bindings/src/reference.rs index 577745060e..1768f29e90 100644 --- a/crates/bevy_mod_scripting_bindings/src/reference.rs +++ b/crates/bevy_mod_scripting_bindings/src/reference.rs @@ -146,6 +146,12 @@ impl ReflectReference { } } + /// Creates a new iterator for Sets specifically. + /// Unlike `into_iter_infinite`, this iterator is finite and will return None when exhausted. + pub fn into_set_iter(self, world: WorldGuard) -> Result { + ReflectSetRefIter::new(self, world) + } + /// If this is a reference to something with a length accessible via reflection, returns that length. pub fn len(&self, world: WorldGuard) -> Result, InteropError> { self.with_reflect(world, |r| match r.reflect_ref() { @@ -1197,6 +1203,65 @@ impl ReflectMapRefIter { } } +/// Iterator specifically for Sets that doesn't use the path system. +/// This collects all set values in a Vec to enable O(1) iteration, but with memory overhead. +pub struct ReflectSetRefIter { + values: Vec, + index: usize, +} + +#[profiling::all_functions] +impl ReflectSetRefIter { + /// Creates a new set iterator by collecting all values from the set in a Vec. + pub fn new(base: ReflectReference, world: WorldGuard) -> Result { + let values = base.with_reflect(world.clone(), |reflect| match reflect.reflect_ref() { + ReflectRef::Set(set) => { + let allocator = world.allocator(); + let mut allocator_guard = allocator.write(); + + let mut collected_values = Vec::with_capacity(set.len()); + + for value in set.iter() { + let owned_value = ::from_reflect(value, world.clone())?; + let value_ref = ReflectReference::new_allocated_boxed( + owned_value, + &mut *allocator_guard, + ); + collected_values.push(value_ref); + } + + drop(allocator_guard); + Ok(collected_values) + } + _ => Err(InteropError::unsupported_operation( + reflect.get_represented_type_info().map(|ti| ti.type_id()), + None, + "set iteration on non-set type".to_owned(), + )), + })??; + + Ok(Self { + values, + index: 0, + }) + } + + /// Returns the next set entry. + /// Returns Ok(None) when there are no more entries. + pub fn next_cloned( + &mut self, + _world: WorldGuard, + ) -> Result, InteropError> { + if self.index < self.values.len() { + let value = self.values[self.index].clone(); + self.index += 1; + Ok(Some(value)) + } else { + Ok(None) + } + } +} + #[profiling::all_functions] impl Iterator for ReflectRefIter { type Item = Result; diff --git a/crates/bevy_mod_scripting_functions/src/core.rs b/crates/bevy_mod_scripting_functions/src/core.rs index ba2ee35e75..0d7e5c24c9 100644 --- a/crates/bevy_mod_scripting_functions/src/core.rs +++ b/crates/bevy_mod_scripting_functions/src/core.rs @@ -7,7 +7,6 @@ use std::ops::Deref; use bevy_app::App; use bevy_asset::{AssetServer, Handle}; use bevy_ecs::{entity::Entity, prelude::AppTypeRegistry, schedule::Schedules, world::World}; -use bevy_reflect::ReflectRef; use bevy_mod_scripting_bindings::{ DynamicScriptFunctionMut, FunctionInfo, GlobalNamespace, InteropError, PartialReflectExt, ReflectReference, ScriptComponentRegistration, ScriptQueryBuilder, ScriptQueryResult, @@ -729,6 +728,10 @@ impl ReflectReference { /// Iterates over the reference, if the reference is an appropriate container type. /// Uses `from_reflect` to create fully reflected clones that can be used with all operations. /// + /// For Maps, returns an iterator function that returns [key, value] pairs. + /// For Sets, returns an iterator function that returns individual values. + /// For other containers (Lists, Arrays), returns an iterator function that returns individual elements. + /// /// Returns an "next" iterator function that returns fully reflected values (Box). /// /// The iterator function should be called until it returns `nil` to signal the end of the iteration. @@ -745,10 +748,8 @@ impl ReflectReference { profiling::function_scope!("iter"); let world = ctxt.world()?; let mut len = reference.len(world.clone())?.unwrap_or_default(); - // Check if this is a Map type - let is_map = reference.with_reflect(world.clone(), |r| { - matches!(r.reflect_ref(), ReflectRef::Map(_)) - })?; + let is_map = reference.is_map(world.clone())?; + let is_set = reference.is_set(world.clone())?; if is_map { // Use special map iterator that clones values @@ -770,6 +771,21 @@ impl ReflectReference { } }; Ok(iter_function.into_dynamic_script_function_mut()) + } else if is_set { + // Use special set iterator that clones values upfront for efficient iteration + let mut set_iter = reference.into_set_iter(world.clone())?; + let iter_function = move || { + if len == 0 { + return Ok(ScriptValue::Unit); + } + len -= 1; + let world = ThreadWorldContainer.try_get_world()?; + match set_iter.next_cloned(world.clone())? { + Some(value_ref) => ReflectReference::into_script_ref(value_ref, world), + None => Ok(ScriptValue::Unit), + } + }; + Ok(iter_function.into_dynamic_script_function_mut()) } else { // Use cloning iterator for lists/arrays let mut infinite_iter = reference.into_iter_infinite(); @@ -780,10 +796,7 @@ impl ReflectReference { len -= 1; let world = ThreadWorldContainer.try_get_world()?; match infinite_iter.next_cloned(world.clone())? { - Some(value_ref) => { - // Convert the cloned value to a script value - ReflectReference::into_script_ref(value_ref, world) - } + Some(value_ref) => ReflectReference::into_script_ref(value_ref, world), None => Ok(ScriptValue::Unit), } }; From 32484f191e4851bdd62e7e32d026a02623180a36 Mon Sep 17 00:00:00 2001 From: aggyomfg Date: Sat, 11 Oct 2025 10:28:44 +0400 Subject: [PATCH 16/16] rm duplicate test --- assets/tests/iter/vec_pairs.lua | 14 -------------- 1 file changed, 14 deletions(-) delete mode 100644 assets/tests/iter/vec_pairs.lua diff --git a/assets/tests/iter/vec_pairs.lua b/assets/tests/iter/vec_pairs.lua deleted file mode 100644 index 2ec25ec144..0000000000 --- a/assets/tests/iter/vec_pairs.lua +++ /dev/null @@ -1,14 +0,0 @@ -local res_type = world.get_type_by_name("TestResourceWithVariousFields") -local res = world.get_resource(res_type) - -local iterated_vals = {} -for v in pairs(res.vec_usize) do - iterated_vals[#iterated_vals + 1] = v -end - -assert(#iterated_vals == 5, "Length is not 5") -assert(iterated_vals[1] == 1, "First value is not 1") -assert(iterated_vals[2] == 2, "Second value is not 2") -assert(iterated_vals[3] == 3, "Third value is not 3") -assert(iterated_vals[4] == 4, "Fourth value is not 4") -assert(iterated_vals[5] == 5, "Fifth value is not 5")