Infrastructure: Editorial fixes to a few Lua API error messages #6871
Conversation
|
Hey there! Thanks for helping Mudlet improve. 🌟 Test versionsYou can directly test the changes here:
No need to install anything - just unzip and run. |
| @@ -2598,7 +2598,7 @@ int TLuaInterpreter::setExitStub(lua_State* L) | |||
| } | |||
| TRoom* pR = host.mpMap->mpRoomDB->getRoom(roomId); | |||
| if (!pR) { | |||
| lua_pushstring(L, "setExitStub: RoomId doesn't exist"); | |||
There was a problem hiding this comment.
It seems strange that we are not reporting the value that roomId was given! Tthough if we fix this so it does, we should report the original argument passed as the first argument and NOT what getVerifiedInt(...) returned.
There was a problem hiding this comment.
Because the "number" that the user entered might not exactly be an "integer" like wot we wuz expecting, or could contain extra things (like commas that are not parsed as the user might expect) e.g. 1,200 is handled not as 1200 but as 1 and 10.6 is handled as 10 not 10.6 or 11, conversely 1.23e2 is processed as 123 but if there is a problem that is the value that will be shown and not what the user entered.
There was a problem hiding this comment.
I don't think we do it like this anywhere else in the API... has this been a problem that has been affecting people a lot?
There was a problem hiding this comment.
I think I started doing it like that for one of my PRs recently, ah yeah, it was #6401 where I added TLuaInterpreter::getVerifiedStringOrInteger(...) which uses lua_tonumber(...) but then applies qRound(...) to convert the double the lua function returns into an integer (and avoid using lua_tointeger(...) because {IIRC} on Windows lua_tointeger(...) returns a long int that may not fit into an int on that platform which the built-in Qt code analyser moans about...!) As such, when then reporting an issue with the argument supplied it has to refer to the passed value rather than what it might have gotten mangled to...!
😀
There was a problem hiding this comment.
Is this addressing a real problem people have been running into though?
There was a problem hiding this comment.
I think I will skip doing this change, I was just looking to do a simple fix and there is a lot of other higher priority things to be working on.
…et#6871) <!-- Keep the title short & concise so anyone non-technical can understand it, the title appears in PTB changelogs --> #### Brief overview of PR changes/additions Editorial fixes to a few Lua API error messages: lowercase after the function name `:` and no period at the end of the message. #### Motivation for adding to Mudlet Consistency and professionalism #### Other info (issues closed, discussion etc)
Brief overview of PR changes/additions
Editorial fixes to a few Lua API error messages: lowercase after the function name
:and no period at the end of the message.Motivation for adding to Mudlet
Consistency and professionalism
Other info (issues closed, discussion etc)