search

authorblues / bizhawk-shuffler-2

A script to randomly shuffle between games played in Bizhawk, with plugins to enhance the experience
MIT License
58 stars 21 forks source link

NLua/KopiLua compatibility workaround #12

Closed kalimag closed 3 years ago

kalimag commented 3 years ago

The shuffler script currently does not work when NLua/KopiLua is enabled in BizHawk. This PR adds a workaround for a bug in KopiLua to make it work.

KopiLua is deprecated and shouldn't be enabled by default, so not supporting it is reasonable. But apparently some people still have it enabled, knowingly or not. The workaround doesn't complicate the rest of the code and on LuaInterface it just falls back to the default implementation. Also, this fix adds prime real estate for code comments complaining about Lua in BizHawk.

If you prefer not to support KopiLua, I recommend refactoring the RECOMMENDED_LUA_CORE check to happen earlier in the script. Currently it happens after the plugin initialization, which calls get_dir_contents and triggers the bug, so the script crashes before it can print the warning.

If you're curious, the problem is that the implementation behind KopiLua's file:lines() (as well as other line-reading functions) is flat out broken. It does not detect the end of file nor does it correctly detect when the string buffer is full, and instead keeps trying to read until it overflows the buffer and causes an exception, which then results in a nonsensical error message referencing a random file or string (?). Additionally, it only trims \n but not \r from the end of the line.

kalimag commented 3 years ago

Nevermind, path_exists also does not work on KopiLua, which causes make_dir to fail and the shuffler fails to work when freshly set up.

authorblues commented 3 years ago

Thanks for trying to address this. I'm not sure if there is any specific reason to want to support Kopi (stability?), but the setup instructions recommend ensuring that Lua+LuaInterface is enabled. I wonder if the warnings about memory leaks are mitigated by the fact that this script is a bit unique in the way that it repeatedly (and unavoidably) restarts.