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

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

Closed
wants to merge 3 commits into from
Closed

Conversation

giusdp
Copy link
Contributor

@giusdp giusdp commented May 6, 2021

This covers issue #2110

It adds the line .add_system(bevy::input::system::exit_on_esc_system.system()) before .run()
to every example that uses a window, so users have a quick way to close the examples.

I used the full name bevy::input::system::exit_on_esc_system, I thought it gave clarity about being a built-in system.

The examples excluded from the change are the ones in the android, ios, wasm folders, the headless
examples and the ecs/system_sets example because it closes itself.

@cart
Copy link
Member

cart commented May 6, 2021

I personally think we should only add this system to "real games" (currently "breakout" and "alien cake addict"). In general I want the examples to be "minimal / noise free". They exist to show exactly what they set out to and nothing else. "Exit on esc" behavior is in a class of thing that we have intentionally excluded from examples in the past.

@alice-i-cecile alice-i-cecile added C-Examples An addition or correction to our examples C-Code-Quality A section of code that is hard to understand or change labels May 6, 2021
@giusdp
Copy link
Contributor Author

giusdp commented May 6, 2021

Following the discussion about ux I thought it'd be handy to close the examples with esc. When looking at the examples I liked going over them fast to see what they were about and the ESC option would have been nice, but I can see why having it only on the game examples is a good idea.

I'll update the pr removing the line to the non game examples

@cart
Copy link
Member

cart commented May 7, 2021

Just needs a cargo fmt --all and we're good to go!

@giusdp
Copy link
Contributor Author

giusdp commented May 7, 2021

I ran it and it fixed a file, thanks for mentioning it

@alice-i-cecile
Copy link
Member

I've added this to #2094 as well, so we should be able to merge this, then my PR and let my PR take priority in any merge conflicts.

@cart
Copy link
Member

cart commented May 14, 2021

bors r+

bors bot pushed a commit that referenced this pull request May 14, 2021
This covers issue #2110

It adds the line `.add_system(bevy::input::system::exit_on_esc_system.system())` before `.run()`
to every example that uses a window, so users have a quick way to close the examples.

I used the full name `bevy::input::system::exit_on_esc_system`, I thought it gave clarity about being a built-in system.

The examples excluded from the change are the ones in the android, ios, wasm folders, the headless 
examples and the ecs/system_sets example because it closes itself.
@bors bors bot changed the title Add exit_on_esc_system to examples with window [Merged by Bors] - Add exit_on_esc_system to examples with window May 14, 2021
@bors bors bot closed this May 14, 2021
ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
This covers issue bevyengine#2110

It adds the line `.add_system(bevy::input::system::exit_on_esc_system.system())` before `.run()`
to every example that uses a window, so users have a quick way to close the examples.

I used the full name `bevy::input::system::exit_on_esc_system`, I thought it gave clarity about being a built-in system.

The examples excluded from the change are the ones in the android, ios, wasm folders, the headless 
examples and the ecs/system_sets example because it closes itself.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Code-Quality A section of code that is hard to understand or change C-Examples An addition or correction to our examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants