ecs: rename ComMut<T> to Track<T> and fix nested change queries

crates/bevy_ecs/hecs/src/query.rs

@@ -203,6 +203,10 @@ impl<'a, T: Fetch<'a>> Fetch<'a> for TryFetch<T> {
     unsafe fn next(&mut self) -> Option<T::Item> {
         Some(self.0.as_mut()?.next())
     }
+
+    unsafe fn should_skip(&self) -> bool {
+        self.0.as_ref().map_or(false, |fetch| fetch.should_skip())
+    }
 }
 
 /// Query transformer skipping entities that have a `T` component
@@ -258,6 +262,10 @@ impl<'a, T: Component, F: Fetch<'a>> Fetch<'a> for FetchWithout<T, F> {
     unsafe fn next(&mut self) -> F::Item {
         self.0.next()
     }
+
+    unsafe fn should_skip(&self) -> bool {
+        self.0.should_skip()
+    }
 }
 
 /// Query transformer skipping entities that do not have a `T` component
@@ -315,6 +323,10 @@ impl<'a, T: Component, F: Fetch<'a>> Fetch<'a> for FetchWith<T, F> {
     unsafe fn next(&mut self) -> F::Item {
         self.0.next()
     }
+
+    unsafe fn should_skip(&self) -> bool {
+        self.0.should_skip()
+    }
 }
 
 /// A borrow of a `World` sufficient to execute the query `Q`
@@ -536,7 +548,7 @@ impl<Q: Query> ChunkIter<Q> {
                 continue;
             }
 
-            break Some(self.fetch.next())
+            break Some(self.fetch.next());
         }
     }
 }
@@ -636,6 +648,10 @@ macro_rules! tuple_impl {
                 let ($($name,)*) = self;
                 ($($name.next(),)*)
             }
+            
+            unsafe fn should_skip(&self) -> bool {
+                false
+            }
         }
 
         impl<$($name: Query),*> Query for ($($name,)*) {

crates/bevy_ecs/src/lib.rs

@@ -15,7 +15,7 @@ pub mod prelude {
         system::{
             Commands, IntoForEachSystem, IntoQuerySystem, IntoThreadLocalSystem, Query, System,
         },
-        world::{WorldBuilderSource, ComMut},
+        world::{WorldBuilderSource, Track},
         Bundle, Component, Entity, Ref, RefMut, With, Without, World,
     };
 }

crates/bevy_ecs/src/world/component.rs

@@ -7,36 +7,36 @@ use std::{
 };
 
 /// Unique borrow of an entity's component
-pub struct ComMut<'a, T: Component> {
+pub struct Track<'a, T: Component> {
     value: &'a mut T,
     modified: &'a mut bool,
 }
 
-unsafe impl<T: Component> Send for ComMut<'_, T> {}
+unsafe impl<T: Component> Send for Track<'_, T> {}
-unsafe impl<T: Component> Sync for ComMut<'_, T> {}
+unsafe impl<T: Component> Sync for Track<'_, T> {}
 
-impl<'a, T: Component> Deref for ComMut<'a, T> {
+impl<'a, T: Component> Deref for Track<'a, T> {
     type Target = T;
     fn deref(&self) -> &T {
         self.value
     }
 }
 
-impl<'a, T: Component> DerefMut for ComMut<'a, T> {
+impl<'a, T: Component> DerefMut for Track<'a, T> {
     fn deref_mut(&mut self) -> &mut T {
         *self.modified = true;
         self.value
     }
 }
 
-impl<'a, T: Component> HecsQuery for ComMut<'a, T> {
+impl<'a, T: Component> HecsQuery for Track<'a, T> {
-    type Fetch = FetchComMut<T>;
+    type Fetch = FetchTrack<T>;
 }
 #[doc(hidden)]
-pub struct FetchComMut<T>(NonNull<T>, NonNull<bool>);
+pub struct FetchTrack<T>(NonNull<T>, NonNull<bool>);
 
-impl<'a, T: Component> Fetch<'a> for FetchComMut<T> {
+impl<'a, T: Component> Fetch<'a> for FetchTrack<T> {
-    type Item = ComMut<'a, T>;
+    type Item = Track<'a, T>;
 
     fn access(archetype: &Archetype) -> Option<Access> {
         if archetype.has::<T>() {
@@ -63,12 +63,12 @@ impl<'a, T: Component> Fetch<'a> for FetchComMut<T> {
         archetype.release_mut::<T>();
     }
 
-    unsafe fn next(&mut self) -> ComMut<'a, T> {
+    unsafe fn next(&mut self) -> Track<'a, T> {
         let component = self.0.as_ptr();
         let modified = self.1.as_ptr();
         self.0 = NonNull::new_unchecked(component.add(1));
         self.1 = NonNull::new_unchecked(modified.add(1));
-        ComMut {
+        Track {
             value: &mut *component,
             modified: &mut *modified,
         }
@@ -114,7 +114,7 @@ impl<'a, T: Component, F: Fetch<'a>> Fetch<'a> for FetchChanged<T, F> {
 
     unsafe fn should_skip(&self) -> bool {
         // skip if the current item wasn't changed
-        !*self.2.as_ref()
+        !*self.2.as_ref() || self.0.should_skip()
     }
 
     unsafe fn next(&mut self) -> F::Item {
@@ -125,52 +125,64 @@ impl<'a, T: Component, F: Fetch<'a>> Fetch<'a> for FetchChanged<T, F> {
 
 #[cfg(test)]
 mod tests {
-    use crate::{Changed, ComMut};
+    use crate::{Changed, Track};
     use hecs::{Entity, World};
 
     struct A(usize);
-    struct B;
+    struct B(usize);
     struct C;
 
     #[test]
     fn modified_trackers() {
         let mut world = World::default();
-        let e1 = world.spawn((A(0), B));
+        let e1 = world.spawn((A(0), B(0)));
-        let e2 = world.spawn((A(0), B));
+        let e2 = world.spawn((A(0), B(0)));
-        let e3 = world.spawn((A(0), B));
+        let e3 = world.spawn((A(0), B(0)));
         world.spawn((A(0), B));
 
-        for (i, mut a) in world.query::<ComMut<A>>().iter().enumerate() {
+        for (i, mut a) in world.query::<Track<A>>().iter().enumerate() {
             if i % 2 == 0 {
                 a.0 += 1;
             }
         }
 
-        fn get_changed(world: &World) -> Vec<Entity> {
+        fn get_changed_a(world: &World) -> Vec<Entity> {
             world
                 .query::<Changed<A, Entity>>()
                 .iter()
                 .collect::<Vec<Entity>>()
         };
 
-        assert_eq!(get_changed(&world), vec![e1, e3]);
+        assert_eq!(get_changed_a(&world), vec![e1, e3]);
 
         // ensure changing an entity's archetypes also moves its modified state
         world.insert(e1, (C,)).unwrap();
 
-        assert_eq!(get_changed(&world), vec![e3, e1], "changed entities list should not change (although the order will due to archetype moves)");
+        assert_eq!(get_changed_a(&world), vec![e3, e1], "changed entities list should not change (although the order will due to archetype moves)");
 
         // spawning a new A entity should not change existing modified state
         world.insert(e1, (A(0), B)).unwrap();
-        assert_eq!(get_changed(&world), vec![e3, e1], "changed entities list should not change");
+        assert_eq!(
+            get_changed_a(&world),
+            vec![e3, e1],
+            "changed entities list should not change"
+        );
 
         // removing an unchanged entity should not change modified state
         world.despawn(e2).unwrap();
-        assert_eq!(get_changed(&world), vec![e3, e1], "changed entities list should not change");
+        assert_eq!(
+            get_changed_a(&world),
+            vec![e3, e1],
+            "changed entities list should not change"
+        );
 
         // removing a changed entity should remove it from enumeration
         world.despawn(e1).unwrap();
-        assert_eq!(get_changed(&world), vec![e3], "e1 should no longer be returned");
+        assert_eq!(
+            get_changed_a(&world),
+            vec![e3],
+            "e1 should no longer be returned"
+        );
 
         world.clear_trackers();
 
@@ -180,4 +192,26 @@ mod tests {
             .collect::<Vec<Entity>>()
             .is_empty());
     }
+
+    #[test]
+    fn nested_changed_query() {
+        let mut world = World::default();
+        world.spawn((A(0), B(0)));
+        let e2 = world.spawn((A(0), B(0)));
+        world.spawn((A(0), B(0)));
+
+        for mut a in world.query::<Track<A>>().iter() {
+            a.0 += 1;
+        }
+
+        for mut b in world.query::<Track<B>>().iter().skip(1).take(1) {
+            b.0 += 1;
+        }
+
+        let a_b_changed = world
+            .query::<Changed<A, Changed<B, Entity>>>()
+            .iter()
+            .collect::<Vec<Entity>>();
+        assert_eq!(a_b_changed, vec![e2]);
+    }
 }