Skip to content

Fix PIE not being cancelled after a cancelled PIEE #8435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
APickledWalrus merged 6 commits into from
Mar 27, 2026
+54 −46
Merged

Fix PIE not being cancelled after a cancelled PIEE #8435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4f4684b
Ensure that a previously cancelled PlayerInteractEntityEvent is propa…
sovdeeth Feb 9, 2026
168bbda
rework how PIE cancellation is determined
sovdeeth Feb 9, 2026
fc19471
Update how we determine PIE cancellation status (again)
sovdeeth Feb 9, 2026
a8908be
Merge branch 'dev/patch' into patch/fix-failure-to-cancel-PIE-after-PIEE
sovdeeth Mar 2, 2026
d4674e9
Update src/main/java/ch/njol/skript/SkriptEventHandler.java
sovdeeth Mar 23, 2026
30742d9
Merge branch 'dev/patch' into patch/fix-failure-to-cancel-PIE-after-PIEE
APickledWalrus Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 26 additions & 23 deletions src/main/java/ch/njol/skript/SkriptEventHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,17 @@
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;
import org.bukkit.event.*;
import org.bukkit.event.Event.Result;
import org.bukkit.event.EventPriority;
import org.bukkit.event.HandlerList;
import org.bukkit.event.Listener;
import org.bukkit.event.block.Action;
import org.bukkit.event.player.PlayerInteractEvent;
import org.bukkit.plugin.EventExecutor;
import org.bukkit.plugin.RegisteredListener;
import org.jetbrains.annotations.Nullable;

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.*;
import java.util.Map.Entry;
import java.util.Set;
import java.util.stream.Collectors;

public final class SkriptEventHandler {
Expand Down Expand Up @@ -130,25 +120,38 @@ private static void check(Event event, EventPriority priority) {
* @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());
if (!(event instanceof Cancellable cancellable))
return false;
// PlayerInteractEvents act differently
if (event instanceof PlayerInteractEvent interactEvent)
return isPlayerInteractEventCancelled(interactEvent);
return cancellable.isCancelled();
}

/**
* 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.
* These events are special in that they are called as cancelled when one of the two interaction types is DENY.
* Their true cancellation value should be as follows:
* LEFT_CLICK_BLOCK -> useItemInHand == DENY || useInteractedBlock == DENY
* RIGHT_CLICK_BLOCK -> useItemInHand == DENY && useInteractedBlock == DENY.
* Note: Some of these events may be cancelled by denying only one of the two, like hoeing dirt.
* However, we cannot reliably determine when this is the case, so we have to err on the
* side of caution to avoid accidentally missing events that are clearly not cancelled,
* like shooting wind charges or enderpearls.
* LEFT_CLICK_AIR -> useItemInHand == DENY
* RIGHT_CLICK_AIR -> useItemInHand == DENY
* PHYSICAL -> useInteractedBlock == DENY
*
* @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 boolean isPlayerInteractEventCancelled(PlayerInteractEvent event) {
return switch (event.getAction()) {
case LEFT_CLICK_AIR, RIGHT_CLICK_AIR -> event.useItemInHand() == Result.DENY;
case RIGHT_CLICK_BLOCK -> event.useItemInHand() == Result.DENY && event.useInteractedBlock() == Result.DENY;
case PHYSICAL -> event.useInteractedBlock() == Result.DENY;
default -> event.useItemInHand() == Result.DENY || event.useInteractedBlock() == Result.DENY;
};
}

/**
Expand Down
26 changes: 14 additions & 12 deletions src/main/java/ch/njol/skript/bukkitutil/ClickEventTracker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,15 @@
package ch.njol.skript.bukkitutil;

import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.UUID;

import ch.njol.skript.effects.EffCancelEvent;
import org.bukkit.Bukkit;
import org.bukkit.entity.Player;
import org.bukkit.event.Cancellable;
import org.bukkit.event.Event;
import org.bukkit.event.player.PlayerInteractEvent;
import org.bukkit.inventory.EquipmentSlot;
import org.bukkit.plugin.java.JavaPlugin;

import ch.njol.skript.effects.EffCancelEvent;
import java.util.*;

/**
* Tracks click events to remove extraneous events for one player click.
Expand Down Expand Up @@ -79,12 +75,18 @@ public boolean checkEvent(Player player, Cancellable event, EquipmentSlot hand)
}

// Ignore this, but set its cancelled status based on one set to first event
if (event instanceof PlayerInteractEvent) { // Handle use item/block separately
if (event instanceof PlayerInteractEvent current) { // Handle use item/block separately
// Failing to do so caused issue SkriptLang/Skript#2303
PlayerInteractEvent firstClick = (PlayerInteractEvent) first.event;
PlayerInteractEvent click = (PlayerInteractEvent) event;
click.setUseInteractedBlock(firstClick.useInteractedBlock());
click.setUseItemInHand(firstClick.useItemInHand());
Cancellable previous = first.event;
if (previous instanceof PlayerInteractEvent prevClick) {
current.setUseInteractedBlock(prevClick.useInteractedBlock());
current.setUseItemInHand(prevClick.useItemInHand());
} else {
// in case the prev was PlayerInteractEntityEvent
Event.Result newResult = previous.isCancelled() ? Event.Result.DENY : Event.Result.DEFAULT;
current.setUseInteractedBlock(newResult);
current.setUseItemInHand(newResult);
}
} else {
event.setCancelled(first.event.isCancelled());
}
Expand Down
25 changes: 14 additions & 11 deletions src/main/java/ch/njol/skript/effects/EffCancelEvent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.bukkit.event.entity.EntityToggleSwimEvent;
import org.bukkit.event.inventory.InventoryInteractEvent;
import org.bukkit.event.player.PlayerDropItemEvent;
import org.bukkit.event.player.PlayerInteractEntityEvent;
import org.bukkit.event.player.PlayerInteractEvent;
import org.bukkit.event.player.PlayerLoginEvent;
import org.jetbrains.annotations.Nullable;
Expand Down Expand Up @@ -74,18 +75,20 @@ public boolean init(Expression<?>[] expressions, int matchedPattern,

@Override
public void execute(Event event) {
if (event instanceof Cancellable)
((Cancellable) event).setCancelled(cancel);
if (event instanceof PlayerInteractEvent) {
if (event instanceof Cancellable cancellable)
cancellable.setCancelled(cancel);
if (event instanceof PlayerInteractEvent playerInteractEvent) {
EvtClick.interactTracker.eventModified((Cancellable) event);
((PlayerInteractEvent) event).setUseItemInHand(cancel ? Event.Result.DENY : Event.Result.DEFAULT);
((PlayerInteractEvent) event).setUseInteractedBlock(cancel ? Event.Result.DENY : Event.Result.DEFAULT);
} else if (event instanceof BlockCanBuildEvent) {
((BlockCanBuildEvent) event).setBuildable(!cancel);
} else if (event instanceof PlayerDropItemEvent) {
PlayerUtils.updateInventory(((PlayerDropItemEvent) event).getPlayer());
} else if (event instanceof InventoryInteractEvent) {
PlayerUtils.updateInventory(((Player) ((InventoryInteractEvent) event).getWhoClicked()));
playerInteractEvent.setUseItemInHand(cancel ? Event.Result.DENY : Event.Result.DEFAULT);
playerInteractEvent.setUseInteractedBlock(cancel ? Event.Result.DENY : Event.Result.DEFAULT);
} else if (event instanceof PlayerInteractEntityEvent) {
EvtClick.interactTracker.eventModified((Cancellable) event);
} else if (event instanceof BlockCanBuildEvent blockCanBuildEvent) {
blockCanBuildEvent.setBuildable(!cancel);
} else if (event instanceof PlayerDropItemEvent playerDropItemEvent) {
PlayerUtils.updateInventory(playerDropItemEvent.getPlayer());
} else if (event instanceof InventoryInteractEvent interactEvent) {
PlayerUtils.updateInventory(((Player) interactEvent.getWhoClicked()));
}
}

Expand Down