bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Fast and correct one-shot systems with a system registry resource

Open Pascualex opened this issue 1 year ago • 7 comments

Objective

  • Working with exclusive world access is not always easy: in many cases, a standard system or three is more ergonomic to write, and more modularly maintainable.
  • For small, one-off tasks (commonly handled with scripting), running an event-reader system incurs a small but flat overhead cost and muddies the schedule.
  • Certain forms of logic (e.g. turn-based games) want very fine-grained linear and/or branching control over logic.
  • SystemState is not automatically cached, and so performance can suffer and change detection breaks.
  • Fixes One-shot systems via Commands for scripting-like logic #2192.
  • Partial workaround for API for dynamic management of systems (adding and removing at runtime) #279.

Solution

  • Adds a SystemRegistry resource to the World, which stores initialized systems keyed by their SystemSet.
  • Allows users to call world.run_system(my_system) and commands.run_system(my_system), without re-initializing or losing state (essential for change detection).
  • Add a Callback type to enable convenient use of dynamic one shot systems and reduce the mental overhead of working with Box<dyn SystemSet>.
  • ~Allow users to run systems based on their SystemSet, enabling more complex user-made abstractions.~

Status

  • [x] Only allow the registry to run single systems, instead of whole sets.

Future work

  • Parameterized one-shot systems would improve reusability and bring them closer to events and commands. The API could be something like run_system_with_input(my_system, my_input) and use the In SystemParam.
  • We should evaluate unification of commands and one-shot systems since they are two different ways to run logic on demand over a World.

Prior attempts

  • #2234
  • #2417
  • #4090

This PR continues the work done in #4090.

Pascualex avatar Mar 09 '23 11:03 Pascualex

@alice-i-cecile I don't like/understand indexing systems by their sets. I like the idea of moving systems out of schedules and into a centralized system registry, but IMO it should only be possible to run them by type.

The use-case of running multiple systems on demand seems better handled by schedules than by this "run in linear registration order".

I know that system labels and sets have been merged into a single concept. So I'm not sure what could be an alternative to sets but with a 1-1 relationship with systems.

Pascualex avatar Mar 09 '23 14:03 Pascualex

IMO it should only be possible to run them by type.

Agreed here: we should only accept a impl Into<SystemTypeSet> and enforce uniqueness. I don't think there's a reasonable use case for "one shot systems but you have multiple distinct copies of the systems and want to keep their internal state distinct". In that case, the user should newtype them.

The use-case of running multiple systems on demand seems better handled by schedules than by this "run in linear registration order".

Strongly agreed.

alice-i-cecile avatar Mar 09 '23 17:03 alice-i-cecile

I'd like to unify the following commands:

pub struct RunSystemCommand<...> {
    _phantom_marker: PhantomData<M>,
    system: S,
}

// formerly RunSystemsBySetCommand
pub struct RunCallback {
    pub callback: Callback,
}

RunCallback stores the SystemTypeSet and requires preregistration, while RunSystemCommand stores the system itself and handles system registration. I don't see a strong reason to employ different strategies here. We could have Callback implement IntoSystem or just have run_callback call run_system if they employed the same strategy.

I personally favor preregistration for explicitness on the fact that this is being stored and cached.

Pascualex avatar Mar 10 '23 12:03 Pascualex

@alice-i-cecile I have another question. #4090 stablishes as future work:

Use this API to store all systems, regardless of whether or not they belong to a schedule, and solve https://github.com/bevyengine/bevy/pull/2777.

Does this still make sense? How would this work with multiple instances of a system inside a world? Should they share change detection or each have their own?

Pascualex avatar Mar 10 '23 15:03 Pascualex

I don't think that proposal makes sense any more :) Like you said, it doesn't play nice with multiple copies of systems, and we already have schedules for that.

alice-i-cecile avatar Mar 10 '23 16:03 alice-i-cecile

Yes, sadly I started to feel the same way after thinking about some of the details involved.

Even if we renounce to this idea of a global system registry where all systems are stored, we may still want a solution for running systems on demand instead of polling. Kind of like schedules but for single systems.

To me it makes sense to call this single-system schedule-like things "Commands", that's what my proposal in #7707 was about.

I know that you said that these systems usually don't require exclusive access, but the fact that they are not scheduled is what removes the possibility of parallelization (unless we can think of some cool solution). Just like this system registry solution requires using commands or direct access to the world to run the systems, even for non-exclusive systems.

The current commands for entity and component management could be renamed to something like exclusive/direct/world commands. That way we'd have exclusive commands (which wouldn't have the overhead of systems) and system commands (which would have the ergonomics of systems).

Edit: Conceptually there would be two ways of running systems of demand, each for a different use-case and with different capabilities:

  • Schedules: allow to run a group of systems on demand handling collisions, ordering and optional parallelization.
  • Commands: allow to run a single system (or direct function call) on demand, with the possibility of passing some arguments.

Pascualex avatar Mar 10 '23 17:03 Pascualex

We really can't reuse the name Commands: that's already taken for Commands.

I agree that we should be able to run systems without pre-registration. I'm currently conflicted between:

  1. Require a distinct method like run_uncached
  2. Just cache it the first time the system is run.

The first is more explicit, the second is substantially more ergonomic.

alice-i-cecile avatar Mar 10 '23 18:03 alice-i-cecile

Sorry for not providing more updates. I was diving deep in Bevy internals but I started on a new position shortly after. I'll try to continue contributing in the future, but right now I don't have the time.

Pascualex avatar Apr 14 '23 16:04 Pascualex

Merged elsewhere!

alice-i-cecile avatar Sep 19 '23 20:09 alice-i-cecile