monk-afk / filterplus

Chat filter and censor for Minetest. Includes mute command, player tags, and mention highlight.
MIT License
1 stars 0 forks source link

provide an API for mods #14

Closed boulbousNaranha closed 2 months ago

boulbousNaranha commented 2 months ago

Hello! i have a feature request so that other mods can use integrate filterplus by exposing an API for them to use. the API im suggesting is very similar to how https://content.minetest.net/packages/sofar/filter/ does things, but its very old and dosent provide a sane default wordlist. the main functions i'd expect are

filterplus.check_text(text) checks the text for swears the first return value is a bool indicating whether the message is allowed the second return value should be the positing of where the bad word starts, possibly return a list of number pairs that show where each swear begins/ends instead

example

local success, swears = filterplus.check_text("frick you witch")
-- success = false
-- swears = 
-- {
--   {1, 5},   -- where "frick" starts and begins
--   {11, 15}, -- where "witch" starts and begins
-- }

also add a way for mods to register callbacks for when players violate

filterplus.on_violation = {}
filterplus.register_on_violation(function(player, message)

end)

other APIs could be provided as well (adding/removing words to blacklist etc), but these are the most important ones

i think the best way to go about doing this is by splitting this core functionality into its own mod filterplus_lib since currently filterplus provides much more than just chat filtering, which can get in the way of mod makers (as it does in my usecase). and make filterplus_lib a dependency of filterplus

filterplus_lib should only implement the APIs, here are some things it shouldnt do

not sure how checking for link URLs and all caps messages fit into this. they should be configurable at least

for sake of completeness, here is how my usecase looks like. im making a mod that completely reimplements everything that has to do with the chat. its comes with a filly featured interpreted language inspired by POSIX shell and minecraft syntax for chat commands, that provides a base API for defining chat commands that are more powerful and easier to write. with that base im also implementing all the commands a user would ever want, reimplementing most builtin commands to use my much more powerful API, this includes moderation tools like muting/banning/blocking etc and i would like to optionally include chat filtering, but i dont want to reinvent the wheel and filterplus adds duplicate features like muting that would break things for me

i cant personally help implement this but i can provide feedback from the perspective of a modder if you decide to implement this

sidenote: on contentDB filterplus has the "API/liberary" tag and says that it provides an API in the description, even though it dosent. my guess is that this was planned but later forgotten

monk-afk commented 2 months ago

Hey, thanks for the suggestion. There is currently a callback for other mods to plug into this, on line 170:

filterplus_api = {}
function filterplus_api.check_word(string)
    local filter_string = filter_message({true, string})
    if filter_string == string then
        return filter_string, false -- not censored
    end
    return filter_string, true
end

It'll take the entire string to be checked and will return both the string (either censored or not), with true or false. The API will also skip the chat message features, like ranks and mention highlighting, allowing for example:

image

Let me know if this is close to what you're looking for!

boulbousNaranha commented 2 months ago

i totally missed that, my bad

this is basically everything i need but it would be nice to have an API to add/remove words from the blacklist and the whitelist

though there is still the issue that filterplus overrides the default chat behavior, which breaks my mod. in my mod chat messages starting with "!" invoke the shell interpreter, but filterplus captures the chat message before my handler and interprets it as a chat message

i could technically rearrange the contents of minetest.registered_on_chat_messages but this is error prone and hacky, this is why im suggesting for filterplus to be split into filterplus_lib that can be used by other mods without registering minetest.register_on_chat_message and other unrelated things to filtering

on a unrelated note, i also took a quick look at the source code and found some weird things that could def be improved in terms of code quality, so here is some feedback

msg_block is a highly confusing, instead of being an array that looks like this

{"joey", "hello joe_doe", {"joe_doe"}}

it should be a dictionary so you dont have to trace the spaghetti to know which index is what (took me like 5 mins)

{name = "joey", message = "hello joe_doe", mentioned_players = {"joe_doe"}}

this also includes not doing hacks like how filter_message() normally returns nil but if you set msg_block[1] to true instead of a player name, it returns the censored word

another weird thing is those seemingly random function calls in a return statement (example: line 127, 141, 167) why are they in a return statement? they all return nil, i dont get it

blacklist is obsolete. its only ever used to check if a word is blacklisted, why not arrange your data like this instead

bpatterns = 
{
  ["frick"] = "(%S*[fF]]+[%s%p]-[rR]]+[%s%p]-[iI]]+[%s%p]-[cC]]+[%s%p]-[kK]]+[%s%p]-%S)",
  ["wack"]  = "(%S*[wW]]+[%s%p]-[aA]]+[%s%p]-[cC]]+[%s%p]-[kK]]+[%s%p]%S)"
}

last thing is that a lot of the functions do way more than the name implies mentioned_players() implies that it just updates msg_block[3] to be accurate with msg_block[2], but it also call send_message() same for filter_message(), it calls mentioned_players() (which in turn calls send_message()) unless you pass a bool value as the player_name these things shouldnt be coupled at all

if you're open to reviewing code, i could clean up the code since i already had to reverse engineer it

monk-afk commented 2 months ago

this is basically everything i need but it would be nice to have an API to add/remove words from the blacklist and the whitelist

Yes there is currently a command which does this already.

msg_block is a highly confusing, instead of being an array that looks like this

{"joey", "hello joe_doe", {"joe_doe"}} it should be a dictionary so you dont have to trace the spaghetti to know which index is what (took me like 5 mins)

{name = "joey", message = "hello joe_doe", mentioned_players = {"joe_doe"}}

Arrays are faster than index tables, or as you say "dictionaries".

There's a total of 3 (three) values in that table, and the first two are inserted from the initial function that creates the table.

It sounds like you're having difficulties keeping track of a single table with three elements; that is beyond my ability to help you with.

this also includes not doing hacks like how filter_message() normally returns nil but if you set msg_block[1] to true instead of a player name, it returns the censored word

filter_message() does never returns nil. You're mistaking the inner loop of the gsub function for the return value of filter_message().

another weird thing is those seemingly random function calls in a return statement (example: line 127, 141, 167)

Here is some literature for you to start your research: https://www.lua.org/pil/6.3.html

blacklist is obsolete. its only ever used to check if a word is blacklisted, why not arrange your data like this instead

bpatterns = { ["frick"] = "(%S[fF]]+[%s%p]-[rR]]+[%s%p]-[iI]]+[%s%p]-[cC]]+[%s%p]-[kK]]+[%s%p]-%S)", ["wack"] = "(%S[wW]]+[%s%p]-[aA]]+[%s%p]-[cC]]+[%s%p]-[kK]]+[%s%p]%S)" }

Yes, the code is currently accomplishing this. If you want to manually rewrite each pattern by hand, go right ahead.

last thing is that a lot of the functions do way more than the name implies mentioned_players() implies that it just updates msg_block[3] to be accurate with msg_block[2], but it also call send_message() same for filter_message(), it calls mentioned_players() (which in turn calls send_message()) unless you pass a bool value as the player_name these things shouldnt be coupled at all

if you're open to reviewing code, i could clean up the code since i already had to reverse engineer it

Since you've got things all figured out, feel free to create a fork off the codebase