Improve code quality of Breakout game example #2094

alice-i-cecile · 2021-05-04T07:08:44Z

Functionality should be nearly an exact match to the old feature. @rhulha, take a look :) This PR was inspired by bevyengine/bevy-website#102, and should make this example much more suitable as a tutorial, whether that's part of the Bevy book or not.

Main improvements:

Better comments
Better practices for magic numbers, which are mostly contained to a config module to allow for easier twiddling and better comprehensibility.
Smaller, more comprehensible systems
Idiomatic use of marker components to make the example easier to extend
A single kinematics system that operates on everything with a velocity
Always use the fixed time step const, rather than sometimes using delta_time
Removed confusing early break in collision system
Demonstrates nested bundles
Much less confusing wall spawning logic by breaking it down into simple parts
Removed redundant add_system(my_system.system()) system naming convention in favor of more descriptive names ;) (Cart feel free to revert this but it will still make me sad)

Working around bitshifter/glam-rs#173 :(

Reduces over-penetration risk

rhulha · 2021-05-05T10:05:37Z

@rhulha, take a look :)

Great effort, unfortunately I am a bit stretched for time this week and won't be able to spend the time on it that it deserves. :-(

examples/game/breakout.rs

@kiiya

Credit to @kiiya for the idea!

alice-i-cecile · 2021-05-08T22:22:21Z

@Kiiyya thanks for the idea on comments for the system ordering! That improves the clarity of the code a lot, and should also be educational. See: 46294c1

alice-i-cecile · 2021-05-09T23:24:20Z

Alright, I think this is ready for final review now :)

alice-i-cecile · 2021-06-16T21:56:08Z

examples/game/breakout.rs

    }
+
+    velocity.x = direction * paddle.speed;


Per #2349, should this be scaled by delta time?

We are moving our other objects on "fixed updates" which means they update X times within the bounds of this frame's "real delta time" and leftover accumulated time from previous frames. Therefore if we move this on "real" delta time, we might "overstep" relative to the other things. I personally think this should also move using the fixed update delta / be a part of the fixed update system set. Its ok to move more than once with the same input (think of this as an input sampling resolution rounding issue), but we do have both the issue of "dropped inputs" (if we had zero fixed updates this frame) and "just_pressed" not getting reset (if we had >1 fixed_updates this frame), which might result in logic repeating that wasn't intended to repeat.

I think this might be something that we need to handle better in-engine.

alice-i-cecile · 2022-03-17T20:41:22Z

Closing this out in favor of several scoped PRs.

# Objective - The Breakout example has a lot of configurable constant values for setup, but these are buried in the source code. - Magic numbers scattered in the source code are hard to follow. - Providing constants up front makes tweaking examples very approachable. ## Solution - Move magic numbers into constants ## Context Part of the changes made in #2094; split out for easier review.

# Objective - The components in the Breakout game are defined in a strange fashion. - Components should decouple behavior wherever possible. - Systems should be as general as possible, to make extending behavior easier. - Marker components are idiomatic and useful, but marker components and query filters were not used. - The existing design makes it challenging for beginners to extend the example into a high-quality game. ## Solution - Refactor component definitions in the Breakout example to reflect principles above. ## Context A small portion of the changes made in #2094. Interacts with changes in #4255; merge conflicts will have to be resolved.

# Objective - The Breakout example has a lot of configurable constant values for setup, but these are buried in the source code. - Magic numbers scattered in the source code are hard to follow. - Providing constants up front makes tweaking examples very approachable. ## Solution - Move magic numbers into constants ## Context Part of the changes made in bevyengine#2094; split out for easier review.

…engine#4261) # Objective - The components in the Breakout game are defined in a strange fashion. - Components should decouple behavior wherever possible. - Systems should be as general as possible, to make extending behavior easier. - Marker components are idiomatic and useful, but marker components and query filters were not used. - The existing design makes it challenging for beginners to extend the example into a high-quality game. ## Solution - Refactor component definitions in the Breakout example to reflect principles above. ## Context A small portion of the changes made in bevyengine#2094. Interacts with changes in bevyengine#4255; merge conflicts will have to be resolved.

# Objective - The Breakout example has a lot of configurable constant values for setup, but these are buried in the source code. - Magic numbers scattered in the source code are hard to follow. - Providing constants up front makes tweaking examples very approachable. ## Solution - Move magic numbers into constants ## Context Part of the changes made in bevyengine#2094; split out for easier review.

…engine#4261) # Objective - The components in the Breakout game are defined in a strange fashion. - Components should decouple behavior wherever possible. - Systems should be as general as possible, to make extending behavior easier. - Marker components are idiomatic and useful, but marker components and query filters were not used. - The existing design makes it challenging for beginners to extend the example into a high-quality game. ## Solution - Refactor component definitions in the Breakout example to reflect principles above. ## Context A small portion of the changes made in bevyengine#2094. Interacts with changes in bevyengine#4255; merge conflicts will have to be resolved.

alice-i-cecile added 28 commits May 3, 2021 20:29

Cleaned up time step by moving to a separate config

cad807b

Organize resources and components into modules

168f33f

Move background color to configurable const

930f6cf

Use init_resource for the Scoreboard

9fbe206

Split setup into many small startup systems

8de74a8

Added marker components

858a7ab

Clarity

bdd9c9c

Refactored Wall code to reduce mess

1d82952

Cleaned up bricks logic

e99feda

More color constants

1193669

Extract Paddle and Ball magic numbers

eb8c8f0

Extract magic numbers from Scoreboard

57976a6

Added kinematics system

8b154b5

Cleaned up ball and paddle movement

2c99de0

Clean up scoreboard_system

88e0eac

Add starting velocity to ball

3e864a6

Rename add_* startup systems to spawn_*

6835d60

Reduced system naming redundancy

120bbdf

Cleaned up collision function

9112742

Respect ownership rules...

4768e9b

Moved many consts to local variables

803c0e7

Working around bitshifter/glam-rs#173 :(

Add missing marker component

400f3a3

Improve comments

3d009c2

Removed magic number

6e37cf4

Fixed paddle input

be14d7a

Collision should always run before kinematics

76c6fba

Reduces over-penetration risk

Fixed walls

c970378

Point to non-duplicate issue

658a8c6

alice-i-cecile added C-Code-Quality A section of code that is hard to understand or change C-Examples An addition or correction to our examples labels May 4, 2021

Satisfy clippy by removing redundant clone

f40c69d

B-Reif mentioned this pull request May 5, 2021

Added a detailed tutorial for the breakout game to the Bevy book. bevyengine/bevy-website#102

Closed

alice-i-cecile commented May 8, 2021

View reviewed changes

examples/game/breakout.rs Outdated Show resolved Hide resolved

alice-i-cecile commented May 8, 2021

View reviewed changes

examples/game/breakout.rs Outdated Show resolved Hide resolved

alice-i-cecile commented May 8, 2021

View reviewed changes

examples/game/breakout.rs Show resolved Hide resolved

Add comments explaining system order rationale

46294c1

Credit to @kiiya for the idea!

Added exit_on_esc_system

bd73201

alice-i-cecile mentioned this pull request May 11, 2021

[Merged by Bors] - Add exit_on_esc_system to examples with window #2121

Closed

This was referenced May 21, 2021

More ergonomic asset loading to avoid "check if asset is loaded" boilerplate #1701

Open

Tracking issue: new Bevy book bevyengine/bevy-website#158

Closed

alice-i-cecile mentioned this pull request Jun 15, 2021

Breakout Example is stuttering #2349

Open

alice-i-cecile commented Jun 16, 2021

View reviewed changes

alice-i-cecile added 2 commits June 17, 2021 18:34

Add delta_time scaling to paddle input

d33564e

Add paddle_input to fixed time step system set

1be6794

cart added the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Jul 23, 2021

mockersf removed the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Jul 24, 2021

alice-i-cecile mentioned this pull request Oct 9, 2021

Add main menu and game over menu to breakout example #2927

Closed

alice-i-cecile closed this Mar 17, 2022

This was referenced Mar 19, 2022

[Merged by Bors] - Move magic numbers into constants in Breakout example #4255

Closed

[Merged by Bors] - Break out Breakout components into a more sensible organization #4261

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve code quality of Breakout game example #2094

Improve code quality of Breakout game example #2094

alice-i-cecile commented May 4, 2021

rhulha commented May 5, 2021 •

edited

Loading

alice-i-cecile commented May 8, 2021

alice-i-cecile commented May 9, 2021

alice-i-cecile Jun 16, 2021

cart Jun 17, 2021

alice-i-cecile commented Mar 17, 2022

Improve code quality of Breakout game example #2094

Improve code quality of Breakout game example #2094

Conversation

alice-i-cecile commented May 4, 2021

rhulha commented May 5, 2021 • edited Loading

alice-i-cecile commented May 8, 2021

alice-i-cecile commented May 9, 2021

alice-i-cecile Jun 16, 2021

Choose a reason for hiding this comment

cart Jun 17, 2021

Choose a reason for hiding this comment

alice-i-cecile commented Mar 17, 2022

rhulha commented May 5, 2021 •

edited

Loading