Add support for Role Tags #1342
Conversation
|
Iirc is there a |
"Is Managed" applies to all 3 of these roles. |
|
Maybe we could add Role#getManagedType() that returns a Enum with the Type |
|
Yeah, perhaps RoleType |
|
Woldn't it be better and simpler to have the RoleType as an enum inside the Role? |
|
Good call, considering how tiny it is. Forgot about classes in classes |
|
It also might be better to rename NORMAL as NONE instead. Since there is no field present, it feels more grammatically correct and follows the patterns set by other enums. |
There was a problem hiding this comment.
This doesn't seem to be an official change yet, since its not yet in the discord docs.
Additionally, if the role tags can include other tags other than type of management, it would make sense to store them as some kind of List/Map similar to the Guild Tags.
But in case of this becoming an official change + the tags only really showing the type of management, i provided some changes
src/main/java/net/dv8tion/jda/internal/entities/EntityBuilder.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
You are ignoring the tag values completely which makes this a bad implementation. You must handle what the tag value is, in order to properly associate roles with their purpose. For example, to retrieve the role of the current bot you must know the bot_id value, which is identical to the bot's id.
In the current state I cannot possibly accept this pull request.
| @@ -77,7 +78,10 @@ | |||
|
|
|||
| /** | |||
| * Whether this {@link net.dv8tion.jda.api.entities.Role Role} is managed by an integration | |||
| * This is any role where the "This role is managed by an integration..." appears in the role list. | |||
There was a problem hiding this comment.
| * This is any role where the "This role is managed by an integration..." appears in the role list. | |
| * <br>This is any role where the "This role is managed by an integration..." appears in the role list. |
| * @see Role#getRoleType() | ||
| * @return True, if this {@link net.dv8tion.jda.api.entities.Role Role} is managed. |
There was a problem hiding this comment.
| * @see Role#getRoleType() | |
| * @return True, if this {@link net.dv8tion.jda.api.entities.Role Role} is managed. | |
| * @return True, if this {@link net.dv8tion.jda.api.entities.Role Role} is managed. | |
| * | |
| * @see Role#getRoleType() |
| * | ||
| * @return this role's type | ||
| */ | ||
| RoleType getRoleType(); |
There was a problem hiding this comment.
| RoleType getRoleType(); | |
| @Nonnull | |
| RoleType getRoleType(); |
| @@ -97,6 +101,14 @@ | |||
| */ | |||
| boolean isMentionable(); | |||
|
|
|||
| /** | |||
| * Return the type of role this is | |||
| * Typically used for finding what kind of managed/integration role this is. | |||
There was a problem hiding this comment.
| * Typically used for finding what kind of managed/integration role this is. | |
| * typically used to determine what kind of managed role this is. |
| @@ -271,4 +283,51 @@ default RoleAction createCopy() | |||
| */ | |||
| @Nonnull | |||
| JDA getJDA(); | |||
|
|
|||
|
|
|||
| */ | ||
| BOT, | ||
| /** | ||
| * A role created from a YouTube Member or Twitch Subscriber integration..\ |
There was a problem hiding this comment.
| * A role created from a YouTube Member or Twitch Subscriber integration..\ | |
| * A role created for an integration. For example, twitch subscribers |
| */ | ||
| INTEGRATION, | ||
| /** | ||
| * The Server Booster role. |
There was a problem hiding this comment.
| * The Server Booster role. | |
| * The Guild Booster role. |
There was a problem hiding this comment.
The role is explicitly called "Server Booster" when it's created. So this should be "The Guild's Server Booster" role
There was a problem hiding this comment.
In API terminology were always use the term "guild".
| public static RoleType fromTags(DataObject tags) { | ||
| if(tags.hasKey("premium_subscriber")) | ||
| { | ||
| return RoleType.BOOSTER; | ||
| } | ||
| else if(tags.hasKey("bot_id")) | ||
| { | ||
| return RoleType.BOT; | ||
| } | ||
| else if(tags.hasKey("integration_id")) | ||
| { | ||
| return RoleType.INTEGRATION; | ||
| } else | ||
| { | ||
| return RoleType.UNKNOWN; | ||
| } | ||
| } |
There was a problem hiding this comment.
| public static RoleType fromTags(DataObject tags) { | |
| if(tags.hasKey("premium_subscriber")) | |
| { | |
| return RoleType.BOOSTER; | |
| } | |
| else if(tags.hasKey("bot_id")) | |
| { | |
| return RoleType.BOT; | |
| } | |
| else if(tags.hasKey("integration_id")) | |
| { | |
| return RoleType.INTEGRATION; | |
| } else | |
| { | |
| return RoleType.UNKNOWN; | |
| } | |
| } | |
| public static RoleType fromTags(DataObject tags) | |
| { | |
| if (tags.hasKey("premium_subscriber")) | |
| return RoleType.BOOSTER; | |
| else if (tags.hasKey("bot_id")) | |
| return RoleType.BOT; | |
| else if (tags.hasKey("integration_id")) | |
| return RoleType.INTEGRATION; | |
| else | |
| return RoleType.UNKNOWN; | |
| } |
| if(roleJson.hasKey("tags")) | ||
| { | ||
| role.setType(Role.RoleType.fromTags(roleJson.getObject("tags"))); | ||
| } | ||
| else | ||
| { | ||
| role.setType(Role.RoleType.NORMAL); | ||
| } |
There was a problem hiding this comment.
This is not a good way to handle tags. You should be storing the tags in a map so that you can also retrieve the associated values. For example, which bot a role belongs to.
| if(roleJson.hasKey("tags")) | |
| { | |
| role.setType(Role.RoleType.fromTags(roleJson.getObject("tags"))); | |
| } | |
| else | |
| { | |
| role.setType(Role.RoleType.NORMAL); | |
| } | |
| if (roleJson.hasKey("tags")) | |
| role.setType(Role.RoleType.fromTags(roleJson.getObject("tags"))); | |
| else | |
| role.setType(Role.RoleType.NORMAL); |
| @@ -56,6 +56,7 @@ | |||
| private boolean managed; | |||
| private boolean hoisted; | |||
| private boolean mentionable; | |||
| private RoleType roleType; | |||
There was a problem hiding this comment.
This is a bad way to handle tags.
|
I'm gonna implement this myself in #1343. I appreciate your efforts, but I have already planned how I would like to implement this a while ago. |
Pull Request Etiquette
Changes
Description
Discord recently added "tags" object to Roles.
Tags appear for any of the following reasons:
"premium_subscriber" will always be null, we only care about the key.
If not any of the above 3, tags will not appear.
I've only implemented the first one (partially) by adding
Role#isBoosterRole(). I plan to move forward and possibly add Role#isBotRole() or Role#isIntegrationRole() or similar. It would also be nice to have a Guild#getBoosterRole() as well.This has not been tested so as of now it's a draft, it's really just here to get feedback as I work on it and to make sure everything is good along the way :)