Skip to content

Commit

Permalink
fix: panic when moving a slice of changes to a change list containing…
Browse files Browse the repository at this point in the history
… newer changes
  • Loading branch information
ten3roberts committed Nov 5, 2024
1 parent 08e9d6c commit e684eb1
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 8 deletions.
40 changes: 32 additions & 8 deletions src/archetype/changes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl ChangeList {
}
}

pub(crate) fn set(&mut self, value: Change) -> &mut Self {
pub(crate) fn set(&mut self, mut new_change: Change) -> &mut Self {
// let orig = self.inner.clone();
let mut insert_point = 0;
let mut i = 0;
Expand All @@ -76,19 +76,19 @@ impl ChangeList {

while i < changes.len() {
let change = &mut changes[i];
let slice = change.slice;
let slice = &mut change.slice;

if slice.start < value.slice.start {
if slice.start < new_change.slice.start {
// TODO: break
insert_point = i + 1;
}

// Merge
match change.tick.cmp(&value.tick) {
match change.tick.cmp(&new_change.tick) {
// Remove the incoming changes range from the existing ones
core::cmp::Ordering::Less => {
// Remove overlaps with existing intervals of previous ticks
match slice.subtract(&value.slice) {
match slice.subtract(&new_change.slice) {
Remainder::NoOverlap => {
i += 1;
}
Expand Down Expand Up @@ -117,7 +117,7 @@ impl ChangeList {
}
core::cmp::Ordering::Equal => {
// Attempt to merge
if let Some(union) = slice.union(&value.slice) {
if let Some(union) = slice.union(&new_change.slice) {
change.slice = union;
// eprintln!("Merge: {slice:?} {value:?} => {change:?}");

Expand All @@ -134,11 +134,35 @@ impl ChangeList {

i += 1;
}
core::cmp::Ordering::Greater => unreachable!(),
// Existing changes are later, don't overwrite
core::cmp::Ordering::Greater => match new_change.slice.subtract(&change.slice) {
Remainder::NoOverlap => {
i += 1;
}
Remainder::FullOverlap => {
// nothing to be done
return self;
}
Remainder::Left(left) => {
new_change.slice = left;
i += 1;
}
Remainder::Right(right) => {
new_change.slice = right;
i += 1;
}
Remainder::Split(left, right) => {
new_change.slice = left;

let tick = new_change.tick;
changes.insert(i + 1, Change::new(right, tick));
i += 2;
}
},
}
}

self.inner.insert(insert_point, value);
self.inner.insert(insert_point, new_change);

// #[cfg(debug_assertions)]
// self.assert_normal(&alloc::format!(
Expand Down
39 changes: 39 additions & 0 deletions tests/change_detection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,3 +275,42 @@ fn added_opt_and() {
assert_eq!(query.borrow(&world).iter().collect_vec(), [(&5, &2)]);
assert_eq!(query.borrow(&world).iter().collect_vec(), []);
}

#[test]
fn move_older_changes() {
component! {
a: (),
b: (),
}

let mut world = World::new();

let id1 = Entity::builder()
.set_default(a())
.set_default(b())
.spawn(&mut world);

let id2 = Entity::builder().set_default(a()).spawn(&mut world);
let mut b_query = Query::new(b().modified());
b_query.borrow(&world);

let mut mark_read = Query::new(a());
let mut query = Query::new((entity_ids()).filtered(a().modified()));

assert_eq!(query.collect_vec(&world), [id2, id1]);

*world.get_mut(id1, a()).unwrap() = ();
mark_read.borrow(&world);

*world.get_mut(id1, b()).unwrap() = ();
mark_read.borrow(&world);

world.get(id1, a()).unwrap();
mark_read.borrow(&world);
*world.get_mut(id2, a()).unwrap() = ();
world.get(id1, a()).unwrap();

world.detach(b().id());

assert_eq!(query.collect_vec(&world), [id2, id1]);
}

0 comments on commit e684eb1

Please sign in to comment.