Editing SCR_GameStart results in "Error Bad Variable" on assembly

[NarryG]

I'm fairly sure it used to work fine.

[NarryG]

Bisect reveals the reversion occurred with commit https://github.com/krzys-h/UndertaleModTool/commit/e78e345

[NarryG]

To expand a bit, it appears to have to do with your hack to deal with arrays. The decompiled code it's failing on is this little block. It fails when it reaches global.spell

{
    if (self.i < 8)
    {
        global.item[self.i] = 0
        global.spell[self.i] = 1
        global.bulletvariable[self.i] = 0
        global.menuno = -1
        global.menucoord[self.i] = 0
        global.bmenuno = 0
        global.bmenucoord[self.i] = 0
        self.i = (self.i + 1)
        continue
    }
    break
}

In the case of global.item, it doesn't fail as global.item is set later so it has a self generated (if I'm not mistaken?).

PreviousInstType is null so there's no value to set, so it stays as undefined. The next check (>= 0 set Self) then sets self

[krzys-h]

Yeah, I knew this hack would cause problems in the long run...

For things like this you should really look at it at assembly level. This language was clearly not designed to be easily assembled from low-level instructions instead of the normal high-level compiler. The problem is in how it handles array push/pop. Normally, the information about whether we are dealing with a global or instance variable is encoded in the instruction itself, but for some reason they decided that in case of arrays this information is put on the stack instead (why? makes no sense at all). The problem is, the pop instruction also needs a reference to the variable itself, and the variable depends on whether it's a global or instance one (don't ask me why they duplicate the information like this, perhaps it's some kind of compatibility thing for older bytecode).

For example: a normal pop instruction looks like this:

pushi.e 1337
pop.v.i global.somevar

and it's clear that we are referring to a global variable. But for an array pop, you get:

pushi.e 1337
pushi.e -5   ; remember that global = -5
pushi.e 42  ; array index
pop.v.i [array]somevar ; no global/self specifier here!

so when compiling the pop instruction you have no idea whether it's a local or global var that you should put in.

There a couple possible ways to solve this:

1. Break the constraint that every line of assembly code is just an exact text representation of the corresponding instruction and inject "global." artifically when decompiling. This would lead to some strange behaviour if somebody tries to change it from global to local and forgets that this part is actually used only for variable reference selection and it should be changed on the stack instead.
2. Make the assembler simulate the stack like the decompiler does (ouch!) so that it knows the exact stack state when executing the instruction. This is rather overcomplicated to put in an assembler tool, but is the only "proper" way that should work no matter what (unless somebody purposefully tries to break it by calculating the self/global value at runtime or something)
3. Just extend my poor hack to look through last instructions to find the last two pushes rather than last two instructions. I didn't look at the exact failing aassembly code yet but I presume that's what goes wrong.

[krzys-h]

Currently, my hack code looks two instructions up expecting something like I shown in the previous post. The code in scr_gamestart is something along the lines of:

00167: pushi.e 0
00168: pushi.e -5
00169: push.v self.i
00171: conv.v.i
00172: pop.v.i [array]item

so this extra 'conv' in the middle breaks everything.

Looks like I'm extending this hack like described in 3, at least for now

[krzys-h]

9e3ff1a2885954136ecb414159b584234912357a should fix it for now, scr_gamestart works fine. If it breaks again feel free to reopen this, I might think about implementing something better than that hack.

[krzys-h]

Example of code that still breaks:

pushi.e -5
pushi.e 0
conv.i.v
push.s "Flag to change"
conv.s.v
call.i get_integer(argc=2)
conv.v.i
pop.v.v [array]flag

[colinator27]

Found another example that I believe is the same issue. This is the disassembly:

pushi.e -5
pushi.e 7
dup.l 0
push.v [array]flag
push.e 1
add.i.v
pop.i.v [array]flag

The GML for this is global.flag[7]++;

This code also completely fails to decompile as well, stating that the stack is empty on line 575 of Decompiler.cs...

[krzys-h]

What the hell is dup.l even, I don't remember ever seeing that variable type... I guess that was meant to be l for long, int64 maybe? Is that from Deltarune? I thought that all scripts decompiled fine.

So I guess it duplicated one 64-bit value instead of two 32-bit ones... implementation-level hacks related to how the real runner stores the stack, that's not going to work in C#'s high-level implementation of Stack. Where did you get that from even?

Normally, that dup was always encoded as dup.i 1

ping @PoroCYon

[colinator27]

I swear I said that it was in another game but I guess I accidentally deleted it when I was formatting the text... Anyway, it was my own game. I just re-tested to see if I got the same result across 1.4 and 2, and the resulting bytecode is exactly the same (and the same error), yes, with the dup.l for some odd reason. I don't understand either...

[colinator27]

By the way, another place that's similar but with a dup.i 1 instruction in Undertale is at the end of gml_Script_scr_torcall. It decompiles fine initially, but reassembly fails.

[krzys-h]

Yeah, I would expect the assembly to still fail. I need to make a better workaround for this stupid thing.

[krzys-h]

The dup.l issue will be tracked in #52 because it seems to be a separate thing

[krzys-h]

All scripts seem to reassemble fine now (with the exception of some rounding errors, see #53)