-
-
Notifications
You must be signed in to change notification settings - Fork 373
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
Ban Entry Syntaxes #7257
base: dev/feature
Are you sure you want to change the base?
Ban Entry Syntaxes #7257
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,218 @@ | ||||||||
package ch.njol.skript.expressions; | ||||||||
|
||||||||
import ch.njol.skript.Skript; | ||||||||
import ch.njol.skript.classes.Changer; | ||||||||
import ch.njol.skript.doc.*; | ||||||||
import ch.njol.skript.lang.Expression; | ||||||||
import ch.njol.skript.lang.ExpressionType; | ||||||||
import ch.njol.skript.lang.SkriptParser; | ||||||||
import ch.njol.skript.lang.util.SimpleExpression; | ||||||||
import ch.njol.skript.util.Timespan; | ||||||||
import ch.njol.util.Kleenean; | ||||||||
import com.destroystokyo.paper.profile.PlayerProfile; | ||||||||
import io.papermc.paper.ban.BanListType; | ||||||||
import org.bukkit.BanEntry; | ||||||||
import org.bukkit.Bukkit; | ||||||||
import org.bukkit.OfflinePlayer; | ||||||||
import org.bukkit.event.Event; | ||||||||
import org.jetbrains.annotations.Nullable; | ||||||||
|
||||||||
import java.net.InetAddress; | ||||||||
import java.net.UnknownHostException; | ||||||||
|
||||||||
@Name("Ban Details") | ||||||||
@Description("Returns data about a player or IP ban. " + | ||||||||
"The source can be any string, but it's usually the player's name. " + | ||||||||
"The expiration date won't return a value if the ban is permanent.") | ||||||||
@Examples({"set {_player} to offlineplayer(\"Notch\", false)", | ||||||||
"set {_expiration} to the date {_player}'s ban expires", | ||||||||
"set {_time.left} to difference between now and {_expiration}", | ||||||||
"set {_reason} to the reason {_player} was banned", | ||||||||
"send \"There is %{_time.left}% before %{_player}% gets unbanned! They were banned for '%{_reason}%'\" to player"}) | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
@Since("INSERT_VERSION") | ||||||||
erenkarakal marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
@RequiredPlugins("Spigot 1.20.1+") | ||||||||
public class ExprBanData extends SimpleExpression<Object> { | ||||||||
|
||||||||
static { | ||||||||
if (Skript.methodExists(BanEntry.class, "remove")) | ||||||||
Skript.registerExpression(ExprBanData.class, Object.class, ExpressionType.SIMPLE, | ||||||||
"[the] date %offlineplayer/string% was banned", | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Syntax is not a good option, it can be very confusing for readers and might affect the parser. I suggest something like Same goes for the rest of syntaxes, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This current one seems much better than ban x of y to me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it'd be improved by adding It buries the lead by like 2 words which i don't really think is an issue, especially when it's so much more natural/fluid imo. I'm not opposed to having it in addition, but i definitely think the current syntax is higher quality There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so do i add another syntax for |
||||||||
"[the] date of %offlineplayer/string%'s ban", | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||
|
||||||||
"[the] source of %offlineplayer/string%'s ban", | ||||||||
|
||||||||
"[the] date %offlineplayer/string%'s ban expires", | ||||||||
"[the] expiration date of %offlineplayer/string%'s ban", | ||||||||
|
||||||||
"[the] reason %offlineplayer/string% was banned", | ||||||||
"[the] reason for %offlineplayer/string%'s ban" | ||||||||
); | ||||||||
} | ||||||||
|
||||||||
private BanEntryType entryType; | ||||||||
private Expression<Object> banTargetExpr; // offline player or string (ip) | ||||||||
|
||||||||
@Override | ||||||||
public boolean init(Expression<?>[] expressions, int matchedPattern, Kleenean isDelayed, SkriptParser.ParseResult parseResult) { | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
entryType = BanEntryType.fromInt(matchedPattern); | ||||||||
banTargetExpr = (Expression<Object>) expressions[0]; | ||||||||
return true; | ||||||||
} | ||||||||
|
||||||||
@Override | ||||||||
protected Object @Nullable [] get(Event event) { | ||||||||
if (banTargetExpr == null) | ||||||||
return null; | ||||||||
|
||||||||
Object banTarget = banTargetExpr.getSingle(event); | ||||||||
BanEntry<?> banEntry; | ||||||||
|
||||||||
if (banTarget instanceof String ipTarget) | ||||||||
banEntry = getBanEntry(ipTarget); | ||||||||
else if (banTarget instanceof OfflinePlayer playerTarget) | ||||||||
banEntry = getBanEntry(playerTarget); | ||||||||
else | ||||||||
return null; | ||||||||
|
||||||||
if (banEntry == null) | ||||||||
return null; | ||||||||
|
||||||||
return switch (entryType) { | ||||||||
case BAN_DATE -> { | ||||||||
java.util.Date creation = banEntry.getCreated(); | ||||||||
ch.njol.skript.util.Date skriptCreation = new ch.njol.skript.util.Date(creation.getTime()); | ||||||||
yield new Object[]{ skriptCreation }; | ||||||||
} | ||||||||
case SOURCE -> new Object[]{ banEntry.getSource() }; | ||||||||
case EXPIRE_DATE -> { | ||||||||
java.util.Date expiration = banEntry.getExpiration(); | ||||||||
if (expiration == null) | ||||||||
yield null; | ||||||||
ch.njol.skript.util.Date skriptExpiration = new ch.njol.skript.util.Date(expiration.getTime()); | ||||||||
yield new Object[]{ skriptExpiration }; | ||||||||
} | ||||||||
case REASON -> new Object[]{ banEntry.getReason() }; | ||||||||
}; | ||||||||
} | ||||||||
|
||||||||
@Override | ||||||||
public boolean isSingle() { | ||||||||
return true; | ||||||||
} | ||||||||
|
||||||||
@Override | ||||||||
public Class<?> getReturnType() { | ||||||||
return Object.class; | ||||||||
} | ||||||||
|
||||||||
@Override | ||||||||
public Class<?> @Nullable [] acceptChange(Changer.ChangeMode mode) { | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
if (mode == Changer.ChangeMode.SET) { | ||||||||
return switch (entryType) { | ||||||||
case EXPIRE_DATE -> new Class<?>[]{ ch.njol.skript.util.Date.class }; | ||||||||
case SOURCE, REASON -> new Class<?>[]{ String.class }; | ||||||||
default -> null; | ||||||||
}; | ||||||||
} else if (mode == Changer.ChangeMode.ADD || mode == Changer.ChangeMode.REMOVE) { | ||||||||
if (entryType == BanEntryType.EXPIRE_DATE) | ||||||||
return new Class<?>[]{ Timespan.class }; | ||||||||
} | ||||||||
return null; | ||||||||
} | ||||||||
|
||||||||
@Override | ||||||||
public void change(Event event, Object @Nullable [] delta, Changer.ChangeMode mode) { | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
Object banTarget = banTargetExpr.getSingle(event); | ||||||||
BanEntry<?> banEntry; | ||||||||
|
||||||||
if (banTarget instanceof String ipTarget) | ||||||||
banEntry = getBanEntry(ipTarget); | ||||||||
else if (banTarget instanceof OfflinePlayer playerTarget) | ||||||||
banEntry = getBanEntry(playerTarget); | ||||||||
else | ||||||||
return; | ||||||||
|
||||||||
if (banEntry == null) | ||||||||
return; | ||||||||
|
||||||||
if (mode == Changer.ChangeMode.SET) { | ||||||||
switch (entryType) { | ||||||||
case SOURCE -> { | ||||||||
String newSource = (String) delta[0]; | ||||||||
if (newSource == null) | ||||||||
return; | ||||||||
banEntry.setSource(newSource); | ||||||||
} | ||||||||
case EXPIRE_DATE -> { | ||||||||
ch.njol.skript.util.Date newDate = (ch.njol.skript.util.Date) delta[0]; | ||||||||
if (newDate == null) | ||||||||
return; | ||||||||
banEntry.setExpiration(new java.util.Date(newDate.getTimestamp())); | ||||||||
} | ||||||||
case REASON -> { | ||||||||
String newReason = (String) delta[0]; | ||||||||
if (newReason == null) | ||||||||
return; | ||||||||
banEntry.setReason(newReason); | ||||||||
} | ||||||||
} | ||||||||
} else { | ||||||||
Timespan timespan = (Timespan) delta[0]; | ||||||||
if (timespan == null) | ||||||||
return; | ||||||||
|
||||||||
if (entryType == BanEntryType.EXPIRE_DATE) { | ||||||||
java.util.Date expiration = banEntry.getExpiration(); | ||||||||
if (expiration == null) | ||||||||
return; | ||||||||
long newExpirationMillis; | ||||||||
if (mode == Changer.ChangeMode.ADD) | ||||||||
newExpirationMillis = expiration.getTime() + timespan.getAs(Timespan.TimePeriod.MILLISECOND); | ||||||||
else if (mode == Changer.ChangeMode.REMOVE) | ||||||||
newExpirationMillis = expiration.getTime() - timespan.getAs(Timespan.TimePeriod.MILLISECOND); | ||||||||
else | ||||||||
return; | ||||||||
banEntry.setExpiration(new java.util.Date(newExpirationMillis)); | ||||||||
} | ||||||||
} | ||||||||
banEntry.save(); | ||||||||
} | ||||||||
|
||||||||
@Override | ||||||||
public String toString(@Nullable Event event, boolean debug) { | ||||||||
return switch (entryType) { | ||||||||
case BAN_DATE -> "the date " + banTargetExpr.toString(event, debug) + " was banned"; | ||||||||
case SOURCE -> "the source of " + banTargetExpr.toString(event, debug) + "'s ban"; | ||||||||
case EXPIRE_DATE -> "the date " + banTargetExpr.toString(event, debug) + "'s ban expires"; | ||||||||
case REASON -> "the reason " + banTargetExpr.toString(event, debug) + " was banned"; | ||||||||
}; | ||||||||
} | ||||||||
|
||||||||
private BanEntry<InetAddress> getBanEntry(String ipTarget) { | ||||||||
try { | ||||||||
InetAddress address = InetAddress.getByName(ipTarget); | ||||||||
return Bukkit.getBanList(BanListType.IP).getBanEntry(address); | ||||||||
} catch (UnknownHostException ignored) { // this only happens when you pass a url and it performs a lookup | ||||||||
return null; | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
private BanEntry<PlayerProfile> getBanEntry(OfflinePlayer playerTarget) { | ||||||||
return Bukkit.getBanList(BanListType.PROFILE).getBanEntry(playerTarget.getPlayerProfile()); | ||||||||
} | ||||||||
|
||||||||
private enum BanEntryType { | ||||||||
BAN_DATE, SOURCE, EXPIRE_DATE, REASON; | ||||||||
|
||||||||
public static BanEntryType fromInt(int value) { | ||||||||
return switch (value) { | ||||||||
case 0, 1 -> BAN_DATE; | ||||||||
case 2 -> SOURCE; | ||||||||
case 3, 4 -> EXPIRE_DATE; | ||||||||
case 5, 6 -> REASON; | ||||||||
default -> null; | ||||||||
}; | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
test "ban entries" when running minecraft "1.20.1": | ||
ban offlineplayer("Njol", false) due to "creating skript" for 5 seconds | ||
assert the date offlineplayer("Njol", false) was banned is now with "getting ban creation date for offline player doesn't work" | ||
assert the reason offlineplayer("Njol", false) was banned is "creating skript" with "getting ban reason doesn't work" | ||
|
||
ban "3.3.3.3" for 3 seconds | ||
assert the date "3.3.3.3" was banned is now with "getting ban creation date for ip doesn't work" | ||
assert the source of "3.3.3.3"'s ban is "Skript ban effect" with "getting ban source doesn't work" | ||
assert the expiration date of "3.3.3.3"'s ban is 3 seconds later with "getting ban expiration date doesn't work" | ||
|
||
# test changers | ||
add 2 seconds to the expiration date of "3.3.3.3"'s ban | ||
assert the expiration date of "3.3.3.3"'s ban is 5 seconds later with "adding to ban expiration date doesn't work" | ||
|
||
remove 1 second from the expiration date of "3.3.3.3"'s ban | ||
assert the expiration date of "3.3.3.3"'s ban is 4 seconds later with "removing from ban expiration date doesn't work" | ||
|
||
set expiration date of "3.3.3.3"'s ban to 1 second later | ||
assert the expiration date of "3.3.3.3"'s ban is 1 second later with "setting ban expiration date doesn't work" | ||
|
||
set the source of "3.3.3.3"'s ban to "skript test" | ||
assert the source of "3.3.3.3"'s ban is "skript test" with "setting ban source doesn't work" | ||
|
||
set the reason "3.3.3.3" was banned to "hello" | ||
assert the reason "3.3.3.3" was banned is "hello" with "setting ban reason doesn't work" |
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.