Skip to content

Commit 6308041

Browse files
committed
Fix Sparse Change Detection (#6896)
# Objective #6547 accidentally broke change detection for SparseSet components by using `Ticks::from_tick_cells` with the wrong argument order. ## Solution Use the right argument order. Add a regression test.
1 parent a5d70b8 commit 6308041

File tree

2 files changed

+85
-1
lines changed

2 files changed

+85
-1
lines changed

crates/bevy_ecs/src/lib.rs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -960,6 +960,90 @@ mod tests {
960960
assert_eq!(get_filtered::<Changed<B>>(&mut world), vec![e4]);
961961
}
962962

963+
#[test]
964+
fn changed_trackers_sparse() {
965+
let mut world = World::default();
966+
let e1 = world.spawn(SparseStored(0)).id();
967+
let e2 = world.spawn(SparseStored(0)).id();
968+
let e3 = world.spawn(SparseStored(0)).id();
969+
world.spawn(SparseStored(0));
970+
971+
world.clear_trackers();
972+
973+
for (i, mut a) in world
974+
.query::<&mut SparseStored>()
975+
.iter_mut(&mut world)
976+
.enumerate()
977+
{
978+
if i % 2 == 0 {
979+
a.0 += 1;
980+
}
981+
}
982+
983+
fn get_filtered<F: ReadOnlyWorldQuery>(world: &mut World) -> Vec<Entity> {
984+
world
985+
.query_filtered::<Entity, F>()
986+
.iter(world)
987+
.collect::<Vec<Entity>>()
988+
}
989+
990+
assert_eq!(
991+
get_filtered::<Changed<SparseStored>>(&mut world),
992+
vec![e1, e3]
993+
);
994+
995+
// ensure changing an entity's archetypes also moves its changed state
996+
world.entity_mut(e1).insert(C);
997+
998+
assert_eq!(get_filtered::<Changed<SparseStored>>(&mut world), vec![e3, e1], "changed entities list should not change (although the order will due to archetype moves)");
999+
1000+
// spawning a new SparseStored entity should not change existing changed state
1001+
world.entity_mut(e1).insert(SparseStored(0));
1002+
assert_eq!(
1003+
get_filtered::<Changed<SparseStored>>(&mut world),
1004+
vec![e3, e1],
1005+
"changed entities list should not change"
1006+
);
1007+
1008+
// removing an unchanged entity should not change changed state
1009+
assert!(world.despawn(e2));
1010+
assert_eq!(
1011+
get_filtered::<Changed<SparseStored>>(&mut world),
1012+
vec![e3, e1],
1013+
"changed entities list should not change"
1014+
);
1015+
1016+
// removing a changed entity should remove it from enumeration
1017+
assert!(world.despawn(e1));
1018+
assert_eq!(
1019+
get_filtered::<Changed<SparseStored>>(&mut world),
1020+
vec![e3],
1021+
"e1 should no longer be returned"
1022+
);
1023+
1024+
world.clear_trackers();
1025+
1026+
assert!(get_filtered::<Changed<SparseStored>>(&mut world).is_empty());
1027+
1028+
let e4 = world.spawn_empty().id();
1029+
1030+
world.entity_mut(e4).insert(SparseStored(0));
1031+
assert_eq!(get_filtered::<Changed<SparseStored>>(&mut world), vec![e4]);
1032+
assert_eq!(get_filtered::<Added<SparseStored>>(&mut world), vec![e4]);
1033+
1034+
world.entity_mut(e4).insert(A(1));
1035+
assert_eq!(get_filtered::<Changed<SparseStored>>(&mut world), vec![e4]);
1036+
1037+
world.clear_trackers();
1038+
1039+
// ensure inserting multiple components set changed state for all components and set added
1040+
// state for non existing components even when changing archetype.
1041+
world.entity_mut(e4).insert(SparseStored(0));
1042+
1043+
assert!(get_filtered::<Added<SparseStored>>(&mut world).is_empty());
1044+
assert_eq!(get_filtered::<Changed<SparseStored>>(&mut world), vec![e4]);
1045+
}
1046+
9631047
#[test]
9641048
fn empty_spawn() {
9651049
let mut world = World::default();

crates/bevy_ecs/src/query/fetch.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T {
771771
.debug_checked_unwrap();
772772
Mut {
773773
value: component.assert_unique().deref_mut(),
774-
ticks: Ticks::from_tick_cells(ticks, fetch.change_tick, fetch.last_change_tick),
774+
ticks: Ticks::from_tick_cells(ticks, fetch.last_change_tick, fetch.change_tick),
775775
}
776776
}
777777
}

0 commit comments

Comments
 (0)