Add: setConfig and getConfig functions #5452
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. |
|
Haven't reviewed your PR yet (will do so later), but I'm favour of the idea. We previously discussed the design here so will definitely work with you on getting this out! |
|
Ah, great, thanks thanks that reference. I didn't see that and looks like my idea is similar to yours. IMHO we should add only a subset of settings - as I said above, the things that are not likely to be user-preference (like font size for example). Basically focus should be put into the config variables that script makers most care about. The documentation for this function will hold a table simmilar to the above, explaining what key changes what thing, what is the expected value type, value ranges etc. Along with some notes on how to use given property, if there are any specific informations regarding given key worth mentioning. |
|
I can just about guarantee there'll be requests for settings we didn't think were worth putting in at first but eventually enough people ask for we feel like we have to add it. I would perhaps just go ahead and include most everything that doesn't seem like a security risk |
|
inputLineCommandSeparator: IIRC We have previously discussed having the ability to set the command separator - and we came to a strong consensus that this should NOT be settable via scripts. As it is, if the user or a script wants to send several commands at once they should instead use the |
|
mapperShow3dView: this one will have to be gated with a build-time |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
GuyD from the #includecpp discord suggested a couple of improvements to getting rid of the wall of It's still on my list to review the PR for what it adds |
|
/refresh links |
| host.mUSE_UNIX_EOL = getVerifiedBool(L, __func__, 2, "value"); | ||
| return success(); | ||
| } | ||
| if (key == "inputLineCommandSeparator") { |
There was a problem hiding this comment.
Lets leave it out for now in order to get the PR through in a timely manner. See #3677 for discussion.
| host.set_USE_IRE_DRIVER_BUGFIX(getVerifiedBool(L, __func__, 2, "value")); | ||
| return success(); | ||
| } | ||
| if (key == "specialForceCompressionOff") { |
There was a problem hiding this comment.
This is duplicated 3x in the description of the PR
| host.mMapperShowRoomBorders = (getVerifiedBool(L, __func__, 2, "value")); | ||
| return success(); | ||
| } | ||
| if (key == "generalEnableGMCP") { |
There was a problem hiding this comment.
I wouldn't include general, just enableGMCP is enough.
| @@ -15835,3 +15836,106 @@ int TLuaInterpreter::getMouseEvents(lua_State * L) | |||
| } | |||
| return 1; | |||
| } | |||
|
|
|||
| int TLuaInterpreter::setMudletConfig(lua_State * L) | |||
There was a problem hiding this comment.
The more concise setPreference sounds nice to me - as well as the ability to parse a table for keys, so you can use one call to set everything. It's a neater API design. Need help with parsing the table from C++ side?
There was a problem hiding this comment.
Second this API decision, being able to batch it by passing a table of all the things to set would be great.
| } | ||
| } | ||
|
|
||
| lua_pushboolean(L, false); |
There was a problem hiding this comment.
Also need to mention in the Debug console what was changed, so scripts aren't sneakily changing things from under you - example https://github.com/Mudlet/Mudlet/blob/development/src/TLuaInterpreter.cpp#L13258-L13260
|
Thanks for all the comments, I had to switch to some other stuff for some time, but I'll get back to this soon. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
I'll adopt this PR, so we can get the config function into Mudlet. |
vadi2
changed the title
|
I don't have the rights to push to your fork @ktunkiewicz so I'll continue the work in a separate branch & PR, see #6074. |
NOTE: Currently this is work-in-progress PR with only the setMudletConfig added - I want to see a feedback on this before I continue the work
Brief overview of PR changes/additions
setMudletConfig(key, value)that allows changing various Mudlet settingsgetMudletConfig(key)that allows reading various Mudlet settingsMotivation for adding to Mudlet
People are lazy. They don't read on-screen instructions carefully and asking to read Readme on github repo of your scripts is usually a waste of time - I'm pretty sure a lot of script creators will tell you that :)
On the other hand, all games are different - they often originate from code made in 80's, and for each game you need a specific config values to be set to make the game work well.
All maps are different, but they usually have one thing in common - they look best with specific settings that the map creator uses. This is most important when there are images and symbols used on the map.
The proposed change is to allow setting various Mudlet settings that are important in these situations. Below is the list of settings it allows changing, and a brief explanation why I think this should be allowed to change from inside scripts.
NOTE: I selected only the settings I believe are safe to allowed being changed on behalf of the player that are game-specific and usually are not opinionated settings that each player sets to what they prefer.
NOTE 2: Please remember that a vast majority Mudlet users are not script makers nor map editors. They just want a nice game experience that someone else prepared for them. People who know how to edit scripts/maps will understand the fact that a script changed some settings because they know how this works, what these settings do, and how to modify scripts not to do this if it annoys them. So, please read the "explanations" I gave from a point of view of typical player.
;in their commands, or as a command itself. For almost all of the players on my MUD that migrated from zMUD the first question they made on our discord was "hey, why the;stopped working for my when using Mudlet?". Presetting this to something else automatically would be a nice experience. This is a setting that is usually a user-preference but I find it really important to be able to preset it, just because of how many people asked me about this, not knowing how to change it. That would be responsibility of script-makers to inform player that this has been changed for them, or to ask them if they want that changed, before changing itWhat's your thoughts on this? Do you think this is good idea?