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
Class decorator version of renpy.register_statement #4846
base: master
Are you sure you want to change the base?
Conversation
Unless I add deco or decorator as a :doc: option, decorators can't be documented by docstring
Wouldn't it make more sense to use an ABC with properly defined signature for this? That way you don't have to look up method names. Also, while making a use out of magic methods looks interesting, it's not as obvious and goes against |
I don't see what the advantages of an ABC would be. What would be the defined signatures, The class attribute/method names corresponding to the function's parameter seems obvious enough to me as a way to do it. As for the "only one" part, Python rarely even respects it itself. It's a bridge between allowing optimization for the class context, since using the constructor directly rather than a classmethod constructor saves a call but you can also make the class method return a different object if you want. As for the name, you need to have the class attribute way if you want to add statements with spaces in their names, yet if you don't it's nice not to have to set it manually. And execute, well |
You'd know that it has to have Using If renpy actually constructed the objects, then I'd understand why you'd use the magic methods. |
The methods do what you tell them to. If you add a parse method (class or static, as I said in the doc), you may make it return an instance of the class, or not. In that case, Ren'Py doesn't create the object, you do. And if you don't add a parse method, the class constructor will be used which, by definition, returns an instance of the class. I believe you can register a statement without execute, if only because of the execute_init, execute_default and label methods. You can also make a statement like |
Is this worth adding to the core API? It's pretty trivial (and a good deal less "magic") for a creator to do this themselves if they want to use a class; directly passing the methods to register_statement, or should they really wish to, with their own decorator. The docs too already have an opinion on how to do this which works whether a creator is using functions or class methods, so I'm not sure this is worth the potential confusion. |
I think there shouldn't be much confusion in that defining CDSs using a class seems like a much simpler recipe, in the general case, than creating a bunch of functions. So, we could have that as the primary way of creating CDS, and have register_statement as the "manual" low-level way. It may make CDS easier to make and understand for creators, with a redesign of the documentation for both functions by putting the decorator first and the function second - taking inspiration from Boop's idea of an abstract base class, in a way, documentation wise. But it's also a very thin wrapper which only relies on documented things, so it may leave us with a maintenance burden for no major benefit. And it may also well live in a cookbook, as an add-on shared alongside renpy. I intended for it to be viable as that as well. |
no need to edit the doc generation script actually
I'm not sure encouraging use of a class for this is a good idea. The reason to choose a class over a module is to retain and share state, something that these functions should not do. |
There is some code duplication as it comes to the accepted parameters to register_statement, because the inspect module does that in py3 only. I suggest using it as soon as we pass to py3-only (the code has been left commented-out), or to wait until then for merging this.
The doc entry looks like this