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

Use chain in breakout example #10124

Merged
merged 2 commits into from
Oct 16, 2023
Merged

Conversation

hymm
Copy link
Contributor

@hymm hymm commented Oct 15, 2023

Objective

  • We should encourage of the simpler to reason about chain.

Solution

  • Use it in the breakout example

Changelog

  • Switch breakout to use chain instead of before and after

Copy link
Contributor

@IceSentry IceSentry left a comment

Choose a reason for hiding this comment

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

I'm not sure if this example needs the comments but the change is definitely worth doing either way

@mgi388
Copy link
Contributor

mgi388 commented Oct 15, 2023

I'm not sure if this example needs the comments but the change is definitely worth doing either way

I’m new to Bevy and the comments in examples help so I would be for it.

@IceSentry
Copy link
Contributor

My point was more that these things should be taught somewhere else like the ecs_guide example. We can't teach the concept of .chain() or every other bevy feature in every example that uses it.

@mgi388
Copy link
Contributor

mgi388 commented Oct 15, 2023

My point was more that these things should be taught somewhere else like the ecs_guide example. We can't teach the concept of .chain() or every other bevy feature in every example that uses it.

In fact (as you probably already knew) that example already talks about chain so seems unnecessary for this PR then doesn’t it. Thanks for entertaining my comment.

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

On board with the move to .chain above: that logical ordering is correct and it's a nice clean way to express it. I think we should cut the second comment though.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Examples An addition or correction to our examples C-Code-Quality A section of code that is hard to understand or change labels Oct 16, 2023
@mockersf mockersf 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 Oct 16, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Oct 16, 2023
Merged via the queue into bevyengine:main with commit 88599d7 Oct 16, 2023
22 checks passed
@hymm hymm deleted the breakout-chain branch October 21, 2023 17:39
ameknite pushed a commit to ameknite/bevy that referenced this pull request Nov 6, 2023
# Objective

- We should encourage of the simpler to reason about chain.

## Solution

- Use it in the breakout example

---

## Changelog

- Switch breakout to use `chain` instead of `before` and `after`
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
# Objective

- We should encourage of the simpler to reason about chain.

## Solution

- Use it in the breakout example

---

## Changelog

- Switch breakout to use `chain` instead of `before` and `after`
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-Code-Quality A section of code that is hard to understand or change C-Examples An addition or correction to our examples 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.

6 participants