Automatically close game when Window close signal is sent

[Xaeroxe]

Slight ergonomics improvement, helps the engine behave in a more obvious manner.

[jojolepro]

1. You could remove the "leave on Escape"
   but
2. You would lose the only examples where you use a key event from a state.

So its fine like that for me.

[torkleyy]

I'm not sure if it's a good thing. Sometimes more explicitness and less magic is better.

[torkleyy]

So how could the state handle a window close event now?

[Xaeroxe]

I could rearrange this so that the state receives this. It just seems odd for our game window to by default not respond to a close event. It confused me for a second when it happened so I'm sure it'd confuse someone not familiar with the engine.

Also it's worth noting that if you want this to be handled properly at all times you have to duplicate the close code for every state your game goes through. Most of our examples have just one state so this isn't as apparent.

[torkleyy]

I see it makes sense, but at least giving the game a notification would be great. Maybe also just call a method which is always when states are stopped? Not sure if we have that already.

[Xaeroxe]

@torkleyy r?

[Xaeroxe]

@torkleyy r?

[Rhuagh]

This removes the ability to have multiple windows. Might not be used a lot but it is possible to do.

[Rhuagh]

This removes the ability to have multiple windows. Might not be used a lot but it is possible to do.

[Xaeroxe]

Huh, does our renderer even support that? An overwhelming volume of the code I've been writing assumes single window. How would we even go about telling just one window to close?

[Xaeroxe]

Huh, does our renderer even support that? An overwhelming volume of the code I've been writing assumes single window. How would we even go about telling just one window to close?

[Rhuagh]

I realize we don't have full support for that yet, but the renderer support it

[Rhuagh]

I realize we don't have full support for that yet, but the renderer support it

[Xaeroxe]

Can we keep the simple solution for now then and address that shortcoming in a larger "multi-window" PR then? As it stands I'm not even sure how I'm supposed to close just one window

[Xaeroxe]

Can we keep the simple solution for now then and address that shortcoming in a larger "multi-window" PR then? As it stands I'm not even sure how I'm supposed to close just one window

[Rhuagh]

There's WindowId in winit but yeah, we can do that in a future PR.

[Rhuagh]

There's WindowId in winit but yeah, we can do that in a future PR.

[Rhuagh]

I just thought of a second issue, what if the user wants to perform some action on exit? Save data to disc or whatnot?

[Rhuagh]

I just thought of a second issue, what if the user wants to perform some action on exit? Save data to disc or whatnot?

[Xaeroxe]

@Rhuagh Then they can use the Window::Closed event they receive just before shutdown.

[Xaeroxe]

@Rhuagh Then they can use the Window::Closed event they receive just before shutdown.

[Rhuagh]

Ah, it still pushes it to the state. Ok then.

[Rhuagh]

Ah, it still pushes it to the state. Ok then.

[omni-viral]

Then they can use the Window::Closed event

But this event is Window::Closed and not Engine::Shutdown.
Explicit is better then implicit.
I'm for keeping manual shutdown.

[Xaeroxe]

@omni-viral From an earlier conversation with @torkleyy

I could rearrange this so that the state receives this. It just seems odd for our game window to by default not respond to a close event. It confused me for a second when it happened so I'm sure it'd confuse someone not familiar with the engine.
Also it's worth noting that if you want this to be handled properly at all times you have to duplicate the close code for every state your game goes through. Most of our examples have just one state so this isn't as apparent.

In addition to this all of the states' on_stop function will be called.

If it absolutely must be called Engine::Shutdown I can do that but it would require a whole new event hierarchy to go along with it.

[Xaeroxe]

@omni-viral From an earlier conversation with @torkleyy

I could rearrange this so that the state receives this. It just seems odd for our game window to by default not respond to a close event. It confused me for a second when it happened so I'm sure it'd confuse someone not familiar with the engine.
Also it's worth noting that if you want this to be handled properly at all times you have to duplicate the close code for every state your game goes through. Most of our examples have just one state so this isn't as apparent.

In addition to this all of the states' on_stop function will be called.

If it absolutely must be called Engine::Shutdown I can do that but it would require a whole new event hierarchy to go along with it.

[omni-viral]

@Xaeroxe I just don't like implicit stuff. Also it limits user to do something after window closes (including continue working with other windows left and nontrivial saving that will require more then few milliseconds)

[omni-viral]

@Xaeroxe I just don't like implicit stuff. Also it limits user to do something after window closes (including continue working with other windows left and nontrivial saving that will require more then few milliseconds)

[jojolepro]

On application creation, just have a setting "QuitOnWindowClosed=bool"
At the same place where you specify resolution and window name.

[jojolepro]

On application creation, just have a setting "QuitOnWindowClosed=bool"
At the same place where you specify resolution and window name.

[Xaeroxe]

Not closing when asked is verging on malware-like behavior. I don't feel making this implicit isn't that unreasonable. The game won't close until all states are finished shutting down anyways.

[Xaeroxe]

Not closing when asked is verging on malware-like behavior. I don't feel making this implicit isn't that unreasonable. The game won't close until all states are finished shutting down anyways.

[Xaeroxe]

I'd also like bring emphasis to this part once again: you have to duplicate the close code for every state your game goes through under our current setup.

[Xaeroxe]

I'd also like bring emphasis to this part once again: you have to duplicate the close code for every state your game goes through under our current setup.

[Xaeroxe]

On application creation, just have a setting "QuitOnWindowClosed=bool"
At the same place where you specify resolution and window name.

I like this idea, so I'll implement it.

[Xaeroxe]

On application creation, just have a setting "QuitOnWindowClosed=bool"
At the same place where you specify resolution and window name.

I like this idea, so I'll implement it.

[Rhuagh]

🎉

[Rhuagh]

🎉

[Xaeroxe]

@omni-viral r?

[Xaeroxe]

@omni-viral r?

[jojolepro]

Could that be in the configs instead?

[jojolepro]

https://github.com/amethyst/amethyst/blob/develop/examples/04_pong/resources/display.ron
Since its related to the rendering/windowing system, it would make more sense if it was an optional value in there.

[jojolepro]

https://github.com/amethyst/amethyst/blob/develop/examples/04_pong/resources/display.ron
Since its related to the rendering/windowing system, it would make more sense if it was an optional value in there.

[Xaeroxe]

@jojolepro Having that in configs exposed to the user doesn't quite seem right either. This should be set by the developer.

[Xaeroxe]

@jojolepro Having that in configs exposed to the user doesn't quite seem right either. This should be set by the developer.

[jojolepro]

optionally?
I think its possible to have default values with serde in a struct that gets deserialized?

[jojolepro]

optionally?
I think its possible to have default values with serde in a struct that gets deserialized?

[Xaeroxe]

When you save a new config it will still write the default value.

[Xaeroxe]

When you save a new config it will still write the default value.

[jojolepro]

Ah true, you win! 🎉

[jojolepro]

Ah true, you win! 🎉

[omni-viral]

@Xaeroxe you can make field to be skipped during serialization if it's equal to default.
There is a ton of programs that don't even close window when X pressed. Most of them ask about saving modifications. Others just hide.
But amethyst is primarily for games and they usually quit when window is closed. Therefore I approve this as you've made it optional

[omni-viral]

@Xaeroxe you can make field to be skipped during serialization if it's equal to default.
There is a ton of programs that don't even close window when X pressed. Most of them ask about saving modifications. Others just hide.
But amethyst is primarily for games and they usually quit when window is closed. Therefore I approve this as you've made it optional

[Rhuagh]

bors r+

[Rhuagh]

bors r+

[bors]

Build succeeded

• continuous-integration/appveyor/branch
• continuous-integration/travis-ci/push

[bors]

Build succeeded

• continuous-integration/appveyor/branch
• continuous-integration/travis-ci/push