From 289c51b5478b37f3685162c05d30c53238e5f804 Mon Sep 17 00:00:00 2001 From: databasedav <31483365+databasedav@users.noreply.github.com> Date: Fri, 9 May 2025 10:10:54 -0700 Subject: [PATCH] fix `.insert_related` index bound (#19134) # Objective resolves #19092 ## Solution - remove the `.saturating_sub` from the index transformation - add `.saturating_add` to the internal offset calculation ## Testing - added regression test, confirming 0 index order + testing max bound --- crates/bevy_ecs/src/hierarchy.rs | 37 +++++++++++++++++++ .../src/relationship/related_methods.rs | 2 +- .../relationship_source_collection.rs | 5 +-- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/hierarchy.rs b/crates/bevy_ecs/src/hierarchy.rs index 9f4b0d0f8f..53be7fc80b 100644 --- a/crates/bevy_ecs/src/hierarchy.rs +++ b/crates/bevy_ecs/src/hierarchy.rs @@ -656,6 +656,43 @@ mod tests { ); } + // regression test for https://github.com/bevyengine/bevy/pull/19134 + #[test] + fn insert_children_index_bound() { + let mut world = World::new(); + let child1 = world.spawn_empty().id(); + let child2 = world.spawn_empty().id(); + let child3 = world.spawn_empty().id(); + let child4 = world.spawn_empty().id(); + + let mut entity_world_mut = world.spawn_empty(); + + let first_children = entity_world_mut.add_children(&[child1, child2]).id(); + let hierarchy = get_hierarchy(&world, first_children); + assert_eq!( + hierarchy, + Node::new_with(first_children, vec![Node::new(child1), Node::new(child2)]) + ); + + let root = world + .entity_mut(first_children) + .insert_children(usize::MAX, &[child3, child4]) + .id(); + let hierarchy = get_hierarchy(&world, root); + assert_eq!( + hierarchy, + Node::new_with( + root, + vec![ + Node::new(child1), + Node::new(child2), + Node::new(child3), + Node::new(child4), + ] + ) + ); + } + #[test] fn remove_children() { let mut world = World::new(); diff --git a/crates/bevy_ecs/src/relationship/related_methods.rs b/crates/bevy_ecs/src/relationship/related_methods.rs index b53fd43943..6f314ffc05 100644 --- a/crates/bevy_ecs/src/relationship/related_methods.rs +++ b/crates/bevy_ecs/src/relationship/related_methods.rs @@ -81,7 +81,7 @@ impl<'w> EntityWorldMut<'w> { let id = self.id(); self.world_scope(|world| { for (offset, related) in related.iter().enumerate() { - let index = index + offset; + let index = index.saturating_add(offset); if world .get::(*related) .is_some_and(|relationship| relationship.get() == id) diff --git a/crates/bevy_ecs/src/relationship/relationship_source_collection.rs b/crates/bevy_ecs/src/relationship/relationship_source_collection.rs index c2c9bd94d8..c9a00c8be8 100644 --- a/crates/bevy_ecs/src/relationship/relationship_source_collection.rs +++ b/crates/bevy_ecs/src/relationship/relationship_source_collection.rs @@ -213,15 +213,14 @@ impl OrderedRelationshipSourceCollection for Vec { fn place_most_recent(&mut self, index: usize) { if let Some(entity) = self.pop() { - let index = index.min(self.len().saturating_sub(1)); + let index = index.min(self.len()); self.insert(index, entity); } } fn place(&mut self, entity: Entity, index: usize) { if let Some(current) = <[Entity]>::iter(self).position(|e| *e == entity) { - // The len is at least 1, so the subtraction is safe. - let index = index.min(self.len().saturating_sub(1)); + let index = index.min(self.len()); Vec::remove(self, current); self.insert(index, entity); };