discordjs / discord.js

A powerful JavaScript library for interacting with the Discord API
https://discord.js.org
Apache License 2.0
25.36k stars 3.97k forks source link

member.addRole(),.removeRole() #2832

Closed brains123456 closed 6 years ago

brains123456 commented 6 years ago

using GuildMember#addRole(role) when a member already has that role should throw errors.likewise, using GuildMember#removeRole(role) when a member doesn't have that role should also throw errors. currently,it doesn't

SpaceEEC commented 6 years ago

Removing a role via GuildMember#removeRole which the member does not have does not throw an error.

PLASMAchicken commented 6 years ago

But would there be any use cases for having this throw an error, as for me it would just mean I have to add extra checks to prevent errors, but I would not get advantages by it

brains123456 commented 6 years ago

plasmachicken. oh,so then why does a bot trying to send messages to a channel without the send messages permission throw errors? why not just pass the error silently cos why not?the coder will perform extra checks to see if the bot has the send messages before sending messages?with your idealogy plasmachicken,nothing in the discordjs lib should throw errors then.

bdistin commented 6 years ago

D.js doesn't throw an error when a bot tries to send a message to a channel without send messages permission, the API request throws. (effectively DiscordApp throws the error) D.js just wraps that error into a useful DiscordAPIError, so you understand why that error has errored.

Similarly, d.js doesn't do runtime checks to see if a role shouldn't be added or removed before sending the API request. It just sends the request without any bias because you are expected to make those checks yourself, just as you would check permissions on a channel; if you are so inclined. Moreso when you add bias checks to a library, and the API changes, you have to fix the bias checks that are now blocking otherwise good requests by arbitrarily throwing an error.

D.js first and foremost is an API wrapper. If the API says error, D.js repeats the error; similarly, if the API doesn't say error, D.js doesn't make up an error based on any arbitrary ideas of what requests should and shouldn't error.

kevinbioj commented 6 years ago

You can easily check if the member has or not the role. GuildMember.roles.has('ID')

didinele commented 6 years ago

In my opinion it is entirely useless to throw an error, just like Plasma said, another thing they could do is to make it so it returns false if they already have the role, but this will once again lead into other errors.

Lewdcario commented 6 years ago

It's basically been agreed on by the developers that this will not be implemented for some of the reasons already stated by @bdistin.