Skip to content

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 <contact@jamessliu.com>
@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