From ece376a69ef52dcdc5539eab332d11a54d761af9 Mon Sep 17 00:00:00 2001 From: JOO200 Date: Thu, 23 Mar 2023 18:58:48 +0100 Subject: [PATCH] Improve NPC handling in WorldGuard's listeners (#1988) * fix: added more npc checks in listeners * Do not listen to events from NPCs such as Citizens. Don't handle NPCs as players. * replaced hasMetaData calls by Entities utilities class --- .../worldguard/bukkit/WorldGuardPlugin.java | 6 +++-- .../sk89q/worldguard/bukkit/cause/Cause.java | 17 +++++++------ .../bukkit/listener/AbstractListener.java | 16 ++++++------ .../listener/InvincibilityListener.java | 14 ++++++----- .../bukkit/listener/RegionFlagsListener.java | 21 ++++++++-------- .../listener/RegionProtectionListener.java | 3 +-- .../listener/WorldGuardEntityListener.java | 25 ++++++++----------- .../listener/WorldGuardPlayerListener.java | 2 ++ .../listener/WorldGuardVehicleListener.java | 2 ++ .../bukkit/session/BukkitSessionManager.java | 7 +++--- .../worldguard/bukkit/util/InteropUtils.java | 2 +- 11 files changed, 61 insertions(+), 54 deletions(-) diff --git a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/WorldGuardPlugin.java b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/WorldGuardPlugin.java index e0f83634..86aefb55 100644 --- a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/WorldGuardPlugin.java +++ b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/WorldGuardPlugin.java @@ -60,6 +60,7 @@ import com.sk89q.worldguard.bukkit.listener.WorldGuardWorldListener; import com.sk89q.worldguard.bukkit.listener.WorldRulesListener; import com.sk89q.worldguard.bukkit.session.BukkitSessionManager; import com.sk89q.worldguard.bukkit.util.ClassSourceValidator; +import com.sk89q.worldguard.bukkit.util.Entities; import com.sk89q.worldguard.bukkit.util.Events; import com.sk89q.worldguard.commands.GeneralCommands; import com.sk89q.worldguard.commands.ProtectionCommands; @@ -419,8 +420,9 @@ public class WorldGuardPlugin extends JavaPlugin { } public Actor wrapCommandSender(CommandSender sender) { - if (sender instanceof Player) { - return wrapPlayer((Player) sender); + if (sender instanceof Player player) { + if (Entities.isNPC(player)) return null; + return wrapPlayer(player); } try { diff --git a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/cause/Cause.java b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/cause/Cause.java index c7821026..0c6003c3 100644 --- a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/cause/Cause.java +++ b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/cause/Cause.java @@ -24,6 +24,7 @@ import com.google.common.collect.Sets; import com.sk89q.worldguard.bukkit.BukkitWorldConfiguration; import com.sk89q.worldguard.bukkit.WorldGuardPlugin; import com.sk89q.worldguard.bukkit.internal.WGMetadata; +import com.sk89q.worldguard.bukkit.util.Entities; import io.papermc.lib.PaperLib; import org.bukkit.Bukkit; import org.bukkit.block.Block; @@ -134,8 +135,8 @@ public final class Cause { @Nullable public Player getFirstPlayer() { for (Object object : causes) { - if (object instanceof Player) { - return (Player) object; + if (object instanceof Player p && !Entities.isNPC(p)) { + return p; } } @@ -145,8 +146,8 @@ public final class Cause { @Nullable public Entity getFirstEntity() { for (Object object : causes) { - if (object instanceof Entity) { - return (Entity) object; + if (object instanceof Entity e) { + return e; } } @@ -156,8 +157,8 @@ public final class Cause { @Nullable public Entity getFirstNonPlayerEntity() { for (Object object : causes) { - if (object instanceof Entity && !(object instanceof Player)) { - return (Entity) object; + if (object instanceof Entity e && (!(object instanceof Player) || Entities.isNPC(e))) { + return e; } } @@ -167,8 +168,8 @@ public final class Cause { @Nullable public Block getFirstBlock() { for (Object object : causes) { - if (object instanceof Block) { - return (Block) object; + if (object instanceof Block b) { + return b; } } diff --git a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/AbstractListener.java b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/AbstractListener.java index e63e01f6..07bb23a0 100644 --- a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/AbstractListener.java +++ b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/AbstractListener.java @@ -29,6 +29,7 @@ import com.sk89q.worldguard.bukkit.BukkitPlayer; import com.sk89q.worldguard.bukkit.BukkitWorldConfiguration; import com.sk89q.worldguard.bukkit.WorldGuardPlugin; import com.sk89q.worldguard.bukkit.cause.Cause; +import com.sk89q.worldguard.bukkit.util.Entities; import com.sk89q.worldguard.config.WorldConfiguration; import com.sk89q.worldguard.domains.Association; import com.sk89q.worldguard.protection.association.DelayedRegionOverlapAssociation; @@ -124,13 +125,12 @@ class AbstractListener implements Listener { if (!cause.isKnown()) { return Associables.constant(Association.NON_MEMBER); - } else if (rootCause instanceof Player) { - return getPlugin().wrapPlayer((Player) rootCause); - } else if (rootCause instanceof OfflinePlayer) { - return getPlugin().wrapOfflinePlayer((OfflinePlayer) rootCause); - } else if (rootCause instanceof Entity) { + } else if (rootCause instanceof Player player && !Entities.isNPC(player)) { + return getPlugin().wrapPlayer(player); + } else if (rootCause instanceof OfflinePlayer offlinePlayer) { + return getPlugin().wrapOfflinePlayer(offlinePlayer); + } else if (rootCause instanceof Entity entity) { RegionQuery query = WorldGuard.getInstance().getPlatform().getRegionContainer().createQuery(); - final Entity entity = (Entity) rootCause; BukkitWorldConfiguration config = getWorldConfig(entity.getWorld()); Location loc; if (PaperLib.isPaper() && config.usePaperEntityOrigin) { @@ -144,9 +144,9 @@ class AbstractListener implements Listener { } return new DelayedRegionOverlapAssociation(query, BukkitAdapter.adapt(loc), config.useMaxPriorityAssociation); - } else if (rootCause instanceof Block) { + } else if (rootCause instanceof Block block) { RegionQuery query = WorldGuard.getInstance().getPlatform().getRegionContainer().createQuery(); - Location loc = ((Block) rootCause).getLocation(); + Location loc = block.getLocation(); return new DelayedRegionOverlapAssociation(query, BukkitAdapter.adapt(loc), getWorldConfig(loc.getWorld()).useMaxPriorityAssociation); } else { diff --git a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/InvincibilityListener.java b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/InvincibilityListener.java index 6a8ec9d0..8ced61e9 100644 --- a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/InvincibilityListener.java +++ b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/InvincibilityListener.java @@ -22,6 +22,7 @@ package com.sk89q.worldguard.bukkit.listener; import com.sk89q.worldguard.LocalPlayer; import com.sk89q.worldguard.WorldGuard; import com.sk89q.worldguard.bukkit.WorldGuardPlugin; +import com.sk89q.worldguard.bukkit.util.Entities; import org.bukkit.entity.Entity; import org.bukkit.entity.LivingEntity; import org.bukkit.entity.Player; @@ -57,9 +58,9 @@ public class InvincibilityListener extends AbstractListener { @EventHandler(ignoreCancelled = true) public void onEntityDamage(EntityDamageEvent event) { Entity victim = event.getEntity(); + if (Entities.isNPC(victim)) return; - if (victim instanceof Player) { - Player player = (Player) victim; + if (victim instanceof Player player) { LocalPlayer localPlayer = WorldGuardPlugin.inst().wrapPlayer(player); if (isInvincible(localPlayer)) { @@ -87,9 +88,9 @@ public class InvincibilityListener extends AbstractListener { @EventHandler(ignoreCancelled = true) public void onEntityCombust(EntityCombustEvent event) { Entity entity = event.getEntity(); + if (Entities.isNPC(entity)) return; - if (entity instanceof Player) { - Player player = (Player) entity; + if (entity instanceof Player player) { LocalPlayer localPlayer = WorldGuardPlugin.inst().wrapPlayer(player); if (isInvincible(localPlayer)) { @@ -100,8 +101,9 @@ public class InvincibilityListener extends AbstractListener { @EventHandler(ignoreCancelled = true) public void onFoodLevelChange(FoodLevelChangeEvent event) { - if (event.getEntity() instanceof Player) { - Player player = (Player) event.getEntity(); + if (Entities.isNPC(event.getEntity())) return; + + if (event.getEntity() instanceof Player player) { LocalPlayer localPlayer = WorldGuardPlugin.inst().wrapPlayer(player); if (event.getFoodLevel() < player.getFoodLevel() && isInvincible(localPlayer)) { diff --git a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/RegionFlagsListener.java b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/RegionFlagsListener.java index e2822f20..3cc05170 100644 --- a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/RegionFlagsListener.java +++ b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/RegionFlagsListener.java @@ -25,6 +25,7 @@ import com.sk89q.worldguard.WorldGuard; import com.sk89q.worldguard.bukkit.WorldGuardPlugin; import com.sk89q.worldguard.bukkit.event.block.BreakBlockEvent; import com.sk89q.worldguard.bukkit.event.block.PlaceBlockEvent; +import com.sk89q.worldguard.bukkit.util.Entities; import com.sk89q.worldguard.bukkit.util.Materials; import com.sk89q.worldguard.config.WorldConfiguration; import com.sk89q.worldguard.protection.association.RegionAssociable; @@ -118,24 +119,24 @@ public class RegionFlagsListener extends AbstractListener { World world = entity.getWorld(); if (!isRegionSupportEnabled(world)) return; // Region support disabled + if (Entities.isNPC(entity)) return; + if (!(entity instanceof Player player)) return; + RegionQuery query = WorldGuard.getInstance().getPlatform().getRegionContainer().createQuery(); - if (entity instanceof Player && event.getCause() == DamageCause.FALL) { - LocalPlayer localPlayer = WorldGuardPlugin.inst().wrapPlayer((Player) entity); + if (event.getCause() == DamageCause.FALL) { + LocalPlayer localPlayer = WorldGuardPlugin.inst().wrapPlayer(player); if (!query.testState(BukkitAdapter.adapt(entity.getLocation()), localPlayer, Flags.FALL_DAMAGE)) { event.setCancelled(true); return; } - } else { - if (entity instanceof Player && event.getCause() == DamageCause.FLY_INTO_WALL) { - LocalPlayer localPlayer = WorldGuardPlugin.inst().wrapPlayer((Player) entity); - if (!query.testState(BukkitAdapter.adapt(entity.getLocation()), localPlayer, Flags.FALL_DAMAGE)) { - event.setCancelled(true); - return; - } + } else if (event.getCause() == DamageCause.FLY_INTO_WALL) { + LocalPlayer localPlayer = WorldGuardPlugin.inst().wrapPlayer(player); + if (!query.testState(BukkitAdapter.adapt(entity.getLocation()), localPlayer, Flags.FALL_DAMAGE)) { + event.setCancelled(true); + return; } } - } /** diff --git a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/RegionProtectionListener.java b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/RegionProtectionListener.java index ed7db2df..5e9ed126 100644 --- a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/RegionProtectionListener.java +++ b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/RegionProtectionListener.java @@ -522,8 +522,7 @@ public class RegionProtectionListener extends AbstractListener { if (!isRegionSupportEnabled(vehicle.getWorld())) return; // Region support disabled Entity exited = event.getExited(); - if (vehicle instanceof Tameable && exited instanceof Player) { - Player player = (Player) exited; + if (vehicle instanceof Tameable && exited instanceof Player player && !Entities.isNPC(player)) { LocalPlayer localPlayer = WorldGuardPlugin.inst().wrapPlayer(player); if (!isWhitelisted(Cause.create(player), vehicle.getWorld(), false)) { RegionQuery query = WorldGuard.getInstance().getPlatform().getRegionContainer().createQuery(); diff --git a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/WorldGuardEntityListener.java b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/WorldGuardEntityListener.java index 74a77c59..ea59fb5d 100644 --- a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/WorldGuardEntityListener.java +++ b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/WorldGuardEntityListener.java @@ -138,8 +138,7 @@ public class WorldGuardEntityListener extends AbstractListener { event.setCancelled(true); return; } - } else if (defender instanceof Player) { - Player player = (Player) defender; + } else if (defender instanceof Player player && !Entities.isNPC(defender)) { LocalPlayer localPlayer = WorldGuardPlugin.inst().wrapPlayer(player); if (wcfg.disableLavaDamage && type == DamageCause.LAVA) { @@ -227,8 +226,7 @@ public class WorldGuardEntityListener extends AbstractListener { } } - if (defender instanceof Player) { - Player player = (Player) defender; + if (defender instanceof Player player && !Entities.isNPC(defender)) { LocalPlayer localPlayer = getPlugin().wrapPlayer(player); if (wcfg.disableLightningDamage && event.getCause() == DamageCause.LIGHTNING) { @@ -289,8 +287,7 @@ public class WorldGuardEntityListener extends AbstractListener { } WorldConfiguration wcfg = getWorldConfig(defender.getWorld()); - if (defender instanceof Player) { - Player player = (Player) defender; + if (defender instanceof Player player && !Entities.isNPC(defender)) { LocalPlayer localPlayer = getPlugin().wrapPlayer(player); @@ -363,8 +360,7 @@ public class WorldGuardEntityListener extends AbstractListener { event.setCancelled(true); return; } - } else if (defender instanceof Player) { - Player player = (Player) defender; + } else if (defender instanceof Player player && !Entities.isNPC(defender)) { LocalPlayer localPlayer = WorldGuardPlugin.inst().wrapPlayer(player); if (type == DamageCause.WITHER) { @@ -665,8 +661,8 @@ public class WorldGuardEntityListener extends AbstractListener { .get(world); if (regionManager == null) return; LocalPlayer associable = null; - if (event.getEntity() instanceof Player) { - associable = getPlugin().wrapPlayer(((Player) event.getEntity())); + if (event.getEntity() instanceof Player player) { + associable = getPlugin().wrapPlayer(player); if (WorldGuard.getInstance().getPlatform().getSessionManager().hasBypass(associable, world)) { return; } @@ -745,10 +741,10 @@ public class WorldGuardEntityListener extends AbstractListener { event.setCancelled(true); return; } - if (wcfg.useRegions && ent instanceof Player + if (wcfg.useRegions && ent instanceof Player player && !Entities.isNPC(ent) && !WorldGuard.getInstance().getPlatform().getRegionContainer().createQuery().testState( BukkitAdapter.adapt(ent.getLocation()), - WorldGuardPlugin.inst().wrapPlayer((Player) ent), + WorldGuardPlugin.inst().wrapPlayer(player), Flags.HEALTH_REGEN)) { event.setCancelled(true); } @@ -758,10 +754,11 @@ public class WorldGuardEntityListener extends AbstractListener { public void onFoodChange(FoodLevelChangeEvent event) { if (event.getItem() != null) return; HumanEntity ent = event.getEntity(); - if (!(ent instanceof Player)) return; + if (Entities.isNPC(ent)) return; + if (!(ent instanceof Player bukkitPlayer)) return; if (event.getFoodLevel() > ent.getFoodLevel()) return; - LocalPlayer player = WorldGuardPlugin.inst().wrapPlayer((Player) ent); + LocalPlayer player = WorldGuardPlugin.inst().wrapPlayer(bukkitPlayer); WorldConfiguration wcfg = getWorldConfig(ent.getWorld()); if (wcfg.useRegions diff --git a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/WorldGuardPlayerListener.java b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/WorldGuardPlayerListener.java index 6c722bcc..f2538062 100644 --- a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/WorldGuardPlayerListener.java +++ b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/WorldGuardPlayerListener.java @@ -314,6 +314,7 @@ public class WorldGuardPlayerListener extends AbstractListener { @EventHandler(priority = EventPriority.HIGHEST) public void onPlayerRespawn(PlayerRespawnEvent event) { Player player = event.getPlayer(); + if (com.sk89q.worldguard.bukkit.util.Entities.isNPC(player)) return; WorldConfiguration wcfg = getWorldConfig(player.getWorld()); if (wcfg.useRegions) { @@ -348,6 +349,7 @@ public class WorldGuardPlayerListener extends AbstractListener { @EventHandler(priority = EventPriority.LOW, ignoreCancelled = true) public void onPlayerTeleport(PlayerTeleportEvent event) { Player player = event.getPlayer(); + if (com.sk89q.worldguard.bukkit.util.Entities.isNPC(player)) return; LocalPlayer localPlayer = getPlugin().wrapPlayer(player); ConfigurationManager cfg = getConfig(); WorldConfiguration wcfg = getWorldConfig(player.getWorld()); diff --git a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/WorldGuardVehicleListener.java b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/WorldGuardVehicleListener.java index 4c044808..fe604dad 100644 --- a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/WorldGuardVehicleListener.java +++ b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/listener/WorldGuardVehicleListener.java @@ -24,6 +24,7 @@ import com.sk89q.worldedit.util.Location; import com.sk89q.worldguard.LocalPlayer; import com.sk89q.worldguard.WorldGuard; import com.sk89q.worldguard.bukkit.WorldGuardPlugin; +import com.sk89q.worldguard.bukkit.util.Entities; import com.sk89q.worldguard.config.WorldConfiguration; import com.sk89q.worldguard.session.MoveType; import com.sk89q.worldguard.util.Locations; @@ -59,6 +60,7 @@ public class WorldGuardVehicleListener extends AbstractListener { // Did we move a block? if (Locations.isDifferentBlock(BukkitAdapter.adapt(event.getFrom()), BukkitAdapter.adapt(event.getTo()))) { for (Player player : playerPassengers) { + if (Entities.isNPC(player)) continue; LocalPlayer localPlayer = getPlugin().wrapPlayer(player); Location lastValid; if ((lastValid = WorldGuard.getInstance().getPlatform().getSessionManager().get(localPlayer) diff --git a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/session/BukkitSessionManager.java b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/session/BukkitSessionManager.java index da23d6bf..e01d5a35 100644 --- a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/session/BukkitSessionManager.java +++ b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/session/BukkitSessionManager.java @@ -25,6 +25,7 @@ import com.sk89q.worldguard.WorldGuard; import com.sk89q.worldguard.bukkit.BukkitPlayer; import com.sk89q.worldguard.bukkit.WorldGuardPlugin; import com.sk89q.worldguard.bukkit.event.player.ProcessPlayerEvent; +import com.sk89q.worldguard.bukkit.util.Entities; import com.sk89q.worldguard.session.AbstractSessionManager; import com.sk89q.worldguard.session.Session; import com.sk89q.worldguard.session.handler.Handler; @@ -81,9 +82,9 @@ public class BukkitSessionManager extends AbstractSessionManager implements Runn @Override public boolean hasBypass(LocalPlayer player, World world) { - if (player instanceof BukkitPlayer) { - if (((BukkitPlayer) player).getPlayer().hasMetadata("NPC") - && WorldGuard.getInstance().getPlatform().getGlobalStateManager().get(world).fakePlayerBuildOverride) { + if (player instanceof BukkitPlayer bukkitPlayer) { + if (Entities.isNPC(bukkitPlayer.getPlayer()) + && WorldGuard.getInstance().getPlatform().getGlobalStateManager().get(world).fakePlayerBuildOverride) { return true; } if (!((BukkitPlayer) player).getPlayer().isOnline()) { diff --git a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/util/InteropUtils.java b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/util/InteropUtils.java index 8bb2ce89..ef53d7b2 100644 --- a/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/util/InteropUtils.java +++ b/worldguard-bukkit/src/main/java/com/sk89q/worldguard/bukkit/util/InteropUtils.java @@ -70,7 +70,7 @@ public final class InteropUtils { UUID uuid = player.getUniqueId(); String name = player.getName(); - if (player.hasMetadata("NPC")) { + if (Entities.isNPC(player)) { return true; }