Merge branch 'dev/patch' into fix/entity-damage-event-1.20.4

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'
 	}
 

gradle.properties

@@ -5,7 +5,7 @@ org.gradle.parallel=true
 
 groupid=ch.njol
 name=skript
-version=2.8.2
+version=2.8.3
 jarName=Skript.jar
 testEnv=java17/paper-1.20.4
 testEnvJavaVersion=17

settings.gradle

@@ -1,6 +1,6 @@
 
 plugins {
-    id 'org.gradle.toolchains.foojay-resolver-convention' version '0.7.0'
+    id 'org.gradle.toolchains.foojay-resolver-convention' version '0.8.0'
 }
 
 rootProject.name = 'Skript'

src/main/java/ch/njol/skript/bukkitutil/HealthUtils.java

@@ -28,12 +28,18 @@
 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.ApiStatus.ScheduledForRemoval;
 import org.jetbrains.annotations.Nullable;
 
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
 public class HealthUtils {
 
 	/**
@@ -126,7 +132,7 @@ public static void setDamage(EntityDamageEvent e, double damage) {
 	public static boolean DAMAGE_SOURCE;
 
 	@Nullable
-	private static Constructor<EntityDamageEvent> DAMAGE_EVENT_CONSTRUCTOR;
+	private static final Constructor<EntityDamageEvent> DAMAGE_EVENT_CONSTRUCTOR;
 
 	static {
 		if (!DAMAGE_SOURCE) {
@@ -153,7 +159,9 @@ public static void setDamageCause(Damageable entity, DamageCause cause) {
 			return;
 		try {
 			entity.setLastDamageCause(DAMAGE_EVENT_CONSTRUCTOR.newInstance(entity, cause, 0));
-		} catch (InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {}
+		} catch (InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
+			Skript.exception(e, "Failed to set last damage cause");
+    }
 	}
 
 }

src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java

@@ -118,6 +118,7 @@
 import org.bukkit.event.inventory.InventoryClickEvent;
 import org.bukkit.event.inventory.InventoryCloseEvent;
 import org.bukkit.event.inventory.InventoryDragEvent;
+import org.bukkit.event.inventory.InventoryMoveItemEvent;
 import org.bukkit.event.inventory.InventoryOpenEvent;
 import org.bukkit.event.inventory.InventoryPickupItemEvent;
 import org.bukkit.event.inventory.PrepareAnvilEvent;
@@ -712,14 +713,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<Block, PlayerBucketFillEvent>() {
 			@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);
@@ -1797,6 +1798,39 @@ public TransformReason get(EntityTransformEvent event) {
 				return event.getTransformReason();
 			}
 		}, EventValues.TIME_NOW);
+
+		// InventoryMoveItemEvent
+		EventValues.registerEventValue(InventoryMoveItemEvent.class, Inventory.class, new Getter<Inventory, InventoryMoveItemEvent>() {
+			@Override
+			public Inventory get(InventoryMoveItemEvent event) {
+				return event.getSource();
+			}
+		}, EventValues.TIME_NOW);
+		EventValues.registerEventValue(InventoryMoveItemEvent.class, Inventory.class, new Getter<Inventory, InventoryMoveItemEvent>() {
+			@Override
+			public Inventory get(InventoryMoveItemEvent event) {
+				return event.getDestination();
+			}
+		}, EventValues.TIME_FUTURE);
+		EventValues.registerEventValue(InventoryMoveItemEvent.class, Block.class, new Getter<Block, InventoryMoveItemEvent>() {
+			@Override
+			public Block get(InventoryMoveItemEvent event) {
+				return event.getSource().getLocation().getBlock();
+			}
+		}, EventValues.TIME_NOW);
+		EventValues.registerEventValue(InventoryMoveItemEvent.class, Block.class, new Getter<Block, InventoryMoveItemEvent>() {
+			@Override
+			public Block get(InventoryMoveItemEvent event) {
+				return event.getDestination().getLocation().getBlock();
+			}
+		}, EventValues.TIME_FUTURE);
+		EventValues.registerEventValue(InventoryMoveItemEvent.class, ItemStack.class, new Getter<ItemStack, InventoryMoveItemEvent>() {
+			@Override
+			public ItemStack get(InventoryMoveItemEvent event) {
+				return event.getItem();
+			}
+		}, EventValues.TIME_NOW);
+
 	}
 
 }

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)

src/main/java/ch/njol/skript/entity/EntityData.java

@@ -37,6 +37,7 @@
 import org.bukkit.RegionAccessor;
 import org.bukkit.World;
 import org.bukkit.entity.Entity;
+import org.bukkit.entity.EntityType;
 import org.bukkit.entity.Player;
 import org.eclipse.jdt.annotation.Nullable;
 
@@ -94,6 +95,7 @@ public abstract class EntityData<E extends Entity> implements SyntaxElement, Ygg
 		} catch (NoSuchMethodException | SecurityException ignored) { /* We already checked if the method exists */ }
 	}
 
+	private static final boolean HAS_ENABLED_BY_FEATURE = Skript.methodExists(EntityType.class, "isEnabledByFeature", World.class);
 	public final static String LANGUAGE_NODE = "entities";
 
 	public final static Message m_age_pattern = new Message(LANGUAGE_NODE + ".age pattern");
@@ -463,6 +465,25 @@ private E apply(E entity) {
 		return entity;
 	}
 
+	/**
+	 * Check if this entity type can spawn.
+	 * <p>Some entity types may be restricted by experimental datapacks.</p>
+	 *
+	 * @param world World to check if entity can spawn in
+	 * @return True if entity can spawn else false
+	 */
+	public boolean canSpawn(@Nullable World world) {
+		if (world == null)
+			return false;
+		if (HAS_ENABLED_BY_FEATURE) {
+			// Check if the entity can actually be spawned
+			// Some entity types may be restricted by experimental datapacks
+			EntityType bukkitEntityType = EntityUtils.toBukkitEntityType(this);
+            return bukkitEntityType.isEnabledByFeature(world);
+		}
+		return true;
+	}
+
 	/**
 	 * Spawn this entity data at a location.
 	 *
@@ -476,7 +497,7 @@ public final E spawn(Location location) {
 
 	/**
 	 * Spawn this entity data at a location.
-	 * The consumer allows for modiciation to the entity before it actually gets spawned.
+	 * The consumer allows for modification to the entity before it actually gets spawned.
 	 * <p>
 	 * Bukkit's own {@link org.bukkit.util.Consumer} is deprecated.
 	 * Use {@link #spawn(Location, Consumer)}
@@ -494,7 +515,7 @@ public E spawn(Location location, org.bukkit.util.@Nullable Consumer<E> consumer
 
 	/**
 	 * Spawn this entity data at a location.
-	 * The consumer allows for modiciation to the entity before it actually gets spawned.
+	 * The consumer allows for modification to the entity before it actually gets spawned.
 	 *
 	 * @param location The {@link Location} to spawn the entity at.
 	 * @param consumer A {@link Consumer} to apply the entity changes to.
@@ -503,10 +524,13 @@ public E spawn(Location location, org.bukkit.util.@Nullable Consumer<E> consumer
 	@Nullable
 	public E spawn(Location location, @Nullable Consumer<E> consumer) {
 		assert location != null;
+		World world = location.getWorld();
+		if (!canSpawn(world))
+			return null;
 		if (consumer != null) {
 			return EntityData.spawn(location, getType(), e -> consumer.accept(this.apply(e)));
 		} else {
-			return apply(location.getWorld().spawn(location, getType()));
+			return apply(world.spawn(location, getType()));
 		}
 	}
 

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<EvtAtTime> {
@@ -54,20 +52,27 @@ public class EvtAtTime extends SkriptEvent implements Comparable<EvtAtTime> {
 	private static final Map<World, EvtAtInfo> 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<EvtAtTime> 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<EvtAtTime> instances = new PriorityQueue<>(EvtAtTime::compareTo);
 	}
 
-	private int tick;
+	private int time;
 
 	@SuppressWarnings("NotNullFieldNotInitialized")
 	private World[] worlds;
 
 	@Override
 	@SuppressWarnings("unchecked")
 	public boolean init(Literal<?>[] args, int matchedPattern, ParseResult parseResult) {
-		tick = ((Literal<Time>) args[0]).getSingle().getTicks();
+		time = ((Literal<Time>) args[0]).getSingle().getTicks();
 		worlds = args[1] == null ? Bukkit.getWorlds().toArray(new World[0]) : ((Literal<World>) args[1]).getAll();
 		return true;
 	}
@@ -78,10 +83,9 @@ public boolean postLoad() {
 			EvtAtInfo info = TRIGGERS.get(world);
 			if (info == null) {
 				TRIGGERS.put(world, info = new EvtAtInfo());
-				info.lastTick = (int) world.getTime() - 1;
+				info.lastCheckedTime = (int) world.getTime() - 1;
 			}
 			info.instances.add(this);
-			Collections.sort(info.instances);
 		}
 		registerListener();
 		return true;
@@ -93,13 +97,11 @@ public void unload() {
 		while (iterator.hasNext()) {
 			EvtAtInfo info = iterator.next();
 			info.instances.remove(this);
-			if (info.currentIndex >= info.instances.size())
-				info.currentIndex--;
 			if (info.instances.isEmpty())
 				iterator.remove();
 		}
 
-		if (taskID == -1 && TRIGGERS.isEmpty()) { // Unregister Bukkit listener if possible
+		if (taskID != -1 && TRIGGERS.isEmpty()) { // Unregister Bukkit listener if possible
 			Bukkit.getScheduler().cancelTask(taskID);
 			taskID = -1;
 		}
@@ -120,59 +122,64 @@ public boolean isEventPrioritySupported() {
 	private static void registerListener() {
 		if (taskID != -1)
 			return;
+		// For each world:
+		// check each instance in order until triggerTime > (worldTime + period)
 		taskID = Bukkit.getScheduler().scheduleSyncRepeatingTask(Skript.getInstance(), () -> {
 			for (Entry<World, EvtAtInfo> entry : TRIGGERS.entrySet()) {
 				EvtAtInfo info = entry.getValue();
-				int tick = (int) entry.getKey().getTime();
+				int worldTime = (int) entry.getKey().getTime();
 
 				// Stupid Bukkit scheduler
-				if (info.lastTick == tick)
+				// TODO: is this really necessary?
+				if (info.lastCheckedTime == worldTime)
 					continue;
 
 				// Check if time changed, e.g. by a command or plugin
-				if (info.lastTick + CHECK_PERIOD * 2 < tick || info.lastTick > tick && info.lastTick - 24000 + CHECK_PERIOD * 2 < tick)
-					info.lastTick = Math2.mod(tick - CHECK_PERIOD, 24000);
+				// if the info was last checked more than 2 cycles ago
+				// then reset the last checked time to the period just before now.
+				if (info.lastCheckedTime + CHECK_PERIOD * 2 < worldTime || (info.lastCheckedTime > worldTime && info.lastCheckedTime - 24000 + CHECK_PERIOD * 2 < worldTime))
+					info.lastCheckedTime = Math2.mod(worldTime - CHECK_PERIOD, 24000);
 
-				boolean midnight = info.lastTick > tick; // actually 6:00
+				// if we rolled over from 23999 to 0, subtract 24000 from last checked
+				boolean midnight = info.lastCheckedTime > worldTime; // actually 6:00
 				if (midnight)
-					info.lastTick -= 24000;
+					info.lastCheckedTime -= 24000;
 
-				int startIndex = info.currentIndex;
-				while (true) {
-					EvtAtTime next = info.instances.get(info.currentIndex);
-					int nextTick = midnight && next.tick > 12000 ? next.tick - 24000 : next.tick;
+				// loop instances from earliest to latest
+				for (EvtAtTime event : info.instances) {
+					// if we just rolled over, the last checked time will be x - 24000, so we need to do the same to the event time
+					int eventTime = midnight && event.time > 12000 ? event.time - 24000 : event.time;
 
-					if (!(info.lastTick < nextTick && nextTick <= tick))
+					// if the event time is in the future, we don't need to check any more events.
+					if (eventTime > worldTime)
 						break;
 
-					// Execute our event
-					ScheduledEvent event = new ScheduledEvent(entry.getKey());
-					SkriptEventHandler.logEventStart(event);
-					SkriptEventHandler.logTriggerEnd(next.trigger);
-					next.trigger.execute(event);
-					SkriptEventHandler.logTriggerEnd(next.trigger);
+					// if we should have already caught this time previously, check the next one
+					if (eventTime <= info.lastCheckedTime)
+						continue;
+
+					// anything that makes it here must satisfy lastCheckedTime < eventTime <= worldTime
+					// and therefore should trigger this event.
+					ScheduledEvent scheduledEvent = new ScheduledEvent(entry.getKey());
+					SkriptEventHandler.logEventStart(scheduledEvent);
+					SkriptEventHandler.logTriggerEnd(event.trigger);
+					event.trigger.execute(scheduledEvent);
+					SkriptEventHandler.logTriggerEnd(event.trigger);
 					SkriptEventHandler.logEventEnd();
-
-					info.currentIndex++;
-					if (info.currentIndex == info.instances.size())
-						info.currentIndex = 0;
-					if (info.currentIndex == startIndex) // All events executed at once
-						break;
 				}
-
-				info.lastTick = tick;
+				info.lastCheckedTime = worldTime;
 			}
 		}, 0, CHECK_PERIOD);
 	}
 
 	@Override
 	public String toString(@Nullable Event event, boolean debug) {
-		return "at " + Time.toString(tick) + " in worlds " + Classes.toString(worlds, true);
+		return "at " + Time.toString(time) + " in worlds " + Classes.toString(worlds, true);
 	}
 
 	@Override
 	public int compareTo(@Nullable EvtAtTime event) {
-		return event == null ? tick : tick - event.tick;
+		return event == null ? time : time - event.time;
 	}
 
 }