From 243281ecad5183ae63d1e36b65662cac2e8d30d8 Mon Sep 17 00:00:00 2001 From: Patrick Miller Date: Wed, 14 Feb 2024 12:49:12 -0500 Subject: [PATCH 01/12] Fix Compilation Issues (Address DamageEvent Constructor Changes) (#6429) --- .../njol/skript/bukkitutil/HealthUtils.java | 33 +++++++++++++++++-- .../syntaxes/expressions/ExprLastDamageCause | 10 ++++++ 2 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 src/test/skript/tests/syntaxes/expressions/ExprLastDamageCause diff --git a/src/main/java/ch/njol/skript/bukkitutil/HealthUtils.java b/src/main/java/ch/njol/skript/bukkitutil/HealthUtils.java index 9af9012eb7f..20b5120a0f1 100644 --- a/src/main/java/ch/njol/skript/bukkitutil/HealthUtils.java +++ b/src/main/java/ch/njol/skript/bukkitutil/HealthUtils.java @@ -18,13 +18,21 @@ */ package ch.njol.skript.bukkitutil; +import ch.njol.skript.Skript; import ch.njol.util.Math2; import org.bukkit.attribute.Attributable; import org.bukkit.attribute.Attribute; import org.bukkit.attribute.AttributeInstance; +import org.bukkit.damage.DamageSource; +import org.bukkit.damage.DamageType; import org.bukkit.entity.Damageable; import org.bukkit.event.entity.EntityDamageEvent; import org.bukkit.event.entity.EntityDamageEvent.DamageCause; +import org.jetbrains.annotations.Nullable; + +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; public class HealthUtils { @@ -107,9 +115,28 @@ public static double getFinalDamage(EntityDamageEvent e) { public static void setDamage(EntityDamageEvent e, double damage) { e.setDamage(damage * 2); } - + + @Nullable + private static final Constructor OLD_DAMAGE_EVENT_CONSTRUCTOR; + + static { + Constructor constructor = null; + try { + constructor = EntityDamageEvent.class.getConstructor(Damageable.class, DamageCause.class, double.class); + } catch (NoSuchMethodException ignored) { } + OLD_DAMAGE_EVENT_CONSTRUCTOR = constructor; + } + public static void setDamageCause(Damageable e, DamageCause cause) { - e.setLastDamageCause(new EntityDamageEvent(e, cause, 0)); + if (OLD_DAMAGE_EVENT_CONSTRUCTOR != null) { + try { + e.setLastDamageCause(OLD_DAMAGE_EVENT_CONSTRUCTOR.newInstance(e, cause, 0)); + } catch (InstantiationException | IllegalAccessException | InvocationTargetException ex) { + Skript.exception("Failed to set last damage cause"); + } + } else { + e.setLastDamageCause(new EntityDamageEvent(e, cause, DamageSource.builder(DamageType.GENERIC).build(), 0)); + } } - + } diff --git a/src/test/skript/tests/syntaxes/expressions/ExprLastDamageCause b/src/test/skript/tests/syntaxes/expressions/ExprLastDamageCause new file mode 100644 index 00000000000..5a9ddf2de67 --- /dev/null +++ b/src/test/skript/tests/syntaxes/expressions/ExprLastDamageCause @@ -0,0 +1,10 @@ +test "last damage cause": + + spawn a pig at spawn of "world" + set {_pig} to last spawned pig + + assert last damage cause of {_pig} is custom with "expected pig to have custom damage cause, but it didn't" + set last damage cause of {_pig} to drowning + assert last damage cause of {_pig} is drowning with "expected pig to have drowning damage cause, but it didn't" + + delete the entity within {_pig} From 817b5a953fd4acea6d1cc344a94f84b38e5f0955 Mon Sep 17 00:00:00 2001 From: Moderocky Date: Sat, 17 Feb 2024 21:50:40 +0000 Subject: [PATCH 02/12] Ask for player's groups off the main thread. (#6192) * Update Vault to 1.7.3 This version should support 1.13 to 1.17+ according to the Spigot page. I'm not sure what this actually adds but it's usually a good idea to keep dependencies updated and all 1.7.X versions of Vault are supposed to be compatible. :) * Do group request from another thread. This change is (basically) pointless -- we still block the main thread while we wait since we absolutely need to, but it prevents the errors from overzealous permissions plugins. * Invert parse tag. --------- Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com> --- build.gradle | 2 +- .../permission/expressions/ExprGroup.java | 31 +++++++++++-------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/build.gradle b/build.gradle index 33f0399632c..92f13c47f46 100644 --- a/build.gradle +++ b/build.gradle @@ -33,7 +33,7 @@ dependencies { implementation group: 'org.eclipse.jdt', name: 'org.eclipse.jdt.annotation', version: '2.2.700' implementation group: 'com.google.code.findbugs', name: 'findbugs', version: '3.0.1' implementation group: 'com.sk89q.worldguard', name: 'worldguard-legacy', version: '7.0.0-SNAPSHOT' - implementation group: 'net.milkbowl.vault', name: 'Vault', version: '1.7.1', { + implementation group: 'net.milkbowl.vault', name: 'Vault', version: '1.7.3', { exclude group: 'org.bstats', module: 'bstats-bukkit' } diff --git a/src/main/java/ch/njol/skript/hooks/permission/expressions/ExprGroup.java b/src/main/java/ch/njol/skript/hooks/permission/expressions/ExprGroup.java index bd043c1c255..b3c12f9f38c 100644 --- a/src/main/java/ch/njol/skript/hooks/permission/expressions/ExprGroup.java +++ b/src/main/java/ch/njol/skript/hooks/permission/expressions/ExprGroup.java @@ -39,6 +39,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.concurrent.CompletableFuture; @Name("Group") @Description({ @@ -53,7 +54,7 @@ public class ExprGroup extends SimpleExpression { static { - PropertyExpression.register(ExprGroup.class, String.class, "group[(1¦s)]", "offlineplayers"); + PropertyExpression.register(ExprGroup.class, String.class, "group[plural:s]", "offlineplayers"); } private boolean primary; @@ -68,21 +69,25 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye return false; } players = (Expression) exprs[0]; - primary = parseResult.mark == 0; + primary = !parseResult.hasTag("plural"); return true; } @SuppressWarnings("null") @Override - protected String[] get(Event e) { - List groups = new ArrayList<>(); - for (OfflinePlayer player : players.getArray(e)) { - if (primary) - groups.add(VaultHook.permission.getPrimaryGroup(null, player)); - else - Collections.addAll(groups, VaultHook.permission.getPlayerGroups(null, player)); - } - return groups.toArray(new String[0]); + protected String[] get(Event event) { + OfflinePlayer[] players = this.players.getArray(event); + return CompletableFuture.supplyAsync(() -> { // #5692: LuckPerms errors for vault requests on main thread + List groups = new ArrayList<>(); + for (OfflinePlayer player : players) { + if (primary) { + groups.add(VaultHook.permission.getPrimaryGroup(null, player)); + } else { + Collections.addAll(groups, VaultHook.permission.getPlayerGroups(null, player)); + } + } + return groups.toArray(new String[0]); + }).join(); } @Override @@ -140,8 +145,8 @@ public Class getReturnType() { @SuppressWarnings("null") @Override - public String toString(Event e, boolean debug) { - return "group" + (primary ? "" : "s") + " of " + players.toString(e, debug); + public String toString(Event event, boolean debug) { + return "group" + (primary ? "" : "s") + " of " + players.toString(event, debug); } } From ba638923d311f68d4dfb765ea85296581fe99698 Mon Sep 17 00:00:00 2001 From: _tud <98935832+UnderscoreTud@users.noreply.github.com> Date: Sun, 18 Feb 2024 01:07:38 +0300 Subject: [PATCH 03/12] Fix Timespan Addition Overflow Exception (#6328) * Fix timespan addition * Fix timespan-number multiplication --------- Co-authored-by: sovdee <10354869+sovdeeth@users.noreply.github.com> --- .../classes/data/DefaultOperations.java | 5 ++-- src/main/java/ch/njol/util/Math2.java | 25 +++++++++++++++++++ ...cause exceptions if there's an overflow.sk | 3 +++ 3 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 src/test/skript/tests/regressions/6328-timespan operations can cause exceptions if there's an overflow.sk diff --git a/src/main/java/ch/njol/skript/classes/data/DefaultOperations.java b/src/main/java/ch/njol/skript/classes/data/DefaultOperations.java index d3f51b29bb9..e6397a1e767 100644 --- a/src/main/java/ch/njol/skript/classes/data/DefaultOperations.java +++ b/src/main/java/ch/njol/skript/classes/data/DefaultOperations.java @@ -21,6 +21,7 @@ import ch.njol.skript.util.Date; import ch.njol.skript.util.Timespan; import ch.njol.skript.util.Utils; +import ch.njol.util.Math2; import org.bukkit.util.Vector; import org.skriptlang.skript.lang.arithmetic.Arithmetics; import org.skriptlang.skript.lang.arithmetic.Operator; @@ -84,7 +85,7 @@ public class DefaultOperations { }); // Timespan - Timespan - Arithmetics.registerOperation(Operator.ADDITION, Timespan.class, (left, right) -> new Timespan(left.getMilliSeconds() + right.getMilliSeconds())); + Arithmetics.registerOperation(Operator.ADDITION, Timespan.class, (left, right) -> new Timespan(Math2.addClamped(left.getMilliSeconds(), right.getMilliSeconds()))); Arithmetics.registerOperation(Operator.SUBTRACTION, Timespan.class, (left, right) -> new Timespan(Math.max(0, left.getMilliSeconds() - right.getMilliSeconds()))); Arithmetics.registerDifference(Timespan.class, (left, right) -> new Timespan(Math.abs(left.getMilliSeconds() - right.getMilliSeconds()))); Arithmetics.registerDefaultValue(Timespan.class, Timespan::new); @@ -95,7 +96,7 @@ public class DefaultOperations { long scalar = right.longValue(); if (scalar < 0) return null; - return new Timespan(left.getMilliSeconds() * scalar); + return new Timespan(Math2.multiplyClamped(left.getMilliSeconds(), scalar)); }, (left, right) -> { long scalar = left.longValue(); if (scalar < 0) diff --git a/src/main/java/ch/njol/util/Math2.java b/src/main/java/ch/njol/util/Math2.java index c16018d341e..d17ac6910d1 100644 --- a/src/main/java/ch/njol/util/Math2.java +++ b/src/main/java/ch/njol/util/Math2.java @@ -155,6 +155,31 @@ public static long round(double value) { public static float safe(float value) { return Float.isFinite(value) ? value : 0; } + + /** + * @param x the first value + * @param y the second value + * @return the sum of x and y, or {@link Long#MAX_VALUE} in case of an overflow + */ + public static long addClamped(long x, long y) { + long result = x + y; + // Logic extracted from Math#addExact to avoid having to catch an expensive exception + boolean causedOverflow = ((x ^ result) & (y ^ result)) < 0; + if (causedOverflow) + return Long.MAX_VALUE; + return result; + } + + public static long multiplyClamped(long x, long y) { + long result = x * y; + long ax = Math.abs(x); + long ay = Math.abs(y); + // Logic extracted from Math#multiplyExact to avoid having to catch an expensive exception + if (((ax | ay) >>> 31 != 0) && (((y != 0) && (result / y != x)) || (x == Long.MIN_VALUE && y == -1))) + // If either x or y is negative return the min value, otherwise return the max value + return x < 0 == y < 0 ? Long.MAX_VALUE : Long.MIN_VALUE; + return result; + } @Deprecated @ScheduledForRemoval diff --git a/src/test/skript/tests/regressions/6328-timespan operations can cause exceptions if there's an overflow.sk b/src/test/skript/tests/regressions/6328-timespan operations can cause exceptions if there's an overflow.sk new file mode 100644 index 00000000000..1267fc7c081 --- /dev/null +++ b/src/test/skript/tests/regressions/6328-timespan operations can cause exceptions if there's an overflow.sk @@ -0,0 +1,3 @@ +test "timespan overflow exception": + assert 1000000000 years + 10000000 years is set with "timespan addition overflow did not return max value" + assert 1000000000 years * 10000000 is set with "timespan addition overflow did not return max value" \ No newline at end of file From 7f61018b6b9813b9c47b8410d48aab06a539e160 Mon Sep 17 00:00:00 2001 From: sovdee <10354869+sovdeeth@users.noreply.github.com> Date: Sat, 17 Feb 2024 23:13:26 +0100 Subject: [PATCH 04/12] Make function parameters respect the case-insensitive-variables setting. (#6388) * Update Parameter.java * Update Parameter.java --- .../ch/njol/skript/lang/function/Parameter.java | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/main/java/ch/njol/skript/lang/function/Parameter.java b/src/main/java/ch/njol/skript/lang/function/Parameter.java index dc61dee6f00..d8682220d77 100644 --- a/src/main/java/ch/njol/skript/lang/function/Parameter.java +++ b/src/main/java/ch/njol/skript/lang/function/Parameter.java @@ -19,6 +19,7 @@ package ch.njol.skript.lang.function; import ch.njol.skript.Skript; +import ch.njol.skript.SkriptConfig; import ch.njol.skript.classes.ClassInfo; import ch.njol.skript.lang.Expression; import ch.njol.skript.lang.ParseContext; @@ -45,8 +46,9 @@ public final class Parameter { /** * Name of this parameter. Will be used as name for the local variable - * that contains value of it inside function. This is always in lower case; - * variable names are case-insensitive. + * that contains value of it inside function. + * If {@link SkriptConfig#caseInsensitiveVariables} is {@code true}, + * then the valid variable names may not necessarily match this string in casing. */ final String name; @@ -69,7 +71,7 @@ public final class Parameter { @SuppressWarnings("null") public Parameter(String name, ClassInfo type, boolean single, @Nullable Expression def) { - this.name = name != null ? name.toLowerCase(Locale.ENGLISH) : null; + this.name = name; this.type = type; this.def = def; this.single = single; @@ -119,6 +121,7 @@ public static Parameter newInstance(String name, ClassInfo type, boole @Nullable public static List> parse(String args) { List> params = new ArrayList<>(); + boolean caseInsensitive = SkriptConfig.caseInsensitiveVariables.value(); int j = 0; for (int i = 0; i <= args.length(); i = SkriptParser.next(args, i, ParseContext.DEFAULT)) { if (i == -1) { @@ -138,8 +141,12 @@ public static List> parse(String args) { return null; } String paramName = "" + n.group(1); + // for comparing without affecting the original name, in case the config option for case insensitivity changes. + String lowerParamName = paramName.toLowerCase(Locale.ENGLISH); for (Parameter p : params) { - if (p.name.toLowerCase(Locale.ENGLISH).equals(paramName.toLowerCase(Locale.ENGLISH))) { + // only force lowercase if we don't care about case in variables + String otherName = caseInsensitive ? p.name.toLowerCase(Locale.ENGLISH) : p.name; + if (otherName.equals(caseInsensitive ? lowerParamName : paramName)) { Skript.error("Each argument's name must be unique, but the name '" + paramName + "' occurs at least twice."); return null; } From a5485389448976146bf8d7709a517c862003b1a6 Mon Sep 17 00:00:00 2001 From: sovdee <10354869+sovdeeth@users.noreply.github.com> Date: Sat, 17 Feb 2024 23:28:47 +0100 Subject: [PATCH 05/12] Make bucket fill events provide the correct event-block. (#6392) * Fix Bucket event-block * Update src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java --- .../classes/data/BukkitEventValues.java | 4 +-- .../java/ch/njol/skript/events/EvtBlock.java | 36 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java b/src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java index 1998e124aa4..18d19ae8fff 100644 --- a/src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java +++ b/src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java @@ -712,14 +712,14 @@ public Block get(final PlayerBedLeaveEvent e) { @Override @Nullable public Block get(final PlayerBucketFillEvent e) { - return e.getBlockClicked().getRelative(e.getBlockFace()); + return e.getBlockClicked(); } }, 0); EventValues.registerEventValue(PlayerBucketFillEvent.class, Block.class, new Getter() { @Override @Nullable public Block get(final PlayerBucketFillEvent e) { - final BlockState s = e.getBlockClicked().getRelative(e.getBlockFace()).getState(); + final BlockState s = e.getBlockClicked().getState(); s.setType(Material.AIR); s.setRawData((byte) 0); return new BlockStateBlock(s, true); diff --git a/src/main/java/ch/njol/skript/events/EvtBlock.java b/src/main/java/ch/njol/skript/events/EvtBlock.java index 98b94d7ee0f..4a393146600 100644 --- a/src/main/java/ch/njol/skript/events/EvtBlock.java +++ b/src/main/java/ch/njol/skript/events/EvtBlock.java @@ -90,9 +90,9 @@ public boolean init(final Literal[] args, final int matchedPattern, final Par @SuppressWarnings("null") @Override - public boolean check(final Event e) { - if (mine && e instanceof BlockBreakEvent) { - if (((BlockBreakEvent) e).getBlock().getDrops(((BlockBreakEvent) e).getPlayer().getItemInHand()).isEmpty()) + public boolean check(final Event event) { + if (mine && event instanceof BlockBreakEvent) { + if (((BlockBreakEvent) event).getBlock().getDrops(((BlockBreakEvent) event).getPlayer().getItemInHand()).isEmpty()) return false; } if (types == null) @@ -101,27 +101,27 @@ public boolean check(final Event e) { ItemType item; BlockData blockData = null; - if (e instanceof BlockFormEvent) { - BlockFormEvent blockFormEvent = (BlockFormEvent) e; + if (event instanceof BlockFormEvent) { + BlockFormEvent blockFormEvent = (BlockFormEvent) event; BlockState newState = blockFormEvent.getNewState(); item = new ItemType(newState); blockData = newState.getBlockData(); - } else if (e instanceof BlockEvent) { - BlockEvent blockEvent = (BlockEvent) e; + } else if (event instanceof BlockEvent) { + BlockEvent blockEvent = (BlockEvent) event; Block block = blockEvent.getBlock(); item = new ItemType(block); blockData = block.getBlockData(); - } else if (e instanceof PlayerBucketFillEvent) { - PlayerBucketFillEvent playerBucketFillEvent = ((PlayerBucketFillEvent) e); - Block relative = playerBucketFillEvent.getBlockClicked().getRelative(playerBucketFillEvent.getBlockFace()); - item = new ItemType(relative); - blockData = relative.getBlockData(); - } else if (e instanceof PlayerBucketEmptyEvent) { - PlayerBucketEmptyEvent playerBucketEmptyEvent = ((PlayerBucketEmptyEvent) e); + } else if (event instanceof PlayerBucketFillEvent) { + PlayerBucketFillEvent playerBucketFillEvent = ((PlayerBucketFillEvent) event); + Block block = playerBucketFillEvent.getBlockClicked(); + item = new ItemType(block); + blockData = block.getBlockData(); + } else if (event instanceof PlayerBucketEmptyEvent) { + PlayerBucketEmptyEvent playerBucketEmptyEvent = ((PlayerBucketEmptyEvent) event); item = new ItemType(playerBucketEmptyEvent.getItemStack()); - } else if (e instanceof HangingEvent) { - final EntityData d = EntityData.fromEntity(((HangingEvent) e).getEntity()); - return types.check(e, o -> { + } else if (event instanceof HangingEvent) { + final EntityData d = EntityData.fromEntity(((HangingEvent) event).getEntity()); + return types.check(event, o -> { if (o instanceof ItemType) return Relation.EQUAL.isImpliedBy(DefaultComparators.entityItemComparator.compare(d, ((ItemType) o))); return false; @@ -134,7 +134,7 @@ public boolean check(final Event e) { final ItemType itemF = item; BlockData finalBlockData = blockData; - return types.check(e, o -> { + return types.check(event, o -> { if (o instanceof ItemType) return ((ItemType) o).isSupertypeOf(itemF); else if (o instanceof BlockData && finalBlockData != null) From 69eb15fa23922c48ce3d2f8a9ea27a087553bfa8 Mon Sep 17 00:00:00 2001 From: sovdee <10354869+sovdeeth@users.noreply.github.com> Date: Fri, 1 Mar 2024 18:48:52 +0100 Subject: [PATCH 06/12] Fix EvtAtTime not triggering when world time has been changed. (#6463) --- .../java/ch/njol/skript/events/EvtAtTime.java | 87 ++++++++++--------- 1 file changed, 47 insertions(+), 40 deletions(-) diff --git a/src/main/java/ch/njol/skript/events/EvtAtTime.java b/src/main/java/ch/njol/skript/events/EvtAtTime.java index 17d37f00c5c..f003b26195c 100644 --- a/src/main/java/ch/njol/skript/events/EvtAtTime.java +++ b/src/main/java/ch/njol/skript/events/EvtAtTime.java @@ -32,12 +32,10 @@ import org.bukkit.event.Event; import org.eclipse.jdt.annotation.Nullable; -import java.util.ArrayList; -import java.util.Collections; import java.util.Iterator; -import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.PriorityQueue; import java.util.concurrent.ConcurrentHashMap; public class EvtAtTime extends SkriptEvent implements Comparable { @@ -54,12 +52,19 @@ public class EvtAtTime extends SkriptEvent implements Comparable { private static final Map TRIGGERS = new ConcurrentHashMap<>(); private static final class EvtAtInfo { - private int lastTick; // as Bukkit's scheduler is inconsistent this saves the exact tick when the events were last checked - private int currentIndex; - private final List instances = new ArrayList<>(); + /** + * Stores the last world time that this object's instances were checked. + */ + private int lastCheckedTime; + + /** + * A list of all {@link EvtAtTime}s in the world this info object is responsible for. + * Sorted by the time they're listening for in increasing order. + */ + private final PriorityQueue instances = new PriorityQueue<>(EvtAtTime::compareTo); } - private int tick; + private int time; @SuppressWarnings("NotNullFieldNotInitialized") private World[] worlds; @@ -67,7 +72,7 @@ private static final class EvtAtInfo { @Override @SuppressWarnings("unchecked") public boolean init(Literal[] args, int matchedPattern, ParseResult parseResult) { - tick = ((Literal