ME3Tweaks / LegendaryExplorer

Editor toolset for Mass Effect Trilogy and Mass Effect Legendary Edition
https://me3tweaks.com
GNU General Public License v3.0
125 stars 27 forks source link

Potential bugs in script decompiling #329

Closed mirh closed 2 years ago

mirh commented 2 years ago

Describe the bug I didn't really parse the hex view to manually confirm LExplorer is at fault here.. Still, it seems more likely for a bug to exist here than in UE3's compiler.

To Reproduce

Expected behavior Unless it was bioware to actually screw these, decompilation should be smooth (putting aside one could argue the "vanilla game state" should be accounted for anyway)

Version information: 6.1 nightly 17/07/2022

SirCxyrtyx commented 2 years ago

BioPlayerInput

The ERRORNULL thing is in fact a problem with the bytecode. Where there should be an iterator reference, there is a null. I suspect this is due to the member it should be referencing (FlyCamPCBinds) being stripped out during cooking. This isn't a problem in-game, since it is contained within an if statement that will only be executed if the game is running in the editor. I will admit that decompiling the EX_FilterEditorOnly token as an error (__IN_EDITOR) was maybe not the best decision. I wanted to force users to delete it if they edited the function, but still show it to provide context. I think a better way to do that is to decompile it commented out. That way you can still see it, there's no syntax error, and recompiling strips it out automatically.

public function SetFlyCam(bool bOn)
{
    local StaticKeyBind Binding;

    if (bOn == TRUE)
    {
        Outer.SetCameraMode('FreeCam');
        Outer.PushState('PlayerFlyCam');
    }
    else
    {
        Outer.ResetCameraMode();
        Outer.PopState();
    }
    if (Outer.Pawn.InFreeCam())
    {
        //if (__<>IN_EDITOR)
        //{
            //foreach ERRORNULL(Binding, )
            //{
                //StaticPCBinds.AddItem(Binding);
            //}
        //}
        foreach FlyCamConsoleBinds(Binding, )
        {
            StaticConsoleBinds.AddItem(Binding);
        }
    }
    else
    {
        //if (__<>IN_EDITOR)
        //{
            //foreach ERRORNULL(Binding, )
            //{
                //StaticPCBinds.RemoveItem(Binding);
            //}
        //}
        foreach FlyCamConsoleBinds(Binding, )
        {
            StaticConsoleBinds.RemoveItem(Binding);
        }
    }
}

BioPlayerSquad

GetOrientationType and GetRestFormation cannot reach the end without returning, the warning is given because the static analysis I do is very simplistic, and only checks for a return statement at the end of the function. Hence why it's a warning instead of an error, and says "might". I leave it up to the user to determine if it's actually a problem.

GetActionString on the other hand, really can reach the end without returning a value. That's not a decompilation error. UE3's compiler allows this because it has a failsafe: Every function that returns something has an implicit return at the end which returns the default value for the type.

Material

Not a decompilation error. The Normal variable in Material does partially shadow the Normal function in Object. Most of the time this won't be a problem and both can be used without issue. Usages of functions and usages of variables are mostly distinct so the parser would have no trouble compiling a function like this in Material:

function foo(){
    //Normalize a vector constant with Object's Normal function, and assign it a field in Material's Normal struct
    Normal.Constant = Normal(vect(5.3, 8.2, 90));
}

However, functions can be used like variables when assigning to a delegate variable. It is in this circumstance that a variable can shadow a function with the same name. Consider this code:

function foo(){
    //declare a variable that can hold a delegate with the same signature as Object's Normal function
    local delegate<Normal> d;
    //This won't compile in Material, since it finds Material's Normal variable first, which is a struct, 
    //and not assignable to a delegate.
    d = Normal;
}

A contrived example to be sure, but the compiler has to consider all possibilities, even unlikely ones.

mirh commented 2 years ago

Well, damn... Hats off for the impressive commentary.

mirh commented 2 years ago

You might be interested to file OnlineSubsystemPC.pcc...

SirCxyrtyx commented 2 years ago

that's fixed in f4376d965bf837cd1a87e0dd0624d53277a86ba7