Skip to content

Examples broken call to View.close on ESC #2538

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

Closed
cdeil opened this issue Feb 1, 2025 · 7 comments
Closed

Examples broken call to View.close on ESC #2538

cdeil opened this issue Feb 1, 2025 · 7 comments

Comments

@cdeil
Copy link
Contributor

cdeil commented Feb 1, 2025

There's a few examples that call View.close on ESC key.

Here:

# Close the window / exit game
elif key == arcade.key.ESCAPE:
self.close()

Here:

# Quit if the player hits escape
elif symbol == arcade.key.ESCAPE:
self.close()

Here, but it's dead code since ESC is tested twice in elif statements and only the first is executed:

elif key == arcade.key.ESCAPE:
self.player.start_pressed = True
# close the window if the user hits the escape key
elif key == arcade.key.ESCAPE:
self.close()

Here:

if key == arcade.key.ESCAPE:
self.close()

I presume this is a recent API change, but I don't see it mentioned here!?

https://github.com/pythonarcade/arcade/blob/development/CHANGELOG.md#window-and-view

What is the proper fix / coding pattern here to process ESC when sub-classing arcade.View?

I can send a PR if you let me know.

@einarf
Copy link
Member

einarf commented Feb 1, 2025

Nice catch.

This is just lingering code after many examples were converted into Views because we try to promote them over subclassing the window itself for more flexibility. We could change it to call self.window.close() or actually make a close() wrapper function in the View that calls self.window.close() to make it simpler for users.

@einarf
Copy link
Member

einarf commented Feb 1, 2025

Be free to make a PR :)

@pushfoo
Copy link
Member

pushfoo commented Feb 1, 2025

We may want to look at some proxying mechanism in classes + sphinx in the future. For the time being, the simplest solution's probably the best.

@einarf
Copy link
Member

einarf commented Feb 2, 2025

For now we should use arcade.close_window() or self.window.close() in views

@einarf
Copy link
Member

einarf commented Feb 2, 2025

Fixed in the referenced PR

@einarf einarf closed this as completed Feb 2, 2025
@cdeil
Copy link
Contributor Author

cdeil commented Feb 2, 2025

This is just lingering code after many examples were converted into Views because we try to promote them over subclassing the window itself for more flexibility. We could change it to call self.window.close()

Ah, good to know that there's View.window :-)

For newcomers like me it's difficult to understand the coding patterns when or why to subclass Window or View or when to create extra classes and how they relate. If any experienced dev has capacity to write a docs page on that I think that would be very useful. I plan to teach a 16 year old Python including OOP using Arcade soon, but to be honest I still haven't figured out what good practices are where to introduce more or less classes.

I did find https://youtu.be/7TdbpIOfosQ?si=JKeoMyNsAYjTR6uV yesterday which explains a coding pattern of having a separate Game class from Window when using pure pyglet. Would be great to have something similar as docs page and/or video for Arcade. The docstring of arcade.View mentions that it's good to have multiple View classes to represent different "screens" like intro/main/game over screens. But then I think the examples I saw so far don't follow that recommendation!?

Fixed in the referenced PR

There's still a bug in this one:

elif key == arcade.key.ESCAPE:
self.player.start_pressed = True
# close the window if the user hits the escape key
elif key == arcade.key.ESCAPE:
self.window.close()

There's two elif testing for ESC and the second one closing the window will never be reached (see https://docs.python.org/3/reference/compound_stmts.html#the-if-statement). For the first condition the game over just flashes and then the game re-starts. Is that intended or should it display and then the game should re-start on some key press?

I'll re-open the ticket. Apologies for not simply sending a PR - as mentioned it's not clear to me what a good solution here is. Introduce a second different key or change the elif so that pressing ESC twice will close the window?

@einarf
Copy link
Member

einarf commented Feb 2, 2025

Fixed the double ESC check and created this #2549

We definitely need better docs. We restructured the entire thing in 3.0.0 and now we can keep improving the content.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants