You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
pass original argument of require call to executed chunk (#85)
Fixes#83
Quoting SWeini:
How the "fix" works:
The first commit makes the argument to `require` available to the executed chunk of code. The require guards introduced by flib rely on the `...` being populated with the argument that was passed to the require call.
The second commit reverts some de-duplication on YAFC-side which made something similar to what the require guards were doing. The caching in `required` dictionary now happens based on the argument, not on the resolved filename. Previously the require guards in flib didn't play nicely with the de-duplication happening in YAFC.
Findings along the way:
- There is not a single `luaL_unref` call in the codebase. Instead the whole lua context is disposed after loading everything.
- The `LuaContext.Require` method looks very hacky. Relying on the stacktrace (`luaL_traceback`) to load relative files is kind of weird, but it works. I'm not sure how much the logic and search priorities mirror what Factorio is doing. But it worked so far.
- `LuaContext.Require` has a weird line: `required[...] = LUA_REFNIL;` just before executing the code might not be exactly what should happen. The documentation says that it stores `nil` afterwards, and only if the executed chunk didn't write to `package.loaded[modname]`. We don't have `package.loaded` so that doesn't matter, but the unfixed caching should have ended up in a stack overflow instead of just returning nil. Was this assignment added to "fix" stack overflows previously?
- The more I looked into this part of the code the more I was astonished that it does work most of the time. I still think that going to use --dump-data and read everything from json would remove a lot of complexity and a whole category of bugs.
0 commit comments