Blightmud / Blightmud

A terminal mud client written in Rust
GNU General Public License v3.0
216 stars 58 forks source link

The `blight` lua module. (Question to users/contributors) #161

Closed LiquidityC closed 3 years ago

LiquidityC commented 3 years ago

Hi,

As Blightmud has been getting more and more features added the api size of the blight module in lua has been getting larger and has a somewhat diverse responsibility. Lately the tts, core, msdp and gmcp modules have been added because I didn't want to grow blight any larger. However with the new modules they make blight feel slightly out of place with it's large API spread across multiple "areas".

What is your opinion on separating some of this into smaller modules? Off the top of my head I'm thinking about moving store and read from both core and blight into a more topical store module with methods for session and disk storage.

There's also some methods regarding logging that could be moved to a log module and a version() function that is a better fit for core.

Is this to invasive or do you agree? Opinions and discussion please. :smile:

LiquidityC commented 3 years ago

The intention is to have the blight module ready for the common use cases, triggers, aliases, timers as well as sending and printing output. In general it's there to interact with the mud.

xvxx commented 3 years ago

I play a Minecraft mod, CC:Tweaked, that adds Lua-powered computers and robots (turtles) to Minecraft. The APIs the mod adds in Lua are broken up into modules, like you are considering.

I love it. It makes finding documentation and functions really easy. I think even if you don’t play Minecraft or know the mod, if you look at the API docs you get a feel for what’s possible right away:

https://tweaked.cc/

They only use their blight-like module (cc) for helper functions, no actual mod functionality.

So I’m :+1: on splitting up the API, it’s been a nice experience for me in the past. I’d also be happy if you took it even further and had, like, trigger.clear_all() and friends.

LiquidityC commented 3 years ago

That's my thought too. Docs will be easier. Help files smaller. The change will require a major version change due to breaking changes. But I think it's better to take that step in order to avoid feature creep and ending up in an even worse scenario.

LiquidityC commented 3 years ago

Ok, below is roughly what I'm thinking will be the goal of version 3.0 (next release after 2.3 is out).

Anyone have any comments or input?

It's a pretty big overhaul and I'm prepared to tackle all of it on my own. But I would be very happy if anyone feels like joining in. I'll set up a full project with a board and bells and whistles and do the whole thing through PRs. Yes even I will send PRs. So if you just wanna review stuff. That's a big help too. There's no deadline for all this so if you want to join in, don't be worried about your availability. We'll just slog through it in the time it takes. It'll be ready when it's ready.

Version 3.0

Based on various discussions in various forums with all of you guys. This is my plan for Version 3.0

Breaking changes

  • Introduce a Lua object Line that mimics the current Line.rs but in lua. lua::Line::from(model::Line) blocker, needs doing first
  • Send all mud output lines to lua as Line objects. I'm thinking mud:add_output_listener(callback)
  • Send all user input to lua as Line objects. mud:add_input_listener(callback)
  • Break out the trigger functions from blight.rs and re-build it in pure lua using the new Regex module. (new triggers module)
  • Break out the alias functions from blight.rs and re-build it in pure lua using the new Regex module. (new aliases module)
  • Move the timer related code out of blight and put it in it's own module timers. This will have to remain a lua/rust hybrid because of the timer thread.
  • Move the logging related stuff from blight into a new logging module.
  • Move all the mud related stuff send, send_bytes, mudoutput, disconnect, connect, on(dis)connect to the new mud module mentioned above.
  • Move script related functions (load, reset) from blight to a new scripts module.

Note, the above should leave blight with only one function: output. I think that will still belong. Because it's about doing stuff with blight. Unrelated to any other system. Also people will most likely only use print now.

Think about

Here are some additional things that should be kept in mind if you're helping out with the next big version.

Naming

Some functions in blight currently have names that pertain to their responsibility. When creating a more logical module system some of these names will be stupid. If you feel that way then change the name. Eg. blight:start_logging() => logging:start()

Lua code

Some of these changes will break existing scripts we have in place. gmcp, msdp, macros etc. Make sure you check these. Simple find/replace will most likely suffice.

Testing

When writing the regex.rs module I introduced a new way of testing in that file. Before all tests for lua modules were done in the lua_script.rs which was ugly an not very unit testy. Since I/we will be making a lot of new modules. Writing as close to 100% testing as possible of those files is mandatory.

xvxx commented 3 years ago

The Version 3.0 plan looks great!

You mentioned it initially but I don't see store on the new list.

I don't feel too strongly about it, but I think I would prefer alias and trigger to aliases and triggers as module names. Mostly because they're shorter, but I also think trigger:remove() and alias:add() read nicely.

Is this an opportunity to add more modules? I wonder about the app settings, and maybe even access to the screen itself. Not sure if either would be useful but I thought of screen when looking at #131.

LiquidityC commented 3 years ago

Yes 'store' I forgot about those. They'll need a module too. This will certainly open up for more modules but I don't think we should add more to this pile so to speak. Perhaps settings could be the one exception. The long term goal is to move all the rust defined "commands" to lua making it as configurable as possible.