From dce6a9a44086b0676bf27a2ed6c174606ff90899 Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Sun, 26 Mar 2023 20:36:41 -0500
Subject: [PATCH 01/25] Add `Schedule.label`

Needed a way to identify a `Schedule` by `ScheduleLabel` for stepping.
This data wasn't currently stored in the `Schedule`, but only used as a
key in the `Schedules` resource.  That resource (and key) are not
available when stepping is evaluating a `Schedule`, so the label needed
to be added.

Added `label` to `Schedule`, and updated `add_schedule()` functions to
set the label.

I would have preferred to implement this as `Schedule::with_label()`,
and have `add_schedule()` pull the label directly from `Schedule`, but
that's a breaking change for the API.  I feel like this is a good
compromise as it doesn't break the API, but the data still gets where it
needs to be.
---
 crates/bevy_app/src/app.rs               |  6 +++++-
 crates/bevy_ecs/src/schedule/schedule.rs | 14 ++++++++++++++
 crates/bevy_ecs/src/world/mod.rs         |  6 +++++-
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs
index d7a4da63cd1b0..4163a31fff3de 100644
--- a/crates/bevy_app/src/app.rs
+++ b/crates/bevy_app/src/app.rs
@@ -875,7 +875,11 @@ impl App {
     /// # Warning
     /// This method will overwrite any existing schedule at that label.
     /// To avoid this behavior, use the `init_schedule` method instead.
-    pub fn add_schedule(&mut self, label: impl ScheduleLabel, schedule: Schedule) -> &mut Self {
+    pub fn add_schedule<Label>(&mut self, label: Label, mut schedule: Schedule) -> &mut Self
+    where
+        Label: ScheduleLabel + Clone,
+    {
+        schedule.set_label(label.clone());
         let mut schedules = self.world.resource_mut::<Schedules>();
         schedules.insert(label, schedule);
 
diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs
index 303ac8dd29479..82b9895d015fc 100644
--- a/crates/bevy_ecs/src/schedule/schedule.rs
+++ b/crates/bevy_ecs/src/schedule/schedule.rs
@@ -151,6 +151,7 @@ fn make_executor(kind: ExecutorKind) -> Box<dyn SystemExecutor> {
 /// }
 /// ```
 pub struct Schedule {
+    label: Option<BoxedScheduleLabel>,
     graph: ScheduleGraph,
     executable: SystemSchedule,
     executor: Box<dyn SystemExecutor>,
@@ -167,6 +168,7 @@ impl Schedule {
     /// Constructs an empty `Schedule`.
     pub fn new() -> Self {
         Self {
+            label: None,
             graph: ScheduleGraph::new(),
             executable: SystemSchedule::new(),
             executor: make_executor(ExecutorKind::default()),
@@ -174,6 +176,18 @@ impl Schedule {
         }
     }
 
+    /// Get the label for this schedule
+    pub fn label(&self) -> &Option<BoxedScheduleLabel> {
+        &self.label
+    }
+
+    /// Set the label for this schedule
+    pub fn set_label(&mut self, label: impl ScheduleLabel) -> &mut Self {
+        debug_assert!(self.label.is_none());
+        self.label = Some(Box::new(label));
+        self
+    }
+
     /// Add a system to the schedule.
     #[deprecated(since = "0.11.0", note = "please use `add_systems` instead")]
     pub fn add_system<M>(&mut self, system: impl IntoSystemConfigs<M>) -> &mut Self {
diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs
index 288650a1b951c..356f705322263 100644
--- a/crates/bevy_ecs/src/world/mod.rs
+++ b/crates/bevy_ecs/src/world/mod.rs
@@ -1711,7 +1711,11 @@ impl World {
     /// Runs the [`Schedule`] associated with the `label` a single time.
     ///
     /// The [`Schedule`] is fetched from the
-    pub fn add_schedule(&mut self, schedule: Schedule, label: impl ScheduleLabel) {
+    pub fn add_schedule<Label>(&mut self, mut schedule: Schedule, label: Label)
+    where
+        Label: ScheduleLabel + Clone,
+    {
+        schedule.set_label(label.clone());
         let mut schedules = self.resource_mut::<Schedules>();
         schedules.insert(label, schedule);
     }

From 8db83c351d06142f08e8dbeba271870c322fbfdf Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Sun, 26 Mar 2023 22:15:11 -0500
Subject: [PATCH 02/25] WIP: Add bevy_ecs::schedule::stepping

`Stepping` Resource for performing system stepping & breakpoints.
---
 crates/bevy_app/src/main_schedule.rs     |   4 +-
 crates/bevy_ecs/src/schedule/mod.rs      |   1 +
 crates/bevy_ecs/src/schedule/schedule.rs |  43 ++
 crates/bevy_ecs/src/schedule/stepping.rs | 611 +++++++++++++++++++++++
 4 files changed, 658 insertions(+), 1 deletion(-)
 create mode 100644 crates/bevy_ecs/src/schedule/stepping.rs

diff --git a/crates/bevy_app/src/main_schedule.rs b/crates/bevy_app/src/main_schedule.rs
index 2042b9e01f4e8..ef449999b990b 100644
--- a/crates/bevy_app/src/main_schedule.rs
+++ b/crates/bevy_app/src/main_schedule.rs
@@ -154,6 +154,8 @@ pub struct MainSchedulePlugin;
 
 impl Plugin for MainSchedulePlugin {
     fn build(&self, app: &mut App) {
+        use bevy_ecs::schedule::Stepping;
+
         // simple "facilitator" schedules benefit from simpler single threaded scheduling
         let mut main_schedule = Schedule::new();
         main_schedule.set_executor_kind(ExecutorKind::SingleThreaded);
@@ -163,6 +165,6 @@ impl Plugin for MainSchedulePlugin {
         app.add_schedule(Main, main_schedule)
             .add_schedule(RunFixedUpdateLoop, fixed_update_loop_schedule)
             .init_resource::<MainScheduleOrder>()
-            .add_systems(Main, Main::run_main);
+            .add_systems(Main, (Stepping::begin_frame, Main::run_main));
     }
 }
diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs
index 05aaba9f8b33c..61b48051d4eaa 100644
--- a/crates/bevy_ecs/src/schedule/mod.rs
+++ b/crates/bevy_ecs/src/schedule/mod.rs
@@ -6,6 +6,7 @@ mod graph_utils;
 mod schedule;
 mod set;
 mod state;
+mod stepping;
 
 pub use self::condition::*;
 pub use self::config::*;
diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs
index 82b9895d015fc..05687e3466e01 100644
--- a/crates/bevy_ecs/src/schedule/schedule.rs
+++ b/crates/bevy_ecs/src/schedule/schedule.rs
@@ -23,6 +23,8 @@ use crate::{
     world::World,
 };
 
+pub use stepping::Stepping;
+
 /// Resource that stores [`Schedule`]s mapped to [`ScheduleLabel`]s.
 #[derive(Default, Resource)]
 pub struct Schedules {
@@ -280,6 +282,11 @@ impl Schedule {
         &mut self.graph
     }
 
+    /// Returns the [`SystemSchedule`].
+    pub fn executable(&self) -> &SystemSchedule {
+        &self.executable
+    }
+
     /// Iterates the change ticks of all systems in the schedule and clamps any older than
     /// [`MAX_CHANGE_AGE`](crate::change_detection::MAX_CHANGE_AGE).
     /// This prevents overflow and thus prevents false positives.
@@ -314,6 +321,36 @@ impl Schedule {
             system.apply_buffers(world);
         }
     }
+
+    /// Returns an iterator over all systems in this schedule.
+    ///
+    /// Note: this method will return [`ScheduleNotInitialized`] if the
+    /// schedule has never been initialized or run.
+    pub fn systems(
+        &self,
+    ) -> Result<impl Iterator<Item = (NodeId, &BoxedSystem)> + Sized, ScheduleNotInitialized> {
+        if !self.executor_initialized {
+            return Err(ScheduleNotInitialized);
+        }
+
+        let iter = self
+            .executable
+            .system_ids
+            .iter()
+            .zip(&self.executable.systems)
+            .map(|(node_id, system)| (*node_id, system));
+
+        Ok(iter)
+    }
+
+    /// Returns the number of systems in this schedule.
+    pub fn systems_len(&self) -> usize {
+        if !self.executor_initialized {
+            self.graph.systems.len()
+        } else {
+            self.executable.systems.len()
+        }
+    }
 }
 
 /// A directed acyclic graph structure.
@@ -1576,3 +1613,9 @@ impl ScheduleBuildSettings {
         }
     }
 }
+
+/// Error to denote that [`Schedule::initialize`] or [`Schedule::run`] has not yet been called for
+/// this schedule.
+#[derive(Error, Debug)]
+#[error("executable schedule has not been built")]
+pub struct ScheduleNotInitialized;
diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs
new file mode 100644
index 0000000000000..e0aa00a296b3a
--- /dev/null
+++ b/crates/bevy_ecs/src/schedule/stepping.rs
@@ -0,0 +1,611 @@
+#![allow(dead_code, unused_mut)]
+#![allow(unused_variables)]
+use fixedbitset::FixedBitSet;
+use std::any::TypeId;
+use std::collections::HashMap;
+
+use crate::{
+    schedule::{BoxedScheduleLabel, IntoSystemConfigs, NodeId, Schedule, ScheduleLabel},
+    system::{IntoSystem, ResMut, Resource, System},
+};
+
+use crate as bevy_ecs;
+
+#[derive(Resource, Default)]
+pub struct Stepping {
+    /// [`ScheduleState`] for each [`Schedule`] with stepping enabled
+    schedule_states: HashMap<BoxedScheduleLabel, ScheduleState>,
+
+    /// Action to perform during this frame
+    action: StepAction,
+
+    /// pending action to be applied at the start of the next frame
+    pending_action: Option<StepAction>,
+
+    /// changes to apply to systems at the start of the next frame
+    system_behavior_updates: Vec<SystemBehaviorUpdate>,
+}
+
+impl Stepping {
+    pub fn new() -> Self {
+        Stepping::default()
+    }
+
+    /// Enable stepping for the provided schedule
+    pub fn add_schedule<Label>(&mut self, schedule: Label) -> &mut Self
+    where
+        Label: ScheduleLabel + Clone,
+    {
+        self.schedule_states
+            .insert(Box::new(schedule), ScheduleState::default());
+        self
+    }
+
+    /// disable stepping for the provided schedule
+    pub fn remove_schedule(&mut self, schedule: impl ScheduleLabel) -> &mut Self {
+        self
+    }
+
+    /// Begin stepping at the start of the next frame
+    pub fn enable(&mut self) -> &mut Self {
+        #[cfg(debug_assertions)]
+        for (_, state) in self.schedule_states.iter() {
+            assert_eq!(state.next, 0);
+        }
+
+        self.action = StepAction::Waiting;
+        self
+    }
+
+    /// Disable stepping, resume normal systems execution
+    pub fn disable(&mut self) -> &mut Self {
+        self.clear_schedule_progress();
+        self.action = StepAction::RunAll;
+        self
+    }
+
+    /// Ensure these systems always run when stepping is enabled
+    pub fn always_run<Marker>(
+        &mut self,
+        schedule: impl ScheduleLabel,
+        system: impl IntoSystem<(), (), Marker>,
+    ) -> &mut Self {
+        self.system_behavior_updates.push(SystemBehaviorUpdate {
+            schedule: Box::new(schedule),
+            system_type: IntoSystem::into_system(system).type_id(),
+            behavior: StepBehavior::AlwaysRun,
+        });
+
+        self
+    }
+
+    /// Ensure this system never runs when stepping is enabled
+    pub fn never_run<Marker>(
+        &mut self,
+        schedule: impl ScheduleLabel,
+        system: impl IntoSystem<(), (), Marker>,
+    ) -> &mut Self {
+        self.system_behavior_updates.push(SystemBehaviorUpdate {
+            schedule: Box::new(schedule),
+            system_type: IntoSystem::into_system(system).type_id(),
+            behavior: StepBehavior::NeverRun,
+        });
+
+        self
+    }
+
+    /// Add a breakpoint for system
+    pub fn set_breakpoint<Marker>(
+        &mut self,
+        schedule: impl ScheduleLabel,
+        system: impl IntoSystem<(), (), Marker>,
+    ) -> &mut Self {
+        self.system_behavior_updates.push(SystemBehaviorUpdate {
+            schedule: Box::new(schedule),
+            system_type: IntoSystem::into_system(system).type_id(),
+            behavior: StepBehavior::Break,
+        });
+
+        self
+    }
+
+    /// Clear a breakpoint for the system
+    pub fn clear_breakpoint<M>(
+        &mut self,
+        schedule: impl ScheduleLabel,
+        system: impl IntoSystemConfigs<M>,
+    ) -> &mut Self {
+        self
+    }
+
+    pub fn build_skip_list(&mut self, schedule: &Schedule) -> Option<FixedBitSet> {
+        if self.action == StepAction::RunAll {
+            return None;
+        }
+        let state = match schedule.label() {
+            Some(label) => self.schedule_states.get_mut(label)?,
+            None => return None,
+        };
+
+        let (list, wait) = state.progress(schedule, self.action);
+        if wait {
+            println!(
+                "build_skip_list({:?}): state {:?} -> {:?}",
+                schedule.label(),
+                self.action,
+                StepAction::Waiting
+            );
+            self.action = StepAction::Waiting;
+        }
+        Some(list)
+    }
+
+    /// continue stepping until the end of the frame or a breakpoint is hit
+    pub fn r#continue(&mut self) {}
+
+    /// run the next system
+    pub fn step(&mut self) {
+        self.pending_action = Some(StepAction::Step);
+    }
+
+    fn next_frame(&mut self) {
+        match self.action {
+            StepAction::RunAll => {
+                if self.pending_action.is_none() {
+                    return;
+                }
+            }
+            StepAction::Step => self.clear_schedule_progress(),
+            StepAction::Continue => (),
+            StepAction::Waiting => (),
+        }
+        if let Some(action) = self.pending_action {
+            println!("next_frame(): state {:?} -> {:?}", self.action, action);
+            self.action = action;
+            self.pending_action = None;
+        }
+
+        for update in self.system_behavior_updates.drain(..) {
+            let state = self
+                .schedule_states
+                .get_mut(&update.schedule)
+                .unwrap_or_else(|| {
+                    panic!(
+                        "stepping is not enabled for schedule {:?}; use
+                `.add_stepping({:?})` to enable stepping",
+                        update.schedule, update.schedule
+                    )
+                });
+            state.set_behavior(update.system_type, update.behavior);
+        }
+    }
+
+    pub fn begin_frame(stepping: Option<ResMut<Self>>) {
+        if let Some(mut stepping) = stepping {
+            stepping.next_frame();
+        }
+    }
+
+    /// clears all schedule progress, starting a new stepping frame.
+    fn clear_schedule_progress(&mut self) {
+        println!("clearing schedule progress");
+        // clear any progress we've made in the stepping frame
+        for (_, state) in self.schedule_states.iter_mut() {
+            state.next = 0;
+        }
+    }
+}
+
+struct SystemBehaviorUpdate {
+    schedule: BoxedScheduleLabel,
+    system_type: TypeId,
+    behavior: StepBehavior,
+}
+
+#[derive(Default)]
+struct ScheduleState {
+    /// Index of the next system to run in the system schedule; if set to
+    /// greater than the number of systems in the schedule, then don't run
+    /// any systems.
+    /// NOTE: [`AlwaysRun`] systems ignore this value and always run
+    next: usize,
+
+    /// per-system [`StepBehavior`]
+    systems: HashMap<NodeId, StepBehavior>,
+
+    /// changes to system behavior that should be applied the next time
+    /// [`ScheduleState::progress()`] is called
+    system_updates: HashMap<TypeId, StepBehavior>,
+}
+
+impl ScheduleState {
+    // set the stepping behavior for a system in this schedule
+    fn set_behavior(&mut self, system_type: TypeId, behavior: StepBehavior) {
+        self.system_updates.insert(system_type, behavior);
+    }
+
+    // apply system behavior updates by looking up the node id of the system in
+    // the schedule, and updating `systems`
+    fn update_behaviors(&mut self, schedule: &Schedule) {
+        for (node_id, system) in schedule.systems().unwrap() {
+            if let Some(behavior) = self.system_updates.get(&system.type_id()) {
+                println!(
+                    "update_behaviors(): {} ({:?}) -- {:?}",
+                    system.name(),
+                    node_id,
+                    behavior
+                );
+                self.systems.insert(node_id, *behavior);
+            }
+        }
+        self.system_updates.clear();
+
+        println!("update_behaviors(): {:?}", self.systems);
+    }
+
+    // progress the schedule for the given action type, returning a skip list,
+    // and if the caller should update state to waiting
+    fn progress(&mut self, schedule: &Schedule, mut action: StepAction) -> (FixedBitSet, bool) {
+        // Now that we have the schedule, apply any pending system behavior
+        // updates.  The schedule is required to map from system `TypeId` to
+        // `NodeId`.
+        if !self.system_updates.is_empty() {
+            self.update_behaviors(schedule);
+        }
+
+        let systems_schedule = schedule.executable();
+
+        // let skip = FixedBitSet::with_capacity(systems_schedule.systems.len());
+        let skip = FixedBitSet::with_capacity(schedule.systems_len());
+        match action {
+            StepAction::RunAll => unreachable!(),
+            StepAction::Waiting => self.skip_list_waiting(schedule),
+            StepAction::Continue => self.skip_list_continue(schedule),
+            StepAction::Step => self.skip_list_step(schedule),
+        }
+    }
+
+    fn skip_list_waiting(&mut self, schedule: &Schedule) -> (FixedBitSet, bool) {
+        let mut skip = FixedBitSet::with_capacity(schedule.systems_len());
+
+        for (i, (node_id, system)) in schedule.systems().unwrap().enumerate() {
+            println!(
+                "skip_list_waiting(): {} ({:?}) -- {:?}",
+                system.name(),
+                node_id,
+                self.systems.get(&node_id),
+            );
+
+            let mut advance = false;
+            match self
+                .systems
+                .get(&node_id)
+                .unwrap_or(&StepBehavior::Continue)
+            {
+                StepBehavior::AlwaysRun => advance = true,
+                StepBehavior::NeverRun => {
+                    advance = true;
+                    skip.insert(i);
+                }
+                StepBehavior::Continue | StepBehavior::Break => skip.insert(i),
+            }
+
+            if advance && self.next == i {
+                self.next += 1;
+            }
+        }
+
+        (skip, false)
+    }
+
+    fn skip_list_step(&mut self, schedule: &Schedule) -> (FixedBitSet, bool) {
+        let mut skip = FixedBitSet::with_capacity(schedule.systems_len());
+        let mut stepped = false;
+        for (i, (node_id, system)) in schedule.systems().unwrap().enumerate() {
+            println!(
+                "skip_list_step(): {} ({:?}) -- {:?}",
+                system.name(),
+                node_id,
+                self.systems.get(&node_id),
+            );
+
+            let mut advance = false;
+            match self
+                .systems
+                .get(&node_id)
+                .unwrap_or(&StepBehavior::Continue)
+            {
+                StepBehavior::AlwaysRun => advance = true,
+                StepBehavior::NeverRun => {
+                    advance = true;
+                    skip.insert(i);
+                }
+                StepBehavior::Continue | StepBehavior::Break => {
+                    if i < self.next || stepped {
+                        skip.insert(i);
+                    } else {
+                        advance = true;
+                        stepped = true;
+                    }
+                }
+            }
+
+            if advance && self.next == i {
+                self.next += 1;
+            }
+        }
+
+        (skip, stepped)
+    }
+
+    fn skip_list_continue(&mut self, schedule: &Schedule) -> (FixedBitSet, bool) {
+        let mut skip = FixedBitSet::with_capacity(schedule.systems_len());
+        for (pos, node_id) in schedule.executable().system_ids.iter().enumerate() {
+            use StepBehavior::*;
+            match self.systems.get(node_id).unwrap_or(&Continue) {
+                AlwaysRun => (),
+                NeverRun => (),
+                Continue => (),
+                Break => (),
+            }
+        }
+        (skip, false)
+    }
+}
+
+#[derive(Debug, Copy, Clone)]
+enum StepBehavior {
+    AlwaysRun,
+    NeverRun,
+    Continue,
+    Break,
+}
+
+#[derive(Debug, Default, PartialEq, Eq, Copy, Clone)]
+enum StepAction {
+    /// Stepping is disabled; run all systems
+    #[default]
+    RunAll,
+
+    /// Stepping is enabled, but we're only running required systems this frame
+    Waiting,
+
+    /// Stepping is enabled; run all systems until the end of the frame, or
+    /// until we encounter a system marked with [`SystemBehavior::Break`].
+    Continue,
+
+    /// stepping is enabled; only run the next system in our step list
+    Step,
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::prelude::*;
+    use crate::{schedule::ScheduleLabel, world::World};
+
+    pub use crate as bevy_ecs;
+
+    #[derive(ScheduleLabel, Clone, Debug, PartialEq, Eq, Hash)]
+    struct TestSchedule;
+
+    fn first_system() {}
+    fn second_system() {}
+
+    fn setup() -> (Schedule, World) {
+        let mut world = World::new();
+        let mut schedule = Schedule::default();
+        schedule
+            .set_label(TestSchedule)
+            .add_systems((first_system, second_system).chain());
+        schedule.initialize(&mut world).unwrap();
+        (schedule, world)
+    }
+
+    #[test]
+    fn stepping_disabled() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping.disable().add_schedule(TestSchedule);
+
+        // returns None when stepping isn't enabled
+        stepping.disable();
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule);
+        assert!(skip.is_none());
+    }
+
+    // if build_skip_list is caled with an unknown schedule, it will return none
+    #[test]
+    fn unknown_schedule() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping.enable();
+
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule);
+        assert!(skip.is_none());
+    }
+
+    #[test]
+    fn step() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping.enable().add_schedule(TestSchedule);
+
+        // stepping while waiting should result in skipping all systems
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.count_ones(..), schedule.executable().systems.len());
+
+        // single stepping should result in only running the first system
+        println!("step 1");
+        stepping.step();
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.len(), schedule.executable().systems.len());
+        assert!(!skip.contains(0));
+        assert!(skip.contains(1));
+
+        // call it again without issuing another `step()` call, and we should
+        // skip all the systems
+        println!("wait");
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.count_ones(..), schedule.executable().systems.len());
+
+        // doing it again should step the second system
+        println!("step 2");
+        stepping.step();
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.len(), schedule.executable().systems.len());
+        assert!(skip.contains(0));
+        assert!(!skip.contains(1));
+
+        // Now that we've run all of the systems in our stepping schedules, we
+        // want to wrap back around to the first system.  However, stepping only
+        // detects that we need to wrap back around when it runs a system step
+        // pass, but no schedule is able to run a system.
+        //
+        // Let's verify that behavior now first by trying to step again, to see
+        // that all systems are skipped.
+        println!("step 3 -- wrap detect");
+        stepping.step();
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.count_ones(..), schedule.executable().systems.len());
+
+        // Now, call `next_frame()` again, then build the skip list.  This list
+        // should have wrapped, and allow only the first system to run.
+        println!("step 3 -- wrapped");
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.len(), schedule.executable().systems.len());
+        assert!(!skip.contains(0));
+        assert!(skip.contains(1));
+    }
+
+    #[test]
+    fn step_always_run() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping.enable().add_schedule(TestSchedule);
+
+        // Let's make it so the first system always runs
+        stepping.always_run(TestSchedule, first_system);
+
+        // when we're stopped/waiting, only the second system should be in the
+        // skip list, as the first system should always run.
+        println!("next frame; waiting");
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.len(), schedule.executable().systems.len());
+        assert!(!skip.contains(0));
+        assert!(skip.contains(1));
+
+        // let's step and verify neither system is skipped
+        println!("next frame; step");
+        stepping.step();
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.len(), schedule.executable().systems.len());
+        assert!(!skip.contains(0));
+        assert!(!skip.contains(1));
+
+        // verify wrapping occurrs properly
+        println!("next frame; step, detect wrapping");
+        stepping.step();
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.len(), schedule.executable().systems.len());
+        assert!(!skip.contains(0));
+        assert!(skip.contains(1));
+
+        println!("next frame; wrapped step");
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.len(), schedule.executable().systems.len());
+        assert!(!skip.contains(0));
+        assert!(!skip.contains(1));
+    }
+
+    #[test]
+    fn step_never_run() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping.enable().add_schedule(TestSchedule);
+
+        stepping.never_run(TestSchedule, first_system);
+
+        println!("next frame; waiting");
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.len(), schedule.executable().systems.len());
+        assert!(skip.contains(0));
+        assert!(skip.contains(1));
+
+        println!("next frame; step");
+        stepping.step();
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.len(), schedule.executable().systems.len());
+        assert!(skip.contains(0));
+        assert!(!skip.contains(1));
+
+        println!("next frame; step, detect wrapping");
+        stepping.step();
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.len(), schedule.executable().systems.len());
+        assert!(skip.contains(0));
+        assert!(skip.contains(1));
+
+        println!("next frame; wrapped step");
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.len(), schedule.executable().systems.len());
+        assert!(skip.contains(0));
+        assert!(!skip.contains(1));
+    }
+
+    // This test just verifies that stepping stops on breakpoint systems, then
+    // steps past them
+    #[test]
+    fn step_breakpoint() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping.enable().add_schedule(TestSchedule);
+
+        stepping.set_breakpoint(TestSchedule, first_system);
+
+        // stepping while waiting should result in skipping all systems
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.count_ones(..), schedule.executable().systems.len());
+
+        // single stepping should result in only running the first system
+        println!("step 1");
+        stepping.step();
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.len(), schedule.executable().systems.len());
+        assert!(!skip.contains(0));
+        assert!(skip.contains(1));
+
+        // now the second system should run
+        println!("next frame: step");
+        stepping.step();
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.len(), schedule.executable().systems.len());
+        assert!(skip.contains(0));
+        assert!(!skip.contains(1));
+    }
+}

From 4ac6dfabd46d9f30890370616b3d2da95807c0bc Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Mon, 27 Mar 2023 21:36:54 -0500
Subject: [PATCH 03/25] Update `ScheduleExecutor` for stepping

A list of systems to skip was added to `ScheduleExecutor::run()`.  All
executors updated to properly apply the list.  Tests added to verify all
executors apply the skip list.
---
 crates/bevy_ecs/src/schedule/executor/mod.rs  |  7 +-
 .../src/schedule/executor/multi_threaded.rs   | 30 ++++++-
 .../bevy_ecs/src/schedule/executor/simple.rs  | 14 +++-
 .../src/schedule/executor/single_threaded.rs  | 14 +++-
 crates/bevy_ecs/src/schedule/mod.rs           | 79 +++++++++++++++++++
 crates/bevy_ecs/src/schedule/schedule.rs      |  8 +-
 crates/bevy_ecs/src/schedule/stepping.rs      | 11 +--
 7 files changed, 153 insertions(+), 10 deletions(-)

diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs
index 5de07a52f5493..0ce19c90677d7 100644
--- a/crates/bevy_ecs/src/schedule/executor/mod.rs
+++ b/crates/bevy_ecs/src/schedule/executor/mod.rs
@@ -18,7 +18,12 @@ use crate::{
 pub(super) trait SystemExecutor: Send + Sync {
     fn kind(&self) -> ExecutorKind;
     fn init(&mut self, schedule: &SystemSchedule);
-    fn run(&mut self, schedule: &mut SystemSchedule, world: &mut World);
+    fn run(
+        &mut self,
+        schedule: &mut SystemSchedule,
+        skip_systems: Option<FixedBitSet>,
+        world: &mut World,
+    );
     fn set_apply_final_buffers(&mut self, value: bool);
 }
 
diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
index 80c6f27d2e3d6..ac008106f98bc 100644
--- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
+++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
@@ -146,7 +146,12 @@ impl SystemExecutor for MultiThreadedExecutor {
         self.num_dependencies_remaining = Vec::with_capacity(sys_count);
     }
 
-    fn run(&mut self, schedule: &mut SystemSchedule, world: &mut World) {
+    fn run(
+        &mut self,
+        schedule: &mut SystemSchedule,
+        skip_systems: Option<FixedBitSet>,
+        world: &mut World,
+    ) {
         // reset counts
         let num_systems = schedule.systems.len();
         if num_systems == 0 {
@@ -164,6 +169,29 @@ impl SystemExecutor for MultiThreadedExecutor {
             }
         }
 
+        // If stepping is enabled, make sure we skip those systems that should
+        // not be run.
+        if let Some(mut skipped_systems) = skip_systems {
+            // mark skipped systems as completed
+            self.completed_systems |= &skipped_systems;
+            self.num_completed_systems = self.completed_systems.count_ones(..);
+
+            // signal the dependencies for each of the skipped systems, as
+            // though they had run
+            for system_index in skipped_systems.ones() {
+                self.signal_dependents(system_index);
+            }
+
+            // Finally, we need to clear all skipped systems from the ready
+            // list.
+            //
+            // We invert the skipped system mask to get the list of systems
+            // that should be run.  Then we bitwise AND it with the ready list,
+            // resulting in a list of ready systems that aren't skipped.
+            skipped_systems.toggle_range(..);
+            self.ready_systems &= skipped_systems;
+        }
+
         let thread_executor = world
             .get_resource::<MainThreadExecutor>()
             .map(|e| e.0.clone());
diff --git a/crates/bevy_ecs/src/schedule/executor/simple.rs b/crates/bevy_ecs/src/schedule/executor/simple.rs
index 2aaf1777ed033..5411222b915f8 100644
--- a/crates/bevy_ecs/src/schedule/executor/simple.rs
+++ b/crates/bevy_ecs/src/schedule/executor/simple.rs
@@ -33,7 +33,19 @@ impl SystemExecutor for SimpleExecutor {
         self.completed_systems = FixedBitSet::with_capacity(sys_count);
     }
 
-    fn run(&mut self, schedule: &mut SystemSchedule, world: &mut World) {
+    fn run(
+        &mut self,
+        schedule: &mut SystemSchedule,
+        skip_systems: Option<FixedBitSet>,
+        world: &mut World,
+    ) {
+        // If stepping is enabled, make sure we skip those systems that should
+        // not be run.
+        if let Some(skipped_systems) = skip_systems {
+            // mark skipped systems as completed
+            self.completed_systems |= &skipped_systems;
+        }
+
         for system_index in 0..schedule.systems.len() {
             #[cfg(feature = "trace")]
             let name = schedule.systems[system_index].name();
diff --git a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs
index 61b4520684e68..0ce4a0e257c86 100644
--- a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs
+++ b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs
@@ -43,7 +43,19 @@ impl SystemExecutor for SingleThreadedExecutor {
         self.unapplied_systems = FixedBitSet::with_capacity(sys_count);
     }
 
-    fn run(&mut self, schedule: &mut SystemSchedule, world: &mut World) {
+    fn run(
+        &mut self,
+        schedule: &mut SystemSchedule,
+        skip_systems: Option<FixedBitSet>,
+        world: &mut World,
+    ) {
+        // If stepping is enabled, make sure we skip those systems that should
+        // not be run.
+        if let Some(skipped_systems) = skip_systems {
+            // mark skipped systems as completed
+            self.completed_systems |= &skipped_systems;
+        }
+
         for system_index in 0..schedule.systems.len() {
             #[cfg(feature = "trace")]
             let name = schedule.systems[system_index].name();
diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs
index 61b48051d4eaa..a3992183e945f 100644
--- a/crates/bevy_ecs/src/schedule/mod.rs
+++ b/crates/bevy_ecs/src/schedule/mod.rs
@@ -709,4 +709,83 @@ mod tests {
             assert!(matches!(result, Err(ScheduleBuildError::Ambiguity)));
         }
     }
+
+    mod stepping {
+        use super::*;
+
+        #[derive(ScheduleLabel, Clone, Debug, PartialEq, Eq, Hash)]
+        pub struct TestSchedule;
+
+        /// verify the [`SimpleExecutor`] supports stepping
+        #[test]
+        fn simple_executor() {
+            // create a test schedule
+            let mut schedule = Schedule::default();
+            schedule
+                .set_label(TestSchedule)
+                .set_executor_kind(ExecutorKind::Simple)
+                .add_systems(|| panic!("test failed; system should not run"));
+
+            // Add our schedule to stepping & and enable stepping; this should
+            // prevent any systems in the schedule from running
+            let mut stepping = Stepping::default();
+            stepping.add_schedule(TestSchedule).enable().next_frame();
+
+            // create a world, and add the stepping resource
+            let mut world = World::default();
+            world.insert_resource(stepping);
+
+            // now run the schedule; this will panic if the executor doesn't
+            // handle stepping
+            schedule.run(&mut world);
+        }
+
+        /// verify the [`SingleThreadedExecutor`] supports stepping
+        #[test]
+        fn single_threaded_executor() {
+            // create a test schedule
+            let mut schedule = Schedule::default();
+            schedule
+                .set_label(TestSchedule)
+                .set_executor_kind(ExecutorKind::SingleThreaded)
+                .add_systems(|| panic!("test failed; system should not run"));
+
+            // Add our schedule to stepping & and enable stepping; this should
+            // prevent any systems in the schedule from running
+            let mut stepping = Stepping::default();
+            stepping.add_schedule(TestSchedule).enable().next_frame();
+
+            // create a world, and add the stepping resource
+            let mut world = World::default();
+            world.insert_resource(stepping);
+
+            // now run the schedule; this will panic if the executor doesn't
+            // handle stepping
+            schedule.run(&mut world);
+        }
+
+        /// verify the [`MultiThreadedExecutor`] supports stepping
+        #[test]
+        fn multi_threaded_executor() {
+            // create a test schedule
+            let mut schedule = Schedule::default();
+            schedule
+                .set_label(TestSchedule)
+                .set_executor_kind(ExecutorKind::MultiThreaded)
+                .add_systems(|| panic!("test failed; system should not run"));
+
+            // Add our schedule to stepping & and enable stepping; this should
+            // prevent any systems in the schedule from running
+            let mut stepping = Stepping::default();
+            stepping.add_schedule(TestSchedule).enable().next_frame();
+
+            // create a world, and add the stepping resource
+            let mut world = World::default();
+            world.insert_resource(stepping);
+
+            // now run the schedule; this will panic if the executor doesn't
+            // handle stepping
+            schedule.run(&mut world);
+        }
+    }
 }
diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs
index 05687e3466e01..d70f8a0dfe2a2 100644
--- a/crates/bevy_ecs/src/schedule/schedule.rs
+++ b/crates/bevy_ecs/src/schedule/schedule.rs
@@ -248,7 +248,13 @@ impl Schedule {
     pub fn run(&mut self, world: &mut World) {
         world.check_change_ticks();
         self.initialize(world).unwrap_or_else(|e| panic!("{e}"));
-        self.executor.run(&mut self.executable, world);
+
+        let skip_systems = match world.get_resource_mut::<Stepping>() {
+            None => None,
+            Some(mut stepping) => stepping.build_skip_list(self),
+        };
+
+        self.executor.run(&mut self.executable, skip_systems, world);
     }
 
     /// Initializes any newly-added systems and conditions, rebuilds the executable schedule,
diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs
index e0aa00a296b3a..aca0423a446f3 100644
--- a/crates/bevy_ecs/src/schedule/stepping.rs
+++ b/crates/bevy_ecs/src/schedule/stepping.rs
@@ -53,14 +53,14 @@ impl Stepping {
             assert_eq!(state.next, 0);
         }
 
-        self.action = StepAction::Waiting;
+        self.pending_action = Some(StepAction::Waiting);
         self
     }
 
     /// Disable stepping, resume normal systems execution
     pub fn disable(&mut self) -> &mut Self {
         self.clear_schedule_progress();
-        self.action = StepAction::RunAll;
+        self.pending_action = Some(StepAction::RunAll);
         self
     }
 
@@ -144,11 +144,12 @@ impl Stepping {
     pub fn r#continue(&mut self) {}
 
     /// run the next system
-    pub fn step(&mut self) {
+    pub fn step(&mut self) -> &mut Self {
         self.pending_action = Some(StepAction::Step);
+        self
     }
 
-    fn next_frame(&mut self) {
+    pub(crate) fn next_frame(&mut self) {
         match self.action {
             StepAction::RunAll => {
                 if self.pending_action.is_none() {
@@ -379,7 +380,7 @@ enum StepAction {
 }
 
 #[cfg(test)]
-mod test {
+mod tests {
     use super::*;
     use crate::prelude::*;
     use crate::{schedule::ScheduleLabel, world::World};

From 6714bcf2be78ab43cebd71543a5055da7574fb86 Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Tue, 28 Mar 2023 21:29:53 -0500
Subject: [PATCH 04/25] Unify `Stepping` updates into single enum

---
 crates/bevy_ecs/src/schedule/stepping.rs | 284 ++++++++++++++---------
 1 file changed, 171 insertions(+), 113 deletions(-)

diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs
index aca0423a446f3..3b248e8917497 100644
--- a/crates/bevy_ecs/src/schedule/stepping.rs
+++ b/crates/bevy_ecs/src/schedule/stepping.rs
@@ -5,12 +5,50 @@ use std::any::TypeId;
 use std::collections::HashMap;
 
 use crate::{
-    schedule::{BoxedScheduleLabel, IntoSystemConfigs, NodeId, Schedule, ScheduleLabel},
+    schedule::{BoxedScheduleLabel, NodeId, Schedule, ScheduleLabel},
     system::{IntoSystem, ResMut, Resource, System},
 };
 
 use crate as bevy_ecs;
 
+#[derive(Debug, Default, PartialEq, Eq, Copy, Clone)]
+enum StepAction {
+    /// Stepping is disabled; run all systems
+    #[default]
+    RunAll,
+
+    /// Stepping is enabled, but we're only running required systems this frame
+    Waiting,
+
+    /// Stepping is enabled; run all systems until the end of the frame, or
+    /// until we encounter a system marked with [`SystemBehavior::Break`].
+    Continue,
+
+    /// stepping is enabled; only run the next system in our step list
+    Step,
+}
+
+#[derive(Debug, Copy, Clone)]
+enum StepBehavior {
+    AlwaysRun,
+    NeverRun,
+    Continue,
+    Break,
+}
+
+enum SystemIdentifier {
+    Type(TypeId),
+    Node(NodeId),
+}
+
+enum Update {
+    Action(StepAction),
+    AddSchedule(BoxedScheduleLabel),
+    RemoveSchedule(BoxedScheduleLabel),
+    SetBehavior(BoxedScheduleLabel, SystemIdentifier, StepBehavior),
+    ClearBehavior(BoxedScheduleLabel, SystemIdentifier),
+}
+
 #[derive(Resource, Default)]
 pub struct Stepping {
     /// [`ScheduleState`] for each [`Schedule`] with stepping enabled
@@ -19,11 +57,8 @@ pub struct Stepping {
     /// Action to perform during this frame
     action: StepAction,
 
-    /// pending action to be applied at the start of the next frame
-    pending_action: Option<StepAction>,
-
-    /// changes to apply to systems at the start of the next frame
-    system_behavior_updates: Vec<SystemBehaviorUpdate>,
+    /// Updates to stepping state to be applied at the start of the next frame
+    updates: Vec<Update>,
 }
 
 impl Stepping {
@@ -36,31 +71,38 @@ impl Stepping {
     where
         Label: ScheduleLabel + Clone,
     {
-        self.schedule_states
-            .insert(Box::new(schedule), ScheduleState::default());
+        self.updates.push(Update::AddSchedule(Box::new(schedule)));
         self
     }
 
     /// disable stepping for the provided schedule
     pub fn remove_schedule(&mut self, schedule: impl ScheduleLabel) -> &mut Self {
+        self.updates
+            .push(Update::RemoveSchedule(Box::new(schedule)));
         self
     }
 
     /// Begin stepping at the start of the next frame
     pub fn enable(&mut self) -> &mut Self {
-        #[cfg(debug_assertions)]
-        for (_, state) in self.schedule_states.iter() {
-            assert_eq!(state.next, 0);
-        }
-
-        self.pending_action = Some(StepAction::Waiting);
+        self.updates.push(Update::Action(StepAction::Waiting));
         self
     }
 
     /// Disable stepping, resume normal systems execution
     pub fn disable(&mut self) -> &mut Self {
-        self.clear_schedule_progress();
-        self.pending_action = Some(StepAction::RunAll);
+        self.updates.push(Update::Action(StepAction::RunAll));
+        self
+    }
+
+    /// continue stepping until the end of the frame or a breakpoint is hit
+    pub fn r#continue(&mut self) -> &mut Self {
+        self.updates.push(Update::Action(StepAction::Continue));
+        self
+    }
+
+    /// run the next system
+    pub fn step(&mut self) -> &mut Self {
+        self.updates.push(Update::Action(StepAction::Step));
         self
     }
 
@@ -70,11 +112,12 @@ impl Stepping {
         schedule: impl ScheduleLabel,
         system: impl IntoSystem<(), (), Marker>,
     ) -> &mut Self {
-        self.system_behavior_updates.push(SystemBehaviorUpdate {
-            schedule: Box::new(schedule),
-            system_type: IntoSystem::into_system(system).type_id(),
-            behavior: StepBehavior::AlwaysRun,
-        });
+        let type_id = IntoSystem::into_system(system).type_id();
+        self.updates.push(Update::SetBehavior(
+            Box::new(schedule),
+            SystemIdentifier::Type(type_id),
+            StepBehavior::AlwaysRun,
+        ));
 
         self
     }
@@ -85,11 +128,12 @@ impl Stepping {
         schedule: impl ScheduleLabel,
         system: impl IntoSystem<(), (), Marker>,
     ) -> &mut Self {
-        self.system_behavior_updates.push(SystemBehaviorUpdate {
-            schedule: Box::new(schedule),
-            system_type: IntoSystem::into_system(system).type_id(),
-            behavior: StepBehavior::NeverRun,
-        });
+        let type_id = IntoSystem::into_system(system).type_id();
+        self.updates.push(Update::SetBehavior(
+            Box::new(schedule),
+            SystemIdentifier::Type(type_id),
+            StepBehavior::NeverRun,
+        ));
 
         self
     }
@@ -100,21 +144,28 @@ impl Stepping {
         schedule: impl ScheduleLabel,
         system: impl IntoSystem<(), (), Marker>,
     ) -> &mut Self {
-        self.system_behavior_updates.push(SystemBehaviorUpdate {
-            schedule: Box::new(schedule),
-            system_type: IntoSystem::into_system(system).type_id(),
-            behavior: StepBehavior::Break,
-        });
+        let type_id = IntoSystem::into_system(system).type_id();
+        self.updates.push(Update::SetBehavior(
+            Box::new(schedule),
+            SystemIdentifier::Type(type_id),
+            StepBehavior::Break,
+        ));
 
         self
     }
 
     /// Clear a breakpoint for the system
-    pub fn clear_breakpoint<M>(
+    pub fn clear_breakpoint<Marker>(
         &mut self,
         schedule: impl ScheduleLabel,
-        system: impl IntoSystemConfigs<M>,
+        system: impl IntoSystem<(), (), Marker>,
     ) -> &mut Self {
+        let type_id = IntoSystem::into_system(system).type_id();
+        self.updates.push(Update::ClearBehavior(
+            Box::new(schedule),
+            SystemIdentifier::Type(type_id),
+        ));
+
         self
     }
 
@@ -140,44 +191,62 @@ impl Stepping {
         Some(list)
     }
 
-    /// continue stepping until the end of the frame or a breakpoint is hit
-    pub fn r#continue(&mut self) {}
-
-    /// run the next system
-    pub fn step(&mut self) -> &mut Self {
-        self.pending_action = Some(StepAction::Step);
-        self
-    }
-
     pub(crate) fn next_frame(&mut self) {
-        match self.action {
-            StepAction::RunAll => {
-                if self.pending_action.is_none() {
-                    return;
-                }
-            }
-            StepAction::Step => self.clear_schedule_progress(),
-            StepAction::Continue => (),
-            StepAction::Waiting => (),
+        // If we were stepping, and nothing ran, the action will remain Step.
+        // This happens when we've already run everything in the "stepping"
+        // frame.  Now, the user requested a step, let's reset our schedule
+        // state to the beginning and start a new "stepping" frame.
+        if self.action == StepAction::Step {
+            self.clear_schedule_progress();
+
+            // This return here is to ensure that the step is executed.  We
+            // don't want to process updates from the previous frame because
+            // they may overwrite the action, resulting in Step doing nothing.
+            return;
         }
-        if let Some(action) = self.pending_action {
-            println!("next_frame(): state {:?} -> {:?}", self.action, action);
-            self.action = action;
-            self.pending_action = None;
+
+        if self.updates.is_empty() {
+            return;
         }
 
-        for update in self.system_behavior_updates.drain(..) {
-            let state = self
-                .schedule_states
-                .get_mut(&update.schedule)
-                .unwrap_or_else(|| {
-                    panic!(
-                        "stepping is not enabled for schedule {:?}; use
+        let mut clear_schedule = false;
+        for update in self.updates.drain(..) {
+            match update {
+                Update::Action(StepAction::RunAll) => {
+                    self.action = StepAction::RunAll;
+                    clear_schedule = true;
+                }
+                Update::Action(a) => self.action = a,
+                Update::AddSchedule(l) => {
+                    self.schedule_states.insert(l, ScheduleState::default());
+                }
+                Update::RemoveSchedule(l) => {
+                    self.schedule_states.remove(&l);
+                }
+                Update::SetBehavior(label, system, behavior) => {
+                    let state = self.schedule_states.get_mut(&label).unwrap_or_else(|| {
+                        panic!(
+                            "stepping is not enabled for schedule {:?}; use
                 `.add_stepping({:?})` to enable stepping",
-                        update.schedule, update.schedule
-                    )
-                });
-            state.set_behavior(update.system_type, update.behavior);
+                            label, label
+                        )
+                    });
+                    state.set_behavior(system, behavior);
+                }
+                Update::ClearBehavior(label, system) => {
+                    let state = self.schedule_states.get_mut(&label).unwrap_or_else(|| {
+                        panic!(
+                            "stepping is not enabled for schedule {:?}; use
+                `.add_stepping({:?})` to enable stepping",
+                            label, label
+                        )
+                    });
+                    state.clear_behavior(system);
+                }
+            }
+        }
+        if clear_schedule {
+            self.clear_schedule_progress();
         }
     }
 
@@ -197,12 +266,6 @@ impl Stepping {
     }
 }
 
-struct SystemBehaviorUpdate {
-    schedule: BoxedScheduleLabel,
-    system_type: TypeId,
-    behavior: StepBehavior,
-}
-
 #[derive(Default)]
 struct ScheduleState {
     /// Index of the next system to run in the system schedule; if set to
@@ -216,32 +279,52 @@ struct ScheduleState {
 
     /// changes to system behavior that should be applied the next time
     /// [`ScheduleState::progress()`] is called
-    system_updates: HashMap<TypeId, StepBehavior>,
+    updates: HashMap<TypeId, Option<StepBehavior>>,
 }
 
 impl ScheduleState {
     // set the stepping behavior for a system in this schedule
-    fn set_behavior(&mut self, system_type: TypeId, behavior: StepBehavior) {
-        self.system_updates.insert(system_type, behavior);
+    fn set_behavior(&mut self, system: SystemIdentifier, behavior: StepBehavior) {
+        match system {
+            SystemIdentifier::Node(node_id) => {
+                self.systems.insert(node_id, behavior);
+            }
+            SystemIdentifier::Type(type_id) => {
+                self.updates.insert(type_id, Some(behavior));
+            }
+        }
+    }
+
+    // clear the stepping behavior for a system in this schedule
+    fn clear_behavior(&mut self, system: SystemIdentifier) {
+        match system {
+            SystemIdentifier::Node(node_id) => {
+                self.systems.remove(&node_id);
+            }
+            SystemIdentifier::Type(type_id) => {
+                self.updates.insert(type_id, None);
+            }
+        }
     }
 
     // apply system behavior updates by looking up the node id of the system in
     // the schedule, and updating `systems`
-    fn update_behaviors(&mut self, schedule: &Schedule) {
+    fn apply_updates(&mut self, schedule: &Schedule) {
         for (node_id, system) in schedule.systems().unwrap() {
-            if let Some(behavior) = self.system_updates.get(&system.type_id()) {
-                println!(
-                    "update_behaviors(): {} ({:?}) -- {:?}",
-                    system.name(),
-                    node_id,
-                    behavior
-                );
-                self.systems.insert(node_id, *behavior);
+            let behavior = self.updates.get(&system.type_id());
+            match behavior {
+                None => continue,
+                Some(None) => {
+                    self.systems.remove(&node_id);
+                }
+                Some(Some(behavior)) => {
+                    self.systems.insert(node_id, *behavior);
+                }
             }
         }
-        self.system_updates.clear();
+        self.updates.clear();
 
-        println!("update_behaviors(): {:?}", self.systems);
+        println!("apply_updates(): {:?}", self.systems);
     }
 
     // progress the schedule for the given action type, returning a skip list,
@@ -250,8 +333,8 @@ impl ScheduleState {
         // Now that we have the schedule, apply any pending system behavior
         // updates.  The schedule is required to map from system `TypeId` to
         // `NodeId`.
-        if !self.system_updates.is_empty() {
-            self.update_behaviors(schedule);
+        if !self.updates.is_empty() {
+            self.apply_updates(schedule);
         }
 
         let systems_schedule = schedule.executable();
@@ -354,31 +437,6 @@ impl ScheduleState {
     }
 }
 
-#[derive(Debug, Copy, Clone)]
-enum StepBehavior {
-    AlwaysRun,
-    NeverRun,
-    Continue,
-    Break,
-}
-
-#[derive(Debug, Default, PartialEq, Eq, Copy, Clone)]
-enum StepAction {
-    /// Stepping is disabled; run all systems
-    #[default]
-    RunAll,
-
-    /// Stepping is enabled, but we're only running required systems this frame
-    Waiting,
-
-    /// Stepping is enabled; run all systems until the end of the frame, or
-    /// until we encounter a system marked with [`SystemBehavior::Break`].
-    Continue,
-
-    /// stepping is enabled; only run the next system in our step list
-    Step,
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

From 031acd4b1be6a1e7ee8785140c717df1b8484a98 Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Thu, 30 Mar 2023 22:02:14 -0500
Subject: [PATCH 05/25] Stepping::continue_frame() works; test updates

Got `Stepping::continue_frame()` functioning.

Expanded testing of `Stepping`, and added `assert_schedule_runs!` macro
to simplify tests.

Most of the functionality is present here, but I need to expand testing
further, then do an architecture & polish pass on `Stepping`.
---
 crates/bevy_ecs/src/schedule/mod.rs      |  25 +-
 crates/bevy_ecs/src/schedule/stepping.rs | 536 ++++++++++++++++++-----
 2 files changed, 448 insertions(+), 113 deletions(-)

diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs
index a3992183e945f..99f11236cf612 100644
--- a/crates/bevy_ecs/src/schedule/mod.rs
+++ b/crates/bevy_ecs/src/schedule/mod.rs
@@ -712,6 +712,7 @@ mod tests {
 
     mod stepping {
         use super::*;
+        use bevy_ecs::system::SystemState;
 
         #[derive(ScheduleLabel, Clone, Debug, PartialEq, Eq, Hash)]
         pub struct TestSchedule;
@@ -729,12 +730,18 @@ mod tests {
             // Add our schedule to stepping & and enable stepping; this should
             // prevent any systems in the schedule from running
             let mut stepping = Stepping::default();
-            stepping.add_schedule(TestSchedule).enable().next_frame();
+            stepping.add_schedule(TestSchedule).enable();
 
             // create a world, and add the stepping resource
             let mut world = World::default();
             world.insert_resource(stepping);
 
+            // start a new frame by running ihe begin_frame() system
+            let mut system_state: SystemState<Option<ResMut<Stepping>>> =
+                SystemState::new(&mut world);
+            let res = system_state.get_mut(&mut world);
+            Stepping::begin_frame(res);
+
             // now run the schedule; this will panic if the executor doesn't
             // handle stepping
             schedule.run(&mut world);
@@ -753,12 +760,18 @@ mod tests {
             // Add our schedule to stepping & and enable stepping; this should
             // prevent any systems in the schedule from running
             let mut stepping = Stepping::default();
-            stepping.add_schedule(TestSchedule).enable().next_frame();
+            stepping.add_schedule(TestSchedule).enable();
 
             // create a world, and add the stepping resource
             let mut world = World::default();
             world.insert_resource(stepping);
 
+            // start a new frame by running ihe begin_frame() system
+            let mut system_state: SystemState<Option<ResMut<Stepping>>> =
+                SystemState::new(&mut world);
+            let res = system_state.get_mut(&mut world);
+            Stepping::begin_frame(res);
+
             // now run the schedule; this will panic if the executor doesn't
             // handle stepping
             schedule.run(&mut world);
@@ -777,12 +790,18 @@ mod tests {
             // Add our schedule to stepping & and enable stepping; this should
             // prevent any systems in the schedule from running
             let mut stepping = Stepping::default();
-            stepping.add_schedule(TestSchedule).enable().next_frame();
+            stepping.add_schedule(TestSchedule).enable();
 
             // create a world, and add the stepping resource
             let mut world = World::default();
             world.insert_resource(stepping);
 
+            // start a new frame by running ihe begin_frame() system
+            let mut system_state: SystemState<Option<ResMut<Stepping>>> =
+                SystemState::new(&mut world);
+            let res = system_state.get_mut(&mut world);
+            Stepping::begin_frame(res);
+
             // now run the schedule; this will panic if the executor doesn't
             // handle stepping
             schedule.run(&mut world);
diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs
index 3b248e8917497..47108cc60aea3 100644
--- a/crates/bevy_ecs/src/schedule/stepping.rs
+++ b/crates/bevy_ecs/src/schedule/stepping.rs
@@ -66,6 +66,14 @@ impl Stepping {
         Stepping::default()
     }
 
+    /// System that notifies the [`Stepping`] resource that a new frame has
+    /// begun.
+    pub fn begin_frame(stepping: Option<ResMut<Self>>) {
+        if let Some(mut stepping) = stepping {
+            stepping.next_frame();
+        }
+    }
+
     /// Enable stepping for the provided schedule
     pub fn add_schedule<Label>(&mut self, schedule: Label) -> &mut Self
     where
@@ -94,15 +102,15 @@ impl Stepping {
         self
     }
 
-    /// continue stepping until the end of the frame or a breakpoint is hit
-    pub fn r#continue(&mut self) -> &mut Self {
-        self.updates.push(Update::Action(StepAction::Continue));
+    /// run the next system
+    pub fn step_frame(&mut self) -> &mut Self {
+        self.updates.push(Update::Action(StepAction::Step));
         self
     }
 
-    /// run the next system
-    pub fn step(&mut self) -> &mut Self {
-        self.updates.push(Update::Action(StepAction::Step));
+    /// continue stepping until the end of the frame or a breakpoint is hit
+    pub fn continue_frame(&mut self) -> &mut Self {
+        self.updates.push(Update::Action(StepAction::Continue));
         self
     }
 
@@ -169,40 +177,28 @@ impl Stepping {
         self
     }
 
-    pub fn build_skip_list(&mut self, schedule: &Schedule) -> Option<FixedBitSet> {
-        if self.action == StepAction::RunAll {
-            return None;
-        }
-        let state = match schedule.label() {
-            Some(label) => self.schedule_states.get_mut(label)?,
-            None => return None,
-        };
-
-        let (list, wait) = state.progress(schedule, self.action);
-        if wait {
-            println!(
-                "build_skip_list({:?}): state {:?} -> {:?}",
-                schedule.label(),
-                self.action,
-                StepAction::Waiting
-            );
-            self.action = StepAction::Waiting;
-        }
-        Some(list)
-    }
-
-    pub(crate) fn next_frame(&mut self) {
+    /// Advance schedule states for the next render frame
+    fn next_frame(&mut self) {
         // If we were stepping, and nothing ran, the action will remain Step.
         // This happens when we've already run everything in the "stepping"
         // frame.  Now, the user requested a step, let's reset our schedule
         // state to the beginning and start a new "stepping" frame.
-        if self.action == StepAction::Step {
-            self.clear_schedule_progress();
-
-            // This return here is to ensure that the step is executed.  We
-            // don't want to process updates from the previous frame because
-            // they may overwrite the action, resulting in Step doing nothing.
-            return;
+        match self.action {
+            StepAction::Step => {
+                println!("wrapping state");
+                self.next_stepping_frame();
+
+                // This return here is to ensure that the step is executed.  We
+                // don't want to process updates from the previous frame
+                // because they may overwrite the action, resulting in Step
+                // doing nothing.
+                return;
+            }
+            StepAction::Continue => {
+                self.next_stepping_frame();
+                self.action = StepAction::Waiting;
+            }
+            _ => (),
         }
 
         if self.updates.is_empty() {
@@ -245,25 +241,44 @@ impl Stepping {
                 }
             }
         }
-        if clear_schedule {
-            self.clear_schedule_progress();
-        }
-    }
 
-    pub fn begin_frame(stepping: Option<ResMut<Self>>) {
-        if let Some(mut stepping) = stepping {
-            stepping.next_frame();
+        if clear_schedule {
+            self.next_stepping_frame();
         }
     }
 
-    /// clears all schedule progress, starting a new stepping frame.
-    fn clear_schedule_progress(&mut self) {
-        println!("clearing schedule progress");
+    /// Advance schedule states for a new stepping frame
+    fn next_stepping_frame(&mut self) {
+        println!("next_stepping_frame(): resetting schedule states");
         // clear any progress we've made in the stepping frame
         for (_, state) in self.schedule_states.iter_mut() {
             state.next = 0;
         }
     }
+
+    /// Build the list of systems the supplied schedule should skip this render
+    /// frame
+    pub fn build_skip_list(&mut self, schedule: &Schedule) -> Option<FixedBitSet> {
+        if self.action == StepAction::RunAll {
+            return None;
+        }
+        let state = match schedule.label() {
+            Some(label) => self.schedule_states.get_mut(label)?,
+            None => return None,
+        };
+
+        let (list, wait) = state.progress(schedule, self.action);
+        if wait {
+            println!(
+                "build_skip_list({:?}): state {:?} -> {:?}",
+                schedule.label(),
+                self.action,
+                StepAction::Waiting
+            );
+            self.action = StepAction::Waiting;
+        }
+        Some(list)
+    }
 }
 
 #[derive(Default)]
@@ -424,16 +439,52 @@ impl ScheduleState {
 
     fn skip_list_continue(&mut self, schedule: &Schedule) -> (FixedBitSet, bool) {
         let mut skip = FixedBitSet::with_capacity(schedule.systems_len());
-        for (pos, node_id) in schedule.executable().system_ids.iter().enumerate() {
-            use StepBehavior::*;
-            match self.systems.get(node_id).unwrap_or(&Continue) {
-                AlwaysRun => (),
-                NeverRun => (),
-                Continue => (),
-                Break => (),
+        let mut hit_breakpoint = false;
+        let mut pos = self.next;
+
+        for (i, (node_id, system)) in schedule.systems().unwrap().enumerate() {
+            println!(
+                "skip_list_continue(): {} ({:?}) -- {:?}; i {}, pos {}, break {}",
+                system.name(),
+                node_id,
+                self.systems.get(&node_id),
+                i,
+                pos,
+                hit_breakpoint,
+            );
+
+            match self
+                .systems
+                .get(&node_id)
+                .unwrap_or(&StepBehavior::Continue)
+            {
+                StepBehavior::AlwaysRun => (),
+                StepBehavior::NeverRun => skip.insert(i),
+                StepBehavior::Continue => {
+                    if i != pos {
+                        skip.insert(i);
+                    }
+                }
+                StepBehavior::Break => {
+                    if i == self.next {
+                        println!("resuming from breakpoint");
+                    } else if i < pos {
+                        println!("skipping already run system");
+                        skip.insert(i);
+                    } else if i == pos {
+                        println!("hit breakpoint");
+                        hit_breakpoint = true;
+                        skip.insert(i);
+                    }
+                }
+            }
+
+            if !hit_breakpoint && i == pos {
+                pos = i + 1;
             }
         }
-        (skip, false)
+        self.next = pos;
+        (skip, hit_breakpoint)
     }
 }
 
@@ -461,31 +512,267 @@ mod tests {
         (schedule, world)
     }
 
+    // Helper for verifying the expected systems will be run by the schedule
+    //
+    // This macro will construct an expected FixedBitSet for the systems that
+    // should be skipped, and compare it with the results from stepping the
+    // provided schedule.  If they don't match, it generates a human-readable
+    // error message and asserts.
+    macro_rules! assert_schedule_runs {
+        ($schedule:expr, $stepping:expr, $($system:expr),*) => {
+            // pull an ordered list of systems in the schedule, and save the
+            // system TypeId, and name.
+            let systems: Vec<(TypeId, std::borrow::Cow<'static, str>)> = $schedule.systems().unwrap().enumerate()
+                .map(|(i, (_, system))| {
+                    (system.type_id(), system.name())
+                })
+            .collect();
+
+            // construct a list of systems that are expected to run
+            let mut expected = FixedBitSet::with_capacity(systems.len());
+            $(
+                let sys = IntoSystem::into_system($system);
+                for (i, (type_id, _)) in systems.iter().enumerate() {
+                    if sys.type_id() == *type_id {
+                        expected.insert(i);
+                    }
+                }
+            )*
+
+            // flip the run list to get our skip list
+            expected.toggle_range(..);
+
+            // advance stepping to the next frame, and build the skip list for
+            // this schedule
+            $stepping.next_frame();
+            let actual = match $stepping.build_skip_list($schedule) {
+                None => FixedBitSet::with_capacity(systems.len()),
+                Some(b) => b,
+            };
+
+            if (actual != expected) {
+                use std::fmt::Write as _;
+
+                // mismatch, let's construct a human-readable message of what
+                // was returned
+                let mut msg = format!("Schedule:\n    {:9} {:16}{:6} {:6} {:6}\n",
+                    "index", "name", "expect", "actual", "result");
+                for (i, (_, full_name)) in systems.iter().enumerate() {
+                    let (_, name) = full_name.rsplit_once("::").unwrap();
+                    let _ = write!(msg, "    system[{:1}] {:16}", i, name);
+                    match (expected.contains(i), actual.contains(i)) {
+                        (true, true) => msg.push_str("skip   skip   pass\n"),
+                        (true, false) =>
+                            msg.push_str("skip   run    FAILED; system should not have run\n"),
+                        (false, true) =>
+                            msg.push_str("run    skip   FAILED; system should have run\n"),
+                        (false, false) => msg.push_str("run    run    pass\n"),
+                    }
+                }
+                assert_eq!(actual, expected, "{}", msg);
+            }
+        };
+    }
+
     #[test]
     fn stepping_disabled() {
         let (schedule, _world) = setup();
 
         let mut stepping = Stepping::new();
-        stepping.disable().add_schedule(TestSchedule);
+        stepping.add_schedule(TestSchedule).disable().next_frame();
 
-        // returns None when stepping isn't enabled
-        stepping.disable();
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule);
-        assert!(skip.is_none());
+        assert!(stepping.build_skip_list(&schedule).is_none());
     }
 
-    // if build_skip_list is caled with an unknown schedule, it will return none
     #[test]
     fn unknown_schedule() {
         let (schedule, _world) = setup();
 
         let mut stepping = Stepping::new();
-        stepping.enable();
+        stepping.enable().next_frame();
 
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule);
-        assert!(skip.is_none());
+        assert!(stepping.build_skip_list(&schedule).is_none());
+    }
+
+    #[test]
+    fn disabled_always_run() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping
+            .add_schedule(TestSchedule)
+            .disable()
+            .always_run(TestSchedule, first_system);
+
+        assert_schedule_runs!(&schedule, &mut stepping, first_system, second_system);
+    }
+
+    #[test]
+    fn waiting_always_run() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping
+            .add_schedule(TestSchedule)
+            .enable()
+            .always_run(TestSchedule, first_system);
+
+        assert_schedule_runs!(&schedule, &mut stepping, first_system);
+    }
+
+    #[test]
+    fn step_always_run() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping
+            .add_schedule(TestSchedule)
+            .enable()
+            .always_run(TestSchedule, first_system)
+            .step_frame();
+
+        assert_schedule_runs!(&schedule, &mut stepping, first_system, second_system);
+    }
+
+    #[test]
+    fn continue_always_run() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping
+            .add_schedule(TestSchedule)
+            .enable()
+            .always_run(TestSchedule, first_system)
+            .continue_frame();
+
+        assert_schedule_runs!(&schedule, &mut stepping, first_system, second_system);
+    }
+
+    #[test]
+    fn disabled_never_run() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping
+            .add_schedule(TestSchedule)
+            .never_run(TestSchedule, first_system)
+            .disable();
+        assert_schedule_runs!(&schedule, &mut stepping, first_system, second_system);
+    }
+
+    #[test]
+    fn waiting_never_run() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping
+            .add_schedule(TestSchedule)
+            .enable()
+            .never_run(TestSchedule, first_system);
+
+        assert_schedule_runs!(&schedule, &mut stepping,);
+    }
+
+    #[test]
+    fn step_never_run() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping
+            .add_schedule(TestSchedule)
+            .enable()
+            .never_run(TestSchedule, first_system)
+            .step_frame();
+
+        assert_schedule_runs!(&schedule, &mut stepping, second_system);
+    }
+
+    #[test]
+    fn continue_never_run() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping
+            .add_schedule(TestSchedule)
+            .enable()
+            .never_run(TestSchedule, first_system)
+            .continue_frame();
+
+        assert_schedule_runs!(&schedule, &mut stepping, second_system);
+    }
+
+    #[test]
+    fn disabled_breakpoint() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping
+            .add_schedule(TestSchedule)
+            .disable()
+            .set_breakpoint(TestSchedule, second_system);
+
+        assert_schedule_runs!(&schedule, &mut stepping, first_system, second_system);
+    }
+
+    #[test]
+    fn waiting_breakpoint() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping
+            .add_schedule(TestSchedule)
+            .enable()
+            .set_breakpoint(TestSchedule, second_system);
+
+        assert_schedule_runs!(&schedule, &mut stepping,);
+    }
+
+    #[test]
+    fn step_breakpoint() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping
+            .add_schedule(TestSchedule)
+            .enable()
+            .set_breakpoint(TestSchedule, second_system)
+            .step_frame();
+
+        // since stepping stops at every system, breakpoints are ignored during
+        // stepping
+        assert_schedule_runs!(&schedule, &mut stepping, first_system);
+        stepping.step_frame();
+        assert_schedule_runs!(&schedule, &mut stepping, second_system);
+
+        // when stepping, we take one render frame to detect that the stepping
+        // frame has completed.  This looks like nothing runs for the first
+        // render frame, then the second render-frame resets the stepping frame
+        // and we run the first system again.
+        //
+        // XXX this is weird from the UI standpoint; how does UI display what
+        // is the next system to be run, esp when transitioning between
+        // schedules.
+        stepping.step_frame();
+        assert_schedule_runs!(&schedule, &mut stepping,);
+        assert_schedule_runs!(&schedule, &mut stepping, first_system);
+    }
+
+    #[test]
+    fn continue_breakpoint() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping
+            .add_schedule(TestSchedule)
+            .enable()
+            .set_breakpoint(TestSchedule, second_system)
+            .continue_frame();
+
+        assert_schedule_runs!(&schedule, &mut stepping, first_system);
+        stepping.continue_frame();
+        assert_schedule_runs!(&schedule, &mut stepping, second_system);
+        stepping.continue_frame();
+        assert_schedule_runs!(&schedule, &mut stepping, first_system);
     }
 
     #[test]
@@ -502,7 +789,7 @@ mod tests {
 
         // single stepping should result in only running the first system
         println!("step 1");
-        stepping.step();
+        stepping.step_frame();
         stepping.next_frame();
         let skip = stepping.build_skip_list(&schedule).unwrap();
         assert_eq!(skip.len(), schedule.executable().systems.len());
@@ -518,7 +805,7 @@ mod tests {
 
         // doing it again should step the second system
         println!("step 2");
-        stepping.step();
+        stepping.step_frame();
         stepping.next_frame();
         let skip = stepping.build_skip_list(&schedule).unwrap();
         assert_eq!(skip.len(), schedule.executable().systems.len());
@@ -533,7 +820,7 @@ mod tests {
         // Let's verify that behavior now first by trying to step again, to see
         // that all systems are skipped.
         println!("step 3 -- wrap detect");
-        stepping.step();
+        stepping.step_frame();
         stepping.next_frame();
         let skip = stepping.build_skip_list(&schedule).unwrap();
         assert_eq!(skip.count_ones(..), schedule.executable().systems.len());
@@ -549,83 +836,73 @@ mod tests {
     }
 
     #[test]
-    fn step_always_run() {
+    fn step_never_run2() {
         let (schedule, _world) = setup();
 
         let mut stepping = Stepping::new();
         stepping.enable().add_schedule(TestSchedule);
 
-        // Let's make it so the first system always runs
-        stepping.always_run(TestSchedule, first_system);
+        stepping.never_run(TestSchedule, first_system);
 
-        // when we're stopped/waiting, only the second system should be in the
-        // skip list, as the first system should always run.
         println!("next frame; waiting");
         stepping.next_frame();
         let skip = stepping.build_skip_list(&schedule).unwrap();
         assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(!skip.contains(0));
+        assert!(skip.contains(0));
         assert!(skip.contains(1));
 
-        // let's step and verify neither system is skipped
         println!("next frame; step");
-        stepping.step();
+        stepping.step_frame();
         stepping.next_frame();
         let skip = stepping.build_skip_list(&schedule).unwrap();
         assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(!skip.contains(0));
+        assert!(skip.contains(0));
         assert!(!skip.contains(1));
 
-        // verify wrapping occurrs properly
         println!("next frame; step, detect wrapping");
-        stepping.step();
+        stepping.step_frame();
         stepping.next_frame();
         let skip = stepping.build_skip_list(&schedule).unwrap();
         assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(!skip.contains(0));
+        assert!(skip.contains(0));
         assert!(skip.contains(1));
 
         println!("next frame; wrapped step");
         stepping.next_frame();
         let skip = stepping.build_skip_list(&schedule).unwrap();
         assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(!skip.contains(0));
+        assert!(skip.contains(0));
         assert!(!skip.contains(1));
     }
 
+    // This test just verifies that stepping stops on breakpoint systems, then
+    // steps past them
     #[test]
-    fn step_never_run() {
+    fn step_breakpoint2() {
         let (schedule, _world) = setup();
 
         let mut stepping = Stepping::new();
         stepping.enable().add_schedule(TestSchedule);
 
-        stepping.never_run(TestSchedule, first_system);
-
-        println!("next frame; waiting");
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(skip.contains(0));
-        assert!(skip.contains(1));
+        stepping.set_breakpoint(TestSchedule, first_system);
 
-        println!("next frame; step");
-        stepping.step();
+        // stepping while waiting should result in skipping all systems
         stepping.next_frame();
         let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(skip.contains(0));
-        assert!(!skip.contains(1));
+        assert_eq!(skip.count_ones(..), schedule.executable().systems.len());
 
-        println!("next frame; step, detect wrapping");
-        stepping.step();
+        // single stepping should result in only running the first system
+        println!("step 1");
+        stepping.step_frame();
         stepping.next_frame();
         let skip = stepping.build_skip_list(&schedule).unwrap();
         assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(skip.contains(0));
+        assert!(!skip.contains(0));
         assert!(skip.contains(1));
 
-        println!("next frame; wrapped step");
+        // now the second system should run
+        println!("next frame: step");
+        stepping.step_frame();
         stepping.next_frame();
         let skip = stepping.build_skip_list(&schedule).unwrap();
         assert_eq!(skip.len(), schedule.executable().systems.len());
@@ -633,34 +910,73 @@ mod tests {
         assert!(!skip.contains(1));
     }
 
-    // This test just verifies that stepping stops on breakpoint systems, then
-    // steps past them
+    // verify continue_frame behavior
     #[test]
-    fn step_breakpoint() {
+    fn continue_frame() {
         let (schedule, _world) = setup();
 
         let mut stepping = Stepping::new();
         stepping.enable().add_schedule(TestSchedule);
 
-        stepping.set_breakpoint(TestSchedule, first_system);
+        // starting from a fresh frame, continue should run every system in the
+        // frame
+        println!("continue frame");
+        stepping.continue_frame();
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.len(), schedule.executable().systems.len());
+        assert!(!skip.contains(0));
+        assert!(!skip.contains(1));
 
-        // stepping while waiting should result in skipping all systems
+        // if we render the next frame, we should be in a waiting state, and
+        // nothing should run
         stepping.next_frame();
+        assert_eq!(stepping.action, StepAction::Waiting);
         let skip = stepping.build_skip_list(&schedule).unwrap();
         assert_eq!(skip.count_ones(..), schedule.executable().systems.len());
 
-        // single stepping should result in only running the first system
-        println!("step 1");
-        stepping.step();
+        // step to only run the first frame
+        println!("next frame: step");
+        stepping.step_frame();
         stepping.next_frame();
         let skip = stepping.build_skip_list(&schedule).unwrap();
         assert_eq!(skip.len(), schedule.executable().systems.len());
         assert!(!skip.contains(0));
         assert!(skip.contains(1));
 
-        // now the second system should run
-        println!("next frame: step");
-        stepping.step();
+        // continue the frame, and we should see only the second system running
+        println!("continue frame");
+        stepping.continue_frame();
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.len(), schedule.executable().systems.len());
+        assert!(skip.contains(0));
+        assert!(!skip.contains(1));
+
+        // if we continue again, we should run all systems
+        println!("continue frame");
+        stepping.continue_frame();
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.len(), schedule.executable().systems.len());
+        assert!(!skip.contains(0));
+        assert!(!skip.contains(1));
+
+        // if we throw in a breakpoint for the second system only the first
+        // sytem should run
+        println!("continue frame; with breakpoint on second system");
+        stepping.set_breakpoint(TestSchedule, second_system);
+        stepping.continue_frame();
+        stepping.next_frame();
+        let skip = stepping.build_skip_list(&schedule).unwrap();
+        assert_eq!(skip.len(), schedule.executable().systems.len());
+        assert!(!skip.contains(0));
+        assert!(skip.contains(1));
+
+        // now resume the frame we hit the breakpoint on, and only the second
+        // system should run
+        println!("continue frame; resume at second system");
+        stepping.continue_frame();
         stepping.next_frame();
         let skip = stepping.build_skip_list(&schedule).unwrap();
         assert_eq!(skip.len(), schedule.executable().systems.len());

From 2a9688302edce8de9bd3e186d550dae9442150e6 Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Fri, 31 Mar 2023 22:01:46 -0500
Subject: [PATCH 06/25] Stepping dynamic schedule order detection

This is the first part of support for introspection of `Stepping` by
external systems.  As `Stepping#build_skip_list()` is called, we insert
the various `Schedule`s we see in `Stepping.schedule_order`.  This is
exposed to users via `Stepping#schedules()`, and will allow them to
generate an ordered list of schedules & systems that will be stepped
through.
---
 crates/bevy_ecs/src/schedule/stepping.rs | 414 +++++++++++------------
 1 file changed, 206 insertions(+), 208 deletions(-)

diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs
index 47108cc60aea3..0a1b2f34e6fb0 100644
--- a/crates/bevy_ecs/src/schedule/stepping.rs
+++ b/crates/bevy_ecs/src/schedule/stepping.rs
@@ -1,5 +1,4 @@
-#![allow(dead_code, unused_mut)]
-#![allow(unused_variables)]
+use bevy_utils::tracing::warn;
 use fixedbitset::FixedBitSet;
 use std::any::TypeId;
 use std::collections::HashMap;
@@ -21,7 +20,8 @@ enum StepAction {
     Waiting,
 
     /// Stepping is enabled; run all systems until the end of the frame, or
-    /// until we encounter a system marked with [`SystemBehavior::Break`].
+    /// until we encounter a system marked with [`StepBehavior::Break`] or all
+    /// systems in the frame have run.
     Continue,
 
     /// stepping is enabled; only run the next system in our step list
@@ -45,6 +45,7 @@ enum Update {
     Action(StepAction),
     AddSchedule(BoxedScheduleLabel),
     RemoveSchedule(BoxedScheduleLabel),
+    ClearSchedule(BoxedScheduleLabel),
     SetBehavior(BoxedScheduleLabel, SystemIdentifier, StepBehavior),
     ClearBehavior(BoxedScheduleLabel, SystemIdentifier),
 }
@@ -59,6 +60,12 @@ pub struct Stepping {
 
     /// Updates to stepping state to be applied at the start of the next frame
     updates: Vec<Update>,
+
+    /// dynamically generated [`Schedule`] order
+    schedule_order: Vec<BoxedScheduleLabel>,
+
+    /// tracks the index of the last schedule stepped in schedule_order
+    last_schedule: Option<usize>,
 }
 
 impl Stepping {
@@ -74,6 +81,12 @@ impl Stepping {
         }
     }
 
+    /// Return the list of schedules with stepping enabled in the order
+    /// they are executed in.
+    pub fn schedules(&mut self) -> &Vec<BoxedScheduleLabel> {
+        &self.schedule_order
+    }
+
     /// Enable stepping for the provided schedule
     pub fn add_schedule<Label>(&mut self, schedule: Label) -> &mut Self
     where
@@ -90,6 +103,12 @@ impl Stepping {
         self
     }
 
+    /// clear [`StepBehavior`] set for all systems in the provided [`Schedule`]
+    pub fn clear_schedule(&mut self, schedule: impl ScheduleLabel) -> &mut Self {
+        self.updates.push(Update::ClearSchedule(Box::new(schedule)));
+        self
+    }
+
     /// Begin stepping at the start of the next frame
     pub fn enable(&mut self) -> &mut Self {
         self.updates.push(Update::Action(StepAction::Waiting));
@@ -167,6 +186,17 @@ impl Stepping {
         &mut self,
         schedule: impl ScheduleLabel,
         system: impl IntoSystem<(), (), Marker>,
+    ) -> &mut Self {
+        self.clear_system(schedule, system);
+
+        self
+    }
+
+    /// Clear any behavior set for the system
+    pub fn clear_system<Marker>(
+        &mut self,
+        schedule: impl ScheduleLabel,
+        system: impl IntoSystem<(), (), Marker>,
     ) -> &mut Self {
         let type_id = IntoSystem::into_system(system).type_id();
         self.updates.push(Update::ClearBehavior(
@@ -219,25 +249,39 @@ impl Stepping {
                 Update::RemoveSchedule(l) => {
                     self.schedule_states.remove(&l);
                 }
-                Update::SetBehavior(label, system, behavior) => {
-                    let state = self.schedule_states.get_mut(&label).unwrap_or_else(|| {
-                        panic!(
+                Update::ClearSchedule(label) => match self.schedule_states.get_mut(&label) {
+                    Some(state) => state.clear_behaviors(),
+                    None => {
+                        warn!(
                             "stepping is not enabled for schedule {:?}; use
-                `.add_stepping({:?})` to enable stepping",
+                    `.add_stepping({:?})` to enable stepping",
                             label, label
                         )
-                    });
-                    state.set_behavior(system, behavior);
+                    }
+                },
+                Update::SetBehavior(label, system, behavior) => {
+                    match self.schedule_states.get_mut(&label) {
+                        Some(state) => state.set_behavior(system, behavior),
+                        None => {
+                            warn!(
+                                "stepping is not enabled for schedule {:?}; use
+                    `.add_stepping({:?})` to enable stepping",
+                                label, label
+                            )
+                        }
+                    }
                 }
                 Update::ClearBehavior(label, system) => {
-                    let state = self.schedule_states.get_mut(&label).unwrap_or_else(|| {
-                        panic!(
-                            "stepping is not enabled for schedule {:?}; use
-                `.add_stepping({:?})` to enable stepping",
-                            label, label
-                        )
-                    });
-                    state.clear_behavior(system);
+                    match self.schedule_states.get_mut(&label) {
+                        Some(state) => state.clear_behavior(system),
+                        None => {
+                            warn!(
+                                "stepping is not enabled for schedule {:?}; use
+                    `.add_stepping({:?})` to enable stepping",
+                                label, label
+                            )
+                        }
+                    }
                 }
             }
         }
@@ -249,9 +293,12 @@ impl Stepping {
 
     /// Advance schedule states for a new stepping frame
     fn next_stepping_frame(&mut self) {
-        println!("next_stepping_frame(): resetting schedule states");
+        println!("next_stepping_frame(): being new stepping frame");
+
         // clear any progress we've made in the stepping frame
-        for (_, state) in self.schedule_states.iter_mut() {
+        self.last_schedule = None;
+
+        for state in self.schedule_states.values_mut() {
             state.next = 0;
         }
     }
@@ -267,6 +314,25 @@ impl Stepping {
             None => return None,
         };
 
+        // We are stepping through a  schedule that has stepping enabled.
+        //
+        // Let's take a moment to make sure this schedule is in our ordered
+        // schedule list.  We maintain this to give any stepping UI a way to
+        // know what order schedules are called in.
+        let label = schedule.label().as_ref().unwrap();
+        let index = self.schedule_order.iter().position(|l| l == label);
+        match (self.last_schedule, index) {
+            (_, Some(i)) => self.last_schedule = Some(i),
+            (Some(last), None) => {
+                self.schedule_order.insert(last + 1, label.clone());
+                self.last_schedule = Some(last + 1);
+            }
+            (None, None) => {
+                self.schedule_order.insert(0, label.clone());
+                self.last_schedule = Some(0);
+            }
+        }
+
         let (list, wait) = state.progress(schedule, self.action);
         if wait {
             println!(
@@ -286,7 +352,8 @@ struct ScheduleState {
     /// Index of the next system to run in the system schedule; if set to
     /// greater than the number of systems in the schedule, then don't run
     /// any systems.
-    /// NOTE: [`AlwaysRun`] systems ignore this value and always run
+    /// NOTE: [`StepBehavior::AlwaysRun`] systems ignore this value and always
+    /// run
     next: usize,
 
     /// per-system [`StepBehavior`]
@@ -322,6 +389,11 @@ impl ScheduleState {
         }
     }
 
+    // clear all system behaviors
+    fn clear_behaviors(&mut self) {
+        self.systems.clear();
+    }
+
     // apply system behavior updates by looking up the node id of the system in
     // the schedule, and updating `systems`
     fn apply_updates(&mut self, schedule: &Schedule) {
@@ -344,7 +416,7 @@ impl ScheduleState {
 
     // progress the schedule for the given action type, returning a skip list,
     // and if the caller should update state to waiting
-    fn progress(&mut self, schedule: &Schedule, mut action: StepAction) -> (FixedBitSet, bool) {
+    fn progress(&mut self, schedule: &Schedule, action: StepAction) -> (FixedBitSet, bool) {
         // Now that we have the schedule, apply any pending system behavior
         // updates.  The schedule is required to map from system `TypeId` to
         // `NodeId`.
@@ -352,10 +424,6 @@ impl ScheduleState {
             self.apply_updates(schedule);
         }
 
-        let systems_schedule = schedule.executable();
-
-        // let skip = FixedBitSet::with_capacity(systems_schedule.systems.len());
-        let skip = FixedBitSet::with_capacity(schedule.systems_len());
         match action {
             StepAction::RunAll => unreachable!(),
             StepAction::Waiting => self.skip_list_waiting(schedule),
@@ -499,6 +567,18 @@ mod tests {
     #[derive(ScheduleLabel, Clone, Debug, PartialEq, Eq, Hash)]
     struct TestSchedule;
 
+    #[derive(ScheduleLabel, Clone, Debug, PartialEq, Eq, Hash)]
+    struct TestScheduleA;
+
+    #[derive(ScheduleLabel, Clone, Debug, PartialEq, Eq, Hash)]
+    struct TestScheduleB;
+
+    #[derive(ScheduleLabel, Clone, Debug, PartialEq, Eq, Hash)]
+    struct TestScheduleC;
+
+    #[derive(ScheduleLabel, Clone, Debug, PartialEq, Eq, Hash)]
+    struct TestScheduleD;
+
     fn first_system() {}
     fn second_system() {}
 
@@ -522,8 +602,8 @@ mod tests {
         ($schedule:expr, $stepping:expr, $($system:expr),*) => {
             // pull an ordered list of systems in the schedule, and save the
             // system TypeId, and name.
-            let systems: Vec<(TypeId, std::borrow::Cow<'static, str>)> = $schedule.systems().unwrap().enumerate()
-                .map(|(i, (_, system))| {
+            let systems: Vec<(TypeId, std::borrow::Cow<'static, str>)> = $schedule.systems().unwrap()
+                .map(|(_, system)| {
                     (system.type_id(), system.name())
                 })
             .collect();
@@ -755,6 +835,9 @@ mod tests {
         stepping.step_frame();
         assert_schedule_runs!(&schedule, &mut stepping,);
         assert_schedule_runs!(&schedule, &mut stepping, first_system);
+
+        // should be back in a waiting state now that it ran first_system
+        assert_schedule_runs!(&schedule, &mut stepping,);
     }
 
     #[test]
@@ -776,211 +859,126 @@ mod tests {
     }
 
     #[test]
-    fn step() {
+    fn clear_breakpoint() {
         let (schedule, _world) = setup();
 
         let mut stepping = Stepping::new();
-        stepping.enable().add_schedule(TestSchedule);
-
-        // stepping while waiting should result in skipping all systems
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.count_ones(..), schedule.executable().systems.len());
-
-        // single stepping should result in only running the first system
-        println!("step 1");
-        stepping.step_frame();
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(!skip.contains(0));
-        assert!(skip.contains(1));
-
-        // call it again without issuing another `step()` call, and we should
-        // skip all the systems
-        println!("wait");
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.count_ones(..), schedule.executable().systems.len());
+        stepping
+            .add_schedule(TestSchedule)
+            .enable()
+            .set_breakpoint(TestSchedule, second_system)
+            .continue_frame();
 
-        // doing it again should step the second system
-        println!("step 2");
-        stepping.step_frame();
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(skip.contains(0));
-        assert!(!skip.contains(1));
-
-        // Now that we've run all of the systems in our stepping schedules, we
-        // want to wrap back around to the first system.  However, stepping only
-        // detects that we need to wrap back around when it runs a system step
-        // pass, but no schedule is able to run a system.
-        //
-        // Let's verify that behavior now first by trying to step again, to see
-        // that all systems are skipped.
-        println!("step 3 -- wrap detect");
-        stepping.step_frame();
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.count_ones(..), schedule.executable().systems.len());
+        assert_schedule_runs!(&schedule, &mut stepping, first_system);
+        stepping.continue_frame();
+        assert_schedule_runs!(&schedule, &mut stepping, second_system);
 
-        // Now, call `next_frame()` again, then build the skip list.  This list
-        // should have wrapped, and allow only the first system to run.
-        println!("step 3 -- wrapped");
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(!skip.contains(0));
-        assert!(skip.contains(1));
+        stepping.clear_breakpoint(TestSchedule, second_system);
+        stepping.continue_frame();
+        assert_schedule_runs!(&schedule, &mut stepping, first_system, second_system);
     }
 
     #[test]
-    fn step_never_run2() {
+    fn clear_system() {
         let (schedule, _world) = setup();
 
         let mut stepping = Stepping::new();
-        stepping.enable().add_schedule(TestSchedule);
-
-        stepping.never_run(TestSchedule, first_system);
-
-        println!("next frame; waiting");
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(skip.contains(0));
-        assert!(skip.contains(1));
-
-        println!("next frame; step");
-        stepping.step_frame();
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(skip.contains(0));
-        assert!(!skip.contains(1));
-
-        println!("next frame; step, detect wrapping");
-        stepping.step_frame();
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(skip.contains(0));
-        assert!(skip.contains(1));
+        stepping
+            .add_schedule(TestSchedule)
+            .enable()
+            .never_run(TestSchedule, second_system)
+            .continue_frame();
+        assert_schedule_runs!(&schedule, &mut stepping, first_system);
 
-        println!("next frame; wrapped step");
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(skip.contains(0));
-        assert!(!skip.contains(1));
+        stepping.clear_system(TestSchedule, second_system);
+        stepping.continue_frame();
+        assert_schedule_runs!(&schedule, &mut stepping, first_system, second_system);
     }
 
-    // This test just verifies that stepping stops on breakpoint systems, then
-    // steps past them
     #[test]
-    fn step_breakpoint2() {
+    fn clear_schedule() {
         let (schedule, _world) = setup();
 
         let mut stepping = Stepping::new();
-        stepping.enable().add_schedule(TestSchedule);
-
-        stepping.set_breakpoint(TestSchedule, first_system);
-
-        // stepping while waiting should result in skipping all systems
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.count_ones(..), schedule.executable().systems.len());
-
-        // single stepping should result in only running the first system
-        println!("step 1");
-        stepping.step_frame();
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(!skip.contains(0));
-        assert!(skip.contains(1));
+        stepping
+            .add_schedule(TestSchedule)
+            .enable()
+            .never_run(TestSchedule, first_system)
+            .never_run(TestSchedule, second_system)
+            .continue_frame();
+        assert_schedule_runs!(&schedule, &mut stepping,);
 
-        // now the second system should run
-        println!("next frame: step");
-        stepping.step_frame();
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(skip.contains(0));
-        assert!(!skip.contains(1));
+        stepping.clear_schedule(TestSchedule);
+        stepping.continue_frame();
+        assert_schedule_runs!(&schedule, &mut stepping, first_system, second_system);
     }
 
-    // verify continue_frame behavior
     #[test]
-    fn continue_frame() {
-        let (schedule, _world) = setup();
+    fn schedules() {
+        let mut world = World::new();
 
+        // build & initialize three schedules
+        let mut schedule_a = Schedule::default();
+        schedule_a.set_label(TestScheduleA);
+        schedule_a.initialize(&mut world).unwrap();
+        let mut schedule_b = Schedule::default();
+        schedule_b.set_label(TestScheduleB);
+        schedule_b.initialize(&mut world).unwrap();
+        let mut schedule_c = Schedule::default();
+        schedule_c.set_label(TestScheduleC);
+        schedule_c.initialize(&mut world).unwrap();
+        let mut schedule_d = Schedule::default();
+        schedule_d.set_label(TestScheduleD);
+        schedule_d.initialize(&mut world).unwrap();
+
+        // setup stepping and add all three schedules
         let mut stepping = Stepping::new();
-        stepping.enable().add_schedule(TestSchedule);
-
-        // starting from a fresh frame, continue should run every system in the
-        // frame
-        println!("continue frame");
-        stepping.continue_frame();
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(!skip.contains(0));
-        assert!(!skip.contains(1));
-
-        // if we render the next frame, we should be in a waiting state, and
-        // nothing should run
-        stepping.next_frame();
-        assert_eq!(stepping.action, StepAction::Waiting);
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.count_ones(..), schedule.executable().systems.len());
-
-        // step to only run the first frame
-        println!("next frame: step");
-        stepping.step_frame();
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(!skip.contains(0));
-        assert!(skip.contains(1));
-
-        // continue the frame, and we should see only the second system running
-        println!("continue frame");
-        stepping.continue_frame();
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(skip.contains(0));
-        assert!(!skip.contains(1));
-
-        // if we continue again, we should run all systems
-        println!("continue frame");
-        stepping.continue_frame();
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(!skip.contains(0));
-        assert!(!skip.contains(1));
-
-        // if we throw in a breakpoint for the second system only the first
-        // sytem should run
-        println!("continue frame; with breakpoint on second system");
-        stepping.set_breakpoint(TestSchedule, second_system);
-        stepping.continue_frame();
-        stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(!skip.contains(0));
-        assert!(skip.contains(1));
-
-        // now resume the frame we hit the breakpoint on, and only the second
-        // system should run
-        println!("continue frame; resume at second system");
-        stepping.continue_frame();
+        stepping
+            .add_schedule(TestScheduleA)
+            .add_schedule(TestScheduleB)
+            .add_schedule(TestScheduleC)
+            .add_schedule(TestScheduleD)
+            .enable()
+            .next_frame();
+
+        stepping.build_skip_list(&schedule_b);
+        stepping.build_skip_list(&schedule_a);
+        stepping.build_skip_list(&schedule_c);
+        assert_eq!(
+            &stepping.schedules()[0],
+            schedule_b.label().as_ref().unwrap()
+        );
+        assert_eq!(
+            &stepping.schedules()[1],
+            schedule_a.label().as_ref().unwrap()
+        );
+        assert_eq!(
+            &stepping.schedules()[2],
+            schedule_c.label().as_ref().unwrap()
+        );
+
+        // make it complicated, run schedule_d after schedule_b and confirm it
+        // was inserted correctly
         stepping.next_frame();
-        let skip = stepping.build_skip_list(&schedule).unwrap();
-        assert_eq!(skip.len(), schedule.executable().systems.len());
-        assert!(skip.contains(0));
-        assert!(!skip.contains(1));
+        stepping.build_skip_list(&schedule_b);
+        stepping.build_skip_list(&schedule_d);
+        stepping.build_skip_list(&schedule_a);
+        stepping.build_skip_list(&schedule_c);
+        assert_eq!(
+            &stepping.schedules()[0],
+            schedule_b.label().as_ref().unwrap()
+        );
+        assert_eq!(
+            &stepping.schedules()[1],
+            schedule_d.label().as_ref().unwrap()
+        );
+        assert_eq!(
+            &stepping.schedules()[2],
+            schedule_a.label().as_ref().unwrap()
+        );
+        assert_eq!(
+            &stepping.schedules()[3],
+            schedule_c.label().as_ref().unwrap()
+        );
     }
 }

From 9004864f546121b479c0b6a1e5f95b3bc7f9c319 Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Sun, 2 Apr 2023 11:09:43 -0500
Subject: [PATCH 07/25] combine build step list functions

---
 crates/bevy_ecs/src/schedule/stepping.rs | 169 ++++++++---------------
 1 file changed, 58 insertions(+), 111 deletions(-)

diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs
index 0a1b2f34e6fb0..2816a43654e97 100644
--- a/crates/bevy_ecs/src/schedule/stepping.rs
+++ b/crates/bevy_ecs/src/schedule/stepping.rs
@@ -38,6 +38,7 @@ enum StepBehavior {
 
 enum SystemIdentifier {
     Type(TypeId),
+    #[allow(dead_code)]
     Node(NodeId),
 }
 
@@ -416,143 +417,89 @@ impl ScheduleState {
 
     // progress the schedule for the given action type, returning a skip list,
     // and if the caller should update state to waiting
-    fn progress(&mut self, schedule: &Schedule, action: StepAction) -> (FixedBitSet, bool) {
+    fn progress(&mut self, schedule: &Schedule, mut action: StepAction) -> (FixedBitSet, bool) {
+        use std::cmp::Ordering;
+
         // Now that we have the schedule, apply any pending system behavior
         // updates.  The schedule is required to map from system `TypeId` to
         // `NodeId`.
         if !self.updates.is_empty() {
             self.apply_updates(schedule);
         }
-
-        match action {
-            StepAction::RunAll => unreachable!(),
-            StepAction::Waiting => self.skip_list_waiting(schedule),
-            StepAction::Continue => self.skip_list_continue(schedule),
-            StepAction::Step => self.skip_list_step(schedule),
-        }
-    }
-
-    fn skip_list_waiting(&mut self, schedule: &Schedule) -> (FixedBitSet, bool) {
         let mut skip = FixedBitSet::with_capacity(schedule.systems_len());
 
+        let mut pos = self.next;
+        let start_pos = pos;
         for (i, (node_id, system)) in schedule.systems().unwrap().enumerate() {
-            println!(
-                "skip_list_waiting(): {} ({:?}) -- {:?}",
-                system.name(),
-                node_id,
-                self.systems.get(&node_id),
-            );
-
-            let mut advance = false;
-            match self
+            let behavior = self
                 .systems
                 .get(&node_id)
-                .unwrap_or(&StepBehavior::Continue)
-            {
-                StepBehavior::AlwaysRun => advance = true,
-                StepBehavior::NeverRun => {
-                    advance = true;
-                    skip.insert(i);
-                }
-                StepBehavior::Continue | StepBehavior::Break => skip.insert(i),
-            }
-
-            if advance && self.next == i {
-                self.next += 1;
-            }
-        }
-
-        (skip, false)
-    }
-
-    fn skip_list_step(&mut self, schedule: &Schedule) -> (FixedBitSet, bool) {
-        let mut skip = FixedBitSet::with_capacity(schedule.systems_len());
-        let mut stepped = false;
-        for (i, (node_id, system)) in schedule.systems().unwrap().enumerate() {
+                .unwrap_or(&StepBehavior::Continue);
             println!(
-                "skip_list_step(): {} ({:?}) -- {:?}",
-                system.name(),
-                node_id,
-                self.systems.get(&node_id),
-            );
-
-            let mut advance = false;
-            match self
-                .systems
-                .get(&node_id)
-                .unwrap_or(&StepBehavior::Continue)
-            {
-                StepBehavior::AlwaysRun => advance = true,
-                StepBehavior::NeverRun => {
-                    advance = true;
-                    skip.insert(i);
-                }
-                StepBehavior::Continue | StepBehavior::Break => {
-                    if i < self.next || stepped {
-                        skip.insert(i);
-                    } else {
-                        advance = true;
-                        stepped = true;
-                    }
-                }
-            }
-
-            if advance && self.next == i {
-                self.next += 1;
-            }
-        }
-
-        (skip, stepped)
-    }
-
-    fn skip_list_continue(&mut self, schedule: &Schedule) -> (FixedBitSet, bool) {
-        let mut skip = FixedBitSet::with_capacity(schedule.systems_len());
-        let mut hit_breakpoint = false;
-        let mut pos = self.next;
-
-        for (i, (node_id, system)) in schedule.systems().unwrap().enumerate() {
-            println!(
-                "skip_list_continue(): {} ({:?}) -- {:?}; i {}, pos {}, break {}",
-                system.name(),
-                node_id,
-                self.systems.get(&node_id),
+                "progress(): systems[{}], cursor {}, Action::{:?}, Behavior::{:?}, {}",
                 i,
                 pos,
-                hit_breakpoint,
+                action,
+                behavior,
+                system.name()
             );
-
-            match self
-                .systems
-                .get(&node_id)
-                .unwrap_or(&StepBehavior::Continue)
-            {
-                StepBehavior::AlwaysRun => (),
-                StepBehavior::NeverRun => skip.insert(i),
-                StepBehavior::Continue => {
-                    if i != pos {
+            match (action, behavior) {
+                // regardless of which action we're performing, if the system
+                // is marked as NeverRun, add it to the skip list
+                (_, StepBehavior::NeverRun) => skip.insert(i),
+                // similarly, ignore any system marked as AlwaysRun; they should
+                // never be added to the skip list
+                (_, StepBehavior::AlwaysRun) => (),
+                // if we're waiting, no other systems besides AlwaysRun should
+                // be run, so add systems to the skip list
+                (StepAction::Waiting, _) => skip.insert(i),
+                // If we're stepping, the remaining behaviors don't matter,
+                // we're only going to run the system at our cursor.  Any system
+                // not at our cursor is skipped.  Once we encounter the system
+                // at the cursor, we'll move the cursor up, and switch to
+                // Waiting, to skip remaining systems.
+                (StepAction::Step, _) => match i.cmp(&pos) {
+                    Ordering::Less => skip.insert(i),
+                    Ordering::Equal => {
+                        pos += 1;
+                        action = StepAction::Waiting;
+                    }
+                    Ordering::Greater => unreachable!(),
+                },
+                // If we're continuing, and the step behavior is continue, we
+                // want to skip any systems prior to our start position.  That's
+                // where the stepping frame left off last time we ran anything.
+                (StepAction::Continue, StepBehavior::Continue) => {
+                    if i < start_pos {
                         skip.insert(i);
                     }
                 }
-                StepBehavior::Break => {
-                    if i == self.next {
-                        println!("resuming from breakpoint");
-                    } else if i < pos {
-                        println!("skipping already run system");
-                        skip.insert(i);
-                    } else if i == pos {
-                        println!("hit breakpoint");
-                        hit_breakpoint = true;
+                // If we're continuing, and we encounter a breakpoint we want
+                // to stop before executing the system.  So we skip this system,
+                // and set the action to waiting.  Note that we do not move the
+                // cursor.  As a result, the next time we continue, if the
+                // first system is a Break we run it anyway.  This allows the
+                // user to continue, hit a breakpoint to stop before running
+                // the system, then continue again to run the system with the
+                // breakpoint.
+                (StepAction::Continue, StepBehavior::Break) => {
+                    if i != start_pos {
+                        debug_assert!(pos == i);
                         skip.insert(i);
+                        action = StepAction::Waiting;
                     }
                 }
+                // should have never gotten into this method if stepping is
+                // disabled
+                (StepAction::RunAll, _) => unreachable!(),
             }
 
-            if !hit_breakpoint && i == pos {
-                pos = i + 1;
+            if i == pos && action != StepAction::Waiting {
+                pos += 1;
             }
         }
         self.next = pos;
-        (skip, hit_breakpoint)
+        (skip, action == StepAction::Waiting)
     }
 }
 

From 545c5f0663730f51813e612dde5bf230d20a7036 Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Mon, 3 Apr 2023 19:24:55 -0500
Subject: [PATCH 08/25] move stepping frame cursor into Stepping

To support predictable system ordering for external consumers of
stepping, I moved the stepping frame cursor up into the `Stepping`
resource.  `Stepping` will now explicitly follow a schedule order when
stepping systems.  This order is dynamically created based on the order
in which schedules get their skip list.

Still don't like the names of some key functions like
`Stepping::skipped_systems()`.
---
 crates/bevy_app/src/main_schedule.rs          |   4 +-
 .../src/schedule/executor/multi_threaded.rs   |   1 +
 crates/bevy_ecs/src/schedule/schedule.rs      |   2 +-
 crates/bevy_ecs/src/schedule/stepping.rs      | 293 +++++++++---------
 4 files changed, 149 insertions(+), 151 deletions(-)

diff --git a/crates/bevy_app/src/main_schedule.rs b/crates/bevy_app/src/main_schedule.rs
index ef449999b990b..69bf7b91ac19b 100644
--- a/crates/bevy_app/src/main_schedule.rs
+++ b/crates/bevy_app/src/main_schedule.rs
@@ -154,7 +154,7 @@ pub struct MainSchedulePlugin;
 
 impl Plugin for MainSchedulePlugin {
     fn build(&self, app: &mut App) {
-        use bevy_ecs::schedule::Stepping;
+        use bevy_ecs::schedule::{IntoSystemConfigs, Stepping};
 
         // simple "facilitator" schedules benefit from simpler single threaded scheduling
         let mut main_schedule = Schedule::new();
@@ -165,6 +165,6 @@ impl Plugin for MainSchedulePlugin {
         app.add_schedule(Main, main_schedule)
             .add_schedule(RunFixedUpdateLoop, fixed_update_loop_schedule)
             .init_resource::<MainScheduleOrder>()
-            .add_systems(Main, (Stepping::begin_frame, Main::run_main));
+            .add_systems(Main, (Stepping::begin_frame, Main::run_main).chain());
     }
 }
diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
index ac008106f98bc..a06cf9b74c149 100644
--- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
+++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
@@ -172,6 +172,7 @@ impl SystemExecutor for MultiThreadedExecutor {
         // If stepping is enabled, make sure we skip those systems that should
         // not be run.
         if let Some(mut skipped_systems) = skip_systems {
+            debug_assert_eq!(skipped_systems.len(), self.completed_systems.len());
             // mark skipped systems as completed
             self.completed_systems |= &skipped_systems;
             self.num_completed_systems = self.completed_systems.count_ones(..);
diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs
index d70f8a0dfe2a2..a6c94b76548dd 100644
--- a/crates/bevy_ecs/src/schedule/schedule.rs
+++ b/crates/bevy_ecs/src/schedule/schedule.rs
@@ -251,7 +251,7 @@ impl Schedule {
 
         let skip_systems = match world.get_resource_mut::<Stepping>() {
             None => None,
-            Some(mut stepping) => stepping.build_skip_list(self),
+            Some(mut stepping) => stepping.skipped_systems(self),
         };
 
         self.executor.run(&mut self.executable, skip_systems, world);
diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs
index 2816a43654e97..9d4aef5d33bf0 100644
--- a/crates/bevy_ecs/src/schedule/stepping.rs
+++ b/crates/bevy_ecs/src/schedule/stepping.rs
@@ -11,7 +11,7 @@ use crate::{
 use crate as bevy_ecs;
 
 #[derive(Debug, Default, PartialEq, Eq, Copy, Clone)]
-enum StepAction {
+enum Action {
     /// Stepping is disabled; run all systems
     #[default]
     RunAll,
@@ -20,7 +20,7 @@ enum StepAction {
     Waiting,
 
     /// Stepping is enabled; run all systems until the end of the frame, or
-    /// until we encounter a system marked with [`StepBehavior::Break`] or all
+    /// until we encounter a system marked with [`SystemBehavior::Break`] or all
     /// systems in the frame have run.
     Continue,
 
@@ -29,13 +29,17 @@ enum StepAction {
 }
 
 #[derive(Debug, Copy, Clone)]
-enum StepBehavior {
+enum SystemBehavior {
     AlwaysRun,
     NeverRun,
     Continue,
     Break,
 }
 
+// schedule_order index, and schedule start point
+#[derive(Debug, Default)]
+struct Cursor(usize, usize);
+
 enum SystemIdentifier {
     Type(TypeId),
     #[allow(dead_code)]
@@ -43,30 +47,33 @@ enum SystemIdentifier {
 }
 
 enum Update {
-    Action(StepAction),
+    SetAction(Action),
     AddSchedule(BoxedScheduleLabel),
     RemoveSchedule(BoxedScheduleLabel),
     ClearSchedule(BoxedScheduleLabel),
-    SetBehavior(BoxedScheduleLabel, SystemIdentifier, StepBehavior),
+    SetBehavior(BoxedScheduleLabel, SystemIdentifier, SystemBehavior),
     ClearBehavior(BoxedScheduleLabel, SystemIdentifier),
 }
 
 #[derive(Resource, Default)]
 pub struct Stepping {
-    /// [`ScheduleState`] for each [`Schedule`] with stepping enabled
+    // [`ScheduleState`] for each [`Schedule`] with stepping enabled
     schedule_states: HashMap<BoxedScheduleLabel, ScheduleState>,
 
-    /// Action to perform during this frame
-    action: StepAction,
+    // dynamically generated [`Schedule`] order
+    schedule_order: Vec<BoxedScheduleLabel>,
 
-    /// Updates to stepping state to be applied at the start of the next frame
-    updates: Vec<Update>,
+    // current position in the stepping frame
+    cursor: Cursor,
 
-    /// dynamically generated [`Schedule`] order
-    schedule_order: Vec<BoxedScheduleLabel>,
+    // index in [`schedule_order`] of the last schedule to call `build_skip_list`
+    previous_schedule: Option<usize>,
+
+    // Action to perform during this render frame
+    action: Action,
 
-    /// tracks the index of the last schedule stepped in schedule_order
-    last_schedule: Option<usize>,
+    // Updates apply at the start of the next render frame
+    updates: Vec<Update>,
 }
 
 impl Stepping {
@@ -74,8 +81,6 @@ impl Stepping {
         Stepping::default()
     }
 
-    /// System that notifies the [`Stepping`] resource that a new frame has
-    /// begun.
     pub fn begin_frame(stepping: Option<ResMut<Self>>) {
         if let Some(mut stepping) = stepping {
             stepping.next_frame();
@@ -104,7 +109,7 @@ impl Stepping {
         self
     }
 
-    /// clear [`StepBehavior`] set for all systems in the provided [`Schedule`]
+    /// clear behavior set for all systems in the provided [`Schedule`]
     pub fn clear_schedule(&mut self, schedule: impl ScheduleLabel) -> &mut Self {
         self.updates.push(Update::ClearSchedule(Box::new(schedule)));
         self
@@ -112,25 +117,25 @@ impl Stepping {
 
     /// Begin stepping at the start of the next frame
     pub fn enable(&mut self) -> &mut Self {
-        self.updates.push(Update::Action(StepAction::Waiting));
+        self.updates.push(Update::SetAction(Action::Waiting));
         self
     }
 
     /// Disable stepping, resume normal systems execution
     pub fn disable(&mut self) -> &mut Self {
-        self.updates.push(Update::Action(StepAction::RunAll));
+        self.updates.push(Update::SetAction(Action::RunAll));
         self
     }
 
     /// run the next system
     pub fn step_frame(&mut self) -> &mut Self {
-        self.updates.push(Update::Action(StepAction::Step));
+        self.updates.push(Update::SetAction(Action::Step));
         self
     }
 
     /// continue stepping until the end of the frame or a breakpoint is hit
     pub fn continue_frame(&mut self) -> &mut Self {
-        self.updates.push(Update::Action(StepAction::Continue));
+        self.updates.push(Update::SetAction(Action::Continue));
         self
     }
 
@@ -144,7 +149,7 @@ impl Stepping {
         self.updates.push(Update::SetBehavior(
             Box::new(schedule),
             SystemIdentifier::Type(type_id),
-            StepBehavior::AlwaysRun,
+            SystemBehavior::AlwaysRun,
         ));
 
         self
@@ -160,7 +165,7 @@ impl Stepping {
         self.updates.push(Update::SetBehavior(
             Box::new(schedule),
             SystemIdentifier::Type(type_id),
-            StepBehavior::NeverRun,
+            SystemBehavior::NeverRun,
         ));
 
         self
@@ -176,7 +181,7 @@ impl Stepping {
         self.updates.push(Update::SetBehavior(
             Box::new(schedule),
             SystemIdentifier::Type(type_id),
-            StepBehavior::Break,
+            SystemBehavior::Break,
         ));
 
         self
@@ -210,45 +215,36 @@ impl Stepping {
 
     /// Advance schedule states for the next render frame
     fn next_frame(&mut self) {
-        // If we were stepping, and nothing ran, the action will remain Step.
-        // This happens when we've already run everything in the "stepping"
-        // frame.  Now, the user requested a step, let's reset our schedule
-        // state to the beginning and start a new "stepping" frame.
-        match self.action {
-            StepAction::Step => {
-                println!("wrapping state");
-                self.next_stepping_frame();
-
-                // This return here is to ensure that the step is executed.  We
-                // don't want to process updates from the previous frame
-                // because they may overwrite the action, resulting in Step
-                // doing nothing.
-                return;
-            }
-            StepAction::Continue => {
-                self.next_stepping_frame();
-                self.action = StepAction::Waiting;
-            }
-            _ => (),
+        println!("next_frame(): resetting action");
+        self.action = Action::Waiting;
+        self.previous_schedule = None;
+
+        let Cursor(schedule_index, _) = self.cursor;
+        if schedule_index >= self.schedule_order.len() {
+            println!("next_frame(): begin new stepping frame");
+            self.cursor = Cursor(0, 0);
         }
 
         if self.updates.is_empty() {
             return;
         }
 
-        let mut clear_schedule = false;
         for update in self.updates.drain(..) {
             match update {
-                Update::Action(StepAction::RunAll) => {
-                    self.action = StepAction::RunAll;
-                    clear_schedule = true;
+                Update::SetAction(Action::RunAll) => {
+                    self.action = Action::RunAll;
+                    self.cursor = Cursor(0, 0);
                 }
-                Update::Action(a) => self.action = a,
+                Update::SetAction(a) => self.action = a,
                 Update::AddSchedule(l) => {
                     self.schedule_states.insert(l, ScheduleState::default());
                 }
-                Update::RemoveSchedule(l) => {
-                    self.schedule_states.remove(&l);
+                Update::RemoveSchedule(label) => {
+                    self.schedule_states.remove(&label);
+                    if let Some(index) = self.schedule_order.iter().position(|l| l == &label) {
+                        self.schedule_order.remove(index);
+                    }
+                    self.cursor = Cursor(0, 0);
                 }
                 Update::ClearSchedule(label) => match self.schedule_states.get_mut(&label) {
                     Some(state) => state.clear_behaviors(),
@@ -257,7 +253,7 @@ impl Stepping {
                             "stepping is not enabled for schedule {:?}; use
                     `.add_stepping({:?})` to enable stepping",
                             label, label
-                        )
+                        );
                     }
                 },
                 Update::SetBehavior(label, system, behavior) => {
@@ -268,7 +264,7 @@ impl Stepping {
                                 "stepping is not enabled for schedule {:?}; use
                     `.add_stepping({:?})` to enable stepping",
                                 label, label
-                            )
+                            );
                         }
                     }
                 }
@@ -280,94 +276,93 @@ impl Stepping {
                                 "stepping is not enabled for schedule {:?}; use
                     `.add_stepping({:?})` to enable stepping",
                                 label, label
-                            )
+                            );
                         }
                     }
                 }
             }
         }
-
-        if clear_schedule {
-            self.next_stepping_frame();
-        }
     }
 
-    /// Advance schedule states for a new stepping frame
-    fn next_stepping_frame(&mut self) {
-        println!("next_stepping_frame(): being new stepping frame");
-
-        // clear any progress we've made in the stepping frame
-        self.last_schedule = None;
-
-        for state in self.schedule_states.values_mut() {
-            state.next = 0;
-        }
-    }
-
-    /// Build the list of systems the supplied schedule should skip this render
+    /// get the list of systems this schedule should skip for this render
     /// frame
-    pub fn build_skip_list(&mut self, schedule: &Schedule) -> Option<FixedBitSet> {
-        if self.action == StepAction::RunAll {
+    pub fn skipped_systems(&mut self, schedule: &Schedule) -> Option<FixedBitSet> {
+        if self.action == Action::RunAll {
             return None;
         }
-        let state = match schedule.label() {
-            Some(label) => self.schedule_states.get_mut(label)?,
+
+        // grab the label and state for the schedule if they've been added to
+        // stepping
+        let (label, state) = match schedule.label() {
             None => return None,
+            Some(label) => (label, self.schedule_states.get_mut(label)?),
         };
 
-        // We are stepping through a  schedule that has stepping enabled.
+        // Stepping is enabled, and this schedule is supposed to be stepped.
         //
-        // Let's take a moment to make sure this schedule is in our ordered
-        // schedule list.  We maintain this to give any stepping UI a way to
-        // know what order schedules are called in.
-        let label = schedule.label().as_ref().unwrap();
+        // We need to get this schedule's index in the ordered schedule list.
+        // If it's not present, it will be inserted in place after the index
+        // of the previous schedule that called this function.
         let index = self.schedule_order.iter().position(|l| l == label);
-        match (self.last_schedule, index) {
-            (_, Some(i)) => self.last_schedule = Some(i),
-            (Some(last), None) => {
-                self.schedule_order.insert(last + 1, label.clone());
-                self.last_schedule = Some(last + 1);
-            }
+        let index = match (index, self.previous_schedule) {
+            (Some(index), _) => index,
             (None, None) => {
                 self.schedule_order.insert(0, label.clone());
-                self.last_schedule = Some(0);
+                0
             }
-        }
+            (None, Some(last)) => {
+                self.schedule_order.insert(last + 1, label.clone());
+                last + 1
+            }
+        };
+        self.previous_schedule = Some(index);
+        println!(
+            "skipped_systems(): cursor {:?}, label {:?}, index {}",
+            self.cursor, label, index
+        );
 
-        let (list, wait) = state.progress(schedule, self.action);
-        if wait {
-            println!(
-                "build_skip_list({:?}): state {:?} -> {:?}",
-                schedule.label(),
-                self.action,
-                StepAction::Waiting
-            );
-            self.action = StepAction::Waiting;
+        // if the stepping frame cursor is pointing at this schedule, we'll run
+        // the schedule with the current stepping action.  If this is not the
+        // cursor schedule, we'll run the schedule with the waiting action.
+        let Cursor(schedule_index, start) = self.cursor;
+        let (list, resume) = if index == schedule_index {
+            let o = state.skipped_systems(schedule, start, self.action);
+
+            // if we just stepped this schedule, then we'll switch the action
+            // to be waiting
+            if self.action == Action::Step {
+                self.action = Action::Waiting;
+            }
+            o
+        } else {
+            state.skipped_systems(schedule, start, Action::Waiting)
+        };
+
+        // update the stepping frame cursor based on if there are any systems
+        // remaining to be run in the schedule
+        // XXX Maybe not option here to auto-detect the end of the render frame
+        match resume {
+            None => self.cursor = Cursor(schedule_index + 1, 0),
+            Some(resume) => self.cursor.1 = resume,
         }
+
         Some(list)
     }
 }
 
 #[derive(Default)]
 struct ScheduleState {
-    /// Index of the next system to run in the system schedule; if set to
-    /// greater than the number of systems in the schedule, then don't run
-    /// any systems.
-    /// NOTE: [`StepBehavior::AlwaysRun`] systems ignore this value and always
-    /// run
-    next: usize,
-
-    /// per-system [`StepBehavior`]
-    systems: HashMap<NodeId, StepBehavior>,
+    /// per-system [`SystemBehavior`]
+    systems: HashMap<NodeId, SystemBehavior>,
 
     /// changes to system behavior that should be applied the next time
     /// [`ScheduleState::progress()`] is called
-    updates: HashMap<TypeId, Option<StepBehavior>>,
+    updates: HashMap<TypeId, Option<SystemBehavior>>,
 }
 
 impl ScheduleState {
     // set the stepping behavior for a system in this schedule
-    fn set_behavior(&mut self, system: SystemIdentifier, behavior: StepBehavior) {
+    fn set_behavior(&mut self, system: SystemIdentifier, behavior: SystemBehavior) {
         match system {
             SystemIdentifier::Node(node_id) => {
                 self.systems.insert(node_id, behavior);
@@ -415,9 +410,12 @@ impl ScheduleState {
         println!("apply_updates(): {:?}", self.systems);
     }
 
-    // progress the schedule for the given action type, returning a skip list,
-    // and if the caller should update state to waiting
-    fn progress(&mut self, schedule: &Schedule, mut action: StepAction) -> (FixedBitSet, bool) {
+    fn skipped_systems(
+        &mut self,
+        schedule: &Schedule,
+        start: usize,
+        mut action: Action,
+    ) -> (FixedBitSet, Option<usize>) {
         use std::cmp::Ordering;
 
         // Now that we have the schedule, apply any pending system behavior
@@ -428,49 +426,51 @@ impl ScheduleState {
         }
         let mut skip = FixedBitSet::with_capacity(schedule.systems_len());
 
-        let mut pos = self.next;
-        let start_pos = pos;
+        let mut pos = start;
         for (i, (node_id, system)) in schedule.systems().unwrap().enumerate() {
             let behavior = self
                 .systems
                 .get(&node_id)
-                .unwrap_or(&StepBehavior::Continue);
+                .unwrap_or(&SystemBehavior::Continue);
             println!(
-                "progress(): systems[{}], cursor {}, Action::{:?}, Behavior::{:?}, {}",
+                "skipped_systems(): systems[{}], cursor {}, Action::{:?}, Behavior::{:?}, {}",
                 i,
                 pos,
                 action,
                 behavior,
                 system.name()
             );
+            // suppress this clippy warning; we want to keep the arms split to
+            // clearly document whats going on.
+            #[allow(clippy::match_same_arms)]
             match (action, behavior) {
                 // regardless of which action we're performing, if the system
                 // is marked as NeverRun, add it to the skip list
-                (_, StepBehavior::NeverRun) => skip.insert(i),
+                (_, SystemBehavior::NeverRun) => skip.insert(i),
                 // similarly, ignore any system marked as AlwaysRun; they should
                 // never be added to the skip list
-                (_, StepBehavior::AlwaysRun) => (),
+                (_, SystemBehavior::AlwaysRun) => (),
                 // if we're waiting, no other systems besides AlwaysRun should
                 // be run, so add systems to the skip list
-                (StepAction::Waiting, _) => skip.insert(i),
+                (Action::Waiting, _) => skip.insert(i),
                 // If we're stepping, the remaining behaviors don't matter,
                 // we're only going to run the system at our cursor.  Any system
                 // not at our cursor is skipped.  Once we encounter the system
                 // at the cursor, we'll move the cursor up, and switch to
                 // Waiting, to skip remaining systems.
-                (StepAction::Step, _) => match i.cmp(&pos) {
+                (Action::Step, _) => match i.cmp(&pos) {
                     Ordering::Less => skip.insert(i),
                     Ordering::Equal => {
                         pos += 1;
-                        action = StepAction::Waiting;
+                        action = Action::Waiting;
                     }
                     Ordering::Greater => unreachable!(),
                 },
                 // If we're continuing, and the step behavior is continue, we
                 // want to skip any systems prior to our start position.  That's
                 // where the stepping frame left off last time we ran anything.
-                (StepAction::Continue, StepBehavior::Continue) => {
-                    if i < start_pos {
+                (Action::Continue, SystemBehavior::Continue) => {
+                    if i < start {
                         skip.insert(i);
                     }
                 }
@@ -482,24 +482,28 @@ impl ScheduleState {
                 // user to continue, hit a breakpoint to stop before running
                 // the system, then continue again to run the system with the
                 // breakpoint.
-                (StepAction::Continue, StepBehavior::Break) => {
-                    if i != start_pos {
+                (Action::Continue, SystemBehavior::Break) => {
+                    if i != start {
                         debug_assert!(pos == i);
                         skip.insert(i);
-                        action = StepAction::Waiting;
+                        action = Action::Waiting;
                     }
                 }
                 // should have never gotten into this method if stepping is
                 // disabled
-                (StepAction::RunAll, _) => unreachable!(),
+                (Action::RunAll, _) => unreachable!(),
             }
 
-            if i == pos && action != StepAction::Waiting {
+            if i == pos && action != Action::Waiting {
                 pos += 1;
             }
         }
-        self.next = pos;
-        (skip, action == StepAction::Waiting)
+
+        if pos >= schedule.systems_len() {
+            (skip, None)
+        } else {
+            (skip, Some(pos))
+        }
     }
 }
 
@@ -572,7 +576,7 @@ mod tests {
             // advance stepping to the next frame, and build the skip list for
             // this schedule
             $stepping.next_frame();
-            let actual = match $stepping.build_skip_list($schedule) {
+            let actual = match $stepping.skipped_systems($schedule) {
                 None => FixedBitSet::with_capacity(systems.len()),
                 Some(b) => b,
             };
@@ -608,7 +612,7 @@ mod tests {
         let mut stepping = Stepping::new();
         stepping.add_schedule(TestSchedule).disable().next_frame();
 
-        assert!(stepping.build_skip_list(&schedule).is_none());
+        assert!(stepping.skipped_systems(&schedule).is_none());
     }
 
     #[test]
@@ -618,7 +622,7 @@ mod tests {
         let mut stepping = Stepping::new();
         stepping.enable().next_frame();
 
-        assert!(stepping.build_skip_list(&schedule).is_none());
+        assert!(stepping.skipped_systems(&schedule).is_none());
     }
 
     #[test]
@@ -771,16 +775,9 @@ mod tests {
         stepping.step_frame();
         assert_schedule_runs!(&schedule, &mut stepping, second_system);
 
-        // when stepping, we take one render frame to detect that the stepping
-        // frame has completed.  This looks like nothing runs for the first
-        // render frame, then the second render-frame resets the stepping frame
-        // and we run the first system again.
-        //
-        // XXX this is weird from the UI standpoint; how does UI display what
-        // is the next system to be run, esp when transitioning between
-        // schedules.
+        // let's go again to verify that we wrap back around to the start of
+        // the frame
         stepping.step_frame();
-        assert_schedule_runs!(&schedule, &mut stepping,);
         assert_schedule_runs!(&schedule, &mut stepping, first_system);
 
         // should be back in a waiting state now that it ran first_system
@@ -888,9 +885,9 @@ mod tests {
             .enable()
             .next_frame();
 
-        stepping.build_skip_list(&schedule_b);
-        stepping.build_skip_list(&schedule_a);
-        stepping.build_skip_list(&schedule_c);
+        stepping.skipped_systems(&schedule_b);
+        stepping.skipped_systems(&schedule_a);
+        stepping.skipped_systems(&schedule_c);
         assert_eq!(
             &stepping.schedules()[0],
             schedule_b.label().as_ref().unwrap()
@@ -907,10 +904,10 @@ mod tests {
         // make it complicated, run schedule_d after schedule_b and confirm it
         // was inserted correctly
         stepping.next_frame();
-        stepping.build_skip_list(&schedule_b);
-        stepping.build_skip_list(&schedule_d);
-        stepping.build_skip_list(&schedule_a);
-        stepping.build_skip_list(&schedule_c);
+        stepping.skipped_systems(&schedule_b);
+        stepping.skipped_systems(&schedule_d);
+        stepping.skipped_systems(&schedule_a);
+        stepping.skipped_systems(&schedule_c);
         assert_eq!(
             &stepping.schedules()[0],
             schedule_b.label().as_ref().unwrap()

From 9d85b2e916495cca9c3c0858de908ff4f9eb58ee Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Mon, 3 Apr 2023 21:45:14 -0500
Subject: [PATCH 09/25] WIP: add stepping to breakout

need to merge in main for wgpu crash fix
---
 crates/bevy_ecs/src/schedule/stepping.rs |  58 ++++-
 examples/games/breakout.rs               |   2 +
 examples/games/stepping.rs               | 289 +++++++++++++++++++++++
 3 files changed, 338 insertions(+), 11 deletions(-)
 create mode 100644 examples/games/stepping.rs

diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs
index 9d4aef5d33bf0..92744a4bca143 100644
--- a/crates/bevy_ecs/src/schedule/stepping.rs
+++ b/crates/bevy_ecs/src/schedule/stepping.rs
@@ -37,8 +37,11 @@ enum SystemBehavior {
 }
 
 // schedule_order index, and schedule start point
-#[derive(Debug, Default)]
-struct Cursor(usize, usize);
+#[derive(Debug, Default, Clone)]
+pub struct Cursor {
+    pub schedule: usize,
+    pub system: usize,
+}
 
 enum SystemIdentifier {
     Type(TypeId),
@@ -89,10 +92,15 @@ impl Stepping {
 
     /// Return the list of schedules with stepping enabled in the order
     /// they are executed in.
-    pub fn schedules(&mut self) -> &Vec<BoxedScheduleLabel> {
+    pub fn schedules(&self) -> &Vec<BoxedScheduleLabel> {
         &self.schedule_order
     }
 
+    /// Return our current position within the stepping frame
+    pub fn cursor(&self) -> Cursor {
+        self.cursor.clone()
+    }
+
     /// Enable stepping for the provided schedule
     pub fn add_schedule<Label>(&mut self, schedule: Label) -> &mut Self
     where
@@ -127,6 +135,11 @@ impl Stepping {
         self
     }
 
+    /// Check if stepping is enabled
+    pub fn is_enabled(&self) -> bool {
+        self.action != Action::RunAll
+    }
+
     /// run the next system
     pub fn step_frame(&mut self) -> &mut Self {
         self.updates.push(Update::SetAction(Action::Step));
@@ -219,10 +232,16 @@ impl Stepping {
         self.action = Action::Waiting;
         self.previous_schedule = None;
 
-        let Cursor(schedule_index, _) = self.cursor;
+        let Cursor {
+            schedule: schedule_index,
+            ..
+        } = self.cursor;
         if schedule_index >= self.schedule_order.len() {
             println!("next_frame(): begin new stepping frame");
-            self.cursor = Cursor(0, 0);
+            self.cursor = Cursor {
+                schedule: 0,
+                system: 0,
+            };
         }
 
         if self.updates.is_empty() {
@@ -233,7 +252,10 @@ impl Stepping {
             match update {
                 Update::SetAction(Action::RunAll) => {
                     self.action = Action::RunAll;
-                    self.cursor = Cursor(0, 0);
+                    self.cursor = Cursor {
+                        schedule: 0,
+                        system: 0,
+                    };
                 }
                 Update::SetAction(a) => self.action = a,
                 Update::AddSchedule(l) => {
@@ -244,7 +266,10 @@ impl Stepping {
                     if let Some(index) = self.schedule_order.iter().position(|l| l == &label) {
                         self.schedule_order.remove(index);
                     }
-                    self.cursor = Cursor(0, 0);
+                    self.cursor = Cursor {
+                        schedule: 0,
+                        system: 0,
+                    };
                 }
                 Update::ClearSchedule(label) => match self.schedule_states.get_mut(&label) {
                     Some(state) => state.clear_behaviors(),
@@ -324,7 +349,10 @@ impl Stepping {
         // if the stepping frame cursor is pointing at this schedule, we'll run
         // the schedule with the current stepping action.  If this is not the
         // cursor schedule, we'll run the schedule with the waiting action.
-        let Cursor(schedule_index, start) = self.cursor;
+        let Cursor {
+            schedule: schedule_index,
+            system: start,
+        } = self.cursor;
         let (list, resume) = if index == schedule_index {
             let o = state.skipped_systems(schedule, start, self.action);
 
@@ -340,10 +368,18 @@ impl Stepping {
 
         // update the stepping frame cursor based on if there are any systems
         // remaining to be run in the schedule
-        // XXX Maybe not option here to auto-detect the end of the render frame
+        // Note: Don't try to detect the end of the render frame here using the
+        // schedule index.  We don't know all schedules have been added to the
+        // schedule_order, so only next_frame() knows its safe to reset the
+        // cursor.
         match resume {
-            None => self.cursor = Cursor(schedule_index + 1, 0),
-            Some(resume) => self.cursor.1 = resume,
+            None => {
+                self.cursor = Cursor {
+                    schedule: schedule_index + 1,
+                    system: 0,
+                }
+            }
+            Some(resume) => self.cursor.system = resume,
         }
 
         Some(list)
diff --git a/examples/games/breakout.rs b/examples/games/breakout.rs
index a10b194edb9e3..7e1cd6ce6d33d 100644
--- a/examples/games/breakout.rs
+++ b/examples/games/breakout.rs
@@ -6,6 +6,8 @@ use bevy::{
     sprite::MaterialMesh2dBundle,
 };
 
+mod stepping;
+
 // These constants are defined in `Transform` units.
 // Using the default 2D camera they correspond 1:1 with screen pixels.
 const PADDLE_SIZE: Vec3 = Vec3::new(120.0, 20.0, 0.0);
diff --git a/examples/games/stepping.rs b/examples/games/stepping.rs
new file mode 100644
index 0000000000000..8282291292ea4
--- /dev/null
+++ b/examples/games/stepping.rs
@@ -0,0 +1,289 @@
+use bevy::{app::MainScheduleOrder, ecs::schedule::*, prelude::*};
+use std::collections::HashMap;
+
+/// Independent [`Schedule`] for stepping systems.
+///
+/// The stepping systems must run in their own schedule to be able to inspect
+/// all the other schedules in the [`App`].  This is because the currently
+/// executing schedule is removed from the [`Schedules`] resource while it is
+/// being run.
+#[derive(Debug, Hash, PartialEq, Eq, Clone, ScheduleLabel)]
+struct Debug;
+
+/// Plugin to add a stepping UI to an example
+#[derive(Default)]
+pub struct SteppingPlugin {
+    schedule_labels: Vec<Box<dyn ScheduleLabel>>,
+    top: Val,
+    left: Val,
+}
+
+impl SteppingPlugin {
+    /// Initialize the plugin to step the schedules specified in `labels`
+    pub fn for_schedules(labels: Vec<Box<dyn ScheduleLabel>>) -> SteppingPlugin {
+        SteppingPlugin {
+            schedule_labels: labels,
+            ..default()
+        }
+    }
+
+    /// Set the location of the stepping UI when activated
+    pub fn at(self, left: Val, top: Val) -> SteppingPlugin {
+        SteppingPlugin { top, left, ..self }
+    }
+}
+
+impl Plugin for SteppingPlugin {
+    fn build(&self, app: &mut App) {
+        // create and insert our debug schedule into the main schedule order.
+        // We need an independent schedule so we have access to all other
+        // schedules through the `Stepping` resource
+        app.init_schedule(Debug);
+        let mut order = app.world.resource_mut::<MainScheduleOrder>();
+        order.insert_after(Update, Debug);
+
+        // create our stepping resource
+        let mut stepping = Stepping::new();
+        for label in self.schedule_labels.iter() {
+            stepping.add_schedule(label.clone());
+        }
+        app.insert_resource(stepping);
+
+        // add our startup & stepping systems
+        app.insert_resource(State {
+            ui_top: self.top,
+            ui_left: self.left,
+            status: Status::Init,
+            schedule_text_map: HashMap::new(),
+        })
+        .add_systems(
+            Debug,
+            (
+                build_ui.run_if(not(initialized)),
+                handle_input.run_if(initialized),
+                update_ui.run_if(initialized),
+            ),
+        );
+    }
+}
+
+#[derive(Debug, PartialEq)]
+enum Status {
+    // initial state, waiting for build_ui() to complete successfully
+    Init,
+    // game running normally
+    Run,
+    // stepping enabled; value is index into State.schedule_labels
+    Step,
+}
+
+/// Struct for maintaining stepping state
+#[derive(Resource, Debug)]
+struct State {
+    // map of Schedule/NodeId to TextSection index in UI entity
+    // This is used to draw the position indicator as we step
+    schedule_text_map: HashMap<BoxedScheduleLabel, Vec<usize>>,
+
+    // status of the stepping plugin
+    status: Status,
+
+    // ui positioning
+    ui_top: Val,
+    ui_left: Val,
+}
+
+/// condition to check if the stepping UI has been constructed
+fn initialized(state: Res<State>) -> bool {
+    !matches!(state.status, Status::Init)
+}
+
+const FONT_SIZE: f32 = 20.0;
+const FONT_COLOR: Color = Color::rgb(0.2, 0.2, 0.2);
+const FONT_BOLD: &str = "fonts/FiraSans-Bold.ttf";
+const FONT_MEDIUM: &str = "fonts/FiraMono-Medium.ttf";
+
+#[derive(Component)]
+struct SteppingUi;
+
+/// Construct the stepping UI elements from the [`Schedules`] resource.
+///
+/// This system may run multiple times before constructing the UI as all of the
+/// data may not be available on the first run of the system.  This happens if
+/// one of the stepping schedules has not yet been run.
+fn build_ui(
+    mut commands: Commands,
+    asset_server: Res<AssetServer>,
+    schedules: Res<Schedules>,
+    stepping: Res<Stepping>,
+    mut state: ResMut<State>,
+) {
+    let mut text_sections = Vec::new();
+
+    // go through the stepping schedules and construct a list of systems for
+    // each label
+    for label in stepping.schedules() {
+        let schedule = schedules.get(&**label).unwrap();
+        text_sections.push(TextSection::new(
+            format!("{:?}\n", label),
+            TextStyle {
+                font: asset_server.load(FONT_BOLD),
+                font_size: FONT_SIZE,
+                color: FONT_COLOR,
+            },
+        ));
+
+        // grab the list of systems in the schedule, in the order the
+        // single-threaded executor would run them.
+        let systems = match schedule.systems() {
+            Ok(iter) => iter,
+            Err(_) => return,
+        };
+
+        let mut system_index = Vec::new();
+
+        for (_node_id, system) in systems {
+            system_index.push(text_sections.len());
+            text_sections.push(TextSection::new(
+                "   ",
+                TextStyle {
+                    font: asset_server.load(FONT_MEDIUM),
+                    font_size: FONT_SIZE,
+                    color: FONT_COLOR,
+                },
+            ));
+
+            text_sections.push(TextSection::new(
+                format!("{}\n", system.name()),
+                TextStyle {
+                    font: asset_server.load(FONT_MEDIUM),
+                    font_size: FONT_SIZE,
+                    color: FONT_COLOR,
+                },
+            ));
+        }
+
+        state.schedule_text_map.insert(label.clone(), system_index);
+    }
+
+    state.status = Status::Run;
+
+    commands.spawn((
+        SteppingUi,
+        TextBundle {
+            text: Text::from_sections(text_sections),
+            style: Style {
+                position_type: PositionType::Absolute,
+                top: state.ui_top,
+                left: state.ui_left,
+                padding: UiRect::all(Val::Px(10.0)),
+                ..default()
+            },
+            background_color: BackgroundColor(Color::rgba(1.0, 1.0, 1.0, 0.33)),
+            visibility: Visibility::Hidden,
+            ..default()
+        },
+    ));
+
+    // stepping description box
+    commands.spawn((TextBundle::from_sections([TextSection::new(
+        "Press ` to toggle stepping mode (S: step system, Space: step frame)",
+        TextStyle {
+            font: asset_server.load(FONT_MEDIUM),
+            font_size: 15.0,
+            color: FONT_COLOR,
+        },
+    )])
+    .with_style(Style {
+        position_type: PositionType::Absolute,
+        bottom: Val::Px(5.0),
+        left: Val::Px(5.0),
+        ..default()
+    }),));
+}
+
+fn handle_input(
+    keyboard_input: Res<Input<KeyCode>>,
+    mut stepping: ResMut<Stepping>,
+    mut state: ResMut<State>,
+) {
+    // grave key to toggle stepping mode for the FixedUpdate schedule
+    if keyboard_input.just_pressed(KeyCode::Grave) {
+        match state.status {
+            Status::Init => return,
+            Status::Run => {
+                state.status = Status::Step;
+                stepping.enable();
+            }
+            Status::Step => {
+                state.status = Status::Run;
+                stepping.disable();
+            }
+        }
+    }
+
+    if state.status == Status::Run {
+        return;
+    }
+
+    // space key will step the remainder of this frame
+    if keyboard_input.just_pressed(KeyCode::Space) {
+        debug!("continue");
+        stepping.continue_frame();
+        return;
+    } else if keyboard_input.just_pressed(KeyCode::S) {
+        debug!("stepping frame");
+        stepping.step_frame();
+    }
+}
+
+fn update_ui(
+    mut commands: Commands,
+    state: Res<State>,
+    stepping: Res<Stepping>,
+    mut ui: Query<(Entity, &mut Text, &Visibility), With<SteppingUi>>,
+) {
+    if ui.is_empty() {
+        return;
+    }
+
+    // If we're stepping, ensure the UI is visibile, and grab the current
+    // schedule label.  Otherwise, hide the UI and just return.
+    let (entity, mut text, vis) = ui.single_mut();
+    match state.status {
+        Status::Step => {
+            if vis == Visibility::Hidden {
+                commands.entity(entity).insert(Visibility::Inherited);
+            };
+        }
+        _ => {
+            // ensure the UI is hidden if we're not stepping
+            if vis != Visibility::Hidden {
+                commands.entity(entity).insert(Visibility::Hidden);
+            }
+            return;
+        }
+    };
+
+    let cursor = stepping.cursor();
+    for (schedule, label) in stepping.schedules().iter().enumerate() {
+        for (system, text_index) in state
+            .schedule_text_map
+            .get(label)
+            .unwrap()
+            .iter()
+            .enumerate()
+        {
+            let here = schedule == cursor.schedule && system == cursor.system;
+            debug!(
+                "schedule {:?} ({}), system {}, cursor {:?}",
+                label, schedule, system, here
+            );
+            let mark = if schedule == cursor.schedule && system == cursor.system {
+                "-> "
+            } else {
+                "   "
+            };
+            text.sections[*text_index].value = mark.to_string();
+        }
+    }
+}

From bca6c85477c2fc58e7f9e1d19e952f7f77980875 Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Sun, 16 Apr 2023 21:42:11 -0500
Subject: [PATCH 10/25] Added stepping to breakout

Need to rewrite `Stepping::Cursor` to expose NodeId of next system, and
ensure it always contains a valid NodeId.  This means tracking what the
first NodeId of each System will be.
---
 crates/bevy_app/src/app.rs               |  20 +-
 crates/bevy_ecs/src/schedule/schedule.rs |   2 +-
 crates/bevy_ecs/src/schedule/stepping.rs | 262 +++++++++++++++++------
 examples/games/breakout.rs               |  14 +-
 examples/games/stepping.rs               | 110 +++++-----
 5 files changed, 271 insertions(+), 137 deletions(-)

diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs
index c424397c1fc33..a779c7690ab18 100644
--- a/crates/bevy_app/src/app.rs
+++ b/crates/bevy_app/src/app.rs
@@ -388,6 +388,7 @@ impl App {
             schedule.add_systems(systems);
         } else {
             let mut new_schedule = Schedule::new();
+            new_schedule.set_label(schedule.dyn_clone());
             new_schedule.add_systems(systems);
             schedules.insert(schedule, new_schedule);
         }
@@ -459,6 +460,7 @@ impl App {
             schedule.configure_set(set);
         } else {
             let mut new_schedule = Schedule::new();
+            new_schedule.set_label(schedule.dyn_clone());
             new_schedule.configure_set(set);
             schedules.insert(schedule, new_schedule);
         }
@@ -476,6 +478,7 @@ impl App {
             schedule.configure_sets(sets);
         } else {
             let mut new_schedule = Schedule::new();
+            new_schedule.set_label(schedule.dyn_clone());
             new_schedule.configure_sets(sets);
             schedules.insert(schedule, new_schedule);
         }
@@ -869,12 +872,11 @@ impl App {
     /// # Warning
     /// This method will overwrite any existing schedule at that label.
     /// To avoid this behavior, use the `init_schedule` method instead.
-    pub fn add_schedule<Label>(&mut self, label: Label, mut schedule: Schedule) -> &mut Self
-    where
-        Label: ScheduleLabel + Clone,
-    {
-        schedule.set_label(label.clone());
+    pub fn add_schedule(&mut self, label: impl ScheduleLabel, mut schedule: Schedule) -> &mut Self {
         let mut schedules = self.world.resource_mut::<Schedules>();
+        if schedule.label().is_none() {
+            schedule.set_label(label.dyn_clone());
+        }
         schedules.insert(label, schedule);
 
         self
@@ -886,7 +888,9 @@ impl App {
     pub fn init_schedule(&mut self, label: impl ScheduleLabel) -> &mut Self {
         let mut schedules = self.world.resource_mut::<Schedules>();
         if !schedules.contains(&label) {
-            schedules.insert(label, Schedule::new());
+            let mut schedule = Schedule::new();
+            schedule.set_label(label.dyn_clone());
+            schedules.insert(label, schedule);
         }
         self
     }
@@ -916,7 +920,9 @@ impl App {
         let mut schedules = self.world.resource_mut::<Schedules>();
 
         if schedules.get(&label).is_none() {
-            schedules.insert(label.dyn_clone(), Schedule::new());
+            let mut schedule = Schedule::new();
+            schedule.set_label(label.dyn_clone());
+            schedules.insert(label.dyn_clone(), schedule);
         }
 
         let schedule = schedules.get_mut(&label).unwrap();
diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs
index 1b64259993e33..5dcb51aa6723d 100644
--- a/crates/bevy_ecs/src/schedule/schedule.rs
+++ b/crates/bevy_ecs/src/schedule/schedule.rs
@@ -186,7 +186,7 @@ impl Schedule {
     /// Set the label for this schedule
     pub fn set_label(&mut self, label: impl ScheduleLabel) -> &mut Self {
         debug_assert!(self.label.is_none());
-        self.label = Some(Box::new(label));
+        self.label = Some(label.dyn_clone());
         self
     }
 
diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs
index 92744a4bca143..9f2b28034eab8 100644
--- a/crates/bevy_ecs/src/schedule/stepping.rs
+++ b/crates/bevy_ecs/src/schedule/stepping.rs
@@ -7,6 +7,7 @@ use crate::{
     schedule::{BoxedScheduleLabel, NodeId, Schedule, ScheduleLabel},
     system::{IntoSystem, ResMut, Resource, System},
 };
+use bevy_utils::thiserror::Error;
 
 use crate as bevy_ecs;
 
@@ -36,6 +37,14 @@ enum SystemBehavior {
     Break,
 }
 
+#[derive(Debug, Default, Clone)]
+pub enum SystemCursor {
+    #[default]
+    First,
+    Index(usize, NodeId),
+    None,
+}
+
 // schedule_order index, and schedule start point
 #[derive(Debug, Default, Clone)]
 pub struct Cursor {
@@ -58,6 +67,10 @@ enum Update {
     ClearBehavior(BoxedScheduleLabel, SystemIdentifier),
 }
 
+#[derive(Error, Debug)]
+#[error("not available until all configured schedules have been run; try again next frame")]
+pub struct NotReady;
+
 #[derive(Resource, Default)]
 pub struct Stepping {
     // [`ScheduleState`] for each [`Schedule`] with stepping enabled
@@ -79,6 +92,26 @@ pub struct Stepping {
     updates: Vec<Update>,
 }
 
+impl std::fmt::Debug for Stepping {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            "Stepping {{ action: {:?}, schedules: {:?}, order: {:?}",
+            self.action,
+            self.schedule_states.keys(),
+            self.schedule_order
+        )?;
+        if self.action != Action::RunAll {
+            let Cursor { schedule, system } = self.cursor;
+            match self.schedule_order.get(schedule) {
+                Some(label) => write!(f, "cursor: {:?}[{}], ", label, system)?,
+                None => write!(f, "cursor: None, ")?,
+            };
+        }
+        write!(f, "}}")
+    }
+}
+
 impl Stepping {
     pub fn new() -> Self {
         Stepping::default()
@@ -92,8 +125,18 @@ impl Stepping {
 
     /// Return the list of schedules with stepping enabled in the order
     /// they are executed in.
-    pub fn schedules(&self) -> &Vec<BoxedScheduleLabel> {
-        &self.schedule_order
+    pub fn schedules(&self) -> Result<&Vec<BoxedScheduleLabel>, NotReady> {
+        /*
+        println!(
+            "schedule_order {:?}, schedule_states {:?}",
+            self.schedule_order.len(),
+            self.schedule_states.len()
+        ); */
+        if self.schedule_order.len() == self.schedule_states.len() {
+            Ok(&self.schedule_order)
+        } else {
+            Err(NotReady)
+        }
     }
 
     /// Return our current position within the stepping frame
@@ -102,11 +145,8 @@ impl Stepping {
     }
 
     /// Enable stepping for the provided schedule
-    pub fn add_schedule<Label>(&mut self, schedule: Label) -> &mut Self
-    where
-        Label: ScheduleLabel + Clone,
-    {
-        self.updates.push(Update::AddSchedule(Box::new(schedule)));
+    pub fn add_schedule(&mut self, schedule: impl ScheduleLabel) -> &mut Self {
+        self.updates.push(Update::AddSchedule(schedule.dyn_clone()));
         self
     }
 
@@ -152,7 +192,10 @@ impl Stepping {
         self
     }
 
-    /// Ensure these systems always run when stepping is enabled
+    /// Ensure this system always runs when stepping is enabled
+    ///
+    /// Note: if the system is run multiple times in the [`Schedule`], this
+    /// will apply for all instances of the system.
     pub fn always_run<Marker>(
         &mut self,
         schedule: impl ScheduleLabel,
@@ -160,7 +203,7 @@ impl Stepping {
     ) -> &mut Self {
         let type_id = IntoSystem::into_system(system).type_id();
         self.updates.push(Update::SetBehavior(
-            Box::new(schedule),
+            schedule.dyn_clone(),
             SystemIdentifier::Type(type_id),
             SystemBehavior::AlwaysRun,
         ));
@@ -168,6 +211,16 @@ impl Stepping {
         self
     }
 
+    /// Ensure this system always runs when stepping is enabled
+    pub fn always_run_node(&mut self, schedule: impl ScheduleLabel, node: NodeId) -> &mut Self {
+        self.updates.push(Update::SetBehavior(
+            schedule.dyn_clone(),
+            SystemIdentifier::Node(node),
+            SystemBehavior::AlwaysRun,
+        ));
+        self
+    }
+
     /// Ensure this system never runs when stepping is enabled
     pub fn never_run<Marker>(
         &mut self,
@@ -228,20 +281,23 @@ impl Stepping {
 
     /// Advance schedule states for the next render frame
     fn next_frame(&mut self) {
-        println!("next_frame(): resetting action");
-        self.action = Action::Waiting;
-        self.previous_schedule = None;
-
-        let Cursor {
-            schedule: schedule_index,
-            ..
-        } = self.cursor;
-        if schedule_index >= self.schedule_order.len() {
-            println!("next_frame(): begin new stepping frame");
-            self.cursor = Cursor {
-                schedule: 0,
-                system: 0,
-            };
+        // if stepping is enabled; reset our internal state for the start of
+        // the next frame
+        if self.action != Action::RunAll {
+            self.action = Action::Waiting;
+            self.previous_schedule = None;
+
+            // if the cursor passed the last schedule, reset it
+            let Cursor {
+                schedule: schedule_index,
+                ..
+            } = self.cursor;
+            if schedule_index >= self.schedule_order.len() {
+                self.cursor = Cursor {
+                    schedule: 0,
+                    system: 0,
+                };
+            }
         }
 
         if self.updates.is_empty() {
@@ -318,6 +374,24 @@ impl Stepping {
 
         // grab the label and state for the schedule if they've been added to
         // stepping
+        let mut found = false;
+        for label in self.schedule_states.keys() {
+            match schedule.label() {
+                Some(l) => {
+                    if l == label {
+                        found = true;
+                    }
+                }
+                None => (),
+            }
+        }
+        assert!(
+            !found
+                || self
+                    .schedule_states
+                    .get_mut(schedule.label().as_ref().unwrap())
+                    .is_some()
+        );
         let (label, state) = match schedule.label() {
             None => return None,
             Some(label) => (label, self.schedule_states.get_mut(label)?),
@@ -342,7 +416,7 @@ impl Stepping {
         };
         self.previous_schedule = Some(index);
         println!(
-            "skipped_systems(): cursor {:?}, label {:?}, index {}",
+            "Stepping::skipped_systems(): cursor {:?}, label {:?}, index {}",
             self.cursor, label, index
         );
 
@@ -363,7 +437,10 @@ impl Stepping {
             }
             o
         } else {
-            state.skipped_systems(schedule, start, Action::Waiting)
+            // we're not supposed to run any systems in this schedule, so pull
+            // the skip list, but ignore any changes it makes to the cursor.
+            let (skip, _) = state.skipped_systems(schedule, 0, Action::Waiting);
+            (skip, Some(start))
         };
 
         // update the stepping frame cursor based on if there are any systems
@@ -374,6 +451,11 @@ impl Stepping {
         // cursor.
         match resume {
             None => {
+                println!(
+                    "Stepping::skipped_systems(): next schedule {} -> {}",
+                    schedule_index,
+                    schedule_index + 1
+                );
                 self.cursor = Cursor {
                     schedule: schedule_index + 1,
                     system: 0,
@@ -460,16 +542,17 @@ impl ScheduleState {
         if !self.updates.is_empty() {
             self.apply_updates(schedule);
         }
-        let mut skip = FixedBitSet::with_capacity(schedule.systems_len());
 
+        let mut skip = FixedBitSet::with_capacity(schedule.systems_len());
         let mut pos = start;
+
         for (i, (node_id, system)) in schedule.systems().unwrap().enumerate() {
             let behavior = self
                 .systems
                 .get(&node_id)
                 .unwrap_or(&SystemBehavior::Continue);
             println!(
-                "skipped_systems(): systems[{}], cursor {}, Action::{:?}, Behavior::{:?}, {}",
+                "skipped_systems(): systems[{}], pos {}, Action::{:?}, Behavior::{:?}, {}",
                 i,
                 pos,
                 action,
@@ -579,14 +662,10 @@ mod tests {
         (schedule, world)
     }
 
-    // Helper for verifying the expected systems will be run by the schedule
-    //
-    // This macro will construct an expected FixedBitSet for the systems that
-    // should be skipped, and compare it with the results from stepping the
-    // provided schedule.  If they don't match, it generates a human-readable
-    // error message and asserts.
-    macro_rules! assert_schedule_runs {
-        ($schedule:expr, $stepping:expr, $($system:expr),*) => {
+    // Helper for verifying that a set of systems will be run for a given skip
+    // list
+    macro_rules! assert_systems_run {
+        ($schedule:expr, $skipped_systems:expr, $($system:expr),*) => {
             // pull an ordered list of systems in the schedule, and save the
             // system TypeId, and name.
             let systems: Vec<(TypeId, std::borrow::Cow<'static, str>)> = $schedule.systems().unwrap()
@@ -609,10 +688,8 @@ mod tests {
             // flip the run list to get our skip list
             expected.toggle_range(..);
 
-            // advance stepping to the next frame, and build the skip list for
-            // this schedule
-            $stepping.next_frame();
-            let actual = match $stepping.skipped_systems($schedule) {
+            // grab the list of skipped systems
+            let actual = match $skipped_systems {
                 None => FixedBitSet::with_capacity(systems.len()),
                 Some(b) => b,
             };
@@ -641,6 +718,21 @@ mod tests {
         };
     }
 
+    // Helper for verifying the expected systems will be run by the schedule
+    //
+    // This macro will construct an expected FixedBitSet for the systems that
+    // should be skipped, and compare it with the results from stepping the
+    // provided schedule.  If they don't match, it generates a human-readable
+    // error message and asserts.
+    macro_rules! assert_schedule_runs {
+        ($schedule:expr, $stepping:expr, $($system:expr),*) => {
+            // advance stepping to the next frame, and build the skip list for
+            // this schedule
+            $stepping.next_frame();
+            assert_systems_run!($schedule, $stepping.skipped_systems($schedule), $($system),*);
+        };
+    }
+
     #[test]
     fn stepping_disabled() {
         let (schedule, _world) = setup();
@@ -893,6 +985,50 @@ mod tests {
         assert_schedule_runs!(&schedule, &mut stepping, first_system, second_system);
     }
 
+    // Schedules such as FixedUpdate can be called multiple times in a single
+    // render frame.  Ensure we only run steppable systems the first time the
+    // schedule is run
+    #[test]
+    fn multiple_calls_per_frame_continue() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping
+            .add_schedule(TestSchedule)
+            .enable()
+            .always_run(TestSchedule, second_system)
+            .continue_frame();
+
+        // start a new frame, then run the schedule three times; first system
+        // should only run on the first one
+        stepping.next_frame();
+        assert_systems_run!(
+            &schedule,
+            stepping.skipped_systems(&schedule),
+            first_system,
+            second_system
+        );
+        assert_systems_run!(
+            &schedule,
+            stepping.skipped_systems(&schedule),
+            second_system
+        );
+    }
+    #[test]
+    fn multiple_calls_per_frame_step() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping.add_schedule(TestSchedule).enable().step_frame();
+
+        // start a new frame, then run the schedule three times; first system
+        // should only run on the first one
+        stepping.next_frame();
+        assert_systems_run!(&schedule, stepping.skipped_systems(&schedule), first_system);
+        assert_systems_run!(&schedule, stepping.skipped_systems(&schedule),);
+    }
+
+    // verify that Stepping can construct an ordered list of schedules
     #[test]
     fn schedules() {
         let mut world = World::new();
@@ -921,44 +1057,28 @@ mod tests {
             .enable()
             .next_frame();
 
+        assert!(stepping.schedules().is_err());
+
         stepping.skipped_systems(&schedule_b);
+        assert!(stepping.schedules().is_err());
         stepping.skipped_systems(&schedule_a);
+        assert!(stepping.schedules().is_err());
         stepping.skipped_systems(&schedule_c);
-        assert_eq!(
-            &stepping.schedules()[0],
-            schedule_b.label().as_ref().unwrap()
-        );
-        assert_eq!(
-            &stepping.schedules()[1],
-            schedule_a.label().as_ref().unwrap()
-        );
-        assert_eq!(
-            &stepping.schedules()[2],
-            schedule_c.label().as_ref().unwrap()
-        );
+        assert!(stepping.schedules().is_err());
 
-        // make it complicated, run schedule_d after schedule_b and confirm it
-        // was inserted correctly
-        stepping.next_frame();
-        stepping.skipped_systems(&schedule_b);
+        // when we call the last schedule, Stepping should have enough data to
+        // return an ordered list of schedules
         stepping.skipped_systems(&schedule_d);
-        stepping.skipped_systems(&schedule_a);
-        stepping.skipped_systems(&schedule_c);
-        assert_eq!(
-            &stepping.schedules()[0],
-            schedule_b.label().as_ref().unwrap()
-        );
-        assert_eq!(
-            &stepping.schedules()[1],
-            schedule_d.label().as_ref().unwrap()
-        );
-        assert_eq!(
-            &stepping.schedules()[2],
-            schedule_a.label().as_ref().unwrap()
-        );
+        assert!(stepping.schedules().is_ok());
+
         assert_eq!(
-            &stepping.schedules()[3],
-            schedule_c.label().as_ref().unwrap()
+            *stepping.schedules().unwrap(),
+            vec![
+                TestScheduleB.dyn_clone(),
+                TestScheduleA.dyn_clone(),
+                TestScheduleC.dyn_clone(),
+                TestScheduleD.dyn_clone(),
+            ]
         );
     }
 }
diff --git a/examples/games/breakout.rs b/examples/games/breakout.rs
index 7e1cd6ce6d33d..abe34699d0750 100644
--- a/examples/games/breakout.rs
+++ b/examples/games/breakout.rs
@@ -52,6 +52,10 @@ const SCORE_COLOR: Color = Color::rgb(1.0, 0.5, 0.5);
 fn main() {
     App::new()
         .add_plugins(DefaultPlugins)
+        .add_plugin(
+            stepping::SteppingPlugin::for_schedules(vec![Box::new(Update), Box::new(FixedUpdate)])
+                .at(Val::Percent(35.0), Val::Percent(50.0)),
+        )
         .insert_resource(Scoreboard { score: 0 })
         .insert_resource(ClearColor(BACKGROUND_COLOR))
         .add_event::<CollisionEvent>()
@@ -173,6 +177,9 @@ struct Scoreboard {
     score: usize,
 }
 
+#[derive(Component)]
+struct ScoreboardUi;
+
 // Add the game's entities to our world
 fn setup(
     mut commands: Commands,
@@ -220,7 +227,8 @@ fn setup(
     ));
 
     // Scoreboard
-    commands.spawn(
+    commands.spawn((
+        ScoreboardUi,
         TextBundle::from_sections([
             TextSection::new(
                 "Score: ",
@@ -242,7 +250,7 @@ fn setup(
             left: SCOREBOARD_TEXT_PADDING,
             ..default()
         }),
-    );
+    ));
 
     // Walls
     commands.spawn(WallBundle::new(WallLocation::Left));
@@ -345,7 +353,7 @@ fn apply_velocity(mut query: Query<(&mut Transform, &Velocity)>, time_step: Res<
     }
 }
 
-fn update_scoreboard(scoreboard: Res<Scoreboard>, mut query: Query<&mut Text>) {
+fn update_scoreboard(scoreboard: Res<Scoreboard>, mut query: Query<&mut Text, With<ScoreboardUi>>) {
     let mut text = query.single_mut();
     text.sections[1].value = scoreboard.score.to_string();
 }
diff --git a/examples/games/stepping.rs b/examples/games/stepping.rs
index 8282291292ea4..c0e8787db2954 100644
--- a/examples/games/stepping.rs
+++ b/examples/games/stepping.rs
@@ -45,38 +45,29 @@ impl Plugin for SteppingPlugin {
         // create our stepping resource
         let mut stepping = Stepping::new();
         for label in self.schedule_labels.iter() {
-            stepping.add_schedule(label.clone());
+            stepping.add_schedule(label.dyn_clone());
         }
+        assert!(!stepping.is_enabled());
         app.insert_resource(stepping);
 
         // add our startup & stepping systems
         app.insert_resource(State {
             ui_top: self.top,
             ui_left: self.left,
-            status: Status::Init,
             schedule_text_map: HashMap::new(),
         })
         .add_systems(
             Debug,
             (
                 build_ui.run_if(not(initialized)),
-                handle_input.run_if(initialized),
+                handle_input,
                 update_ui.run_if(initialized),
-            ),
+            )
+                .chain(),
         );
     }
 }
 
-#[derive(Debug, PartialEq)]
-enum Status {
-    // initial state, waiting for build_ui() to complete successfully
-    Init,
-    // game running normally
-    Run,
-    // stepping enabled; value is index into State.schedule_labels
-    Step,
-}
-
 /// Struct for maintaining stepping state
 #[derive(Resource, Debug)]
 struct State {
@@ -84,9 +75,6 @@ struct State {
     // This is used to draw the position indicator as we step
     schedule_text_map: HashMap<BoxedScheduleLabel, Vec<usize>>,
 
-    // status of the stepping plugin
-    status: Status,
-
     // ui positioning
     ui_top: Val,
     ui_left: Val,
@@ -94,7 +82,7 @@ struct State {
 
 /// condition to check if the stepping UI has been constructed
 fn initialized(state: Res<State>) -> bool {
-    !matches!(state.status, Status::Init)
+    !state.schedule_text_map.is_empty()
 }
 
 const FONT_SIZE: f32 = 20.0;
@@ -114,14 +102,23 @@ fn build_ui(
     mut commands: Commands,
     asset_server: Res<AssetServer>,
     schedules: Res<Schedules>,
-    stepping: Res<Stepping>,
+    mut stepping: ResMut<Stepping>,
     mut state: ResMut<State>,
 ) {
+    debug_assert!(state.schedule_text_map.is_empty());
+
     let mut text_sections = Vec::new();
+    let mut always_run = Vec::new();
+
+    let schedule_order = match stepping.schedules() {
+        Ok(s) => s,
+        Err(_) => return,
+    };
 
     // go through the stepping schedules and construct a list of systems for
     // each label
-    for label in stepping.schedules() {
+    for label in schedule_order {
+        println!("schedule {:?}", label);
         let schedule = schedules.get(&**label).unwrap();
         text_sections.push(TextSection::new(
             format!("{:?}\n", label),
@@ -141,7 +138,13 @@ fn build_ui(
 
         let mut system_index = Vec::new();
 
-        for (_node_id, system) in systems {
+        for (node_id, system) in systems {
+            if system.name().starts_with("bevy") {
+                always_run.push((label.dyn_clone(), node_id));
+                println!("skipping {:?}", system.name());
+                continue;
+            }
+
             system_index.push(text_sections.len());
             text_sections.push(TextSection::new(
                 "   ",
@@ -152,6 +155,7 @@ fn build_ui(
                 },
             ));
 
+            println!("found system: {:?}/{}\n", label, system.name());
             text_sections.push(TextSection::new(
                 format!("{}\n", system.name()),
                 TextStyle {
@@ -165,7 +169,9 @@ fn build_ui(
         state.schedule_text_map.insert(label.clone(), system_index);
     }
 
-    state.status = Status::Run;
+    for (label, node) in always_run.drain(..) {
+        stepping.always_run_node(label, node);
+    }
 
     commands.spawn((
         SteppingUi,
@@ -201,27 +207,22 @@ fn build_ui(
     }),));
 }
 
-fn handle_input(
-    keyboard_input: Res<Input<KeyCode>>,
-    mut stepping: ResMut<Stepping>,
-    mut state: ResMut<State>,
-) {
+fn handle_input(keyboard_input: Res<Input<KeyCode>>, mut stepping: ResMut<Stepping>) {
+    if keyboard_input.just_pressed(KeyCode::Slash) {
+        debug!("stepping: {:#?}", stepping);
+    }
     // grave key to toggle stepping mode for the FixedUpdate schedule
     if keyboard_input.just_pressed(KeyCode::Grave) {
-        match state.status {
-            Status::Init => return,
-            Status::Run => {
-                state.status = Status::Step;
-                stepping.enable();
-            }
-            Status::Step => {
-                state.status = Status::Run;
-                stepping.disable();
-            }
+        if stepping.is_enabled() {
+            stepping.disable();
+            debug!("disabled stepping");
+        } else {
+            stepping.enable();
+            debug!("enabled stepping");
         }
     }
 
-    if state.status == Status::Run {
+    if !stepping.is_enabled() {
         return;
     }
 
@@ -229,7 +230,6 @@ fn handle_input(
     if keyboard_input.just_pressed(KeyCode::Space) {
         debug!("continue");
         stepping.continue_frame();
-        return;
     } else if keyboard_input.just_pressed(KeyCode::S) {
         debug!("stepping frame");
         stepping.step_frame();
@@ -246,26 +246,26 @@ fn update_ui(
         return;
     }
 
-    // If we're stepping, ensure the UI is visibile, and grab the current
-    // schedule label.  Otherwise, hide the UI and just return.
-    let (entity, mut text, vis) = ui.single_mut();
-    match state.status {
-        Status::Step => {
-            if vis == Visibility::Hidden {
-                commands.entity(entity).insert(Visibility::Inherited);
-            };
+    // ensure the UI is only visible when stepping is enabled
+    let (ui, mut text, vis) = ui.single_mut();
+    match (vis, stepping.is_enabled()) {
+        (Visibility::Hidden, true) => {
+            commands.entity(ui).insert(Visibility::Inherited);
         }
-        _ => {
-            // ensure the UI is hidden if we're not stepping
-            if vis != Visibility::Hidden {
-                commands.entity(entity).insert(Visibility::Hidden);
-            }
-            return;
+        (Visibility::Hidden, false) => (),
+        (_, true) => (),
+        (_, false) => {
+            commands.entity(ui).insert(Visibility::Hidden);
         }
-    };
+    }
+
+    // if we're not stepping, there's nothing more to be done here.
+    if !stepping.is_enabled() {
+        return;
+    }
 
     let cursor = stepping.cursor();
-    for (schedule, label) in stepping.schedules().iter().enumerate() {
+    for (schedule, label) in stepping.schedules().unwrap().iter().enumerate() {
         for (system, text_index) in state
             .schedule_text_map
             .get(label)

From 625e6906a1ce67e44a9653671fd418234ae390a7 Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Thu, 20 Apr 2023 13:25:12 -0400
Subject: [PATCH 11/25] Stepping cursor changed to use label & NodeId

It was challenging to write a stepping UI using justthe label & system index.
So I've updated `Stepping.cursor()` to return `(BoxedScheduleLabel,
NodeId)`.  This required `Stepping` keeping a list of `NodeId`s.
---
 crates/bevy_ecs/src/schedule/stepping.rs | 128 ++++++++++++++++-------
 examples/games/stepping.rs               |  56 ++++------
 2 files changed, 112 insertions(+), 72 deletions(-)

diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs
index 9f2b28034eab8..8d61cd9a8d639 100644
--- a/crates/bevy_ecs/src/schedule/stepping.rs
+++ b/crates/bevy_ecs/src/schedule/stepping.rs
@@ -37,14 +37,6 @@ enum SystemBehavior {
     Break,
 }
 
-#[derive(Debug, Default, Clone)]
-pub enum SystemCursor {
-    #[default]
-    First,
-    Index(usize, NodeId),
-    None,
-}
-
 // schedule_order index, and schedule start point
 #[derive(Debug, Default, Clone)]
 pub struct Cursor {
@@ -140,8 +132,22 @@ impl Stepping {
     }
 
     /// Return our current position within the stepping frame
-    pub fn cursor(&self) -> Cursor {
-        self.cursor.clone()
+    pub fn cursor(&self) -> Option<(BoxedScheduleLabel, NodeId)> {
+        if self.action == Action::RunAll {
+            return None;
+        }
+        let label = match self.schedule_order.get(self.cursor.schedule) {
+            None => return None,
+            Some(label) => label,
+        };
+        let state = match self.schedule_states.get(label) {
+            None => return None,
+            Some(state) => state,
+        };
+        state
+            .node_ids
+            .get(self.cursor.system)
+            .map(|node_id| (label.dyn_clone(), *node_id))
     }
 
     /// Enable stepping for the provided schedule
@@ -279,6 +285,27 @@ impl Stepping {
         self
     }
 
+    /// lookup the first system for the supplied schedule index
+    fn first_system_index_for_schedule(&self, index: usize) -> usize {
+        let label = match self.schedule_order.get(index) {
+            None => return 0,
+            Some(label) => label,
+        };
+        let state = match self.schedule_states.get(label) {
+            None => return 0,
+            Some(state) => state,
+        };
+        state.first.unwrap_or(0)
+    }
+
+    /// Move the cursor to the start of the first schedule
+    fn reset_cursor(&mut self) {
+        self.cursor = Cursor {
+            schedule: 0,
+            system: self.first_system_index_for_schedule(0),
+        };
+    }
+
     /// Advance schedule states for the next render frame
     fn next_frame(&mut self) {
         // if stepping is enabled; reset our internal state for the start of
@@ -293,10 +320,7 @@ impl Stepping {
                 ..
             } = self.cursor;
             if schedule_index >= self.schedule_order.len() {
-                self.cursor = Cursor {
-                    schedule: 0,
-                    system: 0,
-                };
+                self.reset_cursor();
             }
         }
 
@@ -304,14 +328,12 @@ impl Stepping {
             return;
         }
 
+        let mut reset_cursor = false;
         for update in self.updates.drain(..) {
             match update {
                 Update::SetAction(Action::RunAll) => {
                     self.action = Action::RunAll;
-                    self.cursor = Cursor {
-                        schedule: 0,
-                        system: 0,
-                    };
+                    reset_cursor = true;
                 }
                 Update::SetAction(a) => self.action = a,
                 Update::AddSchedule(l) => {
@@ -322,10 +344,7 @@ impl Stepping {
                     if let Some(index) = self.schedule_order.iter().position(|l| l == &label) {
                         self.schedule_order.remove(index);
                     }
-                    self.cursor = Cursor {
-                        schedule: 0,
-                        system: 0,
-                    };
+                    reset_cursor = true;
                 }
                 Update::ClearSchedule(label) => match self.schedule_states.get_mut(&label) {
                     Some(state) => state.clear_behaviors(),
@@ -363,6 +382,10 @@ impl Stepping {
                 }
             }
         }
+
+        if reset_cursor {
+            self.reset_cursor();
+        }
     }
 
     /// get the list of systems this schedule should skip for this render
@@ -374,6 +397,7 @@ impl Stepping {
 
         // grab the label and state for the schedule if they've been added to
         // stepping
+        // XXX Take a second look at this
         let mut found = false;
         for label in self.schedule_states.keys() {
             match schedule.label() {
@@ -385,7 +409,7 @@ impl Stepping {
                 None => (),
             }
         }
-        assert!(
+        debug_assert!(
             !found
                 || self
                     .schedule_states
@@ -427,7 +451,7 @@ impl Stepping {
             schedule: schedule_index,
             system: start,
         } = self.cursor;
-        let (list, resume) = if index == schedule_index {
+        let (list, next_system) = if index == schedule_index {
             let o = state.skipped_systems(schedule, start, self.action);
 
             // if we just stepped this schedule, then we'll switch the action
@@ -449,19 +473,19 @@ impl Stepping {
         // schedule index.  We don't know all schedules have been added to the
         // schedule_order, so only next_frame() knows its safe to reset the
         // cursor.
-        match resume {
+        match next_system {
+            Some(i) => self.cursor.system = i,
             None => {
+                let index = schedule_index + 1;
                 println!(
                     "Stepping::skipped_systems(): next schedule {} -> {}",
-                    schedule_index,
-                    schedule_index + 1
+                    schedule_index, index,
                 );
                 self.cursor = Cursor {
-                    schedule: schedule_index + 1,
-                    system: 0,
+                    schedule: index,
+                    system: self.first_system_index_for_schedule(index),
                 }
             }
-            Some(resume) => self.cursor.system = resume,
         }
 
         Some(list)
@@ -471,19 +495,26 @@ impl Stepping {
 #[derive(Default)]
 struct ScheduleState {
     /// per-system [`SystemBehavior`]
-    systems: HashMap<NodeId, SystemBehavior>,
+    behaviors: HashMap<NodeId, SystemBehavior>,
+
+    /// order of NodeIds in the schedule
+    node_ids: Vec<NodeId>,
 
     /// changes to system behavior that should be applied the next time
     /// [`ScheduleState::progress()`] is called
     updates: HashMap<TypeId, Option<SystemBehavior>>,
+
+    /// This field contains the first steppable system in the schedule.
+    first: Option<usize>,
 }
 
 impl ScheduleState {
     // set the stepping behavior for a system in this schedule
     fn set_behavior(&mut self, system: SystemIdentifier, behavior: SystemBehavior) {
+        self.first = None;
         match system {
             SystemIdentifier::Node(node_id) => {
-                self.systems.insert(node_id, behavior);
+                self.behaviors.insert(node_id, behavior);
             }
             SystemIdentifier::Type(type_id) => {
                 self.updates.insert(type_id, Some(behavior));
@@ -493,9 +524,10 @@ impl ScheduleState {
 
     // clear the stepping behavior for a system in this schedule
     fn clear_behavior(&mut self, system: SystemIdentifier) {
+        self.first = None;
         match system {
             SystemIdentifier::Node(node_id) => {
-                self.systems.remove(&node_id);
+                self.behaviors.remove(&node_id);
             }
             SystemIdentifier::Type(type_id) => {
                 self.updates.insert(type_id, None);
@@ -505,7 +537,8 @@ impl ScheduleState {
 
     // clear all system behaviors
     fn clear_behaviors(&mut self) {
-        self.systems.clear();
+        self.behaviors.clear();
+        self.first = None;
     }
 
     // apply system behavior updates by looking up the node id of the system in
@@ -516,16 +549,16 @@ impl ScheduleState {
             match behavior {
                 None => continue,
                 Some(None) => {
-                    self.systems.remove(&node_id);
+                    self.behaviors.remove(&node_id);
                 }
                 Some(Some(behavior)) => {
-                    self.systems.insert(node_id, *behavior);
+                    self.behaviors.insert(node_id, *behavior);
                 }
             }
         }
         self.updates.clear();
 
-        println!("apply_updates(): {:?}", self.systems);
+        println!("apply_updates(): {:?}", self.behaviors);
     }
 
     fn skipped_systems(
@@ -536,6 +569,12 @@ impl ScheduleState {
     ) -> (FixedBitSet, Option<usize>) {
         use std::cmp::Ordering;
 
+        // if our NodeId list hasn't been populated, copy it over from the
+        // schedule
+        if self.node_ids.len() != schedule.systems_len() {
+            self.node_ids = schedule.executable().system_ids.clone();
+        }
+
         // Now that we have the schedule, apply any pending system behavior
         // updates.  The schedule is required to map from system `TypeId` to
         // `NodeId`.
@@ -543,12 +582,25 @@ impl ScheduleState {
             self.apply_updates(schedule);
         }
 
+        // if we don't have a first system set, set it now
+        if self.first.is_none() {
+            for (i, (node_id, _)) in schedule.systems().unwrap().enumerate() {
+                match self.behaviors.get(&node_id) {
+                    Some(SystemBehavior::AlwaysRun) | Some(SystemBehavior::NeverRun) => continue,
+                    Some(_) | None => {
+                        self.first = Some(i);
+                        break;
+                    }
+                }
+            }
+        }
+
         let mut skip = FixedBitSet::with_capacity(schedule.systems_len());
         let mut pos = start;
 
         for (i, (node_id, system)) in schedule.systems().unwrap().enumerate() {
             let behavior = self
-                .systems
+                .behaviors
                 .get(&node_id)
                 .unwrap_or(&SystemBehavior::Continue);
             println!(
diff --git a/examples/games/stepping.rs b/examples/games/stepping.rs
index c0e8787db2954..f2290fb1ed0b1 100644
--- a/examples/games/stepping.rs
+++ b/examples/games/stepping.rs
@@ -1,5 +1,4 @@
 use bevy::{app::MainScheduleOrder, ecs::schedule::*, prelude::*};
-use std::collections::HashMap;
 
 /// Independent [`Schedule`] for stepping systems.
 ///
@@ -54,7 +53,7 @@ impl Plugin for SteppingPlugin {
         app.insert_resource(State {
             ui_top: self.top,
             ui_left: self.left,
-            schedule_text_map: HashMap::new(),
+            systems: Vec::new(),
         })
         .add_systems(
             Debug,
@@ -71,9 +70,8 @@ impl Plugin for SteppingPlugin {
 /// Struct for maintaining stepping state
 #[derive(Resource, Debug)]
 struct State {
-    // map of Schedule/NodeId to TextSection index in UI entity
-    // This is used to draw the position indicator as we step
-    schedule_text_map: HashMap<BoxedScheduleLabel, Vec<usize>>,
+    // vector of schedule/nodeid -> text index offset
+    systems: Vec<(BoxedScheduleLabel, NodeId, usize)>,
 
     // ui positioning
     ui_top: Val,
@@ -82,7 +80,7 @@ struct State {
 
 /// condition to check if the stepping UI has been constructed
 fn initialized(state: Res<State>) -> bool {
-    !state.schedule_text_map.is_empty()
+    !state.systems.is_empty()
 }
 
 const FONT_SIZE: f32 = 20.0;
@@ -105,8 +103,6 @@ fn build_ui(
     mut stepping: ResMut<Stepping>,
     mut state: ResMut<State>,
 ) {
-    debug_assert!(state.schedule_text_map.is_empty());
-
     let mut text_sections = Vec::new();
     let mut always_run = Vec::new();
 
@@ -136,8 +132,6 @@ fn build_ui(
             Err(_) => return,
         };
 
-        let mut system_index = Vec::new();
-
         for (node_id, system) in systems {
             if system.name().starts_with("bevy") {
                 always_run.push((label.dyn_clone(), node_id));
@@ -145,7 +139,9 @@ fn build_ui(
                 continue;
             }
 
-            system_index.push(text_sections.len());
+            state
+                .systems
+                .push((label.dyn_clone(), node_id, text_sections.len()));
             text_sections.push(TextSection::new(
                 "   ",
                 TextStyle {
@@ -165,8 +161,6 @@ fn build_ui(
                 },
             ));
         }
-
-        state.schedule_text_map.insert(label.clone(), system_index);
     }
 
     for (label, node) in always_run.drain(..) {
@@ -250,11 +244,13 @@ fn update_ui(
     let (ui, mut text, vis) = ui.single_mut();
     match (vis, stepping.is_enabled()) {
         (Visibility::Hidden, true) => {
+            println!("showing UI");
             commands.entity(ui).insert(Visibility::Inherited);
         }
         (Visibility::Hidden, false) => (),
         (_, true) => (),
         (_, false) => {
+            println!("hiding UI");
             commands.entity(ui).insert(Visibility::Hidden);
         }
     }
@@ -264,26 +260,18 @@ fn update_ui(
         return;
     }
 
-    let cursor = stepping.cursor();
-    for (schedule, label) in stepping.schedules().unwrap().iter().enumerate() {
-        for (system, text_index) in state
-            .schedule_text_map
-            .get(label)
-            .unwrap()
-            .iter()
-            .enumerate()
-        {
-            let here = schedule == cursor.schedule && system == cursor.system;
-            debug!(
-                "schedule {:?} ({}), system {}, cursor {:?}",
-                label, schedule, system, here
-            );
-            let mark = if schedule == cursor.schedule && system == cursor.system {
-                "-> "
-            } else {
-                "   "
-            };
-            text.sections[*text_index].value = mark.to_string();
-        }
+    let (cursor_schedule, cursor_system) = match stepping.cursor() {
+        // no cursor means stepping isn't enabled, so we're done here
+        None => return,
+        Some(c) => c,
+    };
+
+    for (schedule, system, text_index) in &state.systems {
+        let mark = if &cursor_schedule == schedule && *system == cursor_system {
+            "-> "
+        } else {
+            "   "
+        };
+        text.sections[*text_index].value = mark.to_string();
     }
 }

From 4a6cea522fb86331d367785286a3c98fa93cc25c Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Thu, 20 Apr 2023 21:31:29 -0400
Subject: [PATCH 12/25] Add cursor tests, and Stepping _node() methods

---
 crates/bevy_ecs/src/schedule/stepping.rs | 177 ++++++++++++++++++++++-
 1 file changed, 173 insertions(+), 4 deletions(-)

diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs
index 8d61cd9a8d639..d6bf6df4e668c 100644
--- a/crates/bevy_ecs/src/schedule/stepping.rs
+++ b/crates/bevy_ecs/src/schedule/stepping.rs
@@ -132,6 +132,11 @@ impl Stepping {
     }
 
     /// Return our current position within the stepping frame
+    ///
+    /// NOTE: This function **will** return `None` during normal execution with
+    /// stepping enabled.  This can happen at the end of the stepping frame
+    /// after the last system has been run, but before the start of the next
+    /// render frame.
     pub fn cursor(&self) -> Option<(BoxedScheduleLabel, NodeId)> {
         if self.action == Action::RunAll {
             return None;
@@ -217,7 +222,7 @@ impl Stepping {
         self
     }
 
-    /// Ensure this system always runs when stepping is enabled
+    /// Ensure this system instance always runs when stepping is enabled
     pub fn always_run_node(&mut self, schedule: impl ScheduleLabel, node: NodeId) -> &mut Self {
         self.updates.push(Update::SetBehavior(
             schedule.dyn_clone(),
@@ -243,6 +248,16 @@ impl Stepping {
         self
     }
 
+    /// Ensure this system instance never runs when stepping is enabled
+    pub fn never_run_node(&mut self, schedule: impl ScheduleLabel, node: NodeId) -> &mut Self {
+        self.updates.push(Update::SetBehavior(
+            schedule.dyn_clone(),
+            SystemIdentifier::Node(node),
+            SystemBehavior::NeverRun,
+        ));
+        self
+    }
+
     /// Add a breakpoint for system
     pub fn set_breakpoint<Marker>(
         &mut self,
@@ -259,6 +274,16 @@ impl Stepping {
         self
     }
 
+    /// Add a breakpoint for system instance
+    pub fn set_breakpoint_node(&mut self, schedule: impl ScheduleLabel, node: NodeId) -> &mut Self {
+        self.updates.push(Update::SetBehavior(
+            schedule.dyn_clone(),
+            SystemIdentifier::Node(node),
+            SystemBehavior::Break,
+        ));
+        self
+    }
+
     /// Clear a breakpoint for the system
     pub fn clear_breakpoint<Marker>(
         &mut self,
@@ -270,6 +295,16 @@ impl Stepping {
         self
     }
 
+    /// clear a breakpoint for system instance
+    pub fn clear_breakpoint_node(
+        &mut self,
+        schedule: impl ScheduleLabel,
+        node: NodeId,
+    ) -> &mut Self {
+        self.clear_node(schedule, node);
+        self
+    }
+
     /// Clear any behavior set for the system
     pub fn clear_system<Marker>(
         &mut self,
@@ -278,13 +313,22 @@ impl Stepping {
     ) -> &mut Self {
         let type_id = IntoSystem::into_system(system).type_id();
         self.updates.push(Update::ClearBehavior(
-            Box::new(schedule),
+            schedule.dyn_clone(),
             SystemIdentifier::Type(type_id),
         ));
 
         self
     }
 
+    /// clear a breakpoint for system instance
+    pub fn clear_node(&mut self, schedule: impl ScheduleLabel, node: NodeId) -> &mut Self {
+        self.updates.push(Update::ClearBehavior(
+            schedule.dyn_clone(),
+            SystemIdentifier::Node(node),
+        ));
+        self
+    }
+
     /// lookup the first system for the supplied schedule index
     fn first_system_index_for_schedule(&self, index: usize) -> usize {
         let label = match self.schedule_order.get(index) {
@@ -617,10 +661,21 @@ impl ScheduleState {
             match (action, behavior) {
                 // regardless of which action we're performing, if the system
                 // is marked as NeverRun, add it to the skip list
-                (_, SystemBehavior::NeverRun) => skip.insert(i),
+                (_, SystemBehavior::NeverRun) => {
+                    skip.insert(i);
+                    // always advance the position past this sytem
+                    if i == pos {
+                        pos += 1;
+                    }
+                }
                 // similarly, ignore any system marked as AlwaysRun; they should
                 // never be added to the skip list
-                (_, SystemBehavior::AlwaysRun) => (),
+                (_, SystemBehavior::AlwaysRun) => {
+                    // always advance the position past this sytem
+                    if i == pos {
+                        pos += 1;
+                    }
+                }
                 // if we're waiting, no other systems besides AlwaysRun should
                 // be run, so add systems to the skip list
                 (Action::Waiting, _) => skip.insert(i),
@@ -793,6 +848,7 @@ mod tests {
         stepping.add_schedule(TestSchedule).disable().next_frame();
 
         assert!(stepping.skipped_systems(&schedule).is_none());
+        assert!(stepping.cursor().is_none());
     }
 
     #[test]
@@ -1133,4 +1189,117 @@ mod tests {
             ]
         );
     }
+
+    // Helper to verify cursor position
+    /*
+    macro_rules! assert_cursor_at {
+        ($stepping:expr, $schedule:expr, $system:expr) => {
+            let type_id = IntoSystem::into_system($system).type_id();
+            let node_id = $schedule
+                .systems()
+                .unwrap()
+                .find(|t| t.1.type_id() == type_id)
+                .unwrap()
+                .0;
+            let label = $schedule.label().as_ref().unwrap();
+            assert_eq!($stepping.cursor(), Some((label.dyn_clone(), node_id)));
+        };
+    } */
+
+    #[test]
+    fn verify_cursor() {
+        // helper to build a cursor tuple for the supplied schedule
+        fn cursor(schedule: &Schedule, index: usize) -> (BoxedScheduleLabel, NodeId) {
+            let node_id = schedule.executable().system_ids[index];
+            (schedule.label().as_ref().unwrap().clone(), node_id)
+        }
+
+        let mut world = World::new();
+
+        // create two schedules with a number of systems in them
+        let mut schedule_a = Schedule::default();
+        schedule_a
+            .set_label(TestScheduleA)
+            .add_systems((|| {}, || {}, || {}, || {}).chain());
+        schedule_a.initialize(&mut world).unwrap();
+        let mut schedule_b = Schedule::default();
+        schedule_b
+            .set_label(TestScheduleB)
+            .add_systems((|| {}, || {}, || {}, || {}).chain());
+        schedule_b.initialize(&mut world).unwrap();
+
+        // setup stepping and add all three schedules
+        let mut stepping = Stepping::new();
+        stepping
+            .add_schedule(TestScheduleA)
+            .add_schedule(TestScheduleB)
+            .enable();
+
+        assert!(stepping.cursor().is_none());
+
+        // step the system nine times, and verify the cursor before & after
+        // each step
+        let mut cursors = Vec::new();
+        for _ in 0..9 {
+            stepping.step_frame().next_frame();
+            cursors.push(stepping.cursor());
+            stepping.skipped_systems(&schedule_a);
+            stepping.skipped_systems(&schedule_b);
+            cursors.push(stepping.cursor());
+        }
+
+        #[rustfmt::skip]
+        assert_eq!(
+            cursors,
+            vec![
+                // before render frame        // after render frame
+                None,                         Some(cursor(&schedule_a, 1)),
+                Some(cursor(&schedule_a, 1)), Some(cursor(&schedule_a, 2)),
+                Some(cursor(&schedule_a, 2)), Some(cursor(&schedule_a, 3)),
+                Some(cursor(&schedule_a, 3)), Some(cursor(&schedule_b, 0)),
+                Some(cursor(&schedule_b, 0)), Some(cursor(&schedule_b, 1)),
+                Some(cursor(&schedule_b, 1)), Some(cursor(&schedule_b, 2)),
+                Some(cursor(&schedule_b, 2)), Some(cursor(&schedule_b, 3)),
+                Some(cursor(&schedule_b, 3)), None,
+                Some(cursor(&schedule_a, 0)), Some(cursor(&schedule_a, 1)),
+            ]
+        );
+
+        // reset our cursor (disable/enable), and update stepping to test if the
+        // cursor properly skips over AlwaysRun & NeverRun systems.  Also set
+        // a Break system to ensure that shows properly in the cursor
+        stepping
+            // disable/enable to reset cursor
+            .disable()
+            .enable()
+            .set_breakpoint_node(TestScheduleA, NodeId::System(1))
+            .always_run_node(TestScheduleA, NodeId::System(3))
+            .never_run_node(TestScheduleB, NodeId::System(0));
+
+        let mut cursors = Vec::new();
+        for _ in 0..9 {
+            stepping.step_frame().next_frame();
+            cursors.push(stepping.cursor());
+            stepping.skipped_systems(&schedule_a);
+            stepping.skipped_systems(&schedule_b);
+            cursors.push(stepping.cursor());
+        }
+
+        #[rustfmt::skip]
+        assert_eq!(
+            cursors,
+            vec![
+                // before render frame        // after render frame
+                Some(cursor(&schedule_a, 0)), Some(cursor(&schedule_a, 1)),
+                Some(cursor(&schedule_a, 1)), Some(cursor(&schedule_a, 2)),
+                Some(cursor(&schedule_a, 2)), Some(cursor(&schedule_b, 1)),
+                Some(cursor(&schedule_b, 1)), Some(cursor(&schedule_b, 2)),
+                Some(cursor(&schedule_b, 2)), Some(cursor(&schedule_b, 3)),
+                Some(cursor(&schedule_b, 3)), None,
+                Some(cursor(&schedule_a, 0)), Some(cursor(&schedule_a, 1)),
+                Some(cursor(&schedule_a, 1)), Some(cursor(&schedule_a, 2)),
+                Some(cursor(&schedule_a, 2)), Some(cursor(&schedule_b, 1)),
+            ]
+        );
+    }
 }

From 16a9e596e53dd7e3a8a412e5af24401e3c5c99eb Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Sat, 22 Apr 2023 15:56:23 -0500
Subject: [PATCH 13/25] Updates from code-review

---
 crates/bevy_app/src/app.rs               |   4 +-
 crates/bevy_ecs/src/schedule/mod.rs      | 108 +++-----
 crates/bevy_ecs/src/schedule/schedule.rs |   5 +-
 crates/bevy_ecs/src/schedule/stepping.rs | 301 ++++++++++++++---------
 examples/games/stepping.rs               |  31 ++-
 5 files changed, 243 insertions(+), 206 deletions(-)

diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs
index b1b56adc89900..05b999a21922c 100644
--- a/crates/bevy_app/src/app.rs
+++ b/crates/bevy_app/src/app.rs
@@ -874,9 +874,7 @@ impl App {
     /// To avoid this behavior, use the `init_schedule` method instead.
     pub fn add_schedule(&mut self, label: impl ScheduleLabel, mut schedule: Schedule) -> &mut Self {
         let mut schedules = self.world.resource_mut::<Schedules>();
-        if schedule.label().is_none() {
-            schedule.set_label(label.dyn_clone());
-        }
+        schedule.set_label(label.dyn_clone());
         schedules.insert(label, schedule);
 
         self
diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs
index 99f11236cf612..c53bc6b2788ab 100644
--- a/crates/bevy_ecs/src/schedule/mod.rs
+++ b/crates/bevy_ecs/src/schedule/mod.rs
@@ -717,94 +717,52 @@ mod tests {
         #[derive(ScheduleLabel, Clone, Debug, PartialEq, Eq, Hash)]
         pub struct TestSchedule;
 
+        macro_rules! assert_executor_supports_stepping {
+            ($executor:expr) => {
+                // create a test schedule
+                let mut schedule = Schedule::default();
+                schedule
+                    .set_label(TestSchedule)
+                    .set_executor_kind($executor)
+                    .add_systems(|| panic!("Executor ignored Stepping"));
+
+                // Add our schedule to stepping & and enable stepping; this should
+                // prevent any systems in the schedule from running
+                let mut stepping = Stepping::default();
+                stepping.add_schedule(TestSchedule).enable();
+
+                // create a world, and add the stepping resource
+                let mut world = World::default();
+                world.insert_resource(stepping);
+
+                // start a new frame by running ihe begin_frame() system
+                let mut system_state: SystemState<Option<ResMut<Stepping>>> =
+                    SystemState::new(&mut world);
+                let res = system_state.get_mut(&mut world);
+                Stepping::begin_frame(res);
+
+                // now run the schedule; this will panic if the executor doesn't
+                // handle stepping
+                schedule.run(&mut world);
+            };
+        }
+
         /// verify the [`SimpleExecutor`] supports stepping
         #[test]
         fn simple_executor() {
-            // create a test schedule
-            let mut schedule = Schedule::default();
-            schedule
-                .set_label(TestSchedule)
-                .set_executor_kind(ExecutorKind::Simple)
-                .add_systems(|| panic!("test failed; system should not run"));
-
-            // Add our schedule to stepping & and enable stepping; this should
-            // prevent any systems in the schedule from running
-            let mut stepping = Stepping::default();
-            stepping.add_schedule(TestSchedule).enable();
-
-            // create a world, and add the stepping resource
-            let mut world = World::default();
-            world.insert_resource(stepping);
-
-            // start a new frame by running ihe begin_frame() system
-            let mut system_state: SystemState<Option<ResMut<Stepping>>> =
-                SystemState::new(&mut world);
-            let res = system_state.get_mut(&mut world);
-            Stepping::begin_frame(res);
-
-            // now run the schedule; this will panic if the executor doesn't
-            // handle stepping
-            schedule.run(&mut world);
+            assert_executor_supports_stepping!(ExecutorKind::Simple);
         }
 
         /// verify the [`SingleThreadedExecutor`] supports stepping
         #[test]
         fn single_threaded_executor() {
-            // create a test schedule
-            let mut schedule = Schedule::default();
-            schedule
-                .set_label(TestSchedule)
-                .set_executor_kind(ExecutorKind::SingleThreaded)
-                .add_systems(|| panic!("test failed; system should not run"));
-
-            // Add our schedule to stepping & and enable stepping; this should
-            // prevent any systems in the schedule from running
-            let mut stepping = Stepping::default();
-            stepping.add_schedule(TestSchedule).enable();
-
-            // create a world, and add the stepping resource
-            let mut world = World::default();
-            world.insert_resource(stepping);
-
-            // start a new frame by running ihe begin_frame() system
-            let mut system_state: SystemState<Option<ResMut<Stepping>>> =
-                SystemState::new(&mut world);
-            let res = system_state.get_mut(&mut world);
-            Stepping::begin_frame(res);
-
-            // now run the schedule; this will panic if the executor doesn't
-            // handle stepping
-            schedule.run(&mut world);
+            assert_executor_supports_stepping!(ExecutorKind::SingleThreaded);
         }
 
         /// verify the [`MultiThreadedExecutor`] supports stepping
         #[test]
         fn multi_threaded_executor() {
-            // create a test schedule
-            let mut schedule = Schedule::default();
-            schedule
-                .set_label(TestSchedule)
-                .set_executor_kind(ExecutorKind::MultiThreaded)
-                .add_systems(|| panic!("test failed; system should not run"));
-
-            // Add our schedule to stepping & and enable stepping; this should
-            // prevent any systems in the schedule from running
-            let mut stepping = Stepping::default();
-            stepping.add_schedule(TestSchedule).enable();
-
-            // create a world, and add the stepping resource
-            let mut world = World::default();
-            world.insert_resource(stepping);
-
-            // start a new frame by running ihe begin_frame() system
-            let mut system_state: SystemState<Option<ResMut<Stepping>>> =
-                SystemState::new(&mut world);
-            let res = system_state.get_mut(&mut world);
-            Stepping::begin_frame(res);
-
-            // now run the schedule; this will panic if the executor doesn't
-            // handle stepping
-            schedule.run(&mut world);
+            assert_executor_supports_stepping!(ExecutorKind::MultiThreaded);
         }
     }
 }
diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs
index 5dcb51aa6723d..5f82871650537 100644
--- a/crates/bevy_ecs/src/schedule/schedule.rs
+++ b/crates/bevy_ecs/src/schedule/schedule.rs
@@ -185,7 +185,10 @@ impl Schedule {
 
     /// Set the label for this schedule
     pub fn set_label(&mut self, label: impl ScheduleLabel) -> &mut Self {
-        debug_assert!(self.label.is_none());
+        #[cfg(debug_assertions)]
+        if let Some(existing) = &self.label {
+            debug_assert!(existing.dyn_eq(&label));
+        }
         self.label = Some(label.dyn_clone());
         self
     }
diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs
index d6bf6df4e668c..f38b4ea7b5bbb 100644
--- a/crates/bevy_ecs/src/schedule/stepping.rs
+++ b/crates/bevy_ecs/src/schedule/stepping.rs
@@ -1,4 +1,3 @@
-use bevy_utils::tracing::warn;
 use fixedbitset::FixedBitSet;
 use std::any::TypeId;
 use std::collections::HashMap;
@@ -7,7 +6,12 @@ use crate::{
     schedule::{BoxedScheduleLabel, NodeId, Schedule, ScheduleLabel},
     system::{IntoSystem, ResMut, Resource, System},
 };
-use bevy_utils::thiserror::Error;
+#[cfg(test)]
+use bevy_utils::tracing::debug;
+use bevy_utils::{
+    thiserror::Error,
+    tracing::{info, warn},
+};
 
 use crate as bevy_ecs;
 
@@ -31,31 +35,53 @@ enum Action {
 
 #[derive(Debug, Copy, Clone)]
 enum SystemBehavior {
+    /// System will always run regardless of stepping action
     AlwaysRun,
+
+    /// System will never run while stepping is enabled
     NeverRun,
-    Continue,
+
+    /// When [`Action::Waiting`] this system will not be run
+    /// When [`Action::Step`] this system will be stepped
+    /// When [`Action::Continue`] system execution will stop before executing
+    /// this system unless its the first system run when continuing
     Break,
+
+    /// When [`Action::Waiting`] this system will not be run
+    /// When [`Action::Step`] this system will be stepped
+    /// When [`Action::Continue`] this system will be run
+    Continue,
 }
 
 // schedule_order index, and schedule start point
-#[derive(Debug, Default, Clone)]
-pub struct Cursor {
+#[derive(Debug, Default, Clone, Copy)]
+struct Cursor {
+    /// index within Stepping.schedule_order
     pub schedule: usize,
+    /// index within the schedule's system list
     pub system: usize,
 }
 
+// Two methods of referring to Systems, via TypeId, or per-Schedule NodeId
 enum SystemIdentifier {
     Type(TypeId),
-    #[allow(dead_code)]
     Node(NodeId),
 }
 
+/// Updates to [`Stepping.schedule_states`] that will be applied at the start
+/// of the next render frame
 enum Update {
+    /// Set the action stepping will perform for this render frame
     SetAction(Action),
+    /// Enable stepping for this schedule
     AddSchedule(BoxedScheduleLabel),
+    /// Disable stepping for this schedule
     RemoveSchedule(BoxedScheduleLabel),
+    /// Clear any system-specific behaviors for this schedule
     ClearSchedule(BoxedScheduleLabel),
+    /// Set a system-specific behavior for this schedule & system
     SetBehavior(BoxedScheduleLabel, SystemIdentifier, SystemBehavior),
+    /// Clear any system-specific behavior for this schedule & system
     ClearBehavior(BoxedScheduleLabel, SystemIdentifier),
 }
 
@@ -74,7 +100,7 @@ pub struct Stepping {
     // current position in the stepping frame
     cursor: Cursor,
 
-    // index in [`schedule_order`] of the last schedule to call `build_skip_list`
+    // index in [`schedule_order`] of the last schedule to call `skipped_systems()`
     previous_schedule: Option<usize>,
 
     // Action to perform during this render frame
@@ -109,6 +135,9 @@ impl Stepping {
         Stepping::default()
     }
 
+    /// System to call denoting that a new render frame has begun
+    ///
+    /// Note: This system is automatically added to the default `MainSchedule`.
     pub fn begin_frame(stepping: Option<ResMut<Self>>) {
         if let Some(mut stepping) = stepping {
             stepping.next_frame();
@@ -118,12 +147,6 @@ impl Stepping {
     /// Return the list of schedules with stepping enabled in the order
     /// they are executed in.
     pub fn schedules(&self) -> Result<&Vec<BoxedScheduleLabel>, NotReady> {
-        /*
-        println!(
-            "schedule_order {:?}, schedule_states {:?}",
-            self.schedule_order.len(),
-            self.schedule_states.len()
-        ); */
         if self.schedule_order.len() == self.schedule_states.len() {
             Ok(&self.schedule_order)
         } else {
@@ -161,16 +184,20 @@ impl Stepping {
         self
     }
 
-    /// disable stepping for the provided schedule
+    /// Disable stepping for the provided schedule
+    ///
+    /// NOTE: This function will also clear any system-specific behaviors that
+    /// may have been configured.
     pub fn remove_schedule(&mut self, schedule: impl ScheduleLabel) -> &mut Self {
         self.updates
-            .push(Update::RemoveSchedule(Box::new(schedule)));
+            .push(Update::RemoveSchedule(schedule.dyn_clone()));
         self
     }
 
-    /// clear behavior set for all systems in the provided [`Schedule`]
+    /// Clear behavior set for all systems in the provided [`Schedule`]
     pub fn clear_schedule(&mut self, schedule: impl ScheduleLabel) -> &mut Self {
-        self.updates.push(Update::ClearSchedule(Box::new(schedule)));
+        self.updates
+            .push(Update::ClearSchedule(schedule.dyn_clone()));
         self
     }
 
@@ -191,13 +218,18 @@ impl Stepping {
         self.action != Action::RunAll
     }
 
-    /// run the next system
+    /// Run the next system during the next render frame
+    ///
+    /// NOTE: This will have no impact unless stepping has been enabled
     pub fn step_frame(&mut self) -> &mut Self {
         self.updates.push(Update::SetAction(Action::Step));
         self
     }
 
-    /// continue stepping until the end of the frame or a breakpoint is hit
+    /// Run all remaining systems in the stepping frame during the next render
+    /// frame
+    ///
+    /// NOTE: This will have no impact unless stepping has been enabled
     pub fn continue_frame(&mut self) -> &mut Self {
         self.updates.push(Update::SetAction(Action::Continue));
         self
@@ -240,7 +272,7 @@ impl Stepping {
     ) -> &mut Self {
         let type_id = IntoSystem::into_system(system).type_id();
         self.updates.push(Update::SetBehavior(
-            Box::new(schedule),
+            schedule.dyn_clone(),
             SystemIdentifier::Type(type_id),
             SystemBehavior::NeverRun,
         ));
@@ -266,7 +298,7 @@ impl Stepping {
     ) -> &mut Self {
         let type_id = IntoSystem::into_system(system).type_id();
         self.updates.push(Update::SetBehavior(
-            Box::new(schedule),
+            schedule.dyn_clone(),
             SystemIdentifier::Type(type_id),
             SystemBehavior::Break,
         ));
@@ -359,11 +391,7 @@ impl Stepping {
             self.previous_schedule = None;
 
             // if the cursor passed the last schedule, reset it
-            let Cursor {
-                schedule: schedule_index,
-                ..
-            } = self.cursor;
-            if schedule_index >= self.schedule_order.len() {
+            if self.cursor.schedule >= self.schedule_order.len() {
                 self.reset_cursor();
             }
         }
@@ -379,7 +407,54 @@ impl Stepping {
                     self.action = Action::RunAll;
                     reset_cursor = true;
                 }
-                Update::SetAction(a) => self.action = a,
+                Update::SetAction(action) => {
+                    // This match block is really just to filter out invalid
+                    // transitions, and add debugging messages for permitted
+                    // transitions.  Any action transition that falls through
+                    // this match block will be performed.
+                    match (self.action, action) {
+                        // ignore non-transition updates, and prevent a call to
+                        // enable() from overwriting a step or continue call
+                        (Action::RunAll, Action::RunAll)
+                        | (Action::Waiting, Action::Waiting)
+                        | (Action::Continue, Action::Continue)
+                        | (Action::Step, Action::Step)
+                        | (Action::Continue, Action::Waiting)
+                        | (Action::Step, Action::Waiting) => continue,
+
+                        // when stepping is disabled
+                        (Action::RunAll, Action::Waiting) => info!("enabled stepping"),
+                        (Action::RunAll, _) => {
+                            warn!(
+                                "stepping not enabled; call Stepping::enable() \
+                                before step_frame() or continue_frame()"
+                            );
+                            continue;
+                        }
+
+                        // stepping enabled; waiting
+                        (Action::Waiting, Action::RunAll) => info!("disabled stepping"),
+                        (Action::Waiting, Action::Continue) => info!("continue frame"),
+                        (Action::Waiting, Action::Step) => info!("step frame"),
+
+                        // stepping enabled; continue frame
+                        (Action::Continue, Action::RunAll) => info!("disabled stepping"),
+                        (Action::Continue, Action::Step) => {
+                            warn!("ignoring step_frame(); already continuing next frame");
+                            continue;
+                        }
+
+                        // stepping enabled; step frame
+                        (Action::Step, Action::RunAll) => info!("disabled stepping"),
+                        (Action::Step, Action::Continue) => {
+                            warn!("ignoring continue_frame(); already stepping next frame");
+                            continue;
+                        }
+                    }
+
+                    // permitted action transition; make the change
+                    self.action = action;
+                }
                 Update::AddSchedule(l) => {
                     self.schedule_states.insert(l, ScheduleState::default());
                 }
@@ -394,8 +469,8 @@ impl Stepping {
                     Some(state) => state.clear_behaviors(),
                     None => {
                         warn!(
-                            "stepping is not enabled for schedule {:?}; use
-                    `.add_stepping({:?})` to enable stepping",
+                            "stepping is not enabled for schedule {:?}; \
+                            use `.add_stepping({:?})` to enable stepping",
                             label, label
                         );
                     }
@@ -405,8 +480,8 @@ impl Stepping {
                         Some(state) => state.set_behavior(system, behavior),
                         None => {
                             warn!(
-                                "stepping is not enabled for schedule {:?}; use
-                    `.add_stepping({:?})` to enable stepping",
+                                "stepping is not enabled for schedule {:?}; \
+                                use `.add_stepping({:?})` to enable stepping",
                                 label, label
                             );
                         }
@@ -417,8 +492,8 @@ impl Stepping {
                         Some(state) => state.clear_behavior(system),
                         None => {
                             warn!(
-                                "stepping is not enabled for schedule {:?}; use
-                    `.add_stepping({:?})` to enable stepping",
+                                "stepping is not enabled for schedule {:?}; \
+                                use `.add_stepping({:?})` to enable stepping",
                                 label, label
                             );
                         }
@@ -439,27 +514,7 @@ impl Stepping {
             return None;
         }
 
-        // grab the label and state for the schedule if they've been added to
-        // stepping
-        // XXX Take a second look at this
-        let mut found = false;
-        for label in self.schedule_states.keys() {
-            match schedule.label() {
-                Some(l) => {
-                    if l == label {
-                        found = true;
-                    }
-                }
-                None => (),
-            }
-        }
-        debug_assert!(
-            !found
-                || self
-                    .schedule_states
-                    .get_mut(schedule.label().as_ref().unwrap())
-                    .is_some()
-        );
+        // grab the label and state for this schedule
         let (label, state) = match schedule.label() {
             None => return None,
             Some(label) => (label, self.schedule_states.get_mut(label)?),
@@ -467,36 +522,40 @@ impl Stepping {
 
         // Stepping is enabled, and this schedule is supposed to be stepped.
         //
-        // We need to get this schedule's index in the ordered schedule list.
-        // If it's not present, it will be inserted in place after the index
-        // of the previous schedule that called this function.
+        // We need to maintain a list of schedules in the order that they call
+        // this function. We'll check the ordered list now to see if this
+        // schedule is present. If not, we'll add it after the last schedule
+        // that called this function. Finally we want to save off the index of
+        // this schedule in the ordered schedule list. This is used to
+        // determine if this is the schedule the cursor is pointed at.
         let index = self.schedule_order.iter().position(|l| l == label);
         let index = match (index, self.previous_schedule) {
             (Some(index), _) => index,
             (None, None) => {
-                self.schedule_order.insert(0, label.clone());
+                self.schedule_order.insert(0, label.dyn_clone());
                 0
             }
             (None, Some(last)) => {
-                self.schedule_order.insert(last + 1, label.clone());
+                self.schedule_order.insert(last + 1, label.dyn_clone());
                 last + 1
             }
         };
+        // Update the index of the previous schedule to be the index of this
+        // schedule for the next call
         self.previous_schedule = Some(index);
-        println!(
-            "Stepping::skipped_systems(): cursor {:?}, label {:?}, index {}",
-            self.cursor, label, index
+
+        #[cfg(test)]
+        debug!(
+            "cursor {:?}, index {}, label {:?}",
+            self.cursor, index, label
         );
 
         // if the stepping frame cursor is pointing at this schedule, we'll run
         // the schedule with the current stepping action.  If this is not the
         // cursor schedule, we'll run the schedule with the waiting action.
-        let Cursor {
-            schedule: schedule_index,
-            system: start,
-        } = self.cursor;
-        let (list, next_system) = if index == schedule_index {
-            let o = state.skipped_systems(schedule, start, self.action);
+        let cursor = self.cursor;
+        let (list, next_system) = if index == cursor.schedule {
+            let o = state.skipped_systems(schedule, cursor.system, self.action);
 
             // if we just stepped this schedule, then we'll switch the action
             // to be waiting
@@ -508,7 +567,7 @@ impl Stepping {
             // we're not supposed to run any systems in this schedule, so pull
             // the skip list, but ignore any changes it makes to the cursor.
             let (skip, _) = state.skipped_systems(schedule, 0, Action::Waiting);
-            (skip, Some(start))
+            (skip, Some(cursor.system))
         };
 
         // update the stepping frame cursor based on if there are any systems
@@ -520,15 +579,14 @@ impl Stepping {
         match next_system {
             Some(i) => self.cursor.system = i,
             None => {
-                let index = schedule_index + 1;
-                println!(
-                    "Stepping::skipped_systems(): next schedule {} -> {}",
-                    schedule_index, index,
-                );
+                let index = cursor.schedule + 1;
                 self.cursor = Cursor {
                     schedule: index,
                     system: self.first_system_index_for_schedule(index),
-                }
+                };
+
+                #[cfg(test)]
+                debug!("advanced schedule index: {} -> {}", cursor.schedule, index);
             }
         }
 
@@ -542,11 +600,15 @@ struct ScheduleState {
     behaviors: HashMap<NodeId, SystemBehavior>,
 
     /// order of NodeIds in the schedule
+    ///
+    /// This is a cached copy of SystemExecutable.system_ids. We need it
+    /// available here to be accessed by Stepping::cursor() so we can return
+    /// NodeIds to the caller.
     node_ids: Vec<NodeId>,
 
     /// changes to system behavior that should be applied the next time
-    /// [`ScheduleState::progress()`] is called
-    updates: HashMap<TypeId, Option<SystemBehavior>>,
+    /// [`ScheduleState::skipped_systems()`] is called
+    behavior_updates: HashMap<TypeId, Option<SystemBehavior>>,
 
     /// This field contains the first steppable system in the schedule.
     first: Option<usize>,
@@ -560,8 +622,11 @@ impl ScheduleState {
             SystemIdentifier::Node(node_id) => {
                 self.behaviors.insert(node_id, behavior);
             }
+            // Behaviors are indexed by NodeId, but we cannot map a system
+            // TypeId to a NodeId without the `Schedule`.  So queue this update
+            // to be processed the next time `skipped_systems()` is called.
             SystemIdentifier::Type(type_id) => {
-                self.updates.insert(type_id, Some(behavior));
+                self.behavior_updates.insert(type_id, Some(behavior));
             }
         }
     }
@@ -573,8 +638,9 @@ impl ScheduleState {
             SystemIdentifier::Node(node_id) => {
                 self.behaviors.remove(&node_id);
             }
+            // queue TypeId updates to be processed later when we have Schedule
             SystemIdentifier::Type(type_id) => {
-                self.updates.insert(type_id, None);
+                self.behavior_updates.insert(type_id, None);
             }
         }
     }
@@ -587,9 +653,12 @@ impl ScheduleState {
 
     // apply system behavior updates by looking up the node id of the system in
     // the schedule, and updating `systems`
-    fn apply_updates(&mut self, schedule: &Schedule) {
+    fn apply_behavior_updates(&mut self, schedule: &Schedule) {
+        // Systems may be present multiple times within a schedule, so we
+        // iterate through all systems in the schedule, and check our behavior
+        // updates for the system TypeId.
         for (node_id, system) in schedule.systems().unwrap() {
-            let behavior = self.updates.get(&system.type_id());
+            let behavior = self.behavior_updates.get(&system.type_id());
             match behavior {
                 None => continue,
                 Some(None) => {
@@ -600,9 +669,10 @@ impl ScheduleState {
                 }
             }
         }
-        self.updates.clear();
+        self.behavior_updates.clear();
 
-        println!("apply_updates(): {:?}", self.behaviors);
+        #[cfg(test)]
+        debug!("apply_updates(): {:?}", self.behaviors);
     }
 
     fn skipped_systems(
@@ -622,8 +692,8 @@ impl ScheduleState {
         // Now that we have the schedule, apply any pending system behavior
         // updates.  The schedule is required to map from system `TypeId` to
         // `NodeId`.
-        if !self.updates.is_empty() {
-            self.apply_updates(schedule);
+        if !self.behavior_updates.is_empty() {
+            self.apply_behavior_updates(schedule);
         }
 
         // if we don't have a first system set, set it now
@@ -642,36 +712,40 @@ impl ScheduleState {
         let mut skip = FixedBitSet::with_capacity(schedule.systems_len());
         let mut pos = start;
 
-        for (i, (node_id, system)) in schedule.systems().unwrap().enumerate() {
+        for (i, (node_id, _system)) in schedule.systems().unwrap().enumerate() {
             let behavior = self
                 .behaviors
                 .get(&node_id)
                 .unwrap_or(&SystemBehavior::Continue);
-            println!(
+
+            #[cfg(test)]
+            debug!(
                 "skipped_systems(): systems[{}], pos {}, Action::{:?}, Behavior::{:?}, {}",
                 i,
                 pos,
                 action,
                 behavior,
-                system.name()
+                _system.name()
             );
+
             // suppress this clippy warning; we want to keep the arms split to
             // clearly document whats going on.
-            #[allow(clippy::match_same_arms)]
             match (action, behavior) {
                 // regardless of which action we're performing, if the system
-                // is marked as NeverRun, add it to the skip list
+                // is marked as NeverRun, add it to the skip list.
+                // Also, advance the cursor past this system if it is our
+                // current position
                 (_, SystemBehavior::NeverRun) => {
                     skip.insert(i);
-                    // always advance the position past this sytem
                     if i == pos {
                         pos += 1;
                     }
                 }
                 // similarly, ignore any system marked as AlwaysRun; they should
                 // never be added to the skip list
+                // Also, advance the cursor past this system if it is our
+                // current position
                 (_, SystemBehavior::AlwaysRun) => {
-                    // always advance the position past this sytem
                     if i == pos {
                         pos += 1;
                     }
@@ -679,11 +753,12 @@ impl ScheduleState {
                 // if we're waiting, no other systems besides AlwaysRun should
                 // be run, so add systems to the skip list
                 (Action::Waiting, _) => skip.insert(i),
+
                 // If we're stepping, the remaining behaviors don't matter,
                 // we're only going to run the system at our cursor.  Any system
-                // not at our cursor is skipped.  Once we encounter the system
-                // at the cursor, we'll move the cursor up, and switch to
-                // Waiting, to skip remaining systems.
+                // prior to the cursor is skipped.  Once we encounter the system
+                // at the cursor, we'll advance the cursor, and set behavior to
+                // Waiting to skip remaining systems.
                 (Action::Step, _) => match i.cmp(&pos) {
                     Ordering::Less => skip.insert(i),
                     Ordering::Equal => {
@@ -720,11 +795,15 @@ impl ScheduleState {
                 (Action::RunAll, _) => unreachable!(),
             }
 
+            // If we're at the cursor position, and not waiting, advance the
+            // cursor.
             if i == pos && action != Action::Waiting {
                 pos += 1;
             }
         }
 
+        // output is the skip list, and the index of the next system to run in
+        // this schedule.
         if pos >= schedule.systems_len() {
             (skip, None)
         } else {
@@ -1136,6 +1215,22 @@ mod tests {
         assert_systems_run!(&schedule, stepping.skipped_systems(&schedule),);
     }
 
+    #[test]
+    fn remove_schedule() {
+        let (schedule, _world) = setup();
+        let mut stepping = Stepping::new();
+        stepping.add_schedule(TestSchedule).enable();
+
+        // run the schedule once and verify all systems are skipped
+        assert_schedule_runs!(&schedule, &mut stepping,);
+        assert!(!stepping.schedules().unwrap().is_empty());
+
+        // remove the test schedule
+        stepping.remove_schedule(TestSchedule);
+        assert_schedule_runs!(&schedule, &mut stepping, first_system, second_system);
+        assert!(stepping.schedules().unwrap().is_empty());
+    }
+
     // verify that Stepping can construct an ordered list of schedules
     #[test]
     fn schedules() {
@@ -1190,22 +1285,6 @@ mod tests {
         );
     }
 
-    // Helper to verify cursor position
-    /*
-    macro_rules! assert_cursor_at {
-        ($stepping:expr, $schedule:expr, $system:expr) => {
-            let type_id = IntoSystem::into_system($system).type_id();
-            let node_id = $schedule
-                .systems()
-                .unwrap()
-                .find(|t| t.1.type_id() == type_id)
-                .unwrap()
-                .0;
-            let label = $schedule.label().as_ref().unwrap();
-            assert_eq!($stepping.cursor(), Some((label.dyn_clone(), node_id)));
-        };
-    } */
-
     #[test]
     fn verify_cursor() {
         // helper to build a cursor tuple for the supplied schedule
diff --git a/examples/games/stepping.rs b/examples/games/stepping.rs
index f2290fb1ed0b1..9c33906600846 100644
--- a/examples/games/stepping.rs
+++ b/examples/games/stepping.rs
@@ -7,19 +7,19 @@ use bevy::{app::MainScheduleOrder, ecs::schedule::*, prelude::*};
 /// executing schedule is removed from the [`Schedules`] resource while it is
 /// being run.
 #[derive(Debug, Hash, PartialEq, Eq, Clone, ScheduleLabel)]
-struct Debug;
+struct DebugSchedule;
 
 /// Plugin to add a stepping UI to an example
 #[derive(Default)]
 pub struct SteppingPlugin {
-    schedule_labels: Vec<Box<dyn ScheduleLabel>>,
+    schedule_labels: Vec<BoxedScheduleLabel>,
     top: Val,
     left: Val,
 }
 
 impl SteppingPlugin {
     /// Initialize the plugin to step the schedules specified in `labels`
-    pub fn for_schedules(labels: Vec<Box<dyn ScheduleLabel>>) -> SteppingPlugin {
+    pub fn for_schedules(labels: Vec<BoxedScheduleLabel>) -> SteppingPlugin {
         SteppingPlugin {
             schedule_labels: labels,
             ..default()
@@ -37,16 +37,15 @@ impl Plugin for SteppingPlugin {
         // create and insert our debug schedule into the main schedule order.
         // We need an independent schedule so we have access to all other
         // schedules through the `Stepping` resource
-        app.init_schedule(Debug);
+        app.init_schedule(DebugSchedule);
         let mut order = app.world.resource_mut::<MainScheduleOrder>();
-        order.insert_after(Update, Debug);
+        order.insert_after(Update, DebugSchedule);
 
         // create our stepping resource
         let mut stepping = Stepping::new();
-        for label in self.schedule_labels.iter() {
+        for label in &self.schedule_labels {
             stepping.add_schedule(label.dyn_clone());
         }
-        assert!(!stepping.is_enabled());
         app.insert_resource(stepping);
 
         // add our startup & stepping systems
@@ -56,7 +55,7 @@ impl Plugin for SteppingPlugin {
             systems: Vec::new(),
         })
         .add_systems(
-            Debug,
+            DebugSchedule,
             (
                 build_ui.run_if(not(initialized)),
                 handle_input,
@@ -114,7 +113,6 @@ fn build_ui(
     // go through the stepping schedules and construct a list of systems for
     // each label
     for label in schedule_order {
-        println!("schedule {:?}", label);
         let schedule = schedules.get(&**label).unwrap();
         text_sections.push(TextSection::new(
             format!("{:?}\n", label),
@@ -133,15 +131,19 @@ fn build_ui(
         };
 
         for (node_id, system) in systems {
+            // skip bevy default systems; we don't want to step those
             if system.name().starts_with("bevy") {
                 always_run.push((label.dyn_clone(), node_id));
-                println!("skipping {:?}", system.name());
                 continue;
             }
 
+            // Add an entry to our systems list so we can find where to draw
+            // the cursor when the stepping cursor is at this system
             state
                 .systems
                 .push((label.dyn_clone(), node_id, text_sections.len()));
+
+            // Add a text section for displaying the cursor for this system
             text_sections.push(TextSection::new(
                 "   ",
                 TextStyle {
@@ -151,7 +153,7 @@ fn build_ui(
                 },
             ));
 
-            println!("found system: {:?}/{}\n", label, system.name());
+            // add the name of the system to the ui
             text_sections.push(TextSection::new(
                 format!("{}\n", system.name()),
                 TextStyle {
@@ -203,7 +205,7 @@ fn build_ui(
 
 fn handle_input(keyboard_input: Res<Input<KeyCode>>, mut stepping: ResMut<Stepping>) {
     if keyboard_input.just_pressed(KeyCode::Slash) {
-        debug!("stepping: {:#?}", stepping);
+        info!("{:#?}", stepping);
     }
     // grave key to toggle stepping mode for the FixedUpdate schedule
     if keyboard_input.just_pressed(KeyCode::Grave) {
@@ -244,13 +246,10 @@ fn update_ui(
     let (ui, mut text, vis) = ui.single_mut();
     match (vis, stepping.is_enabled()) {
         (Visibility::Hidden, true) => {
-            println!("showing UI");
             commands.entity(ui).insert(Visibility::Inherited);
         }
-        (Visibility::Hidden, false) => (),
-        (_, true) => (),
+        (Visibility::Hidden, false) | (_, true) => (),
         (_, false) => {
-            println!("hiding UI");
             commands.entity(ui).insert(Visibility::Hidden);
         }
     }

From 1cb1c5814ddf7f7e6a18fd7635282fb3bc4ce463 Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Sat, 22 Apr 2023 16:19:05 -0500
Subject: [PATCH 14/25] merge fixes, and CI fixes

---
 crates/bevy_ecs/src/schedule/schedule.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs
index 5f82871650537..b406ae8b15abb 100644
--- a/crates/bevy_ecs/src/schedule/schedule.rs
+++ b/crates/bevy_ecs/src/schedule/schedule.rs
@@ -187,7 +187,7 @@ impl Schedule {
     pub fn set_label(&mut self, label: impl ScheduleLabel) -> &mut Self {
         #[cfg(debug_assertions)]
         if let Some(existing) = &self.label {
-            debug_assert!(existing.dyn_eq(&label));
+            debug_assert!(existing.inner_type_id() == label.inner_type_id());
         }
         self.label = Some(label.dyn_clone());
         self

From 4b8b5075f4d702cb092d5911c21f01164c32e21e Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Thu, 27 Apr 2023 17:25:23 -0500
Subject: [PATCH 15/25] Added `bevy_debug_stepping` feature

---
 Cargo.toml                                        |  4 ++++
 crates/bevy_app/Cargo.toml                        |  3 ++-
 crates/bevy_app/src/main_schedule.rs              | 10 +++++++---
 crates/bevy_ecs/Cargo.toml                        |  3 ++-
 .../src/schedule/executor/multi_threaded.rs       |  5 +++--
 crates/bevy_ecs/src/schedule/executor/simple.rs   |  5 +++--
 .../src/schedule/executor/single_threaded.rs      |  5 +++--
 crates/bevy_ecs/src/schedule/mod.rs               |  1 +
 crates/bevy_ecs/src/schedule/schedule.rs          |  4 ++++
 crates/bevy_ecs/src/schedule/stepping.rs          | 15 +++++++++++----
 crates/bevy_internal/Cargo.toml                   |  5 +++++
 11 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index d1cc8713725ce..92136941bcfe8 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -53,6 +53,7 @@ default = [
   "android_shared_stdcxx",
   "tonemapping_luts",
   "default_font",
+  "bevy_debug_stepping",
 ]
 
 # Force dynamic linking, which improves iterative compile times
@@ -229,6 +230,9 @@ glam_assert = ["bevy_internal/glam_assert"]
 # Include a default font, containing only ASCII characters, at the cost of a 20kB binary size increase
 default_font = ["bevy_internal/default_font"]
 
+# Enable stepping-based debugging of Bevy systems
+bevy_debug_stepping = ["bevy_internal/bevy_debug_stepping"]
+
 [dependencies]
 bevy_dylib = { path = "crates/bevy_dylib", version = "0.11.0-dev", default-features = false, optional = true }
 bevy_internal = { path = "crates/bevy_internal", version = "0.11.0-dev", default-features = false }
diff --git a/crates/bevy_app/Cargo.toml b/crates/bevy_app/Cargo.toml
index 2a3f604e7b05f..9c0fa87a71805 100644
--- a/crates/bevy_app/Cargo.toml
+++ b/crates/bevy_app/Cargo.toml
@@ -11,7 +11,8 @@ keywords = ["bevy"]
 [features]
 trace = []
 bevy_ci_testing = ["serde", "ron"]
-default = ["bevy_reflect"]
+bevy_debug_stepping = []
+default = ["bevy_reflect", "bevy_debug_stepping"]
 bevy_reflect = ["dep:bevy_reflect", "bevy_ecs/bevy_reflect"]
 
 [dependencies]
diff --git a/crates/bevy_app/src/main_schedule.rs b/crates/bevy_app/src/main_schedule.rs
index f77ebc5e1e87c..8ef0ab1527a3a 100644
--- a/crates/bevy_app/src/main_schedule.rs
+++ b/crates/bevy_app/src/main_schedule.rs
@@ -154,8 +154,6 @@ pub struct MainSchedulePlugin;
 
 impl Plugin for MainSchedulePlugin {
     fn build(&self, app: &mut App) {
-        use bevy_ecs::schedule::{IntoSystemConfigs, Stepping};
-
         // simple "facilitator" schedules benefit from simpler single threaded scheduling
         let mut main_schedule = Schedule::new();
         main_schedule.set_executor_kind(ExecutorKind::SingleThreaded);
@@ -165,6 +163,12 @@ impl Plugin for MainSchedulePlugin {
         app.add_schedule(Main, main_schedule)
             .add_schedule(RunFixedUpdateLoop, fixed_update_loop_schedule)
             .init_resource::<MainScheduleOrder>()
-            .add_systems(Main, (Stepping::begin_frame, Main::run_main).chain());
+            .add_systems(Main, Main::run_main);
+
+        #[cfg(feature = "bevy_debug_stepping")]
+        {
+            use bevy_ecs::schedule::{IntoSystemConfigs, Stepping};
+            app.add_systems(Main, Stepping::begin_frame.before(Main::run_main));
+        }
     }
 }
diff --git a/crates/bevy_ecs/Cargo.toml b/crates/bevy_ecs/Cargo.toml
index cfcea53b3644c..d7a662a61b963 100644
--- a/crates/bevy_ecs/Cargo.toml
+++ b/crates/bevy_ecs/Cargo.toml
@@ -11,7 +11,8 @@ categories = ["game-engines", "data-structures"]
 
 [features]
 trace = []
-default = ["bevy_reflect"]
+bevy_debug_stepping = []
+default = ["bevy_reflect", "bevy_debug_stepping"]
 
 [dependencies]
 bevy_ptr = { path = "../bevy_ptr", version = "0.11.0-dev" }
diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
index 12c84c01703af..66a9da89455ff 100644
--- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
+++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
@@ -164,7 +164,7 @@ impl SystemExecutor for MultiThreadedExecutor {
     fn run(
         &mut self,
         schedule: &mut SystemSchedule,
-        skip_systems: Option<FixedBitSet>,
+        _skip_systems: Option<FixedBitSet>,
         world: &mut World,
     ) {
         // reset counts
@@ -186,7 +186,8 @@ impl SystemExecutor for MultiThreadedExecutor {
 
         // If stepping is enabled, make sure we skip those systems that should
         // not be run.
-        if let Some(mut skipped_systems) = skip_systems {
+        #[cfg(feature = "bevy_debug_stepping")]
+        if let Some(mut skipped_systems) = _skip_systems {
             debug_assert_eq!(skipped_systems.len(), self.completed_systems.len());
             // mark skipped systems as completed
             self.completed_systems |= &skipped_systems;
diff --git a/crates/bevy_ecs/src/schedule/executor/simple.rs b/crates/bevy_ecs/src/schedule/executor/simple.rs
index bd4e8359bb186..a72825f80574c 100644
--- a/crates/bevy_ecs/src/schedule/executor/simple.rs
+++ b/crates/bevy_ecs/src/schedule/executor/simple.rs
@@ -37,12 +37,13 @@ impl SystemExecutor for SimpleExecutor {
     fn run(
         &mut self,
         schedule: &mut SystemSchedule,
-        skip_systems: Option<FixedBitSet>,
+        _skip_systems: Option<FixedBitSet>,
         world: &mut World,
     ) {
         // If stepping is enabled, make sure we skip those systems that should
         // not be run.
-        if let Some(skipped_systems) = skip_systems {
+        #[cfg(feature = "bevy_debug_stepping")]
+        if let Some(skipped_systems) = _skip_systems {
             // mark skipped systems as completed
             self.completed_systems |= &skipped_systems;
         }
diff --git a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs
index 9c1c008e2f6bb..22953e2a4365c 100644
--- a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs
+++ b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs
@@ -47,12 +47,13 @@ impl SystemExecutor for SingleThreadedExecutor {
     fn run(
         &mut self,
         schedule: &mut SystemSchedule,
-        skip_systems: Option<FixedBitSet>,
+        _skip_systems: Option<FixedBitSet>,
         world: &mut World,
     ) {
         // If stepping is enabled, make sure we skip those systems that should
         // not be run.
-        if let Some(skipped_systems) = skip_systems {
+        #[cfg(feature = "bevy_debug_stepping")]
+        if let Some(skipped_systems) = _skip_systems {
             // mark skipped systems as completed
             self.completed_systems |= &skipped_systems;
         }
diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs
index c53bc6b2788ab..f0e9c35517b62 100644
--- a/crates/bevy_ecs/src/schedule/mod.rs
+++ b/crates/bevy_ecs/src/schedule/mod.rs
@@ -710,6 +710,7 @@ mod tests {
         }
     }
 
+    #[cfg(feature = "bevy_debug_stepping")]
     mod stepping {
         use super::*;
         use bevy_ecs::system::SystemState;
diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs
index b406ae8b15abb..a8f365ac308ee 100644
--- a/crates/bevy_ecs/src/schedule/schedule.rs
+++ b/crates/bevy_ecs/src/schedule/schedule.rs
@@ -252,6 +252,10 @@ impl Schedule {
         world.check_change_ticks();
         self.initialize(world).unwrap_or_else(|e| panic!("{e}"));
 
+        #[cfg(not(feature = "bevy_debug_stepping"))]
+        let skip_systems = None;
+
+        #[cfg(feature = "bevy_debug_stepping")]
         let skip_systems = match world.get_resource_mut::<Stepping>() {
             None => None,
             Some(mut stepping) => stepping.skipped_systems(self),
diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs
index f38b4ea7b5bbb..64517e7086e0c 100644
--- a/crates/bevy_ecs/src/schedule/stepping.rs
+++ b/crates/bevy_ecs/src/schedule/stepping.rs
@@ -6,13 +6,14 @@ use crate::{
     schedule::{BoxedScheduleLabel, NodeId, Schedule, ScheduleLabel},
     system::{IntoSystem, ResMut, Resource, System},
 };
-#[cfg(test)]
-use bevy_utils::tracing::debug;
 use bevy_utils::{
     thiserror::Error,
-    tracing::{info, warn},
+    tracing::{error, info, warn},
 };
 
+#[cfg(test)]
+use bevy_utils::tracing::debug;
+
 use crate as bevy_ecs;
 
 #[derive(Debug, Default, PartialEq, Eq, Copy, Clone)]
@@ -203,7 +204,13 @@ impl Stepping {
 
     /// Begin stepping at the start of the next frame
     pub fn enable(&mut self) -> &mut Self {
+        #[cfg(feature = "bevy_debug_stepping")]
         self.updates.push(Update::SetAction(Action::Waiting));
+        #[cfg(not(feature = "bevy_debug_stepping"))]
+        error!(
+            "Stepping cannot be enabled; \
+            bevy was compiled without the bevy_debug_stepping feature"
+        );
         self
     }
 
@@ -812,7 +819,7 @@ impl ScheduleState {
     }
 }
 
-#[cfg(test)]
+#[cfg(all(test, feature = "bevy_debug_stepping"))]
 mod tests {
     use super::*;
     use crate::prelude::*;
diff --git a/crates/bevy_internal/Cargo.toml b/crates/bevy_internal/Cargo.toml
index 51fe644c92969..4dadb3d255431 100644
--- a/crates/bevy_internal/Cargo.toml
+++ b/crates/bevy_internal/Cargo.toml
@@ -99,6 +99,11 @@ glam_assert = ["bevy_math/glam_assert"]
 
 default_font = ["bevy_text?/default_font"]
 
+bevy_debug_stepping = [
+    "bevy_ecs/bevy_debug_stepping",
+    "bevy_app/bevy_debug_stepping"
+]
+
 [dependencies]
 # bevy
 bevy_a11y = { path = "../bevy_a11y", version = "0.11.0-dev" }

From d746299f400c670d39a7114fc1d3b0013ac6d139 Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Thu, 27 Apr 2023 17:31:28 -0500
Subject: [PATCH 16/25] Fix breakout example so stepping help shows on start

---
 examples/games/stepping.rs | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/examples/games/stepping.rs b/examples/games/stepping.rs
index 9c33906600846..5f6bc55e7d71a 100644
--- a/examples/games/stepping.rs
+++ b/examples/games/stepping.rs
@@ -54,6 +54,7 @@ impl Plugin for SteppingPlugin {
             ui_left: self.left,
             systems: Vec::new(),
         })
+        .add_systems(Startup, build_help)
         .add_systems(
             DebugSchedule,
             (
@@ -185,13 +186,15 @@ fn build_ui(
             ..default()
         },
     ));
+}
 
+fn build_help(mut commands: Commands, asset_server: Res<AssetServer>) {
     // stepping description box
     commands.spawn((TextBundle::from_sections([TextSection::new(
         "Press ` to toggle stepping mode (S: step system, Space: step frame)",
         TextStyle {
             font: asset_server.load(FONT_MEDIUM),
-            font_size: 15.0,
+            font_size: 18.0,
             color: FONT_COLOR,
         },
     )])

From 5b3a77cc48185ffbd746795e99839a43ad7c5235 Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Thu, 27 Apr 2023 18:14:21 -0500
Subject: [PATCH 17/25] update docs/cargo_features.md

---
 docs/cargo_features.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/docs/cargo_features.md b/docs/cargo_features.md
index 28d4b205d9ac1..d9dc66831fefa 100644
--- a/docs/cargo_features.md
+++ b/docs/cargo_features.md
@@ -17,6 +17,7 @@ The default feature set enables most of the expected features of a game engine,
 |bevy_asset|Provides asset functionality|
 |bevy_audio|Provides audio functionality|
 |bevy_core_pipeline|Provides cameras and other basic render pipeline features|
+|bevy_debug_stepping|Enable stepping-based debugging of Bevy systems|
 |bevy_gilrs|Adds gamepad support|
 |bevy_gizmos|Adds support for rendering gizmos|
 |bevy_gltf|[glTF](https://www.khronos.org/gltf/) support|

From 8ec0e20be8fe6be12ef489efbd4ac0c3a26ef426 Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Thu, 27 Apr 2023 18:23:05 -0500
Subject: [PATCH 18/25] `pub(crate)` for `Schedule::executor()`

---
 crates/bevy_ecs/src/schedule/schedule.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs
index a8f365ac308ee..9b73a7011320f 100644
--- a/crates/bevy_ecs/src/schedule/schedule.rs
+++ b/crates/bevy_ecs/src/schedule/schedule.rs
@@ -296,7 +296,7 @@ impl Schedule {
     }
 
     /// Returns the [`SystemSchedule`].
-    pub fn executable(&self) -> &SystemSchedule {
+    pub(crate) fn executable(&self) -> &SystemSchedule {
         &self.executable
     }
 

From 806f0ad5fb7c0d279864ed5c23ebd61bafae286c Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Wed, 29 Nov 2023 17:59:33 -0600
Subject: [PATCH 19/25] stepping: cleanup clippy warning

---
 examples/games/stepping.rs | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/examples/games/stepping.rs b/examples/games/stepping.rs
index c89195ab8de4d..1710469e02d74 100644
--- a/examples/games/stepping.rs
+++ b/examples/games/stepping.rs
@@ -104,9 +104,8 @@ fn build_ui(
     let mut text_sections = Vec::new();
     let mut always_run = Vec::new();
 
-    let schedule_order = match stepping.schedules() {
-        Ok(s) => s,
-        Err(_) => return,
+    let Ok(schedule_order) = stepping.schedules() else {
+        return;
     };
 
     // go through the stepping schedules and construct a list of systems for
@@ -124,9 +123,8 @@ fn build_ui(
 
         // grab the list of systems in the schedule, in the order the
         // single-threaded executor would run them.
-        let systems = match schedule.systems() {
-            Ok(iter) => iter,
-            Err(_) => return,
+        let Ok(systems) = schedule.systems() else {
+            return;
         };
 
         for (node_id, system) in systems {

From 383c6fc9284a7b42093def529d99a5d0ad49f02c Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Wed, 29 Nov 2023 18:46:45 -0600
Subject: [PATCH 20/25] fix for toml file

---
 crates/bevy_internal/Cargo.toml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crates/bevy_internal/Cargo.toml b/crates/bevy_internal/Cargo.toml
index 32e8e3d07babe..c222bbea25281 100644
--- a/crates/bevy_internal/Cargo.toml
+++ b/crates/bevy_internal/Cargo.toml
@@ -146,8 +146,8 @@ embedded_watcher = ["bevy_asset?/embedded_watcher"]
 
 # Enable system stepping support
 bevy_debug_stepping = [
-    "bevy_ecs/bevy_debug_stepping",
-    "bevy_app/bevy_debug_stepping"
+  "bevy_ecs/bevy_debug_stepping",
+  "bevy_app/bevy_debug_stepping",
 ]
 
 [dependencies]

From 7a841c8dfa97821ca1e14c4e7df3d1da2c2b5715 Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Fri, 5 Jan 2024 19:33:50 -0600
Subject: [PATCH 21/25] add system_stepping example; fix stepping issue

Added a straight-forward if wordy system_stepping example.  It
demonstrates the use of the Stepping resource to control the execution
of systems in a command-line app.

That example exposed a bug in how stepping handled continue when the
cursor moved past a breakpoint system.  Error was caught by
debug_assert!(), and was fixed eliminating the need for the
debug_assert!().  Added a test to cover the case.
---
 Cargo.toml                                    |  11 +
 .../src/schedule/executor/multi_threaded.rs   |   4 +-
 .../bevy_ecs/src/schedule/executor/simple.rs  |   4 +-
 .../src/schedule/executor/single_threaded.rs  |   4 +-
 crates/bevy_ecs/src/schedule/stepping.rs      |  50 ++++-
 examples/ecs/system_stepping.rs               | 201 ++++++++++++++++++
 6 files changed, 258 insertions(+), 16 deletions(-)
 create mode 100644 examples/ecs/system_stepping.rs

diff --git a/Cargo.toml b/Cargo.toml
index 526b01392833e..81c9a29b52ad3 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1463,6 +1463,17 @@ description = "Illustrates creating custom system parameters with `SystemParam`"
 category = "ECS (Entity Component System)"
 wasm = false
 
+[[example]]
+name = "system_stepping"
+path = "examples/ecs/system_stepping.rs"
+doc-scrape-examples = true
+
+[package.metadata.example.system_stepping]
+name = "System Stepping"
+description = "Demonstrate stepping through systems in order of execution"
+category = "ECS (Entity Component System)"
+wasm = false
+
 # Time
 [[example]]
 name = "time"
diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
index 0f9faff081dd7..dad217781a764 100644
--- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
+++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
@@ -166,7 +166,7 @@ impl SystemExecutor for MultiThreadedExecutor {
     fn run(
         &mut self,
         schedule: &mut SystemSchedule,
-        _skip_systems: Option<FixedBitSet>,
+        #[allow(unused_variables)] skip_systems: Option<FixedBitSet>,
         world: &mut World,
     ) {
         // reset counts
@@ -189,7 +189,7 @@ impl SystemExecutor for MultiThreadedExecutor {
         // If stepping is enabled, make sure we skip those systems that should
         // not be run.
         #[cfg(feature = "bevy_debug_stepping")]
-        if let Some(mut skipped_systems) = _skip_systems {
+        if let Some(mut skipped_systems) = skip_systems {
             debug_assert_eq!(skipped_systems.len(), self.completed_systems.len());
             // mark skipped systems as completed
             self.completed_systems |= &skipped_systems;
diff --git a/crates/bevy_ecs/src/schedule/executor/simple.rs b/crates/bevy_ecs/src/schedule/executor/simple.rs
index 28b1fb7141f85..e21edc072c362 100644
--- a/crates/bevy_ecs/src/schedule/executor/simple.rs
+++ b/crates/bevy_ecs/src/schedule/executor/simple.rs
@@ -33,13 +33,13 @@ impl SystemExecutor for SimpleExecutor {
     fn run(
         &mut self,
         schedule: &mut SystemSchedule,
-        _skip_systems: Option<FixedBitSet>,
+        #[allow(unused_variables)] skip_systems: Option<FixedBitSet>,
         world: &mut World,
     ) {
         // If stepping is enabled, make sure we skip those systems that should
         // not be run.
         #[cfg(feature = "bevy_debug_stepping")]
-        if let Some(skipped_systems) = _skip_systems {
+        if let Some(skipped_systems) = skip_systems {
             // mark skipped systems as completed
             self.completed_systems |= &skipped_systems;
         }
diff --git a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs
index 2bedc7bd74ad4..9fee0bcd81f6b 100644
--- a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs
+++ b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs
@@ -41,13 +41,13 @@ impl SystemExecutor for SingleThreadedExecutor {
     fn run(
         &mut self,
         schedule: &mut SystemSchedule,
-        _skip_systems: Option<FixedBitSet>,
+        #[allow(unused_variables)] skip_systems: Option<FixedBitSet>,
         world: &mut World,
     ) {
         // If stepping is enabled, make sure we skip those systems that should
         // not be run.
         #[cfg(feature = "bevy_debug_stepping")]
-        if let Some(skipped_systems) = _skip_systems {
+        if let Some(skipped_systems) = skip_systems {
             // mark skipped systems as completed
             self.completed_systems |= &skipped_systems;
         }
diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs
index a8fc9580d1c71..b1e235bc16ccf 100644
--- a/crates/bevy_ecs/src/schedule/stepping.rs
+++ b/crates/bevy_ecs/src/schedule/stepping.rs
@@ -780,19 +780,23 @@ impl ScheduleState {
                         skip.insert(i);
                     }
                 }
-                // If we're continuing, and we encounter a breakpoint we want
-                // to stop before executing the system.  So we skip this system,
-                // and set the action to waiting.  Note that we do not move the
-                // cursor.  As a result, the next time we continue, if the
-                // first system is a Break we run it anyway.  This allows the
-                // user to continue, hit a breakpoint to stop before running
-                // the system, then continue again to run the system with the
-                // breakpoint.
+                // If we're continuing, and we encounter a breakpoint we may
+                // want to stop before executing the system.  To do this we
+                // skip this system and set the action to Waiting.
+                //
+                // Note: if the cursor is pointing at this system, we will run
+                // it anyway.  This allows the user to continue, hit a
+                // breakpoint, then continue again to run the breakpoint system
+                // and any following systems.
                 (Action::Continue, SystemBehavior::Break) => {
                     if i != start {
-                        debug_assert!(pos == i);
                         skip.insert(i);
-                        action = Action::Waiting;
+
+                        // stop running systems if the breakpoint isn't the
+                        // system under the cursor.
+                        if i > start {
+                            action = Action::Waiting;
+                        }
                     }
                 }
                 // should have never gotten into this method if stepping is
@@ -842,6 +846,7 @@ mod tests {
 
     fn first_system() {}
     fn second_system() {}
+    fn third_system() {}
 
     fn setup() -> (Schedule, World) {
         let mut world = World::new();
@@ -1120,6 +1125,31 @@ mod tests {
         assert_schedule_runs!(&schedule, &mut stepping, first_system);
     }
 
+    /// regression test for issue encountered while writing `system_stepping`
+    /// example
+    #[test]
+    fn continue_step_continue_with_breakpoint() {
+        let mut world = World::new();
+        let mut schedule = Schedule::new(TestSchedule);
+        schedule.add_systems((first_system, second_system, third_system).chain());
+        schedule.initialize(&mut world).unwrap();
+
+        let mut stepping = Stepping::new();
+        stepping
+            .add_schedule(TestSchedule)
+            .enable()
+            .set_breakpoint(TestSchedule, second_system);
+
+        stepping.continue_frame();
+        assert_schedule_runs!(&schedule, &mut stepping, first_system);
+
+        stepping.step_frame();
+        assert_schedule_runs!(&schedule, &mut stepping, second_system);
+
+        stepping.continue_frame();
+        assert_schedule_runs!(&schedule, &mut stepping, third_system);
+    }
+
     #[test]
     fn clear_breakpoint() {
         let (schedule, _world) = setup();
diff --git a/examples/ecs/system_stepping.rs b/examples/ecs/system_stepping.rs
new file mode 100644
index 0000000000000..5a6c304d28e73
--- /dev/null
+++ b/examples/ecs/system_stepping.rs
@@ -0,0 +1,201 @@
+use bevy::{ecs::schedule::Stepping, prelude::*};
+
+fn main() {
+    let mut app = App::new();
+
+    app.add_systems(
+        Update,
+        (
+            update_system_one,
+            // establish a dependency here to simplify descriptions below
+            update_system_two.after(update_system_one),
+            update_system_three.after(update_system_two),
+            update_system_four,
+        ),
+    )
+    .add_systems(PreUpdate, pre_update_system);
+
+    // For the simplicity of this example, we directly modify the `Stepping`
+    // resource here and run the systems with `App::update()`.  Each call to
+    // `App::update()` is the equivalent of a single frame render when using
+    // `App::run()`.
+    //
+    // In a real-world situation, the `Stepping` resource would be modified by
+    // a system based on input from the user.  A full demonstration of this can
+    // be found in the breakout example.
+    println!(
+        r#"
+    Actions: call app.update()
+     Result: All systems run normally"#
+    );
+    app.update();
+
+    println!(
+        r#"
+    Actions: Add the Stepping resource then call app.update()
+     Result: All systems run normally.  Stepping has no effect unless explicitly
+             configured for a Schedule, and Stepping has been enabled."#
+    );
+    app.insert_resource(Stepping::new());
+    app.update();
+
+    println!(
+        r#"
+    Actions: Add the Update Schedule to Stepping; enable Stepping; call
+             app.update()
+     Result: Only the systems in PreUpdate run.  When Stepping is enabled,
+             systems in the configured schedules will not run unless:
+             * Stepping::step_frame() is called
+             * Stepping::continue_frame() is called
+             * System has been configured to always run"#
+    );
+    let mut stepping = app.world.resource_mut::<Stepping>();
+    stepping.add_schedule(Update).enable();
+    app.update();
+
+    println!(
+        r#"
+    Actions: call Stepping.step_frame(); call app.update()
+     Result: The PreUpdate systems run, and one Update system will run.  In
+             Stepping, step means run the next system across all the schedules 
+             that have been added to the Stepping resource."#
+    );
+    let mut stepping = app.world.resource_mut::<Stepping>();
+    stepping.step_frame();
+    app.update();
+
+    println!(
+        r#"
+    Actions: call app.update()
+     Result: Only the PreUpdate systems run.  The previous call to
+             Stepping::step_frame() only applies for the next call to
+             app.update()/the next frame rendered.
+    "#
+    );
+    app.update();
+
+    println!(
+        r#"
+    Actions: call Stepping::continue_frame(); call app.update()
+     Result: PreUpdate system will run, and all remaining Update systems will
+             run.  Stepping::continue_frame() tells stepping to run all systems
+             starting after the last run system until it hits the end of the
+             frame, or it encounters a system with a breakpoint set.  In this
+             case, we previously performed a step, running one system in Update.
+             This continue will cause all remaining systems in Update to run."#
+    );
+    let mut stepping = app.world.resource_mut::<Stepping>();
+    stepping.continue_frame();
+    app.update();
+
+    println!(
+        r#"
+    Actions: call Stepping::step_frame() & app.update() four times in a row
+     Result: PreUpdate system runs every time we call app.update(), along with
+             one system from the Update schedule each time.  This shows what
+             execution would look like to step through an entire frame of 
+             systems."#
+    );
+    for _ in 0..4 {
+        let mut stepping = app.world.resource_mut::<Stepping>();
+        stepping.step_frame();
+        app.update();
+    }
+
+    println!(
+        r#"
+    Actions: Stepping::always_run(Update, update_system_two); step through all
+             systems
+     Result: PreUpdate system and update_system_two() will run every time we
+             call app.update().  We'll also only need to step three times to
+             execute all systems in the frame.  Stepping::always_run() allows
+             us to granularly allow systems to run when stepping is enabled."#
+    );
+    let mut stepping = app.world.resource_mut::<Stepping>();
+    stepping.always_run(Update, update_system_two);
+    for _ in 0..3 {
+        let mut stepping = app.world.resource_mut::<Stepping>();
+        stepping.step_frame();
+        app.update();
+    }
+
+    println!(
+        r#"
+    Actions: Stepping::never_run(Update, update_system_two); continue through
+             all systems
+     Result: All systems except update_system_two() will execute.
+             Stepping::never_run() allows us to disable systems while Stepping
+             is enabled."#
+    );
+    let mut stepping = app.world.resource_mut::<Stepping>();
+    stepping.never_run(Update, update_system_two);
+    stepping.continue_frame();
+    app.update();
+
+    println!(
+        r#"
+    Actions: Stepping::set_breakpoint(Update, update_system_two); continue,
+             step, continue
+     Result: During the first continue, pre_update_system() and
+             update_system_one() will run.  update_system_four() may also run
+             as it has no dependency on update_system_two() or
+             update_system_three().  Nether update_system_two() nor
+             update_system_three() will run in the first app.update() call as
+             they form a chained dependency on update_system_one() and run
+             in order of one, two, three.  Stepping stops system execution in
+             the Update schedule when it encounters the breakpoint for
+             update_system_three().
+             During the step we run update_system_two() along with the
+             pre_update_system().
+             During the final continue pre_update_system() and
+             update_system_three() run."#
+    );
+    let mut stepping = app.world.resource_mut::<Stepping>();
+    stepping.set_breakpoint(Update, update_system_two);
+    stepping.continue_frame();
+    app.update();
+    let mut stepping = app.world.resource_mut::<Stepping>();
+    stepping.step_frame();
+    app.update();
+    let mut stepping = app.world.resource_mut::<Stepping>();
+    stepping.continue_frame();
+    app.update();
+
+    println!(
+        r#"
+    Actions: Stepping::clear_breakpoint(Update, update_system_two); continue
+             through all systems
+     Result: All systems will run"#
+    );
+    let mut stepping = app.world.resource_mut::<Stepping>();
+    stepping.clear_breakpoint(Update, update_system_two);
+    stepping.continue_frame();
+    app.update();
+
+    println!(
+        r#"
+    Actions: Stepping::disable(); app.update()
+     Result: All systems will run.  With Stepping disabled, there's no need to
+             call Stepping::step_frame() or Stepping::continue_frame() to run
+             systems in the Update schedule."#
+    );
+    let mut stepping = app.world.resource_mut::<Stepping>();
+    stepping.disable();
+    app.update();
+}
+
+fn pre_update_system() {
+    println!("▶ pre_update_system -- app.update() called");
+}
+fn update_system_one() {
+    println!("▶ update_system_one");
+}
+fn update_system_two() {
+    println!("▶ update_system_two");
+}
+fn update_system_three() {
+    println!("▶ update_system_three");
+}
+fn update_system_four() {
+    println!("▶ update_system_four");
+}

From 4242192477487a2d6f1b6192d0d4363215cfc5f4 Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Fri, 5 Jan 2024 20:10:55 -0600
Subject: [PATCH 22/25] update examples/README.md; add LogPlugin to
 system_stepping

Added LogPlugin to system_stepping example so the Stepping log messages
appear in the output.  This helps clarify what actions stepping is
performing during the example.
---
 examples/README.md              |  1 +
 examples/ecs/system_stepping.rs | 28 ++++++++++++++++------------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/examples/README.md b/examples/README.md
index 0450ef88159b8..fac0b381ccbd0 100644
--- a/examples/README.md
+++ b/examples/README.md
@@ -236,6 +236,7 @@ Example | Description
 [System Closure](../examples/ecs/system_closure.rs) | Show how to use closures as systems, and how to configure `Local` variables by capturing external state
 [System Parameter](../examples/ecs/system_param.rs) | Illustrates creating custom system parameters with `SystemParam`
 [System Piping](../examples/ecs/system_piping.rs) | Pipe the output of one system into a second, allowing you to handle any errors gracefully
+[System Stepping](../examples/ecs/system_stepping.rs) | Demonstrate stepping through systems in order of execution
 
 ## Games
 
diff --git a/examples/ecs/system_stepping.rs b/examples/ecs/system_stepping.rs
index 5a6c304d28e73..695b3ca64c989 100644
--- a/examples/ecs/system_stepping.rs
+++ b/examples/ecs/system_stepping.rs
@@ -1,19 +1,23 @@
 use bevy::{ecs::schedule::Stepping, prelude::*};
+use bevy_internal::log::LogPlugin;
 
 fn main() {
     let mut app = App::new();
 
-    app.add_systems(
-        Update,
-        (
-            update_system_one,
-            // establish a dependency here to simplify descriptions below
-            update_system_two.after(update_system_one),
-            update_system_three.after(update_system_two),
-            update_system_four,
-        ),
-    )
-    .add_systems(PreUpdate, pre_update_system);
+    app
+        // to display log messages from Stepping resource
+        .add_plugins(LogPlugin::default())
+        .add_systems(
+            Update,
+            (
+                update_system_one,
+                // establish a dependency here to simplify descriptions below
+                update_system_two.after(update_system_one),
+                update_system_three.after(update_system_two),
+                update_system_four,
+            ),
+        )
+        .add_systems(PreUpdate, pre_update_system);
 
     // For the simplicity of this example, we directly modify the `Stepping`
     // resource here and run the systems with `App::update()`.  Each call to
@@ -185,7 +189,7 @@ fn main() {
 }
 
 fn pre_update_system() {
-    println!("▶ pre_update_system -- app.update() called");
+    println!("▶ pre_update_system");
 }
 fn update_system_one() {
     println!("▶ update_system_one");

From 5f88d6bfe1b4673a9a77c2e4f541ea2aca4d9129 Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Fri, 5 Jan 2024 20:43:47 -0600
Subject: [PATCH 23/25] convert `bevy_internal` use in system_stepping example

---
 examples/ecs/system_stepping.rs | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/examples/ecs/system_stepping.rs b/examples/ecs/system_stepping.rs
index 695b3ca64c989..989f4ef23851b 100644
--- a/examples/ecs/system_stepping.rs
+++ b/examples/ecs/system_stepping.rs
@@ -1,5 +1,4 @@
-use bevy::{ecs::schedule::Stepping, prelude::*};
-use bevy_internal::log::LogPlugin;
+use bevy::{ecs::schedule::Stepping, log::LogPlugin, prelude::*};
 
 fn main() {
     let mut app = App::new();

From 7cc52facabeca594f1c3e0f610165ef58c4f0f0b Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Sat, 6 Jan 2024 11:56:16 -0600
Subject: [PATCH 24/25] Fixes per code-review comments

---
 crates/bevy_ecs/src/schedule/stepping.rs | 216 +++++++++++++++++++----
 1 file changed, 182 insertions(+), 34 deletions(-)

diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs
index b1e235bc16ccf..dff5434701a31 100644
--- a/crates/bevy_ecs/src/schedule/stepping.rs
+++ b/crates/bevy_ecs/src/schedule/stepping.rs
@@ -559,20 +559,21 @@ impl Stepping {
         // the schedule with the current stepping action.  If this is not the
         // cursor schedule, we'll run the schedule with the waiting action.
         let cursor = self.cursor;
-        let (list, next_system) = if index == cursor.schedule {
-            let o = state.skipped_systems(schedule, cursor.system, self.action);
+        let (skip_list, next_system) = if index == cursor.schedule {
+            let (skip_list, next_system) =
+                state.skipped_systems(schedule, cursor.system, self.action);
 
             // if we just stepped this schedule, then we'll switch the action
             // to be waiting
             if self.action == Action::Step {
                 self.action = Action::Waiting;
             }
-            o
+            (skip_list, next_system)
         } else {
             // we're not supposed to run any systems in this schedule, so pull
             // the skip list, but ignore any changes it makes to the cursor.
-            let (skip, _) = state.skipped_systems(schedule, 0, Action::Waiting);
-            (skip, Some(cursor.system))
+            let (skip_list, _) = state.skipped_systems(schedule, 0, Action::Waiting);
+            (skip_list, Some(cursor.system))
         };
 
         // update the stepping frame cursor based on if there are any systems
@@ -595,7 +596,7 @@ impl Stepping {
             }
         }
 
-        Some(list)
+        Some(skip_list)
     }
 }
 
@@ -653,6 +654,7 @@ impl ScheduleState {
     // clear all system behaviors
     fn clear_behaviors(&mut self) {
         self.behaviors.clear();
+        self.behavior_updates.clear();
         self.first = None;
     }
 
@@ -733,8 +735,6 @@ impl ScheduleState {
                 _system.name()
             );
 
-            // suppress this clippy warning; we want to keep the arms split to
-            // clearly document whats going on.
             match (action, behavior) {
                 // regardless of which action we're performing, if the system
                 // is marked as NeverRun, add it to the skip list.
@@ -856,6 +856,41 @@ mod tests {
         (schedule, world)
     }
 
+    // Helper for verifying skip_lists are equal, and if not, printing a human
+    // readable message.
+    macro_rules! assert_skip_list_eq {
+        ($actual:expr, $expected:expr, $system_names:expr) => {
+            let actual = $actual;
+            let expected = $expected;
+            let systems: &Vec<&str> = $system_names;
+
+            if (actual != expected) {
+                use std::fmt::Write as _;
+
+                // mismatch, let's construct a human-readable message of what
+                // was returned
+                let mut msg = format!(
+                    "Schedule:\n    {:9} {:16}{:6} {:6} {:6}\n",
+                    "index", "name", "expect", "actual", "result"
+                );
+                for (i, name) in systems.iter().enumerate() {
+                    let _ = write!(msg, "    system[{:1}] {:16}", i, name);
+                    match (expected.contains(i), actual.contains(i)) {
+                        (true, true) => msg.push_str("skip   skip   pass\n"),
+                        (true, false) => {
+                            msg.push_str("skip   run    FAILED; system should not have run\n")
+                        }
+                        (false, true) => {
+                            msg.push_str("run    skip   FAILED; system should have run\n")
+                        }
+                        (false, false) => msg.push_str("run    run    pass\n"),
+                    }
+                }
+                assert_eq!(actual, expected, "{}", msg);
+            }
+        };
+    }
+
     // Helper for verifying that a set of systems will be run for a given skip
     // list
     macro_rules! assert_systems_run {
@@ -887,28 +922,12 @@ mod tests {
                 None => FixedBitSet::with_capacity(systems.len()),
                 Some(b) => b,
             };
+            let system_names: Vec<&str> = systems
+                .iter()
+                .map(|(_,n)| n.rsplit_once("::").unwrap().1)
+                .collect();
 
-            if (actual != expected) {
-                use std::fmt::Write as _;
-
-                // mismatch, let's construct a human-readable message of what
-                // was returned
-                let mut msg = format!("Schedule:\n    {:9} {:16}{:6} {:6} {:6}\n",
-                    "index", "name", "expect", "actual", "result");
-                for (i, (_, full_name)) in systems.iter().enumerate() {
-                    let (_, name) = full_name.rsplit_once("::").unwrap();
-                    let _ = write!(msg, "    system[{:1}] {:16}", i, name);
-                    match (expected.contains(i), actual.contains(i)) {
-                        (true, true) => msg.push_str("skip   skip   pass\n"),
-                        (true, false) =>
-                            msg.push_str("skip   run    FAILED; system should not have run\n"),
-                        (false, true) =>
-                            msg.push_str("run    skip   FAILED; system should have run\n"),
-                        (false, false) => msg.push_str("run    run    pass\n"),
-                    }
-                }
-                assert_eq!(actual, expected, "{}", msg);
-            }
+            assert_skip_list_eq!(actual, expected, &system_names);
         };
     }
 
@@ -1205,6 +1224,44 @@ mod tests {
         assert_schedule_runs!(&schedule, &mut stepping, first_system, second_system);
     }
 
+    /// This was discovered in code-review, ensure that `clear_schedule` also
+    /// clears any pending changes too.
+    #[test]
+    fn set_behavior_then_clear_schedule() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping
+            .add_schedule(TestSchedule)
+            .enable()
+            .continue_frame();
+        assert_schedule_runs!(&schedule, &mut stepping, first_system, second_system);
+
+        stepping.never_run(TestSchedule, first_system);
+        stepping.clear_schedule(TestSchedule);
+        stepping.continue_frame();
+        assert_schedule_runs!(&schedule, &mut stepping, first_system, second_system);
+    }
+
+    /// Ensure that if they `clear_schedule` then make further changes to the
+    /// schedule, those changes after the clear are applied.
+    #[test]
+    fn clear_schedule_then_set_behavior() {
+        let (schedule, _world) = setup();
+
+        let mut stepping = Stepping::new();
+        stepping
+            .add_schedule(TestSchedule)
+            .enable()
+            .continue_frame();
+        assert_schedule_runs!(&schedule, &mut stepping, first_system, second_system);
+
+        stepping.clear_schedule(TestSchedule);
+        stepping.never_run(TestSchedule, first_system);
+        stepping.continue_frame();
+        assert_schedule_runs!(&schedule, &mut stepping, second_system);
+    }
+
     // Schedules such as FixedUpdate can be called multiple times in a single
     // render frame.  Ensure we only run steppable systems the first time the
     // schedule is run
@@ -1219,7 +1276,7 @@ mod tests {
             .always_run(TestSchedule, second_system)
             .continue_frame();
 
-        // start a new frame, then run the schedule three times; first system
+        // start a new frame, then run the schedule two times; first system
         // should only run on the first one
         stepping.next_frame();
         assert_systems_run!(
@@ -1241,13 +1298,104 @@ mod tests {
         let mut stepping = Stepping::new();
         stepping.add_schedule(TestSchedule).enable().step_frame();
 
-        // start a new frame, then run the schedule three times; first system
+        // start a new frame, then run the schedule two times; first system
         // should only run on the first one
         stepping.next_frame();
         assert_systems_run!(&schedule, stepping.skipped_systems(&schedule), first_system);
         assert_systems_run!(&schedule, stepping.skipped_systems(&schedule),);
     }
 
+    #[test]
+    fn step_duplicate_systems() {
+        let mut world = World::new();
+        let mut schedule = Schedule::new(TestSchedule);
+        schedule.add_systems((first_system, first_system, second_system).chain());
+        schedule.initialize(&mut world).unwrap();
+
+        let mut stepping = Stepping::new();
+        stepping.add_schedule(TestSchedule).enable();
+
+        // needed for assert_skip_list_eq!
+        let system_names = vec!["first_system", "first_system", "second_system"];
+        // we're going to step three times, and each system in order should run
+        // only once
+        for system_index in 0..3 {
+            // build the skip list by setting all bits, then clearing our the
+            // one system that should run this step
+            let mut expected = FixedBitSet::with_capacity(3);
+            expected.set_range(.., true);
+            expected.set(system_index, false);
+
+            // step the frame and get the skip list
+            stepping.step_frame();
+            stepping.next_frame();
+            let skip_list = stepping
+                .skipped_systems(&schedule)
+                .expect("TestSchedule has been added to Stepping");
+
+            assert_skip_list_eq!(skip_list, expected, &system_names);
+        }
+    }
+
+    #[test]
+    fn step_run_if_false() {
+        let mut world = World::new();
+        let mut schedule = Schedule::new(TestSchedule);
+
+        // This needs to be a system test to confirm the interaction between
+        // the skip list and system conditions in Schedule::run().  That means
+        // all of our systems need real bodies that do things.
+        //
+        // first system will be configured as `run_if(|| false)`, so it can
+        // just panic if called
+        let first_system = move || panic!("first_system should not be run");
+
+        // The second system, we need to know when it has been called, so we'll
+        // add a resource for tracking if it has been run.  The system will
+        // increment the run count.
+        #[derive(Resource)]
+        struct RunCount(usize);
+        world.insert_resource(RunCount(0));
+        let second_system = |mut run_count: ResMut<RunCount>| {
+            println!("I have run!");
+            run_count.0 += 1;
+        };
+
+        // build our schedule; first_system should never run, followed by
+        // second_system.
+        schedule.add_systems((first_system.run_if(|| false), second_system).chain());
+        schedule.initialize(&mut world).unwrap();
+
+        // set up stepping
+        let mut stepping = Stepping::new();
+        stepping.add_schedule(TestSchedule).enable();
+        world.insert_resource(stepping);
+
+        // if we step, and the run condition is false, we should not run
+        // second_system.  The stepping cursor is at first_system, and if
+        // first_system wasn't able to run, that's ok.
+        let mut stepping = world.resource_mut::<Stepping>();
+        stepping.step_frame();
+        stepping.next_frame();
+        schedule.run(&mut world);
+        assert_eq!(
+            world.resource::<RunCount>().0,
+            0,
+            "second_system should not have run"
+        );
+
+        // now on the next step, second_system should run
+        let mut stepping = world.resource_mut::<Stepping>();
+        stepping.step_frame();
+        stepping.next_frame();
+        schedule.run(&mut world);
+        assert_eq!(
+            world.resource::<RunCount>().0,
+            1,
+            "second_system should have run"
+        );
+    }
+
     #[test]
     fn remove_schedule() {
         let (schedule, _world) = setup();
@@ -1269,7 +1417,7 @@ mod tests {
     fn schedules() {
         let mut world = World::new();
 
-        // build & initialize three schedules
+        // build & initialize a few schedules
         let mut schedule_a = Schedule::new(TestScheduleA);
         schedule_a.initialize(&mut world).unwrap();
         let mut schedule_b = Schedule::new(TestScheduleB);
@@ -1279,7 +1427,7 @@ mod tests {
         let mut schedule_d = Schedule::new(TestScheduleD);
         schedule_d.initialize(&mut world).unwrap();
 
-        // setup stepping and add all three schedules
+        // setup stepping and add all the schedules
         let mut stepping = Stepping::new();
         stepping
             .add_schedule(TestScheduleA)
@@ -1332,7 +1480,7 @@ mod tests {
         schedule_b.add_systems((|| {}, || {}, || {}, || {}).chain());
         schedule_b.initialize(&mut world).unwrap();
 
-        // setup stepping and add all three schedules
+        // setup stepping and add all schedules
         let mut stepping = Stepping::new();
         stepping
             .add_schedule(TestScheduleA)

From b45538431107a095c9e94b65d4c2191a635d29d9 Mon Sep 17 00:00:00 2001
From: Carter Anderson <mcanders1@gmail.com>
Date: Fri, 2 Feb 2024 18:29:03 -0800
Subject: [PATCH 25/25] Minor style tweaks

---
 crates/bevy_ecs/src/schedule/executor/multi_threaded.rs  | 4 ++--
 crates/bevy_ecs/src/schedule/executor/simple.rs          | 4 ++--
 crates/bevy_ecs/src/schedule/executor/single_threaded.rs | 4 ++--
 crates/bevy_ecs/src/schedule/stepping.rs                 | 7 ++++++-
 4 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
index dad217781a764..0f9faff081dd7 100644
--- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
+++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
@@ -166,7 +166,7 @@ impl SystemExecutor for MultiThreadedExecutor {
     fn run(
         &mut self,
         schedule: &mut SystemSchedule,
-        #[allow(unused_variables)] skip_systems: Option<FixedBitSet>,
+        _skip_systems: Option<FixedBitSet>,
         world: &mut World,
     ) {
         // reset counts
@@ -189,7 +189,7 @@ impl SystemExecutor for MultiThreadedExecutor {
         // If stepping is enabled, make sure we skip those systems that should
         // not be run.
         #[cfg(feature = "bevy_debug_stepping")]
-        if let Some(mut skipped_systems) = skip_systems {
+        if let Some(mut skipped_systems) = _skip_systems {
             debug_assert_eq!(skipped_systems.len(), self.completed_systems.len());
             // mark skipped systems as completed
             self.completed_systems |= &skipped_systems;
diff --git a/crates/bevy_ecs/src/schedule/executor/simple.rs b/crates/bevy_ecs/src/schedule/executor/simple.rs
index e21edc072c362..28b1fb7141f85 100644
--- a/crates/bevy_ecs/src/schedule/executor/simple.rs
+++ b/crates/bevy_ecs/src/schedule/executor/simple.rs
@@ -33,13 +33,13 @@ impl SystemExecutor for SimpleExecutor {
     fn run(
         &mut self,
         schedule: &mut SystemSchedule,
-        #[allow(unused_variables)] skip_systems: Option<FixedBitSet>,
+        _skip_systems: Option<FixedBitSet>,
         world: &mut World,
     ) {
         // If stepping is enabled, make sure we skip those systems that should
         // not be run.
         #[cfg(feature = "bevy_debug_stepping")]
-        if let Some(skipped_systems) = skip_systems {
+        if let Some(skipped_systems) = _skip_systems {
             // mark skipped systems as completed
             self.completed_systems |= &skipped_systems;
         }
diff --git a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs
index 9fee0bcd81f6b..2bedc7bd74ad4 100644
--- a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs
+++ b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs
@@ -41,13 +41,13 @@ impl SystemExecutor for SingleThreadedExecutor {
     fn run(
         &mut self,
         schedule: &mut SystemSchedule,
-        #[allow(unused_variables)] skip_systems: Option<FixedBitSet>,
+        _skip_systems: Option<FixedBitSet>,
         world: &mut World,
     ) {
         // If stepping is enabled, make sure we skip those systems that should
         // not be run.
         #[cfg(feature = "bevy_debug_stepping")]
-        if let Some(skipped_systems) = skip_systems {
+        if let Some(skipped_systems) = _skip_systems {
             // mark skipped systems as completed
             self.completed_systems |= &skipped_systems;
         }
diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs
index 36c0899afe511..8a91f359713db 100644
--- a/crates/bevy_ecs/src/schedule/stepping.rs
+++ b/crates/bevy_ecs/src/schedule/stepping.rs
@@ -251,6 +251,9 @@ impl Stepping {
         schedule: impl ScheduleLabel,
         system: impl IntoSystem<(), (), Marker>,
     ) -> &mut Self {
+        // PERF: ideally we don't actually need to construct the system to retrieve the TypeId.
+        // Unfortunately currently IntoSystem::into_system(system).type_id() != TypeId::of::<I::System>()
+        // If these are aligned, we can use TypeId::of::<I::System>() here
         let type_id = IntoSystem::into_system(system).type_id();
         self.updates.push(Update::SetBehavior(
             schedule.intern(),
@@ -664,6 +667,8 @@ impl ScheduleState {
         // Systems may be present multiple times within a schedule, so we
         // iterate through all systems in the schedule, and check our behavior
         // updates for the system TypeId.
+        // PERF: If we add a way to efficiently query schedule systems by their TypeId, we could remove the full
+        // system scan here
         for (node_id, system) in schedule.systems().unwrap() {
             let behavior = self.behavior_updates.get(&system.type_id());
             match behavior {
@@ -707,7 +712,7 @@ impl ScheduleState {
         if self.first.is_none() {
             for (i, (node_id, _)) in schedule.systems().unwrap().enumerate() {
                 match self.behaviors.get(&node_id) {
-                    Some(SystemBehavior::AlwaysRun) | Some(SystemBehavior::NeverRun) => continue,
+                    Some(SystemBehavior::AlwaysRun | SystemBehavior::NeverRun) => continue,
                     Some(_) | None => {
                         self.first = Some(i);
                         break;