Schedule enter and exit hooks#23738
Conversation
|
Hi @alice-i-cecile , here's a proposal about schedule hooks. I think this feature can support dynamic systems for schedules. I hope this proposal can be included in the 0.19 milestone. I was wondering if you might be interested. |
kfc35
added
C-Feature
andriyDev
left a comment
andriyDev
left a comment
There was a problem hiding this comment.
I don't really care for this change. Hooking on to the schedule execution seems like it would make things slow for the common case of not using this feature.
Even ignoring that, I think this feature needs more motivation. The issue talks about making schedules more dynamic - but this PR won't help with that since the hook can't access the schedule, because the schedule is removed from the world to run it. If you want to mutate schedules, just change them in a system outside the schedule. So for example, in a system in Startup.
crates/bevy_ecs/src/world/mod.rs
Outdated
| self.try_schedule_scope(label, |world, sched| sched.run(world)) | ||
| self.try_schedule_scope(label.intern(), |world, sched| { | ||
| world.try_resource_scope::<ScheduleHooks, ()>(|world, mut hooks| { | ||
| hooks.run_enter(world, label.intern()); |
There was a problem hiding this comment.
Why not just trigger an observer instead? Then there's no need to make a whole custom "hooking" thing.
There was a problem hiding this comment.
I haven't paid attention to observers for a long time. After completing the rough design, I will give it a try.
Definitely not at this stage. Overall the idea has some promise as a way to simplify our state handling code, but I'm still not convinced of the need (or that this is a correct design. I can't dedicate the time and energy to review this properly right now, but if you can convince others that this is useful I will prioritize it more heavily. |
alice-i-cecile
added
S-Needs-Goal
Performance issues still need optimization. My original goal was to implement two things: transferring a CommandQueue to another schedule for deferred processing, and modifying the system set of the current schedule. I know that schedule modifications can be done in other stages, but that requires the target schedule to have a system that handles and stores the changed data (which would be repetitive if every project had to do it). That brings us back to the first requirement. Deferred merging of CommandQueue across schedules (see #23145) — i.e., putting the queue into a specific ScheduleLabel and merging it — is complicated. Executing certain behaviors on schedule entry and exit is an extended idea. As far as I know, within a schedule, there is no way to guarantee which system runs first, and the Commands queue runs last. However, this extended idea is closely related to the two previous requirements. Therefore, my goal is to bridge the gaps between different schedules and prepare the execution for the next schedule. |
|
@andriyDev You're right, observers are more suitable. I've adjusted the implementation to better meet the requirements. |
4 participants
Objective
Fixes #23191
Solution
#23191 (comment)
Testing
I've done some simple tests to ensure the hooks work as expected.
However, I haven't conducted any performance tests, and I don't have much experience in that area.
The documentation still needs improvement, and some other features also need to be refined.