Update/FixedUpdate Stepping #8168
Conversation
alice-i-cecile
added
C-Feature
| #[derive(Resource)] | ||
| pub enum Stepping { | ||
| Disabled, | ||
| Enabled { frames: usize }, |
There was a problem hiding this comment.
The frames field is hard to understand at a glance. It either needs a clearer name, or we should embed the "this still needs to step" information out into an enum variant.
There was a problem hiding this comment.
Yeah im not convinced either. Just a convenience at this point / provides functionality to help test things.
|
Overall, I like it. This is still quite useful, even if it's not as powerful. Boilerplate and complexity reduction is very nice. I'm generally nervous about making Note that this encourages users to move all of their UI, camera movement code and so on out of If this came with a list of "schedules that are stepped" I think I'd be solidly in favor; being able to modify that list at runtime would be extremely powerful for debugging. Following up on this, I think extending this same approach to system sets would work really well for fine-grained control when you need it. |
With the first two "system based stepping" approaches, you could just opt those systems out (ex:
Makes sense to me. Very straightforward to add / my head was in a similar place. |
|
I just confirmed that this works really nicely:
|
|
I'll start off by saying I'm not committed to the approach used in #8063, but I am committed to pushing for that level of convenience and functionality for bevy users (myself 😀 ). This is undoubtably a more elegant solution for per-frame stepping, but I'm a little worried that the lack of system-stepping in this PR creates an (not intentionally) misleading impression that we won't need some mechanism to say "always run this system" for system-stepping. This is acknowledged in the PR comments, but I want to be clear that as complexity was the main hurtle to #8063 we can't compare the two approaches right now, as they do very different things. Frame stepping in this PR differs from #8063 in that #8063 also supports remaining-frame stepping. As in, I've stepping past the systems I care about, move on to the start of the next frame so I can see it again. Although frame-stepping is helpful, per-system stepping is an order of magnitude more powerful as it allows us to examine intra-frame system interactions. Those interactions simply cannot be seen with per-frame stepping.
This is the part that I struggle with. It looks like the trade-off you've made to remove the cost of If I'm reading this right, I think this may be the key place we differ. From the phrasing "without any need to opt bevy internals or user systems in or out", it sounds like you'd rather limit stepping to a fixed selection of schedules than add any additional complexity at all (understandable; bevy is a lot of code). I would like stepping to have the widest reach possible, but find ways to reduce the complexity for opt-out. Perhaps there's a compromise where
I may be mis-reading this, but are you talking about a combination of running the app in One of the challenges to that right now is it's very hard to get a list of systems due to Overall, I am curious to see what this PR looks like with system-stepping implemented. |
Yup I fully acknowledge that the "hard part" is the system stepping / thats where the nuance is (and where a lot of the value is). I tried to call that out in the description. This PR is intended to be an easy way to start the conversation about high level apis / what this should look like on a system-to-system basis: I believe that defaulting to Update/FixedUpdate being stepped and everything else not being stepped is the correct path forward, as it ensures reasonable UX and features without foisting complexity on Bevy developers. Taking the easy path first enabled me to illustrate that those boundaries work very well (and that this can be implemented very simply at the schedule level). It also opens the door to a controversy-free / easily mergeable middle ground if we're interested in that (while we solve the harder / more controversial system-level stepping problems).
Allowing other schedules to be stepped (in an opt-in basis) is definitely where my head (and @alice-i-cecile's head) is at. See our comments above. I see no reason to hard code this ... this PR is just an initial / quick illustrative impl.
Yeah when actually "in" a breakpoint that would prevent inspector tools from working effectively, as theres no way to send new information to the inspector (unless the inspector logic is running in a different thread / isn't blocked by the breakpoint ... this would be unsafe but possible). You could inspect after leaving the breakpoint (when we go back to the normal "schedule stepping" logic). You could only inspect the end result of a full frame, not the intermediate results. Definitely a downside.
To help illustrate this: I think using the executor changes from your PR are an extremely valid path forward. The only significant difference would be high level defaults (FixedUpdate/Update stepped by default, everything else not stepped by default). I would like us to investigate a run-condition impl, but I think its reasonably likely that will be nonviable. |
|
Ok, I think I understand now. This approach switches to opt-in for stepping, at the schedule level; with Update & FixedUpdate enabled by default. Thank you for taking the time to clarify for me. Are you envisioning system-stepping opt-in only at the schedule level, or at both system & schedule? Or phrased another way, must all systems in a stepping schedule permit stepping? |
Yup!
I think system-level stepping implies every system being "stepped" in a way (at least for normal workflows within Update/FixedUpdate). Probably 3 states for each system:
Where |
|
By default, systems in Update/FixedUpdate would be |
|
We might want |
|
@dmlary: if you're interested in porting your PR to this general approach (while still doing the executor-based stepping), let me know. From my first pass, I don't think it would be particularly difficult. It would also make the change set much smaller / easier to review as it would remove all of the annotations from the engine and example code. |
|
That meets all of my core requirements; it should benefit most bevy users without having to make any changes. There are two pieces I'm not able to envision from where we sit in How do we communicate two pieces down to the ScheduleExecutors:
One of the places I really struggled with the complexity of stepping was tracking which schedule to system-step next. My ugly implementation in #8063 "works", but it's awful to expose to a user: bevy/examples/games/stepping.rs Lines 285 to 308 in 05719d9 If there's any way we could move that state management into the |
Yea, I'm absolutely willing to keep working on the PR. This is functionality I'm invested in. |
In a world where the executor is what supports stepping, I don't see much of a reason to limit this to Main (or use I think most step state could be stored per schedule (ex: where did we stop last frame). But for stuff that needs to be shared / user-configurable, you could just pass that in via some shared ECS Resource (the executor has mutable access to World, so state can be written to / read from some I think the biggest open question is "how to define what systems to break on at runtime". I think some combination of these is the move: A variant of add_systems that returns a list of NodeIds in the order they were defined and a way to add breakpoints: let ids = app.add_systems_and_get_ids(Update, (foo, bar));
// later in the app (maybe in a system somewhere)
stepping.add_breakpoint(Update, ids[1]); // break on bar
// later in app (maybe in a system somewhere)
stepping.remove_breakpoint(Update, ids[1]); // stop breaking on barA way to query node ids based on system name / set (note we don't store this info currently ... it needs to be computed from the graph ... this is also useful for other debugging scenarios). This is actually involved enough that we might want to do it separately: // returns a list of all node ids with the given SystemSet
let ids = update_schedule.get_node_ids(bar);
stepping.add_breakpoint(Update, ids[0]);
I bet Stepping could own this: // default to no stepping at all
app.insert_resource(Stepping::new().step_schedule(Update).step_schedule(FixedUpdate))Internally each schedule executor would check the Stepping resource: it would break if (1) stepping is enabled for the schedule (2) we didn't break on a different schedule this frame (3) a system within this schedule has a break point defined. Ideally we use this to "short circuit" expensive stepping work if stepping is disabled for this schedule or there are no break points defined. |
|
I'd like to suggest an alteration to There are three distinct stepping states I've used with increasing granularity when debugging a problem:
I think we should split the existing Then a run-time piece
|
|
For modifying which systems to stop on: You're bringing up a new interface for me to consider here, and that is the programmatic enabling of stepping on a system. UI ApproachThe primary way I've been thinking about this is a UI that displays all systems in schedules with checkboxes. Example of something rough I've done in the past, the second column of checkmarks is effectively Screen.Recording.2023-02-14.at.9.53.59.PM.movThis approach is possible with the helpers added in #8063 on Programmatic approachFor a programmatic approach, I love this interface: fn bar(...) {}
let ids = update_schedule.get_node_ids(bar);I didn't realize this was possible in rust. If there's an example somewhere doing the comparison of |
I don't see how Stop is different from "break" conceptually. I don't think creating new concepts for runtime vs compiletime is helpful here. In general I think the only difference between the two concepts should be: is it "defined/fixed at compile time" or "added/removed at run time". In fact to start, I think skipping fixed compile-time configuration is a reasonable corner to cut while we sort out the runtime apis. Defining configuration directly on systems should just be a convenience (if we decide that is actually useful relative to the runtime apis). It sounds like the only missing piece here in my proposal to enable all of the scenarios you mentioned above (other than the new suggested Skip state, which seems useful but feels orthogonal) is a We might want the optional concept of "removing breakpoints on hit". To remove the need to manually remove them. Also one reason to keep this runtime configuration out of a Schedule: during the execution of a schedule systems don't have access to that schedule (it is removed from the world). If we store this in the Stepping resource, we can queue up Update system breaks from within Update. Internally, Stepping would contain:
Some examples (using runtime-only apis, pretending static system config doesn't exist). Note that Stepping could be configured in a UI: // Break on a specific system in update
app
.insert_resource(
Stepping::new()
.enable_stepping(Update)
// this will continue to break on foo for each update, until the breakpoint is removed
.with_breakpoint(Update, foo)
)
.add_system(Update, foo)
// Break on the next steppable system in Update
fn break_next(input: Res<Input<KeyCode>>, mut stepping: ResMut<Stepping>) {
if input.just_pressed(KeyCode::Space) {
stepping.remove_breakpoints_on_hit = true;
stepping.break_on_next_system(Update)
}
} |
Yup this is possible! |
I think I'm approaching this from a different direction. It's the concept of I'm trying to implement stepping using If we start with If we start with I would prefer we default to |
|
Oh, or does I think what I'm trying to propose is:
We probably should end up saying that "if you don't want to step through all systems, you need to switch to frame-step mode with breakpoints. Ignore Stepping is an important state because it distinguishes from Continue to determine if system-stepping should stop after running the system. |
|
Does the in-app system stepping approach have some advantage over what debuggers already have, apart from ease of use? I was thinking about the possibility of having a launcher process that also acts as an abstraction over a debugger. It would work as this:
Ultimately, the launcher role can be implemented on the editor itself. |
No, nothing more than ease of use. You can do both system stepping and frame stepping using a debugger. However, it takes a lot of knowledge to be able to do it. To do system-step, you need to put breakpoints after every system. Well, we can do that at the return from This is possible, but not accessible by any means. This difficulty barrier means nobody will do this except in the most complex situations with a lot of effort. |
Ahhh I was thinking break actions would be the only way to stop execution mid-frame. "System stepping" would be implemented via break points (via the That being said, I do see the appeal of a separate "system step" action / that does feel reasonable to me. I'm down for the behaviors in your table. |
|
@cart ok, I’ll start implementing according to that table, with the schedule opt-in we discussed earlier. I’ll open it as a separate PR when it’s functional |
|
@cart I'm hitting an architectural fork and I'd like your input.
I was planning to update // grab the stepping config
let mut stepping = world.resource_mut::<Stepping>();
// build a skip list for this system
let skip = stepping.get_systems_skip_list(&self);
// call the executor, passing it the skip list
executor.run(executable, skip);The problem is that I see two paths:
I would prefer to keep all of this stepping logic in Maybe pass the label in as an argument to |
|
I’m going to move forward with the adding label to Schedule path. I’ll make |
I'm still struggling with stepping.always_run(Update, my_system);And I implemented the following on 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: Box::new(IntoSystem::into_system(system)),
behavior: StepBehavior::AlwaysRun,
});
self
}These updates are queued because we don't want them executing mid-frame; it would cause things like double-stepping. The ProblemI have a error[E0369]: binary operation `==` cannot be applied to type `dyn system::system::System<Out = (), In = ()>`
--> crates/bevy_ecs/src/schedule/stepping.rs:261:28
|
261 | if *system == **inner {
| ------- ^^ ------- dyn system::system::System<Out = (), In = ()>
| |
| dyn system::system::System<Out = (), In = ()>
For more information about this error, try `rustc --explain E0369`.I looked into WorkaroundFor right now, I'm comparing using @cart do you know of a better approach here? |
|
Lol, ok, I just figured out |
|
Sorry for the late reply!
This seems reasonable for now. We might find a better way to handle the dataflow later, but I'm reasonably happy with this.
I think we should generally only use two things for system identity: SystemLabel (a one to many relationship) and NodeId (a resolved reference to a specific system in a schedule). I think |
|
No worries, I'm still making progress; it's just a bit slower, trying to avoid architectural mis-steps.
Agreed.
For the For more control, there will be a
I'm doing this a little differently. The Work on this is on-going here: https://github.com/dmlary/bevy/tree/stepping-resource |
Makes sense. I dig it!
Sounds reasonable to me :) |
|
Still working on this, but minor update:
To handle this from the stepping side, we'll only run systems the first time the schedule is called within a render frame. This is likely gonna have all manner of weird side-effects. Also, the longer you wait between steps, the more times |
|
Closing in favor of #8453. |
# Objective Add interactive system debugging capabilities to bevy, providing step/break/continue style capabilities to running system schedules. * Original implementation: #8063 - `ignore_stepping()` everywhere was too much complexity * Schedule-config & Resource discussion: #8168 - Decided on selective adding of Schedules & Resource-based control ## Solution Created `Stepping` Resource. This resource can be used to enable stepping on a per-schedule basis. Systems within schedules can be individually configured to: * AlwaysRun: Ignore any stepping state and run every frame * NeverRun: Never run while stepping is enabled - this allows for disabling of systems while debugging * Break: If we're running the full frame, stop before this system is run Stepping provides two modes of execution that reflect traditional debuggers: * Step-based: Only execute one system at a time * Continue/Break: Run all systems, but stop before running a system marked as Break ### Demo https://user-images.githubusercontent.com/857742/233630981-99f3bbda-9ca6-4cc4-a00f-171c4946dc47.mov Breakout has been modified to use Stepping. The game runs normally for a couple of seconds, then stepping is enabled and the game appears to pause. A list of Schedules & Systems appears with a cursor at the first System in the list. The demo then steps forward full frames using the spacebar until the ball is about to hit a brick. Then we step system by system as the ball impacts a brick, showing the cursor moving through the individual systems. Finally the demo switches back to frame stepping as the ball changes course. ### Limitations Due to architectural constraints in bevy, there are some cases systems stepping will not function as a user would expect. #### Event-driven systems Stepping does not support systems that are driven by `Event`s as events are flushed after 1-2 frames. Although game systems are not running while stepping, ignored systems are still running every frame, so events will be flushed. This presents to the user as stepping the event-driven system never executes the system. It does execute, but the events have already been flushed. This can be resolved by changing event handling to use a buffer for events, and only dropping an event once all readers have read it. The work-around to allow these systems to properly execute during stepping is to have them ignore stepping: `app.add_systems(event_driven_system.ignore_stepping())`. This was done in the breakout example to ensure sound played even while stepping. #### Conditional Systems When a system is stepped, it is given an opportunity to run. If the conditions of the system say it should not run, it will not. Similar to Event-driven systems, if a system is conditional, and that condition is only true for a very small time window, then stepping the system may not execute the system. This includes depending on any sort of external clock. This exhibits to the user as the system not always running when it is stepped. A solution to this limitation is to ensure any conditions are consistent while stepping is enabled. For example, all systems that modify any state the condition uses should also enable stepping. #### State-transition Systems Stepping is configured on the per-`Schedule` level, requiring the user to have a `ScheduleLabel`. To support state-transition systems, bevy generates needed schedules dynamically. Currently it’s very difficult (if not impossible, I haven’t verified) for the user to get the labels for these schedules. Without ready access to the dynamically generated schedules, and a resolution for the `Event` lifetime, **stepping of the state-transition systems is not supported** --- ## Changelog - `Schedule::run()` updated to consult `Stepping` Resource to determine which Systems to run each frame - Added `Schedule.label` as a `BoxedSystemLabel`, along with supporting `Schedule::set_label()` and `Schedule::label()` methods - `Stepping` needed to know which `Schedule` was running, and prior to this PR, `Schedule` didn't track its own label - Would have preferred to add `Schedule::with_label()` and remove `Schedule::new()`, but this PR touches enough already - Added calls to `Schedule.set_label()` to `App` and `World` as needed - Added `Stepping` resource - Added `Stepping::begin_frame()` system to `MainSchedulePlugin` - Run before `Main::run_main()` - Notifies any `Stepping` Resource a new render frame is starting ## Migration Guide - Add a call to `Schedule::set_label()` for any custom `Schedule` - This is only required if the `Schedule` will be stepped --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective Add interactive system debugging capabilities to bevy, providing step/break/continue style capabilities to running system schedules. * Original implementation: bevyengine#8063 - `ignore_stepping()` everywhere was too much complexity * Schedule-config & Resource discussion: bevyengine#8168 - Decided on selective adding of Schedules & Resource-based control ## Solution Created `Stepping` Resource. This resource can be used to enable stepping on a per-schedule basis. Systems within schedules can be individually configured to: * AlwaysRun: Ignore any stepping state and run every frame * NeverRun: Never run while stepping is enabled - this allows for disabling of systems while debugging * Break: If we're running the full frame, stop before this system is run Stepping provides two modes of execution that reflect traditional debuggers: * Step-based: Only execute one system at a time * Continue/Break: Run all systems, but stop before running a system marked as Break ### Demo https://user-images.githubusercontent.com/857742/233630981-99f3bbda-9ca6-4cc4-a00f-171c4946dc47.mov Breakout has been modified to use Stepping. The game runs normally for a couple of seconds, then stepping is enabled and the game appears to pause. A list of Schedules & Systems appears with a cursor at the first System in the list. The demo then steps forward full frames using the spacebar until the ball is about to hit a brick. Then we step system by system as the ball impacts a brick, showing the cursor moving through the individual systems. Finally the demo switches back to frame stepping as the ball changes course. ### Limitations Due to architectural constraints in bevy, there are some cases systems stepping will not function as a user would expect. #### Event-driven systems Stepping does not support systems that are driven by `Event`s as events are flushed after 1-2 frames. Although game systems are not running while stepping, ignored systems are still running every frame, so events will be flushed. This presents to the user as stepping the event-driven system never executes the system. It does execute, but the events have already been flushed. This can be resolved by changing event handling to use a buffer for events, and only dropping an event once all readers have read it. The work-around to allow these systems to properly execute during stepping is to have them ignore stepping: `app.add_systems(event_driven_system.ignore_stepping())`. This was done in the breakout example to ensure sound played even while stepping. #### Conditional Systems When a system is stepped, it is given an opportunity to run. If the conditions of the system say it should not run, it will not. Similar to Event-driven systems, if a system is conditional, and that condition is only true for a very small time window, then stepping the system may not execute the system. This includes depending on any sort of external clock. This exhibits to the user as the system not always running when it is stepped. A solution to this limitation is to ensure any conditions are consistent while stepping is enabled. For example, all systems that modify any state the condition uses should also enable stepping. #### State-transition Systems Stepping is configured on the per-`Schedule` level, requiring the user to have a `ScheduleLabel`. To support state-transition systems, bevy generates needed schedules dynamically. Currently it’s very difficult (if not impossible, I haven’t verified) for the user to get the labels for these schedules. Without ready access to the dynamically generated schedules, and a resolution for the `Event` lifetime, **stepping of the state-transition systems is not supported** --- ## Changelog - `Schedule::run()` updated to consult `Stepping` Resource to determine which Systems to run each frame - Added `Schedule.label` as a `BoxedSystemLabel`, along with supporting `Schedule::set_label()` and `Schedule::label()` methods - `Stepping` needed to know which `Schedule` was running, and prior to this PR, `Schedule` didn't track its own label - Would have preferred to add `Schedule::with_label()` and remove `Schedule::new()`, but this PR touches enough already - Added calls to `Schedule.set_label()` to `App` and `World` as needed - Added `Stepping` resource - Added `Stepping::begin_frame()` system to `MainSchedulePlugin` - Run before `Main::run_main()` - Notifies any `Stepping` Resource a new render frame is starting ## Migration Guide - Add a call to `Schedule::set_label()` for any custom `Schedule` - This is only required if the `Schedule` will be stepped --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Objective
Alternative to #8063
Note that this is a draft PR and only exists to illustrate an idea. It is not intended to be a final API.
It would be nice if users could "step through" their app logic to debug on a frame-by-frame basis (and in the future ... system-by-system).
This aims to illustrate an alternative to the system stepping impl in #8063 that accomplishes the following goals:
Note that this type of "conditional update/fixedupdate" logic running is how engines like Godot implement their pausing functionality.
Like #8063, this would benefit from event buffers that aren't cleared every other frame.
Next Steps
KeyCode::Spaceto advance by one frameTime:delta()in the context of breakpoints, otherwise things will jump around a lot (and FixedUpdate will have a ton of updates each frame). We would want to clamp it to some max value in this context (or maybe all contexts?).