diff --git a/crates/bevy_reflect/src/array.rs b/crates/bevy_reflect/src/array.rs index c49fddc220..f5f1158c20 100644 --- a/crates/bevy_reflect/src/array.rs +++ b/crates/bevy_reflect/src/array.rs @@ -378,7 +378,7 @@ impl<'a> Iterator for ArrayIter<'a> { #[inline] fn next(&mut self) -> Option { let value = self.array.get(self.index); - self.index += 1; + self.index += value.is_some() as usize; value } @@ -503,3 +503,32 @@ pub fn array_debug(dyn_array: &dyn Array, f: &mut std::fmt::Formatter<'_>) -> st } debug.finish() } +#[cfg(test)] +mod tests { + use crate::{Reflect, ReflectRef}; + #[test] + fn next_index_increment() { + const SIZE: usize = if cfg!(debug_assertions) { + 4 + } else { + // If compiled in release mode, verify we dont overflow + usize::MAX + }; + + let b = Box::new([(); SIZE]).into_reflect(); + + let ReflectRef::Array(array) = b.reflect_ref() else { + panic!("Not an array..."); + }; + + let mut iter = array.iter(); + iter.index = SIZE - 1; + assert!(iter.next().is_some()); + + // When None we should no longer increase index + assert!(iter.next().is_none()); + assert!(iter.index == SIZE); + assert!(iter.next().is_none()); + assert!(iter.index == SIZE); + } +} diff --git a/crates/bevy_reflect/src/enums/enum_trait.rs b/crates/bevy_reflect/src/enums/enum_trait.rs index a741945051..66029923da 100644 --- a/crates/bevy_reflect/src/enums/enum_trait.rs +++ b/crates/bevy_reflect/src/enums/enum_trait.rs @@ -280,7 +280,7 @@ impl<'a> Iterator for VariantFieldIter<'a> { Some(VariantField::Struct(name, self.container.field(name)?)) } }; - self.index += 1; + self.index += value.is_some() as usize; value } @@ -312,3 +312,58 @@ impl<'a> VariantField<'a> { } } } + +// Tests that need access to internal fields have to go here rather than in mod.rs +#[cfg(test)] +mod tests { + use crate as bevy_reflect; + use crate::*; + + #[derive(Reflect, Debug, PartialEq)] + enum MyEnum { + A, + B(usize, i32), + C { foo: f32, bar: bool }, + } + #[test] + fn next_index_increment() { + // unit enums always return none, so index should stay at 0 + let unit_enum = MyEnum::A; + let mut iter = unit_enum.iter_fields(); + let size = iter.len(); + for _ in 0..2 { + assert!(iter.next().is_none()); + assert_eq!(size, iter.index); + } + // tuple enums we iter over each value (unnamed fields), stop after that + let tuple_enum = MyEnum::B(0, 1); + let mut iter = tuple_enum.iter_fields(); + let size = iter.len(); + for _ in 0..2 { + let prev_index = iter.index; + assert!(iter.next().is_some()); + assert_eq!(prev_index, iter.index - 1); + } + for _ in 0..2 { + assert!(iter.next().is_none()); + assert_eq!(size, iter.index); + } + + // struct enums, we iterate over each field in the struct + let struct_enum = MyEnum::C { + foo: 0., + bar: false, + }; + let mut iter = struct_enum.iter_fields(); + let size = iter.len(); + for _ in 0..2 { + let prev_index = iter.index; + assert!(iter.next().is_some()); + assert_eq!(prev_index, iter.index - 1); + } + for _ in 0..2 { + assert!(iter.next().is_none()); + assert_eq!(size, iter.index); + } + } +} diff --git a/crates/bevy_reflect/src/list.rs b/crates/bevy_reflect/src/list.rs index e01709b30d..5b786ee76a 100644 --- a/crates/bevy_reflect/src/list.rs +++ b/crates/bevy_reflect/src/list.rs @@ -405,7 +405,7 @@ impl<'a> Iterator for ListIter<'a> { #[inline] fn next(&mut self) -> Option { let value = self.list.get(self.index); - self.index += 1; + self.index += value.is_some() as usize; value } @@ -533,6 +533,7 @@ pub fn list_debug(dyn_list: &dyn List, f: &mut Formatter<'_>) -> std::fmt::Resul #[cfg(test)] mod tests { use super::DynamicList; + use crate::{Reflect, ReflectRef}; use std::assert_eq; #[test] @@ -547,4 +548,29 @@ mod tests { assert_eq!(index, value); } } + + #[test] + fn next_index_increment() { + const SIZE: usize = if cfg!(debug_assertions) { + 4 + } else { + // If compiled in release mode, verify we dont overflow + usize::MAX + }; + let b = Box::new(vec![(); SIZE]).into_reflect(); + + let ReflectRef::List(list) = b.reflect_ref() else { + panic!("Not a list..."); + }; + + let mut iter = list.iter(); + iter.index = SIZE - 1; + assert!(iter.next().is_some()); + + // When None we should no longer increase index + assert!(iter.next().is_none()); + assert!(iter.index == SIZE); + assert!(iter.next().is_none()); + assert!(iter.index == SIZE); + } } diff --git a/crates/bevy_reflect/src/map.rs b/crates/bevy_reflect/src/map.rs index 085eda219e..5b182c68a9 100644 --- a/crates/bevy_reflect/src/map.rs +++ b/crates/bevy_reflect/src/map.rs @@ -417,7 +417,7 @@ impl<'a> Iterator for MapIter<'a> { fn next(&mut self) -> Option { let value = self.map.get_at(self.index); - self.index += 1; + self.index += value.is_some() as usize; value } @@ -618,4 +618,27 @@ mod tests { assert!(map.get_at(2).is_none()); } + + #[test] + fn next_index_increment() { + let values = ["first", "last"]; + let mut map = DynamicMap::default(); + map.insert(0usize, values[0]); + map.insert(1usize, values[1]); + + let mut iter = map.iter(); + let size = iter.len(); + + for _ in 0..2 { + let prev_index = iter.index; + assert!(iter.next().is_some()); + assert_eq!(prev_index, iter.index - 1); + } + + // When None we should no longer increase index + for _ in 0..2 { + assert!(iter.next().is_none()); + assert_eq!(size, iter.index); + } + } } diff --git a/crates/bevy_reflect/src/struct_trait.rs b/crates/bevy_reflect/src/struct_trait.rs index b342ea92a6..5d13403469 100644 --- a/crates/bevy_reflect/src/struct_trait.rs +++ b/crates/bevy_reflect/src/struct_trait.rs @@ -204,7 +204,7 @@ impl<'a> Iterator for FieldIter<'a> { fn next(&mut self) -> Option { let value = self.struct_val.field_at(self.index); - self.index += 1; + self.index += value.is_some() as usize; value } @@ -562,3 +562,31 @@ pub fn struct_debug(dyn_struct: &dyn Struct, f: &mut Formatter<'_>) -> std::fmt: } debug.finish() } + +#[cfg(test)] +mod tests { + use crate as bevy_reflect; + use crate::*; + #[derive(Reflect, Default)] + struct MyStruct { + a: (), + b: (), + c: (), + } + #[test] + fn next_index_increment() { + let my_struct = MyStruct::default(); + let mut iter = my_struct.iter_fields(); + iter.index = iter.len() - 1; + let prev_index = iter.index; + assert!(iter.next().is_some()); + assert_eq!(prev_index, iter.index - 1); + + // When None we should no longer increase index + let prev_index = iter.index; + assert!(iter.next().is_none()); + assert_eq!(prev_index, iter.index); + assert!(iter.next().is_none()); + assert_eq!(prev_index, iter.index); + } +} diff --git a/crates/bevy_reflect/src/tuple.rs b/crates/bevy_reflect/src/tuple.rs index 50eb9512f9..cf111edcdf 100644 --- a/crates/bevy_reflect/src/tuple.rs +++ b/crates/bevy_reflect/src/tuple.rs @@ -75,7 +75,7 @@ impl<'a> Iterator for TupleFieldIter<'a> { fn next(&mut self) -> Option { let value = self.tuple.field(self.index); - self.index += 1; + self.index += value.is_some() as usize; value } @@ -709,3 +709,24 @@ macro_rules! impl_type_path_tuple { } all_tuples!(impl_type_path_tuple, 0, 12, P); + +#[cfg(test)] +mod tests { + use super::Tuple; + + #[test] + fn next_index_increment() { + let mut iter = (0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11).iter_fields(); + let size = iter.len(); + iter.index = size - 1; + let prev_index = iter.index; + assert!(iter.next().is_some()); + assert_eq!(prev_index, iter.index - 1); + + // When None we should no longer increase index + assert!(iter.next().is_none()); + assert_eq!(size, iter.index); + assert!(iter.next().is_none()); + assert_eq!(size, iter.index); + } +} diff --git a/crates/bevy_reflect/src/tuple_struct.rs b/crates/bevy_reflect/src/tuple_struct.rs index b6cb0b720c..8aeb103984 100644 --- a/crates/bevy_reflect/src/tuple_struct.rs +++ b/crates/bevy_reflect/src/tuple_struct.rs @@ -155,7 +155,7 @@ impl<'a> Iterator for TupleStructFieldIter<'a> { fn next(&mut self) -> Option { let value = self.tuple_struct.field(self.index); - self.index += 1; + self.index += value.is_some() as usize; value } @@ -475,3 +475,26 @@ pub fn tuple_struct_debug( } debug.finish() } + +#[cfg(test)] +mod tests { + use crate as bevy_reflect; + use crate::*; + #[derive(Reflect)] + struct Ts(u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8, u8); + #[test] + fn next_index_increment() { + let mut iter = Ts(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11).iter_fields(); + let size = iter.len(); + iter.index = size - 1; + let prev_index = iter.index; + assert!(iter.next().is_some()); + assert_eq!(prev_index, iter.index - 1); + + // When None we should no longer increase index + assert!(iter.next().is_none()); + assert_eq!(size, iter.index); + assert!(iter.next().is_none()); + assert_eq!(size, iter.index); + } +}