Skip to content

[Merged by Bors] - Revise docs for system set marker traits #7882

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

Closed
wants to merge 5 commits into from

Conversation

joseph-gio
Copy link
Member

@joseph-gio joseph-gio commented Mar 3, 2023

Objective

#7863 introduced a potential footgun. When trying to incorrectly add a user-defined type using in_base_set, the compiler will suggest that the user implement BaseSystemSet for their type. This is a reasonable-sounding suggestion, however this is not the correct way to make a base set, and will lead to a confusing panic message when a marker trait is implemented for the wrong type.

Solution

Rewrite the documentation for these traits, making it more clear that BaseSystemSet is a marker for types that are already base sets, and not a way to define a base set.

@joseph-gio joseph-gio added C-Docs An addition or correction to our documentation A-ECS Entities, components, systems, and events labels Mar 3, 2023
@alice-i-cecile alice-i-cecile added this to the 0.10 milestone Mar 3, 2023
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Mar 3, 2023
@alice-i-cecile
Copy link
Member

I'm comfortable merging as trivial, but if others have opinions on doc wording go ahead.

Co-authored-by: James Liu <[email protected]>
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Mar 3, 2023
# Objective

#7863 introduced a potential footgun. When trying to incorrectly add a user-defined type using `in_base_set`, the compiler will suggest that the user implement `BaseSystemSet` for their type. This is a reasonable-sounding suggestion, however this is not the correct way to make a base set, and will lead to a confusing panic message when a marker trait is implemented for the wrong type.

## Solution

Rewrite the documentation for these traits, making it more clear that `BaseSystemSet` is a marker for types that are already base sets, and not a way to define a base set.
@bors bors bot changed the title Revise docs for system set marker traits [Merged by Bors] - Revise docs for system set marker traits Mar 3, 2023
@bors bors bot closed this Mar 3, 2023
@joseph-gio joseph-gio deleted the base-set-trait-docs branch March 3, 2023 15:10
ProfLander pushed a commit to shfty-rust/bevy that referenced this pull request Mar 19, 2023
# Objective

bevyengine#7863 introduced a potential footgun. When trying to incorrectly add a user-defined type using `in_base_set`, the compiler will suggest that the user implement `BaseSystemSet` for their type. This is a reasonable-sounding suggestion, however this is not the correct way to make a base set, and will lead to a confusing panic message when a marker trait is implemented for the wrong type.

## Solution

Rewrite the documentation for these traits, making it more clear that `BaseSystemSet` is a marker for types that are already base sets, and not a way to define a base set.
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 S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants