If-Else statement scope incorrectly decompiled when Else path begins with another If-Else statement

[0HMyC]

When writing an if-else statement as so: if (condition) { code; } else { if (condition) { ... } }, the compiler will fail to recognize that the latter if statement(s) are within the scope of the parent if-else statement, and will merge the if-else statements into the same chain within the same scope, ending the chain whenever the first instance of non-scope (meaning not an if-else statement or similar) code appears.

For example, the following source code: does compile correctly, however decompiling the compiled file results in the following incorrect code: In the above example, if that code were to be compiled again, it would result in the line SET_MSG_VAR(0, 4, 0); always being executed after all preceding if-else statements, when it is only supposed to execute if testVar is equal to 0. This becomes problematic when writing code far more dependent on code-scoping; for example, if we wanted a message to be displayed only when testVar != 0, but after any other conditions within the relevant scope are run, the message call would be made regardless of what testVar is actually set to, which would not be correct.

Interestingly, if the parent else { ... } portion of the chain begins with code that is not another if-else statement chain, the compiler does correctly recognize and then prefer the proper scope, which in this example results in the decompiled code being truly functionally identical to the source it was originally compiled from, as seen in the below images (original source, decompiled source)

For a practical example, the following code in the field_tbox_get() procedure from the field.bf file used in Persona 3 Portable when opening chests (typically when within Tartarus) has an if-else statement which checks if FLD_FUNCTION_0035 is equal to 0 to determine if the "tbox" contains an item, or money within. The intended functionality is that a "tbox" can only contain either an item or money, and thus the scope of the if-else statments prevents the code which determines the amount of money to receive from running if the player is supposed to get an item. Due to the "else" path containing code to set variables var3, var4 and var5 at the start, the scope is preserved when decompiling the file.

However, decompiling the FIELD.BF file used in Persona 3 FES (which is largely identical otherwise to the Persona 3 Portable version of the file) results in the scope not being preserved, as variables var3, var4 and var5 are set outside of the if-else statements entirely, resulting in the else path mentioned before starting with an if-else statement, resulting in all the code in that path being moved up to the parent scope. If this code is then recompiled (either from compiling the decompiled .flow or by copying the code to a new .flow file as-is or lightly modified via hooking,) it will result in the code to give the player money always executing even if the player isn't supposed to receive money at all, usually then showing a message claiming the player has acquired 0 money due to this.

Given that the decompiling function is used on the compiled BF files present in Atlus games in order to create modifications to the scripts, this will very easily result in the decompiled scripts having incorrect functionality that then carries over to a hypothetical mod, which can be missed by those seeking to create mods. Ideally, the decompiler should recognize and preserve the scope of if-else statements whether or not code or an if-else statements appears at the start of an else path, so as to preserve the functionality of decompiled scripts.

[matelgaard]

This was recently fixed by #69, but it's not in the latest release yet so you have to compile the source yourself for this fix. Maybe we should have a new release soon @tge-was-taken?

[0HMyC]

Oh wow, I didn't even know about that. Guess I wasn't looking closely enough at the commit history haha 😅

In that case I guess I can just close this issue if it's apparently been fixed already; I would definitely say that it would be nice to have a new proper release out if it's apparently been fixed, especially considering that support for P3R was added since the last release (a pretty major feature,) although from what I remember that does have some issues still which may make a full release unwise? Hard to say myself.

I suppose for now though I ought to be able to get by with the appveyor build artifacts, at least until they expire lol