Skip to content

Write real docs for SystemSet #19538

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

alice-i-cecile
Copy link
Member

Objective

SystemSets are surprisingly rich and nuanced, but are extremely poorly documented.

Fixes #19536.

Solution

Explain the basic concept of system sets, how to create them, and give some opinionated advice about their more advanced functionality.

Follow-up

I'd like proper module level docs on system ordering that I can link to here, but they don't exist. Punting to follow-up!

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation A-ECS Entities, components, systems, and events X-Uncontroversial This work is generally agreed upon D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 8, 2025
@viridia
Copy link
Contributor

viridia commented Jun 8, 2025

Show an example of adding a system to a system set. (Or link to a place which explains it).

@viridia
Copy link
Contributor

viridia commented Jun 8, 2025

Nit: I would probably want to steer novices in the direction of using app.add_system(MySystem.in_set(MySet)) rather than a custom schedule, which seems like an advanced topic. However, maybe that's intentional.

@alice-i-cecile
Copy link
Member Author

Nit: I would probably want to steer novices in the direction of using app.add_system(MySystem.in_set(MySet)) rather than a custom schedule, which seems like an advanced topic. However, maybe that's intentional.

Me too! Unfortunately, this is in bevy_ecs, which does not rely on bevy_app, so I can't pull in the required types.

@viridia
Copy link
Contributor

viridia commented Jun 8, 2025

Ugh, I have run into that problem as well. Makes it hard to tell a coherent story if you can't talk about the purposes for which a given thing exists.

Copy link
Contributor

@theotherphil theotherphil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One typo, but otherwise LGTM!

Co-authored-by: theotherphil <[email protected]>
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 8, 2025
@alice-i-cecile alice-i-cecile enabled auto-merge June 8, 2025 21:06
@alice-i-cecile alice-i-cecile disabled auto-merge June 8, 2025 21:06
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-Docs An addition or correction to our documentation D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API docs for SystemSet are rather austere
3 participants