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

Replace Sets with Schedules / unify adding sets to schedules #8353

Open
vonforum opened this issue Apr 11, 2023 · 1 comment
Open

Replace Sets with Schedules / unify adding sets to schedules #8353

vonforum opened this issue Apr 11, 2023 · 1 comment
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Controversial There is active debate or serious implications around merging this PR

Comments

@vonforum
Copy link

vonforum commented Apr 11, 2023

What problem does this solve or what need does it fill?

After #8079, adding systems to schedules is unified under a single API, which is great. Configuring sets, however, still has multiple ways of doing it. Plus, systems belong in multiple different "types" of "collections" (schedules and sets). This was the case before as well, with systems having to belong in a CoreSet plus the main schedule, but now the core sets have been replaced with separate schedules.

What solution would you like?

Do the same for all notions of "sets", i.e. unify sets and schedules. Since systems have to belong to a schedule anyway, I propose replacing sets with nested schedules. Instead of configuring a set as you would now, you instead can configure schedules and nest them. In function, this would be the same as the current way of configuring a tree of sets and adding your systems to those, inside a schedule. In addition to making the API smaller and cleaner, it would simplify code for structuring your systems. Currently, if you want to create a system in a set, you have to annotate every system with the set it's in, in addition to its schedule

app.configure_sets(PreUpdate, (A, B).chain());

app.add_systems(
  PreUpdate,
  (
    a.in_set(A),
    b.in_set(B)
  )
);

However, after this, you could add directly to your nested schedule (just an example)

app.add_schedules(PreUpdate, (A, B).chain());

app.add_systems(A, a);
app.add_systems(B, b);

This also removes the need of having to "know" what schedules each set belongs to if you want to add your system to it, e.g. in plugins.

What alternative(s) have you considered?

Just configure the configure_sets function to also take a single set, to at least be consistent in not having many functions do the same thing.

@vonforum vonforum added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Apr 11, 2023
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Controversial There is active debate or serious implications around merging this PR and removed C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Apr 12, 2023
@mscofield0
Copy link

Has anyone done anything with regards to this and #9792?

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-Usability A targeted quality-of-life change that makes Bevy easier to use X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

No branches or pull requests

3 participants