CensoredUsername / unrpyc

A ren'py script decompiler
Other
861 stars 157 forks source link

RenPy cfg assignment incorrect decompiled #113

Closed madeddy closed 3 years ago

madeddy commented 3 years ago

A game uses this dict assignment for font changing in his gui.rpy config: define config.font_replacement_map["fonts/RobotoCondensed-Regular.ttf", False, True] = ("fonts/RobotoCondensed-Italic.ttf", False, False)

After it was compiled with renpy and decompiled with unrpyc-1.1.5 we get this line instead: define -2 config.font_replacement_map = ("fonts/RobotoCondensed-Italic.ttf", False, False) and RenPy errors logically on it. (As should normally the triple dict key but renpy works somehow with it.)

Here are the traceback and the original gui.rpy/rpyc files. example_ files_rpy_font_err.tar.gz traceback.txt

The trace is from another user in W10, but i got the same in Kubuntu 20.04.2.

CensoredUsername commented 3 years ago

a[b, c, d] = e is essentially shorthand for a[(b, c, d)] = e.

The issue here is that the ren'py define statement used to be more restricted on what appeared on the left hand side of the = sign. It used to only include a variable name, and at one point was expanded to at least take a scoped varname, but item assignments were previously not possible.

I'll have to look in the .rpyc file to see what exactly is going on in there but should be a decently easy fix.

CensoredUsername commented 3 years ago

fixed on dev!

madeddy commented 3 years ago

a[b, c, d] = e is essentially shorthand for a[(b, c, d)] = e.

Must be some specialty of the renpy language to be able to do this without making it a tuple or something else. Strange thing.

fixed on dev!

Very good . Tested it and works for me.

Something i noticed: The "-2" after every define... i guess its the "init offset" originally on top of the file, yes? Why is it after decompile changed like this? The original state with just the one statement would imho be better. Or is this compiled like this?

CensoredUsername commented 3 years ago

Must be some specialty of the renpy language to be able to do this without making it a tuple or something else. Strange thing.

Nah that's just how python handles indexing. In some situations tuples do not have to be ()-delimited in python. See this:

>>> class Test:
    def __getitem__(self, item):
        return item
>>> a = Test()
>>> a[1,2,3]
(1, 2, 3)

Something i noticed: The "-2" after every define... i guess its the "init offset" originally on top of the file, yes? Why is it after decompile changed like this? The original state with just the one statement would imho be better. Or is this compiled like this?

The init offset statement is a parser directive, it disappears at parse time and is compiled as individual offsets at every statement.

There's no real way to differentiate between individual offsets and init offset-caused offsets, and trying to guess would complicate decompilation significantly.

madeddy commented 3 years ago

Thanks for explaining!

I did try to init it like this:

d = {"akey", True, False: ('aval', 'another'), "bkey, False, True:('bval', 'bah')}

What(unsurprisingly) not worked and wondered. But doing this does it:

d = {}
d["akey", True, False] = ('aval', 'another')

Learned something again. Successful day. Greets

CensoredUsername commented 3 years ago

Bare tuples are an interesting thing in python's parsing. In a lot of places you can construct a tuple without the delimiting ()'s, but some are not allowed.

d = {"akey", True, False: ('aval', 'another'), "bkey, False, True:('bval', 'bah')}

The reason this doesn't work is that it would results in a parsing ambiguity that is unsolvable. Look at the following example. Should this be parsed as {a: (b, c), d: e} or {a: b, (c, d): e}

d = {a: b, c, d: e}

So to deal with these possible ambiguities, it's disallowed to construct bare tuples in several situations where they would result in an ambiguity or violate LL(1) parser lookahead rules.

Notably they are allowed in:

madeddy commented 3 years ago

Thanks again for the additional really good explanation. More complex as one thinks at first.

Your code changes on the issue are working for me and some other people which use it in "the" forum. From my side this could be closed as resolved. Don't know if the seldom used font replace and the few fix lines for it are enough for another release. Also, i would like to refactor a few things in unrpyc and maybe they could go then together. If accepted.

CensoredUsername commented 3 years ago

I'll make a release some time, this plus your multiprocessing changes is enough, just a bit busy this week.

Also you're not going to piss me off by mentioning f95 or anything lol,you guys do know that github has redirect analytics right ;) I just got annoyed that one time because people were suggesting unren to fix an actual issue a user was having with purely unrpyc, which was rather annoying. Either way I don't mind you filtering issue reports into something I can actually work with so thanks for that.

madeddy commented 3 years ago

I'll make a release some time, this plus your multiprocessing changes is enough, just a bit busy this week.

No stress! Being busy in real life is absolute understandable. :smile: Wanted just to get back with "me thinks it's stable" infos.

And about multiprocessing: To me you seemed a bit reluctant to implement it and i understand the reasoning. So, its not really needed to do it and your decision. I am ok with it anyway.

Also you're not going to piss me off by mentioning f95 or anything lol,you guys do know that github has redirect analytics right ;)

Yes. I figured... i mean which of the bigger companys doesnt sniff like a randy dog. I did just not know if it causes probs because of the nature of some things there. I am not much of a "blackbeard" with a "Jolly Roger" at my window. I hang there up for the fun with the tools and some laughs about dumb/lazy/crazy people there. (...and for scientific reasons about naked facts... Really. :joy:)

madeddy commented 3 years ago

Issue was successful fixed by https://github.com/CensoredUsername/unrpyc/commit/4e3b67b763e7436b0c58b50df98464e81e8a057a