Basic Lua Scripting Support #1671
Conversation
nadiaholmquist
force-pushed
the
master
branch
3 times, most recently
from
e6c2fb6 to
df0ab90
Compare
src/frontend/qt_sdl/LuaMain.cpp
Outdated
| luaThread = new LuaThread(); | ||
| connect(luaThread,&LuaThread::signalPrint,console,&LuaConsole::onGetText); | ||
| connect(luaThread,&LuaThread::signalClearConsole,console,&LuaConsole::onClear); | ||
| connect(buttonPausePlay,&QPushButton::clicked,luaThread,&LuaThread::luaTogglePause); |
There was a problem hiding this comment.
placement of spacing here and in a bunch of other places.
|
Still some more changes needed but majority of the refactoring is done, no more LuaThread, the Lua state is created and run by the emuThread. This should hopefully address any concerns around potential race conditions. I plan on cleaning up other issues on Sunday. |
|
I think this should be good for another review, I believe I have addressed all the major issues that have been brought up. If there is anything else that I need to change or fix please let me know. There are still more features / functions I'd like to add, but this is the kind of project that could easily lead to feature creep, so I'm gonna cut myself off here. |
|
there's probably still some more things, but this is what I found from a glance. |
|
OK addressed those issues, still some bugs I found that I need to fix, I plan on working on that tomorrow. |
|
OK ready for another review @RSDuck when you have time. |
|
OK went over everything, double checked to make sure the spacing was good. Fixed spelling errors in the markdown. Will probably need someone else to take a look at the merge conflicts at least for the .yml and CMakeLists files. |
|
blah something went wrong while resolving the merge conflict. |
|
hm I don't really understand how dependencies are managed on macOS, I can't find anything on how it's installed in the code. |
nadiaholmquist
force-pushed
the
master
branch
from
10c70eb to
a7575ec
Compare
|
To the person who talked about this PR on our IRC: If you are reading this, we are very sorry, the ban was due to the anti spam bot bot for some reason flagging your messages. Let us know how you can communicate again with us. |
|
adding on Generic's message: I removed the ban, so you can come back to the IRC. sorry about it. |
|
Hey sorry I have been kind of a ghost for a while! It's been over a year since I started this fork, I'm not super up to date on all the changes since then. I'd like to get this project working again with the current version, since I am able to continue working on it now. |
|
there have been a ton of changes on the frontend especially with regards to multi instance within one process support. At this point it would probably smarter to not rebase/merge and instead start fresh while moving over the more independent parts like the Lua interface. |
|
Understood! I'd prefer that option honestly. Most of the work was on the Lua interpreter side of things, I tried to keep the front end stuff to a minimum while working on this since I figured I'd probably need to remake the front end entirely at some point. So I won't be loosing much. Also gives me a chance to refresh my memory on how everything works. I'll make a new fork later today, once that's up I'll add a link to the new one and close this pull request. |
Hello, sorry I just saw this. I don't really know how to use IRC, and I just saw you guys added a link to the discord so I'll probably stick to that since it's more familiar. I actually started a very rudimentary merge of this branch to main. (It definitely doesn't work, I was just using the merge conflicts to track which parts of the code base I hadn't yet looked at.) Because I've been busy IRL, I haven't touched anything in about 2 weeks, but before that I was mainly trying to get an overview of all the changes made externally to the Lua scripting module itself (i.e. what kind of integration was even necessary to begin with) as well as trying to understand the general structure of the codebase (and also what pieces got refactored, which were many).
I don't see the new fork yet on your profile, but I'll push the half-merge (and some of my notes) I've got so far to mine if you want to take a look (I was in the middle of figuring out how/where to associate a Lua scripting engine for each instance, before starting to refactor the Lua module as a class that could have multiple instances). my fork with the partial merge |
|
I'll be sure to take a look at your notes, although I think for right now my goal is simply build that can get a Although yes, one of the first things that will need to be figured out is how to handle multiple instances, since that is the biggest change from back when I first started. I should be able to work on it some today, I originally wanted to start on this like 2 weeks ago but my availability has been... to keep it simple, rather unpredictable as of recently. Hopefully I will have the time to get a good start this week, however, just so everyone is aware, my progress will likely remain somewhat scattered for now. I'll be sure to post a link to new fork here once I can get, at the bare minimum, "hello world" working on a single instance so we can have a starting point. In the meantime feel free to ask me any questions! Also thanks again to everyone for the help and patience. |
|
Hello, any update on this project ? Thanks ! |
|
Small update for those interested, (thanks to everyone who reached out about this) For those who have reached out asking if they can help in any way, I only ask for a little more time to get a minimal viable version working that is compatible with the recent updates to melonDS. Once that basic frame work is setup, I will be sure to let everyone know, and will then gladly appreciate any help! |
|
Hype! |
|
Finished testing on Linux and Mac using a VM, everything seems to run as expected on Linux, had to change the default font for one of the drawing functions, since it seemed to not work on Mac. I wrote up a simple Lua script and placed it under tools to test all the different functions, everything seems to be working on all platforms! With that finished that's everything on my to-do list crossed off, (sans better documentation) so really all that's needed now is to get someone else to give it a look over and let me know what needs to be improved / changed. |
There's another gotcha that i noticed and you should be wary about (and is present in yoru code), lua errors in a c function will corrupt the stack if there are objects with constructors present on the stack frame because lua (unless compiled as c++) uses longjmp/setjmp to handle script errors. This is not an issue on windows because on there longjmp also calls destructors, but is an issue on every other platform. In this pr the only function exhibiting this issue is drawImage melonDS/src/frontend/qt_sdl/LuaMain.cpp Lines 589 to 595 in bc40d89 If you pass a valid string but invalid other arguments, that QString won't be properly destroyed, so you should move its creation after all the lua_check functions have been executed (since in your case you're not calling anything else that would call into lua and possibly raise lua errors) |
|
@edo9300 The last push should have resolved the issue you pointed out. I also took a look at using a C++ compiled version of the lua library, but as you already mentioned on discord since we aren't shipping it as a statically linked library we can't assume that a C++ compiled version of lua will always be available... There may be a way to setup Cmake to compile lua from source ourselves, or something to that effect, but I don't really think that the benefits of doing so would outweigh the added complexity, at least not currently. |
Created a basic Documentation of currently implemented lua functions.
|
Finished writing up the documentation for the currently implemented Lua functions. |
|
Please make the use of whitespaces consistent with the rest of the code. |
Round one of clean up
Round 1-2 of code clean up
Pass 1-3 should be ready for another round of review.
Sorry about that, thanks for the reminder @RSDuck! Let me know if there is anything else I need to go back and take a look at! 😄 |
Renamed "MelonPrint" to just "print", TIL you can redefine pretty much any lua funct... Also fixed the lua console not re-opening after closing it the first time.
Switched running the LUA script from the emuThread to the main window / main GUI thread... Should have just done this from the start, this makes things much simpler to implement, and will make future planned GUI functions much easier to add.
Made for loop more readable in Screen.cpp Removed include for LuaMain in EmuThread as it is no longer needed. Cleaned up some whitespace / old commented out code.
|
With that, I should be ready for another review. If @RSDuck or someone has time this weekend to look over it, I'd appreciate it! |
|
Is there any way we can get a compiled version of this branch in the releases page on this repo in the meantime? |
|
Also, could you upload some examples of how to use it? |
|
@gaithern @AntonioND , made a test release of the current build, you can find the current documentation under the ./tools/LuaScripts/ folder here. let me know if you have any trouble! As of now the currently implemented Lua functions are still rather bare bones, most of the work so far has been just setting up the foundation. If there are specific features / functions that you would find useful to have that aren't implemented yet, then let me know and I can prioritize adding those features next. I agree with RSDuck about waiting until after the 1.0 release before we think about merging this onto main, It simply doesn't make sense to worry about something that's still very much in the experimental phase like this with such a major release on the horizon. That being said I do plan to continue working on this in the mean time. 😄 |
Changed KeyboardMask to HeldKeys function. Changed filesystem::current_path to QDir::setCurrent to avoid use of 'c_str'...
Here is a proposed starting point to adding Lua script support for MelonDS natively. Currently has bare minimal functionality, as I would like to have others review my work before making to many additions. This is my first project in c++ so please let me know if there are any errors, or parts that I need to re-do. The aim for this is to support accessibility features and trackers, not as a comprehensive debugging tool.