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

Untested on Linux and Mac systems #2

Closed authorblues closed 3 years ago

authorblues commented 3 years ago

Some features depend on using os.execute() to list and make directories, as well as deleting folders. This code checks what OS the system is running and tries to run the appropriate commands for those platforms, but it has not been tested.

authorblues commented 3 years ago

Relevant lines: https://github.com/authorblues/bizhawk-shuffler-2/blob/86a99da292aa439411f1214dbc7f7c7281300042/shuffler.lua#L21-L23 https://github.com/authorblues/bizhawk-shuffler-2/blob/86a99da292aa439411f1214dbc7f7c7281300042/shuffler.lua#L66-L70 https://github.com/authorblues/bizhawk-shuffler-2/blob/86a99da292aa439411f1214dbc7f7c7281300042/shuffler.lua#L106-L113 A warning that this last one involves an rm -rf on the savestates folder. I believe this line is safe, but I felt it important to point out.

dennisrijsdijk commented 3 years ago

@authorblues I'll be able to test this for you on linux soon, if you still want this.

authorblues commented 3 years ago

@dennisrijsdijk Yeah, just a basic confirmation of whether things work would be appreciated. If not, that'll probably motivate me to bust out a VM to debug it myself.

dennisrijsdijk commented 3 years ago

@authorblues I have not been able to test yet (will in the coming days) but I see one critical issue already. currently you're using -rf, which will force deletion and never cause a prompt, and presumably, if accidentally ran as superuser/root may cause essential folders to be removed. I would recommend instead to use just r (for recursive) or rd (recursive and remove empty directories)

https://man7.org/linux/man-pages/man1/rm.1.html

authorblues commented 3 years ago

Looks like there are a host of reasons why this might not be viable, not the least of which being that Lua may not be enabled in Bizhawk on Linux. Furthermore, if it were enabled, I have been told that LuaInterface doesn't have Windows versions, so it would have to use KopiLua, which has not worked with the shuffler on Windows at least, so I am not hopeful for Linux.

Anyways, closing unless this warrants further consideration. Thanks for all the insight offered.

YoshiRulz commented 1 year ago

Lua is enabled on Linux from 2.7, with the caveat of requiring KopiLua as LuaInterface tends to crash. I tried the 2.6.3+ branch (sans LuaInterface check) on a 2.9 dev build but it fails with

xdg-open: unexpected argument '/C'
Try 'xdg-open --help' for more information.
NLua.Exceptions.LuaScriptException: shuffler-src/.cmd-output.txt

which seems to be the result of os.execute-ing Command Prompt commands in BASH.

edit: Just to note, you would get cross-platform compat "for free" by migrating to an external tool. But that creates a new problem, the same as I noted with ScriptHawk: either plugins would need to be compiled or you'd need to manage a Lua interpreter instance from your tool.

kalimag commented 1 year ago

I tried getting KopiLua compatibility to work (on Windows) a long time ago, but gave up when I ran into fundamental deficiencies in the IO library (file:lines() being outright broken was one of them, iirc).

Another solution to the external tool/lua issue would be to run the plugin scripts in the regular environment and have them talk to the tool through client.gettool. I think you made this possible after I asked about it. But this would likely be rather fragile in practice.

dennisrijsdijk commented 1 year ago

I've been looking into the possibility of opening plugins (as .NET tools) from a main tool and communicate with those. It would require the tools to be pre-compiled as you said, but with enough documentation, it should be feasible. It looks like I can load tools with Tools.LoadExternalToolForm, and I should be able to then check if the main tool is available, and if it is, create an instance and attach to events. I need to work out a PoC of this.