mt-mods / beerchat

beerchat mod for minetest
GNU Lesser General Public License v3.0
9 stars 7 forks source link

Refactor program flow for channel messages #83

Closed S-S-X closed 1 year ago

S-S-X commented 1 year ago

Combine:

beerchat.on_channel_message(channel_name, name, msg)
beerchat.send_on_channel(name, channel_name, msg)

so that beerchat.send_on_channel(name, channel_name, msg) handles both.

Also add new event handler before_send_on_channel which should be executed once before doing anything currently done by those functions.

Also switch from on_send_on_channel to new before_send_on_channel wherever possible, allow editing message details at once for all platforms.

Also doing this will cause duplicate remote messages from some mods but that's very simple fix: just remove all beerchat.on_channel_message(channel_name, name, msg) from other mods and only keep beerchat.send_on_channel(name, channel_name, msg). I think even while changing flow like that causes duplicate messages on remote systems it does make a lot more sense than current requirement for dual call and updating it is worth the trouble.

Underlying API is still available for special needs, doing this wont really take anything away but instead just simplifies every use case I know and brings tiny performance improvement through allowing to skip on_send_on_channel x connected players callbacks / message sent. Yes those callbacks are pretty well optimized and fast but still removing maybe 10 to 40 function calls per message delivered sound good.

S-S-X commented 1 year ago

If explicit high level local message function with automatic formatting and such is wanted then I think it would be best to make it clear for API, by that I mean adding new function that communicates intent very clearly so whole set would consist of following:

Anyway, new local chat limited function might not be needed really:

Remote-only messages have been used for few things, it very much seems like that is needed and that wont change. Just function naming is technical debt, not too bad. Local-only messages have not been used that much, I think minetest.chat_send_player(name, message) been enough so far in most cases.