Skip to content
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

Required ATL transform's keyword-only parameter can't be curryfied #4371

Closed
Tracked by #5102
Gouvernathor opened this issue Feb 20, 2023 · 3 comments
Closed
Tracked by #5102
Assignees
Labels
crash or exception Issues causing renpy to quit unexpectedly, or to show the grey error screen. unintended behavior Something not working the way it's supposed to, without causing a crash or exception

Comments

@Gouvernathor
Copy link
Member

transform al(child, b, *, c, d=5):
    pass

al(b=5) works, and returns a (sort-of) curryfied version of al (sort-of because al(b=5)(b=3) works)
al(d=6) works and does (sort-of) the same
al(c=5) fails, for no apparent reason.

This is because the related section of atl.py was written when defaultless (=required) keyword-only arguments weren't a thing, and it wasn't updated in 13 years when Andy upgraded the parameters syntax to py3 standards.

This should/will be fixed when upgrading the parameters system to using the inspect module.

Also, atl doesn't take *args or **kwargs (the parser checks against it), and I'm not sure why.

@Gouvernathor Gouvernathor added the enhancement A proposed or requested enhancement or improvement to renpy's features. label Feb 20, 2023
@Gouvernathor Gouvernathor self-assigned this Feb 20, 2023
@Gouvernathor
Copy link
Member Author

Gouvernathor commented Feb 20, 2023

Unbound required keyword-only parameters are also forgotten when currying the transform : al(b=7) has an equivalent signature of (child, b=7, d=6). Not only has the star been omitted (d is now positional-or-keyword, instead of keyword-only), but c has simply vanished.

Gouvernathor added a commit that referenced this issue Feb 20, 2023
numerous improvements
fixes #4371
keyword-only parameters are now actually supported in ATL
@Gouvernathor
Copy link
Member Author

Gouvernathor commented Feb 21, 2023

pos-only arguments (placed before a slash in the signature) are parsed and compiled correctly, but are treated as pos-or-keyword by the atl code and turned into that in the curryfied version. For transform t(a, /, b), t(a=1) and t(b=5)(a=1) will work when they're supposed to fail.

@Gouvernathor
Copy link
Member Author

Maybe this deserves a separate issue, because it has to do with child is managed.

transform h(a, child, b=5):
    pass

h(child=Null()).child is None. The Null() has been passed to the child variable in the ATL block's namespace, but it was not set to be the child attribute of the resulting transform (it or a duplicated version, whatever).
One could argue that it makes sense, if the creator added a child parameter, to trust them with it hands-off. I'd note that when calling h(1, Null()) the attribute does get set, so it's still a discrepancy, but ok. What's actually a problem, is that it breaks At.
At(Null(), h) returns that same child-less transform. chil != At(chil, t).child should be an invariant, and here that invariant is broken.

@Gouvernathor Gouvernathor added unintended behavior Something not working the way it's supposed to, without causing a crash or exception crash or exception Issues causing renpy to quit unexpectedly, or to show the grey error screen. and removed enhancement A proposed or requested enhancement or improvement to renpy's features. labels Jun 30, 2023
renpytom added a commit that referenced this issue Jun 26, 2024
Adresses most of the main points of #5102, and #4371.
The parameters that get fixed are still forbidden though, as are var-pos and var-kw parameters. That will be for another PR. But the implementation that goes with handling those is already here, commented-out for performance.

The solution to #4405 is there too but also commented out. That change will also go in separately.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crash or exception Issues causing renpy to quit unexpectedly, or to show the grey error screen. unintended behavior Something not working the way it's supposed to, without causing a crash or exception
Projects
None yet
Development

No branches or pull requests

1 participant