mt-mods / mail

Mail mod for Luanti
https://content.minetest.net/packages/mt-mods/mail/
Other
14 stars 23 forks source link

Antispam features #128

Closed Athozus closed 9 months ago

Athozus commented 10 months ago

This is WIP. It follows suggestions listed in #102.

Note that it contains many changes, such as :

It has a dependency with merge request #127

I will rebase (with some squashes) because it has some commits that are technically independent from the antispam itself.

More details about changes, check commits messages.

screenshot_20231226_174319 screenshot_20231226_190759

Athozus commented 10 months ago

There are some problems with luacheck (I don't really know how to fix it) but the table.indexof feature is available since 5.0.0.

Niklp09 commented 10 months ago

(I don't really know how to fix it) but the table.indexof feature is available since 5.0.0.

Luacheck doesn't know some functions since they don't belong to the lua specification. You must add them manually to the read_globals table (table = {fields = {"indexof"}}).

Athozus commented 10 months ago

Thanks. All is ok for the review ?

Athozus commented 10 months ago

Until I do some fixes for that you can report, I plan to do the following steps :

S-S-X commented 10 months ago

Disable beerchat syncing until muting gets fixed

API should be fixed when https://github.com/mt-mods/beerchat/pull/116 is merged. If you want you could test it with upgrade-mute-bugs branch.

Athozus commented 10 months ago

API should be fixed when mt-mods/beerchat#116 is merged. If you want you could test it with upgrade-mute-bugs branch.

It successfully works, thanks ! I git force push to remove the latest commit, because it is soon merged, or I let this fix and re-enable sync when it is merged in beerchat master ?

Athozus commented 9 months ago

@S-S-X @BuckarooBanzay

I followed what I said, following the suggestion from Buckaroo to squash #127 (I haven't understood when I answered, but after reading again the message it was clear).

Here's my working tree. I want to be sure that it's not polluting master branch, but I'm quite satisfied of I what I did, so I consider one approval so one of you will be ok for rebasing.

S-S-X commented 9 months ago

Don't really want approve as long as direct player metadata reading or writing is included. There's anyway one approval already...

If you believe it wont cause issues when metadata is obsoleted and/or cleaned up then probably fine, just keep in mind that it is completely private data that is not considered being part of beerchat API and should be refactored soon to allow missing features.

Additionally just a reminder, I didn't read code well enough to be sure but it seems to loop over all players and then read/write their metadata. Is there a checks in place to make sure it wont try to do it for offline players, I think no which might cause crash if sync is called while any of the registered players is offline. edit. Did read a bit more and seems it just grabs names for offline players and wont try to use their meta which should be fine (but unnecessary cpu hog).