Merge branch 'dev/feature' into enhancement/permission-changer

gradle.properties

@@ -5,7 +5,7 @@ org.gradle.parallel=true
 
 groupid=ch.njol
 name=skript
-version=2.8.3
+version=2.8.4
 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/SkriptEventHandler.java

@@ -18,17 +18,12 @@
  */
 package ch.njol.skript;
 
-import java.lang.ref.WeakReference;
-import java.lang.reflect.Method;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.Map.Entry;
-import java.util.Set;
-import java.util.stream.Collectors;
-
+import ch.njol.skript.lang.SkriptEvent;
+import ch.njol.skript.lang.Trigger;
+import ch.njol.skript.timings.SkriptTimings;
+import ch.njol.skript.util.Task;
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.Multimap;
 import org.bukkit.Bukkit;
 import org.bukkit.event.Cancellable;
 import org.bukkit.event.Event;
@@ -42,13 +37,16 @@
 import org.bukkit.plugin.RegisteredListener;
 import org.eclipse.jdt.annotation.Nullable;
 
-import com.google.common.collect.ArrayListMultimap;
-import com.google.common.collect.Multimap;
-
-import ch.njol.skript.lang.SkriptEvent;
-import ch.njol.skript.lang.Trigger;
-import ch.njol.skript.timings.SkriptTimings;
-import ch.njol.skript.util.Task;
+import java.lang.ref.WeakReference;
+import java.lang.reflect.Method;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 public final class SkriptEventHandler {
 
@@ -112,66 +110,94 @@ private static List<Trigger> getTriggers(Class<? extends Event> event) {
 	 * @param priority The priority of the Event.
 	 */
 	private static void check(Event event, EventPriority priority) {
+		// get all triggers for this event, return if none
 		List<Trigger> triggers = getTriggers(event.getClass());
 		if (triggers.isEmpty())
 			return;
 
-		if (Skript.logVeryHigh()) {
-			boolean hasTrigger = false;
-			for (Trigger trigger : triggers) {
-				SkriptEvent triggerEvent = trigger.getEvent();
-				if (
-					triggerEvent.getEventPriority() == priority
-					&& triggerEvent.canExecuteAsynchronously() ? triggerEvent.check(event) : Boolean.TRUE.equals(Task.callSync(() -> triggerEvent.check(event)))
-				) {
-					hasTrigger = true;
-					break;
-				}
-			}
-			if (!hasTrigger)
-				return;
+		// Check if this event should be treated as cancelled
+		boolean isCancelled = isCancelled(event);
 
-			logEventStart(event);
-		}
-
-		boolean isCancelled = event instanceof Cancellable && ((Cancellable) event).isCancelled() && !listenCancelled.contains(event.getClass());
-		boolean isResultDeny = !(event instanceof PlayerInteractEvent && (((PlayerInteractEvent) event).getAction() == Action.LEFT_CLICK_AIR || ((PlayerInteractEvent) event).getAction() == Action.RIGHT_CLICK_AIR) && ((PlayerInteractEvent) event).useItemInHand() != Result.DENY);
-
-		if (isCancelled && isResultDeny) {
-			if (Skript.logVeryHigh())
-				Skript.info(" -x- was cancelled");
-			return;
-		}
+		// This logs events even if there isn't a trigger that's going to run at that priority.
+		// However, there should only be a priority listener IF there's a trigger at that priority.
+		// So the time will be logged even if no triggers pass check(), which is still useful information.
+		logEventStart(event, priority);
 
 		for (Trigger trigger : triggers) {
 			SkriptEvent triggerEvent = trigger.getEvent();
+
+			// check if the trigger is at the right priority
 			if (triggerEvent.getEventPriority() != priority)
 				continue;
 
-			// these methods need to be run on whatever thread the trigger is
-			Runnable execute = () -> {
-				logTriggerStart(trigger);
-				Object timing = SkriptTimings.start(trigger.getDebugLabel());
-				trigger.execute(event);
-				SkriptTimings.stop(timing);
-				logTriggerEnd(trigger);
-			};
-
-			if (trigger.getEvent().canExecuteAsynchronously()) {
-				if (triggerEvent.check(event))
-					execute.run();
-			} else { // Ensure main thread
-				Task.callSync(() -> {
-					if (triggerEvent.check(event))
-						execute.run();
-					return null; // we don't care about a return value
-				});
-			}
+			// check if the cancel state of the event is correct
+			if (!triggerEvent.getListeningBehavior().matches(isCancelled))
+				continue;
+
+			// execute the trigger
+			execute(trigger, event);
 		}
 
 		logEventEnd();
 	}
 
+	/**
+	 * Helper method to check if we should treat the provided Event as cancelled.
+	 *
+	 * @param event The event to check.
+	 * @return Whether the event should be treated as cancelled.
+	 */
+	private static boolean isCancelled(Event event) {
+		return event instanceof Cancellable &&
+			(((Cancellable) event).isCancelled() && isResultDeny(event)) &&
+			// TODO: listenCancelled is deprecated and should be removed in 2.10
+			!listenCancelled.contains(event.getClass());
+	}
+
+	/**
+	 * Helper method for when the provided Event is a {@link PlayerInteractEvent}.
+	 * These events are special in that they are called as cancelled when the player is left/right clicking on air.
+	 * We don't want to treat those as cancelled, so we need to check if the {@link PlayerInteractEvent#useItemInHand()} result is DENY.
+	 * That means the event was purposefully cancelled, and we should treat it as cancelled.
+	 *
+	 * @param event The event to check.
+	 * @return Whether the event was a PlayerInteractEvent with air and the result was DENY.
+	 */
+	private static boolean isResultDeny(Event event) {
+		return !(event instanceof PlayerInteractEvent &&
+			(((PlayerInteractEvent) event).getAction() == Action.LEFT_CLICK_AIR || ((PlayerInteractEvent) event).getAction() == Action.RIGHT_CLICK_AIR) &&
+			((PlayerInteractEvent) event).useItemInHand() != Result.DENY);
+	}
+
+	/**
+	 * Executes the provided Trigger with the provided Event as context.
+	 *
+	 * @param trigger The Trigger to execute.
+	 * @param event The Event to execute the Trigger with.
+	 */
+	private static void execute(Trigger trigger, Event event) {
+		// these methods need to be run on whatever thread the trigger is
+		Runnable execute = () -> {
+			logTriggerStart(trigger);
+			Object timing = SkriptTimings.start(trigger.getDebugLabel());
+			trigger.execute(event);
+			SkriptTimings.stop(timing);
+			logTriggerEnd(trigger);
+		};
+
+		if (trigger.getEvent().canExecuteAsynchronously()) {
+			if (trigger.getEvent().check(event))
+				execute.run();
+		} else { // Ensure main thread
+			Task.callSync(() -> {
+				if (trigger.getEvent().check(event))
+					execute.run();
+				return null; // we don't care about a return value
+			});
+		}
+	}
+
+
 	private static long startEvent;
 
 	/**
@@ -180,11 +206,30 @@ private static void check(Event event, EventPriority priority) {
 	 * @param event The Event that started.
 	 */
 	public static void logEventStart(Event event) {
+		logEventStart(event, null);
+	}
+
+	/**
+	 * Logs that the provided Event has started with a priority.
+	 * Requires {@link Skript#logVeryHigh()} to be true to log anything.
+	 * @param event The Event that started.
+	 * @param priority The priority of the Event.
+	 */
+	public static void logEventStart(Event event, @Nullable EventPriority priority) {
 		startEvent = System.nanoTime();
 		if (!Skript.logVeryHigh())
 			return;
 		Skript.info("");
-		Skript.info("== " + event.getClass().getName() + " ==");
+
+		String message = "== " + event.getClass().getName();
+
+		if (priority != null)
+			message += " with priority " + priority;
+
+		if (event instanceof Cancellable && ((Cancellable) event).isCancelled())
+			message += " (cancelled)";
+
+		Skript.info(message + " ==");
 	}
 
 	/**
@@ -307,8 +352,10 @@ public static void unregisterBukkitEvents(Trigger trigger) {
 	}
 
 	/**
-	 * Events which are listened even if they are cancelled.
+	 * Events which are listened even if they are cancelled. This should no longer be used.
+	 * @deprecated Users should specify the listening behavior in the event declaration. "on any %event%:", "on cancelled %event%:".
 	 */
+	@Deprecated
 	public static final Set<Class<? extends Event>> listenCancelled = new HashSet<>();
 
 	/**

src/main/java/ch/njol/skript/aliases/ItemData.java

@@ -31,6 +31,7 @@
 import org.bukkit.Material;
 import org.bukkit.block.Block;
 import org.bukkit.block.BlockState;
+import org.bukkit.block.data.BlockData;
 import org.bukkit.enchantments.Enchantment;
 import org.bukkit.inventory.ItemFactory;
 import org.bukkit.inventory.ItemFlag;
@@ -188,15 +189,23 @@ public ItemData(ItemStack stack) {
 		this(stack, BlockCompat.INSTANCE.getBlockValues(stack));
 		this.itemForm = true;
 	}
-
-	public ItemData(BlockState block) {
-		this.type = ItemUtils.asItem(block.getType());
+
+	/**
+	 * @deprecated Use {@link ItemData#ItemData(BlockData)} instead
+	 */
+	@Deprecated
+	public ItemData(BlockState blockState) {
+		this(blockState.getBlockData());
+	}
+
+	public ItemData(BlockData blockData) {
+		this.type = blockData.getMaterial();
 		this.stack = new ItemStack(type);
-		this.blockValues = BlockCompat.INSTANCE.getBlockValues(block);
+		this.blockValues = BlockCompat.INSTANCE.getBlockValues(blockData);
 	}
 
 	public ItemData(Block block) {
-		this(block.getState());
+		this(block.getBlockData());
 	}
 
 	/**

src/main/java/ch/njol/skript/aliases/ItemType.java

@@ -43,6 +43,7 @@
 import org.bukkit.block.Block;
 import org.bukkit.block.BlockState;
 import org.bukkit.block.Skull;
+import org.bukkit.block.data.BlockData;
 import org.bukkit.enchantments.Enchantment;
 import org.bukkit.entity.Player;
 import org.bukkit.inventory.Inventory;
@@ -184,10 +185,16 @@ public ItemType(ItemStack i) {
 		add_(new ItemData(i));
 	}
 
-	public ItemType(BlockState b) {
-//		amount = 1;
-		add_(new ItemData(b));
-		// TODO metadata - spawners, skulls, etc.
+	/**
+	 * @deprecated Use {@link #ItemType(BlockData)} instead
+	 */
+	@Deprecated
+	public ItemType(BlockState blockState) {
+		this(blockState.getBlockData());
+	}
+
+	public ItemType(BlockData blockData) {
+		add_(new ItemData(blockData));
 	}
 
 	/**
@@ -211,7 +218,7 @@ public void setTo(ItemType i) {
 	}
 
 	public ItemType(Block block) {
-		this(block.getState());
+		this(block.getBlockData());
 	}
 
 	/**
@@ -272,17 +279,25 @@ public boolean isOfType(@Nullable ItemStack item) {
 		return isOfType(new ItemData(item));
 	}
 
-	public boolean isOfType(@Nullable BlockState block) {
-		if (block == null)
+	/**
+	 * @deprecated Use {@link #isOfType(BlockData)} instead
+	 */
+	@Deprecated
+	public boolean isOfType(@Nullable BlockState blockState) {
+		return blockState != null && isOfType(blockState.getBlockData());
+	}
+
+	public boolean isOfType(@Nullable BlockData blockData) {
+		if (blockData == null)
 			return isOfType(Material.AIR, null);
 
-		return isOfType(new ItemData(block));
+		return isOfType(new ItemData(blockData));
 	}
 
 	public boolean isOfType(@Nullable Block block) {
 		if (block == null)
 			return isOfType(Material.AIR, null);
-		return isOfType(block.getState());
+		return isOfType(block.getBlockData());
 	}
 
 	public boolean isOfType(ItemData type) {

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

@@ -74,13 +74,15 @@ public static Material asBlock(Material type) {
 			return null;
 		}
 	}
-	
+
 	/**
 	 * Gets an item material corresponding to given block material, which might
 	 * be the given material.
 	 * @param type Material.
 	 * @return Item version of material or null.
+	 * @deprecated This just returns itself and has no use
 	 */
+	@Deprecated
 	public static Material asItem(Material type) {
 		// Assume (naively) that all types are valid items
 		return type;