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.

Sign up for GitHub

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

Jump to bottom

Scripting: Some issues building with Lua 5.1 #2967

Open
nuive opened this issue Jun 30, 2023 · 4 comments · May be fixed by #3376
Open

Scripting: Some issues building with Lua 5.1 #2967

nuive opened this issue Jun 30, 2023 · 4 comments · May be fixed by #3376

Comments

@nuive
Copy link
Contributor

nuive commented Jun 30, 2023 •
edited
Loading

I'm finding some issues trying to use mGBA with Lua 5.1.

What I've found and what I've fixed:

  1. Building using Lua 5.1 gave me some errors and the build process was broken. The missing function was luaL_tolstring. I fixed it by doing the following:

script/engines/lua.c:
line 224:

#if LUA_VERSION_NUM < 502
#define luaL_traceback(L, M, S, level) lua_pushstring(L, S)
#define lua_pushglobaltable(L) lua_pushvalue(L, LUA_GLOBALSINDEX)

const char* luaL_tolstring (lua_State* lua, int idx, size_t* len) {
  if (!luaL_callmeta(lua, idx, "__tostring")) {
    int t = lua_type(lua, idx), tt = 0;
    char const* name = NULL;
    switch (t) {
      case LUA_TNIL:
        lua_pushliteral(lua, "nil");
        break;
      case LUA_TSTRING:
      case LUA_TNUMBER:
        lua_pushvalue(lua, idx);
        break;
      case LUA_TBOOLEAN:
        if (lua_toboolean(lua, idx))
          lua_pushliteral(lua, "true");
        else
          lua_pushliteral(lua, "false");
        break;
      default:
        tt = luaL_getmetafield(lua, idx, "__name");
        name = (tt == LUA_TSTRING) ? lua_tostring(lua, -1) : lua_typename(lua, t);
        lua_pushfstring(lua, "%s: %p", name, lua_topointer(lua, idx));
        if (tt != LUA_TNIL)
          lua_replace(lua, -2);
        break;
    }
  } else {
    if (!lua_isstring(lua, -1))
      luaL_error(lua, "'__tostring' must return a string");
  }
  return lua_tolstring(lua, -1, len);
}
#endif
  1. After the previous bug was fixed, the build completed successfully, but everytime I loaded some Lua Script the Emu crashed. I found the problem was with how Lua handles the _ENV variable (environments are different in 5.1 than 5.2 and posterior versions.
    In order to fix this, I tried to do:

Original code:

script/engines/lua.c:
...

    case LUA_OK:
        // Create new _ENV
        lua_newtable(luaContext->lua);

        // Make the old _ENV the index in the metatable
        lua_newtable(luaContext->lua);
        lua_pushliteral(luaContext->lua, "index");
        lua_getupvalue(luaContext->lua, -4, 1); // this doesn't add any value to the stack, because _ENV doesn't exist on Lua 5.1.
        lua_rawset(luaContext->lua, -3);

        lua_pushliteral(luaContext->lua, "__newindex");
        lua_getupvalue(luaContext->lua, -4, 1); // same here
        lua_rawset(luaContext->lua, -3); // -with Lua 5.1, this causes an Emu crash.

My solution:
As lua_getupvalue is getting the _ENV upvalue which lua_load creates when loading a script on Lua 5.2+, for Lua 5.1 I replaced those two lua_getupvalue calls with getfenv calls, which get the _G variable of the loaded script, which should be equivalent to the _ENV variable from Lua 5.2+:

#if LUA_VERSION_NUM >= 502
        lua_getupvalue(luaContext->lua, -4, 1);
#else
        lua_getfenv(luaContext->lua, -4);
#endif

This fixed the crash, but now I'm finding another error when loading any script: [ERROR] attempt to call a table value.

I think this is related to environments too (something with Lua 5.1 creating an environment for each function it declares or something like that, but I'm a bit lost here.

Any idea?
@nuive nuive changed the title I'm finding some issues trying to use mGBA with Lua 5.1. Scripting: Some issues building with Lua 5.1 Jun 30, 2023
@Aarushb
Copy link

Aarushb commented Oct 6, 2023

Bumping this up because I'm super interested in the problem as well as the script op is building that greatly improves accessibility for a game series I play a lot, and the issue seems to have been berried.

@nuive
Copy link
Contributor Author

nuive commented Nov 23, 2023

I've edited first comment. For some reason, part of the code became broken.

By the way, now I'm almost sure the error is related to how Lua 5.1 handles environments, metatables and all of that. I think that in some momment the engine creates some table which is OK for Lua 5.2+ structure, but not for Lua 5.1, and Lua 5.1 finds a table when its looking for a function. The problem is I cannot find where I can start in order to fix that.

@nuive
Copy link
Contributor Author

nuive commented Jan 2, 2025

PR #3376 would fix this.

@nuive nuive closed this as completed Jan 2, 2025
@endrift endrift linked a pull request Jan 3, 2025 that will close this issue
Scripting: Fix Lua 5.1/LuaJIT and Lua 5.2 problems #3376
Open
@endrift endrift reopened this Jan 3, 2025
@endrift
Copy link
Member

endrift commented Jan 3, 2025

Please don't close it until after the PR is merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Scripting: Fix Lua 5.1/LuaJIT and Lua 5.2 problems nuive/mgba
3 participants
@endrift @Aarushb @nuive