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

Decorator examples don't work #193

Open
pvcraven opened this Issue Apr 23, 2018 · 14 comments

Comments

Projects
None yet
3 participants
@pvcraven
Owner

pvcraven commented Apr 23, 2018

Bug Report

Decorators need some help.

@pvcraven pvcraven added the bug label Apr 23, 2018

@pvcraven pvcraven self-assigned this Apr 23, 2018

pvcraven added a commit that referenced this issue Apr 23, 2018

@Mahi

This comment has been minimized.

Mahi commented May 9, 2018

Just curious, is there any need for the decorators in the first place? I mean, is there any advantage over just using the Window class? It seems more "correct" to me anyways, since

@pvcraven

This comment has been minimized.

Owner

pvcraven commented May 9, 2018

Some people prefer a more functional-based approach, rather than classes.

Unfortunately I'm not great at writing decorators yet, so that's something I hope to work on.

@Mahi

This comment has been minimized.

Mahi commented May 11, 2018

Right, I believe I could help with that. What is the desired API for this, you wish to keep the arcade.decorator.something?

@arcade.decorator.setup
def setup_game(window):
    do_stuff()

Not that there's anything wrong with it, I'm just making sure you've thought through it :) Another option would be something like:

@arcade.on_setup
def setup_game(...):
    ...

but I don't know if that's any better.

And is there a reason it's all in a class rather than a module called decorator (in which case the syntax would be the same), do you intent this to support subclassing or something?

@pvcraven

This comment has been minimized.

Owner

pvcraven commented May 11, 2018

I'm at PyCon right now, I'll try to get back to you sooner on on that.

@Mahi

This comment has been minimized.

Mahi commented May 12, 2018

Sure, no rush :)

Yet another idea I came up with is to use a window object with decorators rather than subclassing:

window = arcade.Window(...)

@window.on_setup  # or window.decorator.setup or so
def setup_game():
    do_stuff(window)

@window.on_update
def update_game(delta_time):
    more_stuff(window)

arcade.run()

I feel like this is the best solution, but I'm not sure yet

@Mahi

This comment has been minimized.

Mahi commented May 14, 2018

Implementing the latest suggestion I proposed would be as simple as adding a decorator method to the arcade.application.Window class which shadow the Window.update method with an instance attribute of the same name:

class Window(pyglet.window.Window):
    ...
    def on_update(self, callback):
        self.update = callback
        return callback

window = Window(...)

@window.on_update
def update(delta_time):
    window.do_stuff(delta_time)
@pvcraven

This comment has been minimized.

Owner

pvcraven commented May 15, 2018

I'm kind of hoping to sit and spend some time thinking though this over the summer. I'm not much of a decorator expert, but hopefully with some time I can examine what Pyglet has done and learn more. Then come up with several examples to help prove it out.

@Mahi

This comment has been minimized.

Mahi commented May 15, 2018

I've coded a lot of them so feel free to ask for guidance if you need any, I'm just not sure what would be the ideal API here for users, although I'm highly leaning on using the @window.on_update.

@pauleveritt

This comment has been minimized.

Collaborator

pauleveritt commented May 21, 2018

@Mahi I had a big branch last year where I did some experiments. Here's a prototype. Too ambitious though, I was trying to solve multiple problems at once. I'm still attracted though to the ideas you're discussing.

First, I was trying to get rid of a global window and have the decorator support implicitly create a window, store it inside the decorator, and pass into functions. I wanted a better test writing experience and not have a global window.

Second, and the beginning of the too-ambitious...I was trying to craft a "Game API" that let us sit in between pyglet and a user's game and simplify some things. We are currently bound by pyglet's contract. I'd still like @game.on_update or even @game.some_other_method_completely but I think @pvcraven rightfully wants evolution not revolution.

@Mahi

This comment has been minimized.

Mahi commented May 22, 2018

I've had a look at pyglet since I last posted here, and it seems like pyglet already has a support for both multiple windows and decorators for registering handlers. To me it looks like arcade has explicitly created a limitation of using only one window in the window_commands.py file, so I'd start off by getting rid of this limitation.

@pauleveritt

This comment has been minimized.

Collaborator

pauleveritt commented May 22, 2018

@Mahi When you say limitation, do you mean Arcade should support multiple windows? IMO this is a feature, not a bug...simple games (the Arcade target) don't need to think about which window is being used. But perhaps you meant something else.

@Mahi

This comment has been minimized.

Mahi commented May 22, 2018

@pauleveritt What I'm saying is that pyglet supports multiple windows, and arcade uses pyglet for its windowing, so arcade should support multiple windows. But it doesn't, because the window_commands.py file has explicitly defined a single global window instance. If we get rid of using this global instance, and instead move any required functionality into the Window class, we automatically inherit the multi-window-support from pyglet.

@pvcraven

This comment has been minimized.

Owner

pvcraven commented May 23, 2018

This was debated when I started Arcade. Short answer, using a global keeps the user from having to pass around the window context to every drawing command and everything else. Keeping things simple and easy is an important part of this library. This simplifies the 99.9% (or whatever) of programs that run on one window. It forces multi-window programs to use something else. At this point, I'm ok with that decision.

@Mahi

This comment has been minimized.

Mahi commented May 23, 2018

using a global keeps the user from having to pass around the window context to every drawing command and everything else.

I believe pyglet already takes care of this, it has its own internal global window, which can be changed with the Window.switch_to function. So literally none of the commands would change if we got rid of the global instance.

Edit: As a matter of fact, if we take a look at the viewport functions, we see that the global _window variable is not being used at all. All the other functions seem rather pointless to me, or they're just utility functions and don't really need the global _window anyways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment