diff --git a/assets/tests/map_get/can_get_hashmap_value.lua b/assets/tests/hashmap/can_get_hashmap_value.lua similarity index 51% rename from assets/tests/map_get/can_get_hashmap_value.lua rename to assets/tests/hashmap/can_get_hashmap_value.lua index 2bd4a4459e..972cff850b 100644 --- a/assets/tests/map_get/can_get_hashmap_value.lua +++ b/assets/tests/hashmap/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/hashmap/can_get_hashmap_value.rhai similarity index 51% rename from assets/tests/map_get/can_get_hashmap_value copy.rhai rename to assets/tests/hashmap/can_get_hashmap_value.rhai index 33f2dc1525..4057ecf0a6 100644 --- a/assets/tests/map_get/can_get_hashmap_value copy.rhai +++ b/assets/tests/hashmap/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/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/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/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/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 diff --git a/assets/tests/iter/hashmap.lua b/assets/tests/iter/hashmap.lua new file mode 100644 index 0000000000..0d27ae93b0 --- /dev/null +++ b/assets/tests/iter/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() +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/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/assets/tests/iter/hashmap_pairs.lua b/assets/tests/iter/hashmap_pairs.lua new file mode 100644 index 0000000000..c27cd113c5 --- /dev/null +++ b/assets/tests/iter/hashmap_pairs.lua @@ -0,0 +1,22 @@ +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 + +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/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/function/magic_functions.rs b/crates/bevy_mod_scripting_bindings/src/function/magic_functions.rs index 54387fe8e0..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::{ReflectReference, ReflectionPathExt, ScriptValue, error::InteropError}; +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}; /// 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 +49,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 +61,8 @@ 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) + reference.get_indexed(key, world, ctxt.convert_to_0_indexed()) } /// Sets the value under the specified path on the underlying value. @@ -91,22 +82,7 @@ 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(); - } - 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>(()) - })? + 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 664cabcd43..1768f29e90 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. @@ -134,6 +137,21 @@ 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, + } + } + + /// 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() { @@ -343,6 +361,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, @@ -598,6 +733,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 @@ -924,11 +1104,164 @@ 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> { 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, 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> { + 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(), + )), + })? + } +} + +/// 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; @@ -956,9 +1289,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/bevy_mod_scripting_functions/src/core.rs b/crates/bevy_mod_scripting_functions/src/core.rs index c3ffd8584a..0d7e5c24c9 100644 --- a/crates/bevy_mod_scripting_functions/src/core.rs +++ b/crates/bevy_mod_scripting_functions/src/core.rs @@ -604,46 +604,6 @@ impl ReflectReference { Ok(format!("{reference:#?}")) } - /// Gets and clones the value under the specified key if the underlying type is a map type. - /// - /// 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_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), - })? - } - /// Pushes the value into the reference, if the reference is an appropriate container type. /// /// Arguments: @@ -694,51 +654,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. @@ -811,8 +726,13 @@ 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. + /// 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. /// @@ -820,7 +740,7 @@ impl ReflectReference { /// * `ctxt`: The function call context. /// * `reference`: The reference to iterate over. /// Returns: - /// * `iter`: The iterator function. + /// * `iter`: The iterator function that returns cloned values. fn iter( ctxt: FunctionCallContext, reference: ReflectReference, @@ -828,25 +748,60 @@ 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 - }; - - Ok(iter_function.into_dynamic_script_function_mut()) + 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 + 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 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(); + 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) => 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. 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..b8fc06e43a 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,35 @@ 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) => { + 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(()) }], 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(), } } }