From af513b03b079ad1a8e611fa6b81348b52384bb3b Mon Sep 17 00:00:00 2001 From: Andreas Troelsen Date: Tue, 24 Oct 2023 01:26:49 +0200 Subject: [PATCH] Prevent unauthorized sign edits. Since Minecraft 1.20, players can edit signs by right-clicking on them, and that poses a problem for the sign-centric portions of the plugin, such as class selection signs and the various types of arena signs. This commit refactors the PlayerInteractEvent handler in ArenaListener in order to break open the possibility of handling non-lobby players as well. We're a little more strict with lobby players, and we still want to handle class sign clicks and iron block clicks here. For players who aren't in the lobby, we're really just blocking the event according to the regular protection rules (block is inside region, protect is on, and arena is not in edit mode). It also blanket cancels events in the HandlesSignClicks event handler, because there is no meaningful way to edit an arena sign, since their contents come from the template file and not from what is written on them by the sign renderer. Ideally, we'd refactor some of this event handler logic, because it's a lot easier to take care of the individual responsibilities in separate event handlers. Fixes #765 --- changelog.md | 1 + .../garbagemule/MobArena/ArenaListener.java | 57 +++++++++++++++---- .../MobArena/signs/HandlesSignClicks.java | 1 + 3 files changed, 47 insertions(+), 12 deletions(-) diff --git a/changelog.md b/changelog.md index db192da..fa79747 100644 --- a/changelog.md +++ b/changelog.md @@ -15,6 +15,7 @@ These changes will (most likely) be included in the next version. - Explosion damage caused by Exploding Sheep now correctly counts as monster damage. This means that the explosions only affect other mobs if the per-arena setting `monster-infight` is set to `true`. - Explosion damage caused by the boss ability `obsidian-bomb` now correctly counts as monster damage. This means that the explosions only affect other mobs if the per-arena setting `monster-infight` is set to `true`. - An old discrepancy with auto start timers in the auto-ready logic has been removed. This fixes an issue in MobArenaStats where the extension would throw errors in arenas with `auto-ready: true` and a non-zero auto start timer. Note that the combination of `auto-ready: true` and a `default-class` now _requires_ the use of a `start-delay-timer` to prevent the arena from starting immediately when the first player joins. +- Signs in arena regions, as well as Arena Signs anywhere, can no longer be edited by right-clicking. - Signs now correctly restore themselves again in arenas with `soft-restore: true`. ## [0.107] - 2022-07-30 diff --git a/src/main/java/com/garbagemule/MobArena/ArenaListener.java b/src/main/java/com/garbagemule/MobArena/ArenaListener.java index 3092846..42b2a3d 100644 --- a/src/main/java/com/garbagemule/MobArena/ArenaListener.java +++ b/src/main/java/com/garbagemule/MobArena/ArenaListener.java @@ -44,6 +44,7 @@ import org.bukkit.entity.TNTPrimed; import org.bukkit.entity.ThrownPotion; import org.bukkit.entity.Vehicle; import org.bukkit.event.Event.Result; +import org.bukkit.event.block.Action; import org.bukkit.event.block.BlockBreakEvent; import org.bukkit.event.block.BlockBurnEvent; import org.bukkit.event.block.BlockEvent; @@ -1089,26 +1090,58 @@ public class ArenaListener } public void onPlayerInteract(PlayerInteractEvent event) { - Player p = event.getPlayer(); - if (!arena.inLobby(p)) return; + if (arena.inLobby(event.getPlayer())) { + onLobbyPlayerInteract(event); + } else { + onNonLobbyPlayerInteract(event); + } + } - // Prevent placing blocks and using held items + private void onLobbyPlayerInteract(PlayerInteractEvent event) { if (event.hasItem()) { event.setUseItemInHand(Result.DENY); } - // Bail if off-hand or if there's no block involved. - if (event.getHand() == EquipmentSlot.OFF_HAND || !event.hasBlock()) + if (event.getHand() == EquipmentSlot.OFF_HAND) { return; - - // Iron block - if (event.getClickedBlock().getType() == Material.IRON_BLOCK) { - handleReadyBlock(p); } - // Sign - else if (event.getClickedBlock().getState() instanceof Sign) { + + Block block = event.getClickedBlock(); + if (block == null) { + return; + } + + if (block.getType() == Material.IRON_BLOCK) { + handleReadyBlock(event.getPlayer()); + } else if (block.getState() instanceof Sign) { + if (event.getAction() == Action.RIGHT_CLICK_BLOCK) { + event.setCancelled(true); + } Sign sign = (Sign) event.getClickedBlock().getState(); - handleSign(sign, p); + handleSign(sign, event.getPlayer()); + } + } + + private void onNonLobbyPlayerInteract(PlayerInteractEvent event) { + if (!protect) { + return; + } + + Block block = event.getClickedBlock(); + if (block == null) { + return; + } + if (!region.contains(block.getLocation())) { + return; + } + if (arena.inEditMode()) { + return; + } + + if (block.getState() instanceof Sign) { + if (event.getAction() == Action.RIGHT_CLICK_BLOCK) { + event.setCancelled(true); + } } } diff --git a/src/main/java/com/garbagemule/MobArena/signs/HandlesSignClicks.java b/src/main/java/com/garbagemule/MobArena/signs/HandlesSignClicks.java index 375719d..c5b8804 100644 --- a/src/main/java/com/garbagemule/MobArena/signs/HandlesSignClicks.java +++ b/src/main/java/com/garbagemule/MobArena/signs/HandlesSignClicks.java @@ -41,6 +41,7 @@ class HandlesSignClicks implements Listener { ArenaSign sign = signStore.findByLocation(block.getLocation()); if (sign != null) { + event.setCancelled(true); purgeAndInvoke(sign, event.getPlayer()); } }