Skip to content

Add Optional Duration Argument to Kickban Command - #227

Open
MrCoolPotato wants to merge 2 commits into
beyond-all-reason:mainfrom
MrCoolPotato:kickban-duration
Open

Add Optional Duration Argument to Kickban Command#227
MrCoolPotato wants to merge 2 commits into
beyond-all-reason:mainfrom
MrCoolPotato:kickban-duration

Conversation

@MrCoolPotato

Copy link
Copy Markdown

Add optional duration argument to !kickban

Summary

Adds an optional <duration> argument to the !kickban command:

!kickban <playerName> [<duration>]

<duration> is in minutes, in [0,60]. If duration is omitted then the server default of 15 minutes is used.
If an already banned player is banned again, this will change it so the newer ban takes precedence whereas before both bans took effect, effectively setting the ban duration to be the max of the two.

This is done with a SPADS plugin that overrides the kickban command handler. It checks if a duration argument was specified, if so then it updates the config to equal that duration, calls the spads hkickban then resets the config to its previous value.
It also checks for runtime errors or version differences and will run the original kickban handler in its place if either of these are present. This allows the old kickban command to work if this code breaks in the future.

Why

This would be helpful to keep out people spamming in chat for longer, or it could also be used as an unban command via !kickban player_name 0 to reset their ban duration to 0 minutes allowing them to rejoin a lobby.
This has been requested by players: https://discordapp.com/channels/549281623154229250/1524568970336669851
Adding an optional argument would not remove any existing functionality and !kickban player_name works (almost) the same as it did before.

Details

Before anything is done, it checks the current spads version against the last version the plugin was manually tested against, and if spads version is newer it calls the previous kickban handler. Also if there are runtime errors it calls the previous handler
If a player already on the banned list is banned again, before this would create a new entry in the list and one player would have 2 entries and would only become unbanned once both expire. In order to allow for unbanning, This code will remove any existing ban entry which could have been added by a kickban command (the remaining duration is <=60) and then it will issue the new kickban. This is practically the same as the old behavior.
The actual banning is implemented by temporarily changing the banDuration config value, calling the previous handler and then changing the value back to what it was

Potential Issues

  • The biggest concern is the unban part which could possibly remove a ban placed not by a kickban vote. I am not sure how different types of bans are implemented so going off of the duration seemed like a good heuristic to minimize impact even if the ban list contains kickban entries and manual entries placed by mods. This could also lead to issues with spad clusters that possess a shared bans list, however it does not change the current kickban implementation so it should not cause players to be banned from multiple lobbies from one command.
  • Also, when a new version of SPADS comes out which would be unlikely to stop this plugin from working, it disables itself automatically until the new version is manually reviewed, tested and the max tested version constant is updated.
  • 60 minutes was somewhat arbitrarily chosen as a max ban duration value. This is higher than the current duration to allow for griefing players or spammers to be banned for longer, without the possibility of something like a boss banning someone from a lobby for a very long time and that ban persisting even after the boss leaves

Testing

Each of these have been verified in a local Teiserver and SPADS environment
-when duration is omitted, server default is used.
-when duration is specified, it is actually used in place of server default
-when duration is specified for a user that is already banned, the new duration takes precedence
-version difference and error fallbacks both work as intended
-after a kickban of a different duration, the config default is restored

@MrCoolPotato

Copy link
Copy Markdown
Author

I forgot to add this in the PR description, but this was AI-assisted with Claude because I suck at Perl.

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.

1 participant