-
-
Notifications
You must be signed in to change notification settings - Fork 413
A Complete Whitelist Module #8205
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
base: dev/feature
Are you sure you want to change the base?
A Complete Whitelist Module #8205
Conversation
|
PS. MC 1.19.4 tests are expected to fail due to classes doesn't exist in that version, but the next Skript release drops support of that version |
src/main/java/org/skriptlang/skript/bukkit/whitelist/WhitelistModule.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/whitelist/elements/CondWillBeWhitelisted.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/whitelist/elements/CondWillBeWhitelisted.java
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/whitelist/elements/EvtWhitelist.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/whitelist/elements/EvtWhitelist.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/whitelist/elements/EvtWhitelist.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/whitelist/elements/CondWillBeWhitelisted.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/whitelist/elements/EvtPlayerWhitelist.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/whitelist/elements/EvtPlayerWhitelist.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/whitelist/elements/EvtPlayerWhitelist.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/whitelist/elements/EvtServerWhitelist.java
Outdated
Show resolved
Hide resolved
| return new SyntaxStringBuilder(event, debug) | ||
| .append("the") | ||
| .append(isServer ? "server" : "player") | ||
| .append(isNegated() ? "will not" : "will") | ||
| .append("be whitelisted") | ||
| .toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should use if statements instead of ternary to improve readability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was this resolved?
src/main/java/org/skriptlang/skript/bukkit/whitelist/elements/EvtPlayerWhitelist.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/whitelist/elements/ExprWhitelist.java
Show resolved
Hide resolved
…eature/whitelist-module
…elist-module # Conflicts: # src/main/java/ch/njol/skript/Skript.java
src/main/java/org/skriptlang/skript/bukkit/whitelist/elements/CondIsWhitelisted.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/whitelist/elements/CondWillBeWhitelisted.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/whitelist/elements/CondWillBeWhitelisted.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/whitelist/elements/EffEnforceWhitelist.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/bukkit/whitelist/elements/ExprWhitelist.java
Outdated
Show resolved
Hide resolved
| public class EvtServerWhitelist extends SkriptEvent { | ||
|
|
||
| private enum EventState { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| public class EvtPlayerWhitelist extends SkriptEvent { | ||
|
|
||
| private enum EventState { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
||
| @Name("Will Be Whitelisted") | ||
| @Description("Checks whether the server or a player will be whitelisted in a <a href='events.html#whitelist'>whitelist</a> event.") | ||
| @Keywords("server, player") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @Keywords("server, player") | |
| @Keywords({"server", "player"}) |
| return new SyntaxStringBuilder(event, debug) | ||
| .append("the") | ||
| .append(isServer ? "server" : "player") | ||
| .append(isNegated() ? "will not" : "will") | ||
| .append("be whitelisted") | ||
| .toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was this resolved?
Problem
Skript is lacking certain coverage on whitelist related APIs, especially whitelist events.
Solution
This PR focuses more on completing the whitelist module, such as:
Testing Completed
The current test coverage for whitelist only covers manipulating player list in whitelist and server whitelist state. Alongside with new additions added in this PR, JUnit tests were also added -- in EvtWhitelistTest.java and EvtWhitelistTest.sk, marking this a complete test coverage specifically for whitelist.
Supporting Information
Completes: none
Related: none