Bubble sync points if ignore_deferred, do not ignore if target system is exclusive

crates/bevy_ecs/src/schedule/auto_insert_apply_deferred.rs

@@ -82,39 +82,54 @@ impl ScheduleBuildPass for AutoInsertApplyDeferredPass {
 
         // calculate the number of sync points each sync point is from the beginning of the graph
         // use the same sync point if the distance is the same
-        let mut distances: HashMap<usize, Option<u32>> =
+        // distance means here how many sync points are placed between the schedule start and a node
+        // if a sync point is ignored for an edge, add it to a later edge
+        let mut distances_and_pending_sync: HashMap<usize, (u32, bool)> =
             HashMap::with_capacity_and_hasher(topo.len(), Default::default());
         for node in &topo {
-            let add_sync_after = graph.systems[node.index()].get().unwrap().has_deferred();
+            let (node_distance, mut add_sync_after) = distances_and_pending_sync
+                .get(&node.index())
+                .copied()
+                .unwrap_or_default();
+
+            if !add_sync_after {
+                add_sync_after = graph.systems[node.index()].get().unwrap().has_deferred();
+            }
 
             for target in dependency_flattened.neighbors_directed(*node, Direction::Outgoing) {
-                let add_sync_on_edge = add_sync_after
-                    && !is_apply_deferred(graph.systems[target.index()].get().unwrap())
-                    && !self.no_sync_edges.contains(&(*node, target));
-
-                let weight = if add_sync_on_edge { 1 } else { 0 };
-
-                let distance = distances
-                    .get(&target.index())
-                    .unwrap_or(&None)
-                    .or(Some(0))
-                    .map(|distance| {
-                        distance.max(
-                            distances.get(&node.index()).unwrap_or(&None).unwrap_or(0) + weight,
-                        )
-                    });
-
-                distances.insert(target.index(), distance);
-
-                if add_sync_on_edge {
-                    let sync_point =
-                        self.get_sync_point(graph, distances[&target.index()].unwrap());
-                    sync_point_graph.add_edge(*node, sync_point);
-                    sync_point_graph.add_edge(sync_point, target);
-
-                    // edge is now redundant
-                    sync_point_graph.remove_edge(*node, target);
+                let (target_distance, target_pending_sync) = distances_and_pending_sync
+                    .entry(target.index())
+                    .or_default();
+
+                *target_distance = node_distance.max(*target_distance);
+
+                if !add_sync_after {
+                    continue;
+                }
+
+                let target_system = graph.systems[target.index()].get().unwrap();
+
+                if is_apply_deferred(target_system) {
+                    // if target system is `ApplyDeferred` there is no point in adding another sync point
+                    continue;
+                }
+
+                if !target_system.is_exclusive() && self.no_sync_edges.contains(&(*node, target)) {
+                    // `node` has deferred commands to apply, but this edge is a no sync edge
+                    // Mark the `target` node as 'delaying' those commands to a future edge
+                    *target_pending_sync = true;
+                    continue;
                 }
+
+                // add sync point at this edge, target distance may increase
+                *target_distance = (node_distance + 1).max(*target_distance);
+
+                let sync_point = self.get_sync_point(graph, *target_distance);
+                sync_point_graph.add_edge(*node, sync_point);
+                sync_point_graph.add_edge(sync_point, target);
+
+                // edge is now redundant
+                sync_point_graph.remove_edge(*node, target);
             }
         }
 

crates/bevy_ecs/src/schedule/schedule.rs

@@ -2140,6 +2140,61 @@ mod tests {
         assert_eq!(schedule.executable.systems.len(), 5);
     }
 
+    #[test]
+    fn do_not_consider_ignore_deferred_before_exclusive_system() {
+        let mut schedule = Schedule::default();
+        let mut world = World::default();
+        // chain_ignore_deferred adds no sync points usually but an exception is made for exclusive systems
+        schedule.add_systems(
+            (
+                |_: Commands| {},
+                // no sync point is added here because the following system is not exclusive
+                |mut commands: Commands| commands.insert_resource(Resource1),
+                // sync point is added here because the following system is exclusive which expects to see all commands to that point
+                |world: &mut World| assert!(world.contains_resource::<Resource1>()),
+                // no sync point is added here because the following system is not exclusive
+                |_: Commands| {},
+            )
+                .chain_ignore_deferred(),
+        );
+        schedule.run(&mut world);
+
+        assert_eq!(schedule.executable.systems.len(), 5); // 4 systems + 1 sync point
+    }
+
+    #[test]
+    fn bubble_sync_point_through_ignore_deferred_node() {
+        let mut schedule = Schedule::default();
+        let mut world = World::default();
+
+        let insert_resource_config = (
+            // the first system has deferred commands
+            |mut commands: Commands| commands.insert_resource(Resource1),
+            // the second system has no deferred commands
+            || {},
+        )
+            // the first two systems are chained without a sync point in between
+            .chain_ignore_deferred();
+
+        schedule.add_systems(
+            (
+                insert_resource_config,
+                // the third system would panic if the command of the first system was not applied
+                |_: Res<Resource1>| {},
+            )
+                // the third system is chained after the first two, possibly with a sync point in between
+                .chain(),
+        );
+
+        // To add a sync point between the second and third system despite the second having no commands,
+        // the first system has to signal the second system that there are unapplied commands.
+        // With that the second system will add a sync point after it so the third system will find the resource.
+
+        schedule.run(&mut world);
+
+        assert_eq!(schedule.executable.systems.len(), 4); // 3 systems + 1 sync point
+    }
+
     #[test]
     fn disable_auto_sync_points() {
         let mut schedule = Schedule::default();