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,42 @@ 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);
-
-		if (isCancelled && isResultDeny) {
-			if (Skript.logVeryHigh())
-				Skript.info(" -x- was cancelled");
-			return;
-		}
+		// This handles cancelled left/right clicks on air. Left/right clicks on air are called as cancelled,
+		// but we want to listen to them as if they are uncancelled unless they're specifically cancelled,
+		// so we can't rely on isCancelled() alone.
+		// Checking useItemInHand() is a reliable method, as it will be DENY if the event was specifically cancelled.
+		// Note that right clicks on air with nothing in hand aren't ever sent to the server.
+		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
+		// listenCancelled is deprecated and should be removed in 2.9
+		boolean isCancelled = event instanceof Cancellable &&
+			(((Cancellable) event).isCancelled() && isResultDeny) &&
+			!listenCancelled.contains(event.getClass());
+
+		// 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.matchesListeningBehavior(isCancelled))
+				continue;
+
 			// these methods need to be run on whatever thread the trigger is
 			Runnable execute = () -> {
 				logTriggerStart(trigger);
@@ -175,16 +176,35 @@ private static void check(Event event, EventPriority priority) {
 	private static long startEvent;
 
 	/**
-	 * Logs that the provided Event has started.
+	 * Logs that the provided Event has started, using the default {@link EventPriority}.
 	 * Requires {@link Skript#logVeryHigh()} to be true to log anything.
 	 * @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 +327,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,17 @@
  */
 package ch.njol.skript.events;
 
+import ch.njol.skript.Skript;
+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 +41,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 +118,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
  */
@@ -473,15 +471,14 @@ public class SimpleEvents {
 				.examples("on slime split:")
 				.since("2.2-dev26");
 		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.")
+				.description("Called when an entity dies, always. If they are not holding a totem, the event will be cancelled - you can, however, uncancel it. Remember to use \"on any\" or \"on cancelled\" to listen to cancelled events.")
 				.examples(
-						"on resurrect attempt:",
-						"\tentity is player",
-						"\tentity has permission \"admin.undying\"",
-						"\tuncancel the event"
+						"on any 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
 		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

@@ -54,6 +54,7 @@ public abstract class SkriptEvent extends Structure {
 	private String expr;
 	@Nullable
 	protected EventPriority eventPriority;
+	protected ListeningBehavior listeningBehavior;
 	private SkriptEventInfo<?> skriptEventInfo;
 
 	/**
@@ -65,7 +66,11 @@ 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);
+
+		listeningBehavior = eventData.getListenerBehavior();
+
+		EventPriority priority = eventData.getPriority();
 		if (priority != null && !isEventPrioritySupported()) {
 			Skript.error("This event doesn't support event priority");
 			return false;
@@ -199,6 +204,22 @@ public boolean isEventPrioritySupported() {
 		return true;
 	}
 
+	/**
+	 * @return the {@link ListeningBehavior} to be used for this event.
+	 */
+	public ListeningBehavior getListeningBehavior() {
+		return listeningBehavior;
+	}
+
+	/**
+	 * @return whether this SkriptEvent should be run for the given cancelled state.
+	 */
+	public boolean matchesListeningBehavior(boolean isCancelled) {
+		return listeningBehavior == ListeningBehavior.ANY
+				|| (listeningBehavior == ListeningBehavior.UNCANCELLED && !isCancelled)
+				|| (listeningBehavior == ListeningBehavior.CANCELLED && isCancelled);
+	}
+
 	/**
 	 * Override this method to allow Skript to not force synchronization.
 	 */
@@ -241,4 +262,22 @@ 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
+	}
+
 }

src/main/java/ch/njol/skript/structures/StructEvent.java

@@ -22,6 +22,7 @@
 import ch.njol.skript.doc.NoDoc;
 import ch.njol.skript.lang.Literal;
 import ch.njol.skript.lang.SkriptEvent;
+import ch.njol.skript.lang.SkriptEvent.ListeningBehavior;
 import ch.njol.skript.lang.SkriptParser.ParseResult;
 import ch.njol.skript.lang.parser.ParserInstance;
 import org.bukkit.event.Event;
@@ -37,7 +38,7 @@ public class StructEvent extends Structure {
 
 	static {
 		Skript.registerStructure(StructEvent.class,
-				"[on] <.+> [with priority (:(lowest|low|normal|high|highest|monitor))]");
+				"[on] [uncancelled|:cancelled|any:(any|all)] <.+> [priority:with priority (:(lowest|low|normal|high|highest|monitor))]");
 	}
 
 	private SkriptEvent event;
@@ -46,8 +47,21 @@ public class StructEvent extends Structure {
 	@SuppressWarnings("ConstantConditions")
 	public boolean init(Literal<?>[] args, int matchedPattern, ParseResult parseResult, EntryContainer entryContainer) {
 		String expr = parseResult.regexes.get(0).group();
-		if (!parseResult.tags.isEmpty())
-			getParser().getData(EventData.class).priority = EventPriority.valueOf(parseResult.tags.get(0).toUpperCase(Locale.ENGLISH));
+
+		EventData data = getParser().getData(EventData.class);
+
+		data.behavior = ListeningBehavior.UNCANCELLED;
+		if (parseResult.hasTag("cancelled")) {
+			data.behavior = ListeningBehavior.CANCELLED;
+		} else if (parseResult.hasTag("any")) {
+			data.behavior = ListeningBehavior.ANY;
+		}
+
+		if (parseResult.hasTag("priority")) {
+			String lastTag = parseResult.tags.get(parseResult.tags.size() - 1);
+			data.priority = EventPriority.valueOf(lastTag.toUpperCase(Locale.ENGLISH));
+		}
+
 		event = SkriptEvent.parse(expr, entryContainer.getSource(), null);
 		return event != null;
 	}
@@ -102,6 +116,8 @@ public static class EventData extends ParserInstance.Data {
 
 		@Nullable
 		private EventPriority priority;
+		@Nullable
+		private ListeningBehavior behavior;
 
 		public EventData(ParserInstance parserInstance) {
 			super(parserInstance);
@@ -112,6 +128,13 @@ public EventPriority getPriority() {
 			return priority;
 		}
 
+		/**
+		 * @return the listening behavior that should be used for the event. Defaults to {@link ListeningBehavior#UNCANCELLED}
+		 */
+		public ListeningBehavior getListenerBehavior() {
+			return behavior == null ? ListeningBehavior.UNCANCELLED : behavior;
+		}
+
 	}
 
 }