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

Fix decompiler else-if chain bug #69

Merged
merged 2 commits into from
Apr 4, 2024

Conversation

matelgaard
Copy link
Contributor

Fixes #40, fixes #9

This fixes an else-if chain bug in the decompiler incorrectly removing brackets { } from else-statements whose bodies contain more than one statement, other than a single if-statement.

It is caused by a special case when writing a compilation unit, which omits { } from else clauses if the first statement in the else body is another if-statement, which ensures pretty else-if chains in the output. However, it would incorrectly do this if the else-body had additional statements following the if-statement, leading to those statements being left out of the else body in the output.

This would cause decompiled and recompiled scripts to have inconsistent logic, leading to strange behavior in the game and sometimes crashing. I have tested the situations where crashes happened in issue #9, before and after the fix, and can confirm that this fix resolves those crashes.

The fix was changing a single line in CompilationUnitWriter.cs, such that it only omits the brackets if the if-statement is the only statement in the else body. Otherwise the brackets will be left in.

An example from P5R's FIELD.BF (located at CPK\EN.CPK\FIELD\ETC\FIELD.BF)
The correct logic for the fld_r1_menu() procedure is as follows:

void fld_r1_menu()
{
    if ( BIT_CHK( ( 0x20000000 + 65 ) ) )
    {
        return;
    }
    else if ( ( CHK_DAYS( 11, 22 ) == 1 ) && ( GET_TIME() == 4 ) )
    {
        return;
    }
    else if ( BIT_CHK( ( 0 + 96 ) ) )
    {
        KeyfreeEvent_SHORTCUT();
    }
    else if ( BIT_CHK( ( 1342177280 + 270 ) ) )
    {
        MyPalace_SHORTCUT();
    }
    else 
    {
        
        if ( ( BIT_CHK( ( 0x10000000 + 546 ) ) == 1 ) && ( BIT_CHK( ( 0 + 744 ) ) == 0 ) )
        {
            BIT_ON( ( 0 + 744 ) );
        }

        Field_SHORTCUT();
    }

}

However, the decompiler would produce:

void fld_r1_menu()
{
    if ( BIT_CHK( ( 0x20000000 + 65 ) ) )
    {
        return;
    }
    else if ( ( CHK_DAYS( 11, 22 ) == 1 ) && ( GET_TIME() == 4 ) )
    {
        return;
    }
    else if ( BIT_CHK( ( 0 + 96 ) ) )
    {
        KeyfreeEvent_SHORTCUT();
    }
    else if ( BIT_CHK( ( 1342177280 + 270 ) ) )
    {
        MyPalace_SHORTCUT();
    }
    else if ( ( BIT_CHK( ( 0x10000000 + 546 ) ) == 1 ) && ( BIT_CHK( ( 0 + 744 ) ) == 0 ) )
    {
        BIT_ON( ( 0 + 744 ) );
    }

    Field_SHORTCUT();

}

Leading to Field_SHORTCUT() being called in any case, which is not correct

Fixed an else-if chain bug in the decompiler incorrectly removing brackets {  } from else-statements whose bodies contain more than one statement
@tge-was-taken
Copy link
Owner

Nice find.

@tge-was-taken tge-was-taken merged commit b10bf35 into tge-was-taken:master Apr 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants