Add support for coroutines#955
Add support for coroutines#955vadi2 merged 7 commits intoMudlet:developmentfrom ahmedcharles:luastate
Conversation
|
Hey you might be implementing https://blueprints.launchpad.net/mudlet/+spec/coroutines-for-c-api with this. Gonna test. |
|
Yeah, unintentionally. My main reason for doing it is that I really want to split up the lua api functions into multiple files and doing that currently would be a huge pain. |
|
You did. The following test works:
On a Mudlet that doesn't have this improvement, it just locks up. Well this'll be super-awesome news to a super-small subset of people! I'm included so I'll be happy. |
|
Does 5.1's library functions support coroutines? Like, yielding in the comparator of a sort? |
|
We'll need to test this with all of Mudlets functions so expect the approval to take a bit - but it'll be my priority to do so, as this is a pretty great change. a) gets rid of the boilerplate and b) allows calling Mudlet API from coroutines. |
|
I've updated the title of the PR to better reflect the really awesome feature that's happening here. |
src/TLuaInterpreter.cpp
Outdated
| @@ -13723,3 +13439,19 @@ int TLuaInterpreter::startPermSubstringTrigger(const QString & name, const QStri | |||
|
|
|||
| } | |||
|
|
|||
| static int host_key = 0; | |||
|
|
|||
| static void store_host_in_lua_state(lua_State* L, Host* h) { | |||
There was a problem hiding this comment.
🤔 Would it be applicable to store/access any other data like this? What are the pros and cons of this technique?
If it works (and you must think it does) then 👍, it is just that the Lua-fu is not string in me so I cannot really judge... 😉
There was a problem hiding this comment.
Not only do I think it works, I know it works.
The registry is explained: https://www.lua.org/manual/5.1/manual.html#3.5
Just run:
for k, v in pairs(debug.getregistry()) do print(k, v) endBoth before and after this change.
SlySven
left a comment
There was a problem hiding this comment.
I like this, I do not really understand it but I like it (which is why I cannot say yah or nay and have to leave it to others).
|
Needed to resolve some merge conflicts. |
|
The PR is ready but without any testing progress for two weeks which is unacceptable so I'll drop the idea of adding all tests to Squish as I go and just test functions on the fly. I've updated the first comment to keep track of which functions have been tested. Help would be appreciated! |
|
I've got #1006 while testing, but I can't reproduce it anymore, so just something to watch out for. |
|
@SlySven while I am working through testing this, are you able to review it as well and give your tick? |
|
Vadi2 wrote:
Um, see above, where I put:
I really do not feel confident that I understand what is happening here - so I do not think I can offer any input - sorry... 😐 |
# Conflicts: # src/TLuaInterpreter.cpp
|
Merging in for the time being since all of the functionalty is fine and most of the time is spent improving our documentation instead I went through every function. |
I'm not a fan of camelcasing too much but it is the C++ style and consistency is important!
This makes it easier to write functions which can be independent of
TLuaInterpreter.
Get Mudlet from this branch, test each function in a normal and a coroutine call, and if everything works tick if off this list. Here's a template for calling from inside a coroutine: