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

Placeholders for all roles #1511

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

supercoolspy
Copy link

Adds placeholders for all roles based on #1494 placeholders would end up being:

%discordsrv_role_roleid_name% = Cool Role Name
%discordsrv_role_roleid_color_hex% = #FFFFFF
%discordsrv_role_roleid_color_code% = &F&F&F&F&F&F (Converted to color and would not show up as text)

@supercoolspy
Copy link
Author

supercoolspy commented Mar 17, 2023

@Vankka can you re-run the build? I updated from dev

@Dinty1 Dinty1 requested a review from Vankka December 17, 2023 16:59
Copy link
Member

@Vankka Vankka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your PR and sorry about the late response

I would prefer if the roleId logic was shared for these placeholders, the regular expression could also use groups to catch the role id and "sub placeholder". Example of what I mean:

private static final Pattern SPECIFIC_ROLE_PATTERN = Pattern.compile("role_(\\d+)_(\\w+)");

Matcher rolePlaceholderMatcher = SPECIFIC_ROLE_PATTERN.matcher(identifier);
if (rolePlaceholderMatcher.matches()) {
    String roleId = rolePlaceholderMatcher.group(1);
    Role role = DiscordUtil.getRole(roleId);
    String subPlaceholder = rolePlaceholderMatcher.group(2);
    
    if (subPlaceholder.equals("name")) {
        ...
    }
}

(The above is untested code)

@@ -113,6 +113,26 @@ public class PlaceholderAPIExpansion extends PlaceholderExpansion {
return String.valueOf(accountLinkManager.getLinkedAccountCount());
}

if(identifier.startsWith("role_")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if(identifier.startsWith("role_")) {
if (identifier.startsWith("role_")) {

Styling nitpick

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

Successfully merging this pull request may close these issues.

None yet

3 participants