Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run system from &mut World #2417

Closed
wants to merge 8 commits into from

Conversation

alice-i-cecile
Copy link
Member

@alice-i-cecile alice-i-cecile commented Jun 30, 2021

Objective

Systems are an ergonomic and expressive API for performing well-encapsulated complex logic.
However, you could not easily use them while working directly with &mut World for custom commands, exclusive systems, writing tests or bare-bones bevy_ecs usage.

Solution

Adds a World::run_system method, which immediately executes a single system in sequence and then flushes any Commands (or similar system parameters).

This is also very useful for users who may want to construct their own game loop in part or whole, sacrificing parallelism for simplicity and control.

Notes

Closely related to #2234, and borrows aggressively from the implementation there. Thanks @mockersf for pointing me in the right direction <3

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Jun 30, 2021
@alice-i-cecile
Copy link
Member Author

I considered an API for chainable systems, accepting Box<dyn System> as the method argument type. However, like basically every other time I've examined system chaining, I could not find real world applications for this.

If others have compelling ideas, it can be added in this or a future PR.

alice-i-cecile added a commit to alice-i-cecile/bevy-website that referenced this pull request Jun 30, 2021
@NathanSWard NathanSWard added A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible and removed S-Needs-Triage This issue needs to be labelled labels Jun 30, 2021
@cart
Copy link
Member

cart commented Jul 1, 2021

I'm not convinced that this biases toward the right outcomes. It encourages people to re-create systems every frame because it requires taking ownership of the system. Instead I think we should make Systems themselves easier to run with the correct lifecycle (with a mutable system borrow). Ex: System::run_but_do_everything_a_schedule_would(&mut self, &mut world).

@alice-i-cecile
Copy link
Member Author

I hadn't noticed the ownership issue. I agree, that needs to be fixed one way or another.

Ah, so make this a method on systems instead? I can get behind that; the reduced discoverability isn't bad since I'm aiming to include this in the new ECS chapter.

I don't think I can pass in the argument here by reference since it's impl IntoSystemDescriptor, but I'll try that first...

@alice-i-cecile
Copy link
Member Author

    pub fn run_system<Params>(&mut self, system: &(impl IntoSystemDescriptor<Params> + Clone)) {

So this compiles, but requires Clone. But then the examples no longer work due to Clone being missing on the functions. We could make Clone required by IntoSystemDescriptor (which could be handy for dynamically arranging the schedule later??) but I think the proposed approach is superior.

@Trashtalk217 Trashtalk217 mentioned this pull request Jun 26, 2023
github-merge-queue bot pushed a commit that referenced this pull request Sep 19, 2023
I'm adopting this ~~child~~ PR.

# 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 #2192.
- Partial workaround for #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.

## 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 the 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
- #7999

This PR continues the work done in
#7999.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Federico Rinaldi <gisquerin@gmail.com>
Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>
Co-authored-by: Aevyrie <aevyrie@gmail.com>
Co-authored-by: Alejandro Pascual Pozo <alejandro.pascual.pozo@gmail.com>
Co-authored-by: Rob Parrett <robparrett@gmail.com>
Co-authored-by: François <mockersf@gmail.com>
Co-authored-by: Dmytro Banin <banind@cs.washington.edu>
Co-authored-by: James Liu <contact@jamessliu.com>
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
I'm adopting this ~~child~~ PR.

# 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 bevyengine#2192.
- Partial workaround for bevyengine#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.

## 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 the unification of commands and one-shot systems
since they are two different ways to run logic on demand over a World.

### Prior attempts

- bevyengine#2234
- bevyengine#2417
- bevyengine#4090
- bevyengine#7999

This PR continues the work done in
bevyengine#7999.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Federico Rinaldi <gisquerin@gmail.com>
Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>
Co-authored-by: Aevyrie <aevyrie@gmail.com>
Co-authored-by: Alejandro Pascual Pozo <alejandro.pascual.pozo@gmail.com>
Co-authored-by: Rob Parrett <robparrett@gmail.com>
Co-authored-by: François <mockersf@gmail.com>
Co-authored-by: Dmytro Banin <banind@cs.washington.edu>
Co-authored-by: James Liu <contact@jamessliu.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants