BuildTheEarth / main-bot

The main bot for the BuildTheEarth Discord servers.
https://buildtheearth.net
MIT License
22 stars 13 forks source link

Purge command limit #72

Closed jkPngu closed 3 years ago

jkPngu commented 3 years ago

100 messages purged is max limit, if limit is surpassed tos is broken i recommend saying this at line 18

if (amount > 100) return message.channel.sendError("Say something"); //i cant code for my life

Because some of my friends have been ban of using the discord application (bot) due to tos abuse.

I GOT YOU CATTE

HPaulson commented 3 years ago

Hey! You actually can bulkDelete more than 100 messages by sending multiple requests. I recommend allowing more than 100, but ensuring ratelimits are not exceeded; This is the equivalent to running the command twice (in terms of stress on the API) so it wouldn't be abuse;

jkPngu commented 3 years ago

Thats pretty much what i said... um#

HPaulson commented 3 years ago

@jkPngu This isn't what you said; What you wrote above, and the PR you submitted, both return if the amount is above 100. Discord will throw 400 if you send more than 100 (or less than 2, or messages two weeks old) to POST /channels/{channel.id}/messages/bulk-delete- as you mentioned. However, the way to avoid this (without abusing the API) would be to simply divide the amount divided by 100 & send the bulk delete for said amount of times (then a last one for the remainder, amount % 100). The max should probably be 500 or 700. Using this method is equivalent to simply running the command multiple times, and as long as it isn't abused by staff, the bot will never hit a 429 with this feature (the rate limit for bulk delete is quite small). This is much better than simply returning a message when the amount is over 100 (as you did above and in your PR) as the BTE guild can be fairly spammy and busy, to the extent in which bulkDeleting more than 100 messages is necessary- and this makes that easier without running the command multiple times.

HPaulson commented 3 years ago

@cAttte If it would be beneficial for me to create a PR for the above (as a replacement for #73) I can; Please advise 👍

cAttte commented 3 years ago

@HPaulson sure! as i said in the other comment, you should probably set a hard limit of like 500, and display the error message when the amount provided is higher than that

XboxBedrock commented 3 years ago

dosent discord.js already have handling for this ....

HPaulson commented 3 years ago

Ah, I had forgotten this project uses Djs. I still don't know why we aren't using Eris I'm unsure if there's built-in handling from the library, but knowing Djs, they do probably have some overly-bloated and unnecessary method that will take care of it.

ghost commented 3 years ago

Fixed in #488f45d