Initial version of new style Lua argument parsing

[sbx320]

Initial stuff for automated Lua argument parsing by applying template magic.

Basically this allows us to directly use C++ functions without needing to handle the argument parsing via argstreams. For example

int CLuaCryptDefs::Hash(lua_State* luaVM)
{
    //  string hash ( string type, string data )
    EHashFunctionType hashFunction;
    SString           strSourceData;

    CScriptArgReader argStream(luaVM);
    argStream.ReadEnumString(hashFunction);
    argStream.ReadString(strSourceData);

    if (!argStream.HasErrors())
    {
        SString strResult = GenerateHashHexString(hashFunction, strSourceData);
        lua_pushstring(luaVM, strResult.ToLower());
        return 1;
    }
    else
        m_pScriptDebugging->LogCustom(luaVM, argStream.GetFullErrorMessage());

    lua_pushboolean(luaVM, false);
    return 1;
}

becomes

std::string CLuaCryptDefs::Hash(EHashFunctionType hashFunction, std::string strSourceData)
{
    return GenerateHashHexString(hashFunction, strSourceData);
}

which avoids error and reduces the required amount of code massively.

This PR also allows registering functions via the ArgumentParser template (rather than the ArgumentParserWarn template), which causes usage mistakes to be considered a hard error in accordance with #821. As a usage example I mostly converted CLuaCryptDefs to use the new style.

Supported Types:

• bool
• int, (todo: ushort, char, uchar)
• std::string (todo: SString?)
• CLuaFunctionRef (argument only)
• enums (with StringToEnum)
• Script entities (via UserDataCast)
• std::vector<T> if T is supported (expects a table with consistent value types) (todo: as result)
• std::unordered_map<K, V> if K and V are supported (todo: as result)
• std::optional<T> if T is supported (expects either a T or nil/none) (todo: as result)
• std::variant<Ts...> if all Ts... are supported
• nullptr (return type only) to indicate nil

Additionally a lua_State* may be taken as an argument, which will be passed through directly. (We use that in some places to access things based on the running script VM)

Restrictions:

• Currently this only supports default-constructible complex types. For example one can not read a std::unique_ptr (unlikely to change)
• No full support for overloads yet. Using std::variant and std::optional we have some support for overloading, but a function that expects very different types is not fun to handle
• No vararg support (yet)

Notes:

• See CLuaCryptDefs for usage examples
• Functions may throw a std::invalid_argument exception to indicate argument failures inside the functions.
• This requires VS2019 on Windows with the v142 toolset (VS2017 does not work due to compiler bugs)

Todos:

• Add debug assertions for Lua stack manipulation (compare gettop vs the expected value)
• Support additional types (see above, maybe more?)
• Figure out a way to handle overloads nicely
• Error messages are not generated yet
• Test on Linux
• Figure out why LuaBasic.h is detected as ObjectiveC by clang format (help?)

[ghost]

Brilliant! So, will we be using ArgumentParserWarn for old functions and ArgumentParser for newer ones?

[sbx320]

Yes that is the plan. We need to be careful with converting existing code though, as the behavior of this new stuff may be different in some edge cases.

[qaisjp]

Looks good.

Almost all of these suggestions are readability/documentation related - I've gone with the approach that if I am having trouble skim-reading something, it should probably be explained in a comment. I hope that's OK.

I've also pointed out some methods that would benefit from guarding (or "short-circuiting"... I'm not really sure what it's called). I think it reduces cognitive load. Let me know what you think about this style.

Also, note that my suggestions to Client/.../CLuaDefs.h also apply to Server/.../CLuaDefs.h.

Still to review:

• I have not (yet) checked to make sure you exhaust all types and include the full range of types in all areas of TypeMatch/Push/Pop.
• Deeper read into CLuaFunctionParser::PopUnsafe, CLuaFunctionParser::Call, and CLuaOverloadParser.

Optional:

• Include a short guide somewhere that tells you what you need to edit when adding a new type. This is probably unnecessary as you cover types in the most general case.

[sbx320]

Thanks for the review. I applied some changes to resolve most of it.

I have not (yet) checked to make sure you exhaust all types and include the full range of types in all areas of TypeMatch/Push/Pop.

I most likely do not cover all of them yet. Adding further types is fairly simple though, so I'd suggest we just add them as we go.

Deeper read into CLuaFunctionParser::PopUnsafe, CLuaFunctionParser::Call, and CLuaOverloadParser.

If you have questions, ask me :)

Include a short guide somewhere that tells you what you need to edit when adding a new type. This is probably unnecessary as you cover types in the most general case.

Adding new types is rare, as we usually only introduce new script entities which are all handled already. But as a quick reference for the future:

• New arguments would need to be added in TypeMatch (check if type is valid here), TypeToName (for error messages) and PopUnsafe (read type from Lua stack).
• New return types would need to be added as a Push overload.

Details of these operations depend heavily on the type that is to be added.

[qaisjp]

What is this for?

[sbx320]

Call recursively calls itself until it has reached the desired number of arguments. In each step we pop the next parameter from the Lua stack and append it to the list of arguments.

[qaisjp]

And then Push the return value?

[sbx320]

Yup. So if we're trying to call int foo(std::string, float) we get the following calls:
Call(L);
Call(L, str); (with str being the string returned from Pop)
Call(L, str, flt) (flt being the float from Pop)
Push(Func(str, flt));

[qaisjp]

What's blocking this?

[ghost]

What's blocking this?

Some DX rendering stuff gets broken, if you compile MTA with VS 2019. Other than that, this PR looks complete.

[sbx320]

What's blocking this?

VS2019 on the build server is the big thing.

But if @saml1er knows about some rendering issues we should investigate those first before upgrading. Probably some new optimization breaking some wrong code. Could you file an issue for those @saml1er so we can track/investigate them?

[qaisjp]

Some DX rendering stuff gets broken

I think @botder fixed one of them in 53121a3. I don't know what the others are

[Dutchman101]

Now that VS2019 is on the build server, and no one created new issues for any DX issues (nor did I observe any bugs in recent nightlies as a result of using vs2019), can we get a fresh look on what's required to proceed with this?

Edit: wait for #1135 results

[sbx320]

I'd want to get proper handling for std::string_view in (should be significantly faster than using std::string as it avoids copying the Lua string) as well as some useful tests for the crypt defs before merging it.

But no real blockers other than that anymore.

[qaisjp]

clang complains

/mtasa/blue/Build/../Shared/mods/deathmatch/logic/lua/LuaBasic.h:25:14: warning: inline function 'lua::PopPrimitive<std::__1::basic_string<char> >' is not defined
      [-Wundefined-inline]
    inline T PopPrimitive(lua_State* L, std::size_t& index);
             ^
../Shared/mods/deathmatch/logic/lua/CLuaFunctionParser.h:263:25: note: used here
            return lua::PopPrimitive<T>(L, index);
                        ^

[ghost]

clang complains

Any other issue? I want to merge this.

[ghost]

I'd want to get proper handling for std::string_view in (should be significantly faster than using std::string as it avoids copying the Lua string) as well as some useful tests for the crypt defs before merging it.

After reading more about std::string_view, looks like we can easily replace const string & with std::string_view. Should I do it for this PR? @sbx320

[Citizen01]

🚀