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

Fix bevy_ecs README.md #3279

Closed
wants to merge 1 commit into from
Closed

Conversation

redwarp
Copy link
Contributor

@redwarp redwarp commented Dec 8, 2021

Objective

  • The README.md for bevy_ecs contains an example for building a system that would not work as is. This change addresses that.

Solution

  • For using iter_mut(), the query parameter in the system function needs to be mut as well.
  • For adding a fun as a system, system() needs to be called.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Dec 8, 2021
@@ -131,7 +131,7 @@ fn main() {
// Add a Stage to our schedule. Each Stage in a schedule runs all of its systems
// before moving on to the next Stage
schedule.add_stage("update", SystemStage::parallel()
.with_system(movement)
.with_system(movement.system())
Copy link
Member

Choose a reason for hiding this comment

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

.system() is not required on main.

@@ -109,7 +109,7 @@ struct Position {
}

// This system moves each entity with a Position and Velocity component
fn movement(query: Query<(&mut Position, &Velocity)>) {
fn movement(mut query: Query<(&mut Position, &Velocity)>) {
Copy link
Member

Choose a reason for hiding this comment

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

This is correct; good catch.

@@ -131,7 +131,7 @@ fn main() {
// Add a Stage to our schedule. Each Stage in a schedule runs all of its systems
// before moving on to the next Stage
schedule.add_stage("update", SystemStage::parallel()
.with_system(movement)
.with_system(movement.system())
Copy link
Member

Choose a reason for hiding this comment

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

No longer needed on main, see #2398

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Docs An addition or correction to our documentation and removed S-Needs-Triage This issue needs to be labelled labels Dec 8, 2021
@DJMcNab
Copy link
Member

DJMcNab commented Dec 8, 2021

Duplicate of #2575, #3083, #3100.

See also #2398, unless you're claiming it regressed.

@cart
Copy link
Member

cart commented Dec 8, 2021

Yup adding .system() isn't necessary, but adding mut to the query variable is still needed, so we should merge that.

@redwarp
Copy link
Contributor Author

redwarp commented Dec 8, 2021

Ah, I see, and as I'm using bevy_ecs 0.5, the patch from june is not live yet, gotcha. Will amend to just show the mut then.

@DJMcNab
Copy link
Member

DJMcNab commented Dec 8, 2021

Indeed, although that fix is already in #2575

@redwarp
Copy link
Contributor Author

redwarp commented Dec 8, 2021

@DJMcNab indeed, closing it then.

@redwarp redwarp closed this Dec 8, 2021
@redwarp redwarp deleted the bevy_ecs_doc_fix branch December 8, 2021 21:49
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants