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

Tracking issue for removal of ren'py 6/7 backwards compatibility in the ren'py 8 / python 3 move. #185

Open
8 of 13 tasks
CensoredUsername opened this issue Feb 20, 2024 · 24 comments
Labels

Comments

@CensoredUsername
Copy link
Owner

CensoredUsername commented Feb 20, 2024

  • RPYC v1 support: ren'py 8 always generates RPYC v2 files
  • Screen language v1 support: ren'py 8 cannot even parse these.
  • --sl1-as-python: see above
  • --tag-outside-block: this flag is only relevant for ren'py 6.99-7.2
  • Revertable classes in renpy.python: these were moved to renpy.revertable in 8.0
  • codegen in decompiler. This was needed to support SL1 screens.
  • A lot of hasattr checks in Decompiler that aren't necessary anymore.
  • updating un.rpyc to account for all the above
  • astdump: go through should_print_key and see what is still relevant.
  • Implement the third way of specifying a parameter signature that apparently existed in 8.0/8.1 / 7.5/7.6
  • Update testcases. Add some more interesting files (ren'py tutorial and the question are MIT-licensed, and thus an ideal candidate to include here).
  • Make --init-offset default to being on. It's hardly ever not preferable,

More to come as I go through files.

Additional notes:

  • UserStatement seems to have grown a lot of properties that we don't know about.
@madeddy
Copy link
Contributor

madeddy commented Feb 20, 2024

It seems to me the have blown userstatement out of proportion.

Seeing my py3_port is in things of codebase outdated now, i don't think you will merge it like this. Would be to stressful.

Some changes regarding py3 to keep in mind:

  • imports: from __future__ import unicode_literals can all go
  • py switch:
    • magic.py has some py2 -py3 switch code
    • the fake_class definition around L105 is hybrid -> Py3 only
  • unicode:
    • cases of string = string(unicode)
    • u' prefixes
    • bytes/utf encoding args
  • classes inherit always from object, so class Example(object): should go -> class Example:
  • type checks should whenever possible use isinstance() instead type()
  • super calls: py3 has super().__init__() instead of super(Child, self).__init__()

@CensoredUsername
Copy link
Owner Author

CensoredUsername commented Feb 20, 2024

You can find the state of things on the dev branch rn, most things have already been processed. magic.py was always python 2/3 compatible as it's borrowed from another project where I did put in that effort, so nothing changed there. I think I've covered most of what you mentioned already, except for the super() thing, but that's a single search/replace for sometime.

I'm looking through your py3 codebase for reference stuff, the actual changes are luckily not that invasive.

@madeddy
Copy link
Contributor

madeddy commented Feb 21, 2024

Looks already good. I wasn't aware the codegen van also go besides sl1.

  • Implement the third way of specifying a parameter signature

No wonder i pulled my hair over this if there are three ways. I wanted to find "the one" way after RenPy v8.0 and the set() didn't help.

  • UserStatement seems to have grown a lot of properties that we don't know about.

This seems to have grown a bit powerful over time.


I have some proposals beyond the port if you're interested. Do you want me to add it here or should i make a new ish.? Alternatively could you open the GH discussions feature for the repo, so we do not need to clutter the ishs for questions etc,

@CensoredUsername
Copy link
Owner Author

I opened discussions, feel free to use it.

@madeddy
Copy link
Contributor

madeddy commented Feb 22, 2024

Thanks!

@CensoredUsername
Copy link
Owner Author

Man, I was not looking forward to handling this one (due to the sheer amount of black magic that I poured into it), but un.rpyc and variants have been fixed. The astdump review is very low priority (it is purely a development tool), and the UserStatement changes are something to be seen for the future mostly, I at least don't have any testcases for it.

I'm going to merge dev into master, and cut new un.rpyc releases for both legacy and master, and we should be reasonably up to date again!

@madeddy
Copy link
Contributor

madeddy commented Feb 24, 2024

Man, I was not looking forward to handling this one

I can relate. Did try my hand on this ~two years back out of curiosity and just got frustrated following the errors through the "strange" code. Not easy. Didn't know if the bugs came from codegen/pickleast etc.
The mainapp was already good running on py3, so i needed to out-comment build for this in my pull to get rid of the repeat build errors.

we should be reasonably up to date again!

Good thing! Thanks for your work to get this running again!

The pulls can be closed i think. The other is with four years also outdated, same for the game branches.

@CensoredUsername
Copy link
Owner Author

The pulls can be closed i think. The other is with four years also outdated, same for the game branches.

Aight.

@CensoredUsername
Copy link
Owner Author

I can relate. Did try my hand on this ~two years back out of curiosity and just got frustrated following the errors through the "strange" code. Not easy. Didn't know if the bugs came from codegen/pickleast etc. The mainapp was already good running on py3, so i needed to out-comment build for this in my pull to get rid of the repeat build errors.

it is maybe a little more complicated than it has to be 😅 When I originally built it the idea was to do as much work inside the pickle machinery as possible, but that proved to be rather complicated. I also really liked the idea of just making it as tiny as possible (the whole minimize-codegen step could just be skipped if you don't feel like it).

And debugging is, well, not great. Ren'py has a tendency to swallow errors when loading rpyc files so you need to edit the source a little if you want those error messages. and even then, you're likely to have to deal with stuff like "On line 1, in something went wrong".

Un.rpy is a little easier to debug as it's not quite as magical, although it still has to go through the steps of constructing a python package tree in software, which is what required the changes for python 3 (in python 2 relative imports and absolute could use the same syntax, so all modules were just loaded absolutely there, while now we actually construct the package properly).

That said, all pickleast changes were basically an unrelated issue I found with the library, and I dealt with a deprecation warning. Minimize needed to deal with the new way arguments appear in the nodes, but in the end, the total changeset to the machinery was quite small since codegen/pickleast were already (mostly) py3 compatible.

@madeddy
Copy link
Contributor

madeddy commented Feb 24, 2024

"On line 1, in something went wrong".

LOL. I had my share of this uselessness. Thats why i applauded the improvements in the last years in main-python versions for error outputs. Really useful!


Something came to mind: Did you not mention something about the new ren.py files and how we need to implement support for them? They where forgotten or not?

Another note: So far i could not manage to get something useful out of this translate function of JackMcBarn. Not sure if I'm just trying it wrong or this thing not works. 🤷🏻

@CensoredUsername
Copy link
Owner Author

Something came to mind: Did you not mention something about the new ren.py files and how we need to implement support for them? They where forgotten or not?

Any _ren.py file will get compiled to a normal .rpyc file, and that .rpyc file will then be decompiled to a normal .rpy file with exactly identical functionality. Ren'py internally just converts the "_ren.py" file to a .rpy file internally before compiling so there's no way to figure out what it was at the start, but there's also little reason why to do so.

Another note: So far i could not manage to get something useful out of this translate function of JackMcBarn. Not sure if I'm just trying it wrong or this thing not works. 🤷🏻

Time to dig into the code then to figure out what it's doing ;)

LOL. I had my share of this uselessness. Thats why i applauded the improvements in the last years in main-python versions for error outputs. Really useful!

Just imagine the quality of them when you're messing with the import machinery inside of a pickle bytecode.

@madeddy
Copy link
Contributor

madeddy commented Feb 25, 2024

...so there's no way to figure out what it was at the start..

Understood. Thanks for explaining.
I figured we get maybe something like Ren'Py flavored *.pyc files, but i was obviously wrong.

Time to dig into the code...

Thanks a lot, but there is no need to overdo it. I am merely curious.

Just imagine the quality of them when you're messing with the import machinery inside of a pickle bytecode.

Oh boy. This sounds hurtful. Uhhh...

@madeddy
Copy link
Contributor

madeddy commented Mar 6, 2024

Ok, i have an ish which should be right here. I put my money on some py2 -> py3 residue and should be related to the open taskpoint about astdump.

If i try to astdump this file from a Ren'Py v8.1.4 app, with unrpyc v2.0.0, on system-py v3.10:

Error while decompiling /home/olli/.xlib/RPG/_test/Maeve-0.5.2-pc/game/characters_wardrobe/goth_wardrobe.rpyc:
Traceback (most recent call last):
File "/home/olli/Code/Git/unrpyc/unrpyc.py", line 183, in worker
return decompile_rpyc(filename, args.clobber, args.dump, no_pyexpr=args.no_pyexpr,
File "/home/olli/Code/Git/unrpyc/unrpyc.py", line 117, in decompile_rpyc
astdump.pprint(out_file, ast, comparable=comparable,
File "/home/olli/Code/Git/unrpyc/decompiler/astdump.py", line 29, in pprint
AstDumper(out_file, comparable=comparable, no_pyexpr=no_pyexpr).dump(ast)
File "/home/olli/Code/Git/unrpyc/decompiler/astdump.py", line 52, in dump
self.print_ast(ast)
File "/home/olli/Code/Git/unrpyc/decompiler/astdump.py", line 66, in print_ast
self.print_list(ast)
File "/home/olli/Code/Git/unrpyc/decompiler/astdump.py", line 102, in print_list
self.print_ast(obj)
File "/home/olli/Code/Git/unrpyc/decompiler/astdump.py", line 80, in print_ast
self.print_object(ast)
File "/home/olli/Code/Git/unrpyc/decompiler/astdump.py", line 216, in print_object
self.print_ast(getattr(ast, key))
File "/home/olli/Code/Git/unrpyc/decompiler/astdump.py", line 66, in print_ast
self.print_list(ast)
File "/home/olli/Code/Git/unrpyc/decompiler/astdump.py", line 102, in print_list
self.print_ast(obj)
File "/home/olli/Code/Git/unrpyc/decompiler/astdump.py", line 80, in print_ast
self.print_object(ast)
File "/home/olli/Code/Git/unrpyc/decompiler/astdump.py", line 216, in print_object
self.print_ast(getattr(ast, key))
File "/home/olli/Code/Git/unrpyc/decompiler/astdump.py", line 80, in print_ast
self.print_object(ast)
File "/home/olli/Code/Git/unrpyc/decompiler/astdump.py", line 206, in print_object
print(f"astdump print_object: {dir(ast)} ast: {ast}")
TypeError: '<' not supported between instances of 'str' and 'bytes'

Decompilation of 1 file failed

goth_wardrobe.zip

@CensoredUsername
Copy link
Owner Author

What the hell, it's failing in the dir(ast) call. That's new.

Oh, I figured it out. That RPYC file seems to have been compiled with ren'py 7. And then for some reason they upgraded the engine, and recompiled everything except for that file. If you look in the archive that file is from 14/03/2023 while the rest is from 06/01/2024. Truly delightful.

@CensoredUsername
Copy link
Owner Author

Going through it, I'm surprised it fails this late honestly. For some reason, the normal AST nodes do get their attributes loaded as actual unicode strings, but atl nodes don't. I think this is due to a funny interaction between from __future__ import unicode_literals and __slots__, which really should be causing errors on ren'py's side, but, for some reason it doesn't.

Adding an encoding=ASCII parameter in the call to magic.safe_loads in renpycompat.pickle_safe_loads causes it to be loaded correctly, as now any strings that were encoded on the python 2 side now get decoded to unicode on the py3 side. Of course, this still means that any pre-7.5.0 things do not work properly as they're no longer implemented. I'm not sure what the implications of this would be. It'd be nice if rpyc files recorded properly what version they were compiled in...

@madeddy
Copy link
Contributor

madeddy commented Mar 6, 2024

What the hell, it's failing in the dir(ast) call. That's new.

This stubbed me also and i could not even read this to get a handle on things.

...that file is from 14/03/2023 while the rest is from 06/01/2024. Truly delightful.

Yeah. You said it all.
So my guess from the other thread was right. This behavior to put apps from v7 just for fun on v8 happens a lot more often as you think. This and errors from it happen "on this other forum" since 2022@v8-release.

It'd be nice if rpyc files recorded properly what version they were compiled in...

Agreed. Maybe @Gouvernathor could comment on the possibility of this?

Adding an encoding=ASCII parameter in the call to magic.safe_loads in renpycompat.pickle_safe_loads causes it to be loaded correctly...

I don't think unrpyc should support such chimeras. This way Py3-unrpyc is fast back where py2 was in ways of hacks and extra-code massacres. 😨 Or not?

@Gouvernathor
Copy link

Gouvernathor commented Mar 6, 2024

I can't help you with that. My knowledge about the rpyc format is very limited, and any such indication which would only help unauthorized decompilings will not be implemented within renpy, unless we require it for our purposes.

That being said, there's a script_version file that's supposed to be included in all game distribs. We load it after ast and parsing time, but you could look for it directly and implement your own rules about it.

@CensoredUsername
Copy link
Owner Author

Yeah I don't want to bother renpy devs with this, sorry for that. I'd like to keep my contributions to ren'py useful as well (I did prototype the first version of the ingame debugger with some friends), and not a burden to support. Besides, at this point it wouldn't fix the problem any more anyway, as we'd only know old files aren't versioned yet.

I'm more inclined to just add a small option that detects most of these cases (BINSTRING/SHORT_BINSTRING opcodes in the pickle format are a tell, as python 3 doesn't generate those) and give a warning.

We could still enable the option, and backport possibly some things back (old-style argument/parameter handling mainly). After all, when decompiling such a chimera we're not required to be backwards compatible with what we emit, which is what most ugly hacks were really for.

You'd think old-style screen language support would be needed, but it actually isn't, because those are broken for several other reasons if you'd try to chimera it. Because the engine stores a python ast/bytecode blob directly for old-style screens, and that ast uses 2.7 nodes, while 3.9 ast nodes/bytecode are different.

So it wouldn't be the worst to support it. Just adding back old style param/argument support and rpyc v1 handling (which is literally 1 line of code).

@madeddy
Copy link
Contributor

madeddy commented Mar 6, 2024

any such indication which would only help unauthorized decompilings will not be implemented

Fair enough. Forget i asked.

Yeah I don't want to bother renpy devs with this, sorry for that.

No prob and my bad. Sry. I just didn't know the stance on this.

@Gouvernathor
Copy link

Because the engine stores a python ast/bytecode blob directly for old-style screens, and that ast uses 2.7 nodes, while 3.9 ast nodes/bytecode are different.

Yes, but they still exist in the py3 pickle module, right ? Or is old-style screen support broken under Ren'py 8 ?

@CensoredUsername
Copy link
Owner Author

CensoredUsername commented Mar 7, 2024

Yes, but they still exist in the py3 pickle module, right ? Or is old-style screen support broken under Ren'py 8 ?

Not sure what the py3 pickle module has to do with ast nodes, those are located in the ast module. Unless you mean the pickle import fixing machinery between py2-py3, that one doesn't handle this either.

Just tested to confirm it, but yeah. Rpyc files with screenlang version 1 support are broken in ren'py 8. It'll fail to even load the rpyc file as it errors during unpickling.

On a testcase I had, the result was:

AttributeError: Can't get attribute 'Num' on <module '_ast' (built-in)>

(Or well, that's what it'd show if errors weren't swallowed if they happen in rpyc loading)

Which makes sense. Ren'py stores very little data for old-style screen support in the rpyc file. A screenlang version 1 screen was internally a small piece of generated python code calling a bunch of ui. family functions after all. The engine would then compile that in advance, and only store the resulting python ast directly in the rpyc file (and compiled bytecode of that in the bytecode cache). It would've been more future compatible to also store the source, but sadly it isn't.

The ast format of python has changed through the years, and it isn't the same between python 2 and 3. So when you unpickle a python 2 ast in python 3 it'll likely complain that it simply cannot find the definitions for a bunch of classes as they do not exist in python 3. This at least applies to the following ast nodes that exist in python 2.7, but not in 3.9:

ast.TryExcept
ast.TryFinally
ast.Exec
ast.Repr
ast.Num
ast.Str
ast.Print

Furthermore the following have changed their format and will likely fail to unpickle as well.

ast.arguments
ast.keyword
ast.Call
ast.Raise
ast.With

Now I don't think old-style screenlang generates any Try/Exec/Repr/Print/Raise/With nodes but it does practically always generate Num/Str/Call/keyword/argument nodes.

I can think of a few ways to deal with this, but none of them are easy or pretty. It can be done though, the legacy branch of this project actually supports decompiling these back to the internal python code, or even to the original source code. Feel free to borrow some of that.

Edit: and of course the bytecode is completely incompatible as well, so no luck there either.

@CensoredUsername
Copy link
Owner Author

@Gouvernathor I ended up thinking about it a bit more, and I think there's a solution that's not too terrible that could get these loaded again (just with a little rewriting of that ast at pickle load time). Sounds like a fun challenge, would there be any interest in that for Ren'py?

@Gouvernathor
Copy link

Yes, if you can show that it fails to load now you can open it on the repo

@CensoredUsername
Copy link
Owner Author

To get back to the original issue, I've removed some items that I'm considering reverting in #199

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

No branches or pull requests

3 participants