Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chat #463

Closed
wants to merge 10 commits into from
Closed

Chat #463

wants to merge 10 commits into from

Conversation

Nico8340
Copy link
Contributor

@Nico8340 Nico8340 commented Jan 27, 2024

Chat

This Pull Request removes chat management from all game modes and adds a new, separate solution that is highly configurable and customizable, with automatic moderation by today's standards.

Fixes #460
Follows #461

Preps

  • Remove onPlayerChat handlers from gamemodes
  • Remove onPlayerChat handlers from admin and admin2
  • Remove onPlayerChat handler from playercolors

Tasks

  • Implement message sending
  • Implement message logging
  • Implement chat filter config
  • Implement chat filter
  • Implement sanctions
  • Implement anti-spam
  • Implement anti-repeat

Backlog

There was discussion that it would be nice to include a user interface in the existing admin panel to configure moderation. This will be implemented optionally after the later works.

@Dutchman101
Copy link
Member

Dutchman101 commented Jan 27, 2024

Copying over my proposal from the previous PR..

How about some advanced anti-spam features, like:

  • Configurable limits with action, like: mute, kick, or ban player for x duration if they repeatedly get flagged for spamming. Mute is the first line defense, to store serials of repeat offenders for persisting spamming rap sheet, an SQLite .db can be used. The more times they get a lighter automatic punishment (mute, kick), the sooner it will escalate to a more severe punishment, Mute > kick > 30 mins ban > longer bans as defined in meta.xml. In this case, also add a toggle for automatic .db clearing (choice between interval or to do it on server launch)

Measures against coordinated spamming, like:
Multiple players are sending the same or similar message (not a single source, but it'll be big) or different ones; measures like anti-repeat or chat limits per player won't be effective

  • If chat activity exceeds a certain amount (specified in meta.xml) of messages per the polling window (specified in meta.xml, default: 10 secs), an abnormality has started and the system will offer a meta.xml toggle-based resolution: slowmode (limit chat messages globally, from anyone.. and return "You need to wait, slowmode has been activated due to possible spam" to anyone that tries to talk while it's still active

Server owners can tweak the # of messages/polling interval based on what's normal for their server. For example, if it's a gamemode some people can get hyped up and send a burst of happy messages, like end-of-round, they would need to make the polling interval longer. If the server gets more popular (leading to increased chat activity), they would need a higher message count tolerance.

So a spamming incident can be resolved without the presence of a server admin. If one is present when slowmode is activated, they should be able to use a command to lift it if they go to manually resolve it (like: ban the coordinated spammers)

Bonus points: every time slowmode has been applied, automatically send a "suspected spamming incident report" into the admin panel reports list (submitter: not player, but system) eventually with a recap of the chat lines ( + author) around the time it got triggered. Using the same functionality that already captures a player's last chat messages if they got reported with /report & selected in the target player dropdown.

@Dutchman101
Copy link
Member

In the upcoming code reviews (when all commits are added to the new PR), there should be extra attention for conditions/optimal branching of the code in onPlayerChat, to ensure that there's no grave performance consequences that comes with complexity; ideally, most chat messages (to say: ones that don't trigger any detection or additional checks) shouldn't take much more CPU time/be slower than 'clean' onPlayerChat handlers present in various gamemodes before this.

I didn't review or test this, but we need to keep our eye on this aspect, it has to be checked off on when the PR is in its final state.

@Nico8340
Copy link
Contributor Author

A few people said it during the previous PR and also on Discord, they think it's not a good solution to implement the chat in the admin resource, since quite a lot of people use the admin resource in their own projects, so they need to turn off the chat or change the code if the latest version is used. In addition, if we want to implement all of this in the admin, two resources must be managed at the same time.

What do you think, wouldn't it be easier to create a new resource called 'chat' for this?
@Dutchman101

@Dutchman101
Copy link
Member

Dutchman101 commented Jan 27, 2024

What do you think, wouldn't it be easier to create a new resource called 'chat' for this? @Dutchman101

yes. Then also output a console warning if one of the gamemodes are running (that now rely on the chat resource) but the chat resource isn't.. or even preserve the original chat handlers in those resources, but attach/detach them based on whether or not the new chat resource is in use; this will provide server owners the freedom which road to walk entirely. It can also be compounded with an opt-in or opt-out (meta.xml) above the running state of 'chat', and since the old chat handlers containin the old code will be present as well, it can just fall back on it. Any server owners that run into issues can conveniently return to the old behavior.

@jlillis
Copy link
Contributor

jlillis commented Jan 31, 2024

A few people said it during the previous PR and also on Discord, they think it's not a good solution to implement the chat in the admin resource, since quite a lot of people use the admin resource in their own projects, so they need to turn off the chat or change the code if the latest version is used. In addition, if we want to implement all of this in the admin, two resources must be managed at the same time.

What do you think, wouldn't it be easier to create a new resource called 'chat' for this? @Dutchman101

I think if you are going to go forward with these expanded features it makes sense for it to be in a separate resource. However, I would personally like to see a section in the admin2 panel added to interface with chat and manage these settings in a way that gives appears to the user that it is part of the admin panel rather than an add-on. I think that can be covered as a separate scope of work in a later PR.

@Nico8340 Nico8340 changed the title Improved Chat Chat Feb 2, 2024
@Nico8340
Copy link
Contributor Author

Nico8340 commented Feb 4, 2024

I think if you are going to go forward with these expanded features it makes sense for it to be in a separate resource. However, I would personally like to see a section in the admin2 panel added to interface with chat and manage these settings in a way that gives appears to the user that it is part of the admin panel rather than an add-on. I think that can be covered as a separate scope of work in a later PR.

It's a good idea, I'll think about it after completing the remaining tasks.

@Fernando-A-Rocha
Copy link
Contributor

Fernando-A-Rocha commented Jun 25, 2024

Ok so a new chat resource needs to be created with Meta xml settings to manage the moderation and spam features.

I don't see the need for a special panel in admin2 to control the settings because the admin panel already has a resource settings editing panel. You just double click the resource name in the list and it shows a pretty list of settings.

@Fernando-A-Rocha
Copy link
Contributor

I'm willing to finish this asap if y'all agree with above ,

@jlillis
Copy link
Contributor

jlillis commented Jun 25, 2024

I don't see the need for a special panel in admin2 to control the settings because the admin panel already has a resource settings editing panel. You just double click the resource name in the list and it shows a pretty list of settings.

My argument here is that because the chat is a "core functionality" and this resource is replacing functionality that was previously in admin, so it would be convenient for admins to have access to these settings directly in the main part of the admin panel rather than having to dig into resource settings.

I would be fine with omitting it for now. Perhaps it could be saved as a new feature just for admin2 in the future.

@Fernando-A-Rocha
Copy link
Contributor

I don't see the need for a special panel in admin2 to control the settings because the admin panel already has a resource settings editing panel. You just double click the resource name in the list and it shows a pretty list of settings.

My argument here is that because the chat is a "core functionality" and this resource is replacing functionality that was previously in admin, so it would be convenient for admins to have access to these settings directly in the main part of the admin panel rather than having to dig into resource settings.

I would be fine with omitting it for now. Perhaps it could be saved as a new feature just for admin2 in the future.

For the sake of pushing progress to fix the chat bug ASAP, I think this main panel for configuring chat should be done in another PR for admin2 indeed.

@Nico8340
Copy link
Contributor Author

I'm willing to finish this asap if y'all agree with above ,

I can continue working with it from today, so I will finish it soon.

@Nico8340
Copy link
Contributor Author

For the sake of pushing progress to fix the chat bug ASAP, I think this main panel for configuring chat should be done in another PR for admin2 indeed.

I think that the admin2 user interface for configuration is an important and good feature, and since its implementation does not take much time, it fits in this pull request. If anyone has an idea about it, feel free to share.

@Fernando-A-Rocha
Copy link
Contributor

For the sake of pushing progress to fix the chat bug ASAP, I think this main panel for configuring chat should be done in another PR for admin2 indeed.

I think that the admin2 user interface for configuration is an important and good feature, and since its implementation does not take much time, it fits in this pull request. If anyone has an idea about it, feel free to share.

Okey, we can brainstorm ideas for the admin2 chat control panel.

@Nico8340
Copy link
Contributor Author

Nico8340 commented Jul 10, 2024

I was busy earlier..
I will continue working on this pull request this week 🚀

@Nico8340
Copy link
Contributor Author

I need some advice on how to store message logs. I thought of using an SQLite database, but the table cannot be filled with messages indefinitely, there should be a limit.

@jlillis
Copy link
Contributor

jlillis commented Jul 11, 2024

SQL will be your best option. Add a column to the table for the timestamp of each message. In the resource, have a function to DELETE FROM messages WHERE timestamp < [whatever the limit is]. Execute this function on resource start and set a timer to trigger it from time to time.

@Fernando-A-Rocha
Copy link
Contributor

@Nico8340 I suggest logging to an SQLite DB, and periodically running a query that deletes log entries that are older than X days (e.g. 30 days). Alternatively, to keep a maximum number of rows in the table, you can save the last row ID and run this query to periodically delete the rows with IDs inferior to the lastRowID - Y: DELETE FROM yourtable WHERE rowid < (last_row_id - 1000)

@Nico8340
Copy link
Contributor Author

I'm going to open a new pull request, because the preps weren't right, since we agreed to leave the old handlers as a fallback option, and with this modification I would completely overturn the current structure.
In order to preserve the conversation, I will link this pr to the new one.

@Nico8340 Nico8340 closed this Jul 13, 2024
@Nico8340 Nico8340 deleted the chat branch July 13, 2024 21:19
@Nico8340
Copy link
Contributor Author

I will continue working on it as soon as I get home from vacation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chat messages sent twice
4 participants