Resolve [Bug]/#24 #26

Closed

csm-kb wants to merge 11 commits from master into mc/1.20.2

csm-kb commented

2023-12-01 02:24:03 +00:00

(Migrated from github.com)

Resolves #24

@Awakened-Redstone @kaydenvg
When executing the whitelist register command in Discord, we take the member's guild's getPublicRole() (which isn't returned in the list from getRoles() because it is documented as implicit):

The @everyone role; synonymous with the guild's ID.

and add it explicitly to the roles we're checking on the member.

This allows users to configure the mod using their guild's @everyone role, and have it behave as expected.

Resolves #24 @Awakened-Redstone @kaydenvg When executing the whitelist register command in Discord, we take the member's guild's `getPublicRole()` (which isn't returned in the list from `getRoles()` because it is documented as implicit): > The `@everyone` role; synonymous with the guild's ID. and add it explicitly to the roles we're checking on the member. This allows users to configure the mod using their guild's `@everyone` role, and have it behave as expected.

Awakened-Redstone (Migrated from github.com) reviewed

2023-12-01 02:24:03 +00:00

csm-kb commented

2023-12-01 02:26:45 +00:00

(Migrated from github.com)

Thought that signed... going to do an interactive rebase to have that sign

csm-kb commented

2023-12-01 02:38:32 +00:00

(Migrated from github.com)

done! should be good to go @Awakened-Redstone

Awakened-Redstone commented

2023-12-01 02:55:08 +00:00

(Migrated from github.com)

Seems good, I just need to check it a bit more later as the mod runs periodic checks, so may need to add that line to some other parts of the code, like CoreEvents:80 and DiscordDataProcessor:59

I'm sorry for the confusing branches, I'm working on some changes to the repo as AutoWhitelist Lite has been deprecated

Seems good, I just need to check it a bit more later as the mod runs periodic checks, so may need to add that line to some other parts of the code, like [CoreEvents:80](https://github.com/Awakened-Redstone/AutoWhitelist/blob/334184a3ed7e34560dfb75a3eacbbd81666745d5/src/main/java/com/awakenedredstone/autowhitelist/discord/events/CoreEvents.java#L80C66-L80C66) and [DiscordDataProcessor:59](https://github.com/Awakened-Redstone/AutoWhitelist/blob/404961649f1aef7ed749924cf38235fb1290dcf7/src/main/java/com/awakenedredstone/autowhitelist/discord/DiscordDataProcessor.java#L59C38-L59C38) I'm sorry for the confusing branches, I'm working on some changes to the repo as AutoWhitelist Lite has been deprecated

❤️ 1

Awakened-Redstone commented

2023-12-01 03:04:46 +00:00

(Migrated from github.com)

I forgot to mention, the major branch currently is mc/1.20.2
Once I have more time I'll remove unused branches and rename master to archive/lite

I forgot to mention, the major branch currently is `mc/1.20.2` Once I have more time I'll remove unused branches and rename master to `archive/lite`

Awakened-Redstone (Migrated from github.com) requested changes

2023-12-02 21:02:36 +00:00

Awakened-Redstone (Migrated from github.com) left a comment

The roles list is immutable, it has to either be copied to a mutable list or use another method to verify for the @everyone role.
The user can use a bot to give all users a role, but supporting @everyone is a nice QoL, also needs to add the extra code to other places that may change the user whitelist, as mentioned on the last comment

The roles list is immutable, it has to either be copied to a mutable list or use another method to verify for the `@everyone` role. The user can use a bot to give all users a role, but supporting `@everyone` is a nice QoL, also needs to add the extra code to other places that may change the user whitelist, as mentioned on the last comment

csm-kb commented

2023-12-08 18:10:10 +00:00

(Migrated from github.com)

mutable list

my bad, it's been years since I touched java... damn ArrayList 😂

> mutable list my bad, it's been years since I touched java... damn `ArrayList` 😂

csm-kb commented

2023-12-08 21:05:52 +00:00

(Migrated from github.com)

This should be better, was fighting Gradle to use my jdk-17 path (little gremlin)

I pulled the get roles logic into its own helper function and modified the other files making use of .getRoles() in that capacity to use this
I did a rebase onto mc/1.20.2 to catch back up

Please take a look!

This should be better, was fighting Gradle to use my jdk-17 path (little gremlin) - I pulled the get roles logic into its own helper function and modified the other files making use of `.getRoles()` in that capacity to use this - I did a rebase onto `mc/1.20.2` to catch back up Please take a look!

Commenting is not possible because the repository is archived.

No reviewers

Awakened-Redstone