Adds ability to listen to cancelled events

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,39 +110,30 @@ 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 && Boolean.TRUE.equals(Task.callSync(() -> triggerEvent.check(event)))) {
-					hasTrigger = true;
-					break;
-				}
-			}
-			if (!hasTrigger)
-				return;
-
-			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);
+		// Check if this event should be treated as cancelled
+		boolean isCancelled = isCancelled(event);
 
-		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;
 
+			// check if the cancel state of the event is correct
+			if (!triggerEvent.getListeningBehavior().matches(isCancelled))
+				continue;
+
 			// these methods need to be run on whatever thread the trigger is
 			Runnable execute = () -> {
 				logTriggerStart(trigger);
@@ -172,6 +161,34 @@ private static void check(Event event, EventPriority priority) {
 		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)) &&
+			// listenCancelled is deprecated and should be removed in 2.9
+			!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);
+	}
+
 	private static long startEvent;
 
 	/**
@@ -180,11 +197,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 +343,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/events/SimpleEvents.java

@@ -18,9 +18,18 @@
  */
 package ch.njol.skript.events;
 
+import ch.njol.skript.Skript;
+import ch.njol.skript.lang.SkriptEvent.ListeningBehavior;
+import ch.njol.skript.lang.util.SimpleEvent;
 import com.destroystokyo.paper.event.block.AnvilDamagedEvent;
+import com.destroystokyo.paper.event.entity.EntityJumpEvent;
+import com.destroystokyo.paper.event.entity.ProjectileCollideEvent;
+import com.destroystokyo.paper.event.player.PlayerArmorChangeEvent;
+import com.destroystokyo.paper.event.player.PlayerJumpEvent;
+import com.destroystokyo.paper.event.server.PaperServerListPingEvent;
 import io.papermc.paper.event.player.PlayerDeepSleepEvent;
 import io.papermc.paper.event.player.PlayerInventorySlotChangeEvent;
+import io.papermc.paper.event.player.PlayerTradeEvent;
 import org.bukkit.event.block.BlockCanBuildEvent;
 import org.bukkit.event.block.BlockDamageEvent;
 import org.bukkit.event.block.BlockFertilizeEvent;
@@ -33,9 +42,9 @@
 import org.bukkit.event.block.BlockSpreadEvent;
 import org.bukkit.event.block.LeavesDecayEvent;
 import org.bukkit.event.block.SignChangeEvent;
+import org.bukkit.event.block.SpongeAbsorbEvent;
 import org.bukkit.event.enchantment.EnchantItemEvent;
 import org.bukkit.event.enchantment.PrepareItemEnchantEvent;
-import org.bukkit.event.block.SpongeAbsorbEvent;
 import org.bukkit.event.entity.AreaEffectCloudApplyEvent;
 import org.bukkit.event.entity.CreeperPowerEvent;
 import org.bukkit.event.entity.EntityBreakDoorEvent;
@@ -110,16 +119,6 @@
 import org.spigotmc.event.entity.EntityDismountEvent;
 import org.spigotmc.event.entity.EntityMountEvent;
 
-import com.destroystokyo.paper.event.entity.ProjectileCollideEvent;
-import com.destroystokyo.paper.event.player.PlayerArmorChangeEvent;
-import com.destroystokyo.paper.event.player.PlayerJumpEvent;
-import com.destroystokyo.paper.event.server.PaperServerListPingEvent;
-import com.destroystokyo.paper.event.entity.EntityJumpEvent;
-import io.papermc.paper.event.player.PlayerTradeEvent;
-import ch.njol.skript.Skript;
-import ch.njol.skript.SkriptEventHandler;
-import ch.njol.skript.lang.util.SimpleEvent;
-
 /**
  * @author Peter Güttinger
  */
@@ -475,13 +474,13 @@ public class SimpleEvents {
 		Skript.registerEvent("Resurrect Attempt", SimpleEvent.class, EntityResurrectEvent.class, "[entity] resurrect[ion] [attempt]")
 				.description("Called when an entity dies, always. If they are not holding a totem, this is cancelled - you can, however, uncancel it.")
 				.examples(
-						"on resurrect attempt:",
+					"on resurrect attempt:",
 						"\tentity is player",
 						"\tentity has permission \"admin.undying\"",
 						"\tuncancel the event"
 				)
-				.since("2.2-dev28");
-		SkriptEventHandler.listenCancelled.add(EntityResurrectEvent.class); // Listen this even when cancelled
+				.since("2.2-dev28")
+				.listeningBehavior(ListeningBehavior.ANY);
 		Skript.registerEvent("Player World Change", SimpleEvent.class, PlayerChangedWorldEvent.class, "[player] world chang(ing|e[d])")
 				.description("Called when a player enters a world. Does not work with other entities!")
 				.examples("on player world change:",

src/main/java/ch/njol/skript/lang/SkriptEvent.java

@@ -26,12 +26,14 @@
 import ch.njol.skript.events.EvtClick;
 import ch.njol.skript.lang.SkriptParser.ParseResult;
 import ch.njol.skript.structures.StructEvent.EventData;
-import org.skriptlang.skript.lang.script.Script;
-import org.skriptlang.skript.lang.entry.EntryContainer;
-import org.skriptlang.skript.lang.structure.Structure;
+import ch.njol.skript.util.Utils;
+import org.bukkit.event.Cancellable;
 import org.bukkit.event.Event;
 import org.bukkit.event.EventPriority;
 import org.eclipse.jdt.annotation.Nullable;
+import org.skriptlang.skript.lang.entry.EntryContainer;
+import org.skriptlang.skript.lang.script.Script;
+import org.skriptlang.skript.lang.structure.Structure;
 
 import java.util.List;
 import java.util.Locale;
@@ -54,6 +56,9 @@ public abstract class SkriptEvent extends Structure {
 	private String expr;
 	@Nullable
 	protected EventPriority eventPriority;
+	@Nullable
+	protected ListeningBehavior listeningBehavior;
+	protected boolean supportsListeningBehavior;
 	private SkriptEventInfo<?> skriptEventInfo;
 
 	/**
@@ -65,7 +70,9 @@ public abstract class SkriptEvent extends Structure {
 	public final boolean init(Literal<?>[] args, int matchedPattern, ParseResult parseResult, EntryContainer entryContainer) {
 		this.expr = parseResult.expr;
 
-		EventPriority priority = getParser().getData(EventData.class).getPriority();
+		EventData eventData = getParser().getData(EventData.class);
+
+		EventPriority priority = eventData.getPriority();
 		if (priority != null && !isEventPrioritySupported()) {
 			Skript.error("This event doesn't support event priority");
 			return false;
@@ -77,6 +84,27 @@ public final boolean init(Literal<?>[] args, int matchedPattern, ParseResult par
 			throw new IllegalStateException();
 		skriptEventInfo = (SkriptEventInfo<?>) syntaxElementInfo;
 
+		// evaluate whether this event supports listening to cancelled events
+		supportsListeningBehavior = true;
+		for (Class<? extends Event> eventClass : getEventClasses()) {
+			if (!Cancellable.class.isAssignableFrom(eventClass)) {
+				supportsListeningBehavior = false;
+				break;
+			}
+		}
+
+		ListeningBehavior behavior = eventData.getListenerBehavior();
+		// only set the listening behavior if it's not null,
+		// so children can know if the user has set a listening behavior or not
+		if (behavior != null) {
+			if (behavior == ListeningBehavior.CANCELLED && !isListeningBehaviorSupported()) {
+				String eventName = skriptEventInfo.name.toLowerCase(Locale.ENGLISH);
+				Skript.error(Utils.A(eventName) + " event cannot be cancelled, so listening for the cancelled event isn't allowed.");
+				return false;
+			}
+			listeningBehavior = behavior;
+		}
+
 		return init(args, matchedPattern, parseResult);
 	}
 
@@ -199,6 +227,21 @@ public boolean isEventPrioritySupported() {
 		return true;
 	}
 
+	/**
+	 * @return the {@link ListeningBehavior} to be used for this event. Defaults to the default listening behavior
+	 * of the SkriptEventInfo for this SkriptEvent.
+	 */
+	public ListeningBehavior getListeningBehavior() {
+		return listeningBehavior != null ? listeningBehavior : skriptEventInfo.getListeningBehavior();
+	}
+
+	/**
+	 * @return whether this SkriptEvent supports listening behaviors
+	 */
+	public boolean isListeningBehaviorSupported() {
+		return supportsListeningBehavior;
+	}
+
 	/**
 	 * Override this method to allow Skript to not force synchronization.
 	 */
@@ -241,4 +284,41 @@ public static SkriptEvent parse(String expr, SectionNode sectionNode, @Nullable
 		return (SkriptEvent) Structure.parse(expr, sectionNode, defaultError, Skript.getEvents().iterator());
 	}
 
+	/**
+	 * The listening behavior of a Skript event. This determines whether the event should be run for cancelled events, uncancelled events, or both.
+	 */
+	public enum ListeningBehavior {
+		/**
+		 * This Skript event should be run for any uncancelled event.
+		 */
+		UNCANCELLED,
+		/**
+		 * This Skript event should be run for any cancelled event.
+		 */
+		CANCELLED,
+		/**
+		 * This Skript event should be run for any event, cancelled or uncancelled.
+		 */
+		ANY;
+
+		/**
+		 * Checks whether this listening behavior matches the given cancelled state.
+		 * @param cancelled Whether the event is cancelled.
+		 * @return Whether an event with the given cancelled state should be run for this listening behavior.
+		 */
+		public boolean matches(final boolean cancelled) {
+			switch (this) {
+				case CANCELLED:
+					return cancelled;
+				case UNCANCELLED:
+					return !cancelled;
+				case ANY:
+					return true;
+				default:
+					assert false;
+					return false;
+			}
+		}
+	}
+
 }