-
Notifications
You must be signed in to change notification settings - Fork 193
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
Add on_exit() hook #166
base: main
Are you sure you want to change the base?
Add on_exit() hook #166
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I think my suggestion "on_before_exit()" may have been inspired by "onbeforeunload" in Javascript:
https://developer.mozilla.org/en-US/docs/Web/API/WindowEventHandlers/onbeforeunload
Javascript has "onunload" and "onbeforeunload", with "onbeforeunload" offering the ability to prompt for confirmation. So perhaps I was thinking that on_before_exit()
would offer the ability to cancel.
In any case, that sounds like quite an advanced feature for a Pygame Zero game, and I can think of various use case for on_exit()
as defined, so I'm happy to go with that.
@@ -213,6 +212,55 @@ def get_draw_func(self): | |||
) | |||
return draw | |||
|
|||
def _call_users_on_enter_func(self): | |||
# Calls the user's on_enter function if defined. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like on_enter()
means that "there is more than one way to do it" because we also promote using the module scope to set up the initial game state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think an on_enter()
function is useful for encapsulating start up code, but I understand your point.
Would you like me to remove it?
"""Cleanly exits pgzero and pygame. | ||
|
||
Args: | ||
exit_status (int): Exit status. The default value of 0 indicates |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should bother with exit statuses. What you've done makes perfect sense for a general app development framework, but for Pygame Zero I think this adds unnecessary complexity.
We pay a cost for any additional complexity. It makes the documentation longer and adds terms that beginners may not be familiar with (and are unlikely to need). Here I suspect they will not know what an exit status is, and there are very few cases where it would be useful for them.
There is a very slightly stronger case that you can pair up exit(n)
with on_exit(n)
, so you can signal different exit conditions. But there are other ways to deal with this, eg. setting global flags.
I guess our expectations differ. I expect exit()
to be very rarely used, because most games don't need to manage their own quitting. Meanwhile on_exit()
would be used rarely, eg to save scores or game state.
It looks like this issue has stalled. I've seen someone else asking for this feature on reddit. I don't see much need for a status code for most users. Perhaps there is someone that it may be helpful to, but it think a simple on_exit() would be sufficient. I'm happy to have a go at making the changes and creating a pull-request, if that will help. |
This update adds support for an on_exit() hook.
Overview of changes contained in this pull request:
on_exit()
function which is called after the game loop is exited. This function can be defined with 0 parameters or 1 parameter (error_status).on_enter()
function which is called before the game loop is entered.exit()
function has been changed to use the common exiting path.Notes:
on_exit()
seemed better thanon_before_exit()
.on_enter()
support for symmetry.PGZeroGame
(intest_event_dispatch.py
). If you would like test cases for these changes, I can add them to this pull request or open a new issue to track them.System details (used for testing):
Resolves #51.