From dfd916dc97ed64ba67a67c7d58dc4b48f8a736a9 Mon Sep 17 00:00:00 2001 From: A248 Date: Sun, 19 Apr 2020 18:56:47 -0400 Subject: [PATCH 1/3] No need to listen for PlayerKickEvent - quit event fires anyway The PlayerQuitEvent is fired even when the player is kicked. --- .../net/jitse/npclib/listeners/PlayerListener.java | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/api/src/main/java/net/jitse/npclib/listeners/PlayerListener.java b/api/src/main/java/net/jitse/npclib/listeners/PlayerListener.java index 5f87379..d39553e 100755 --- a/api/src/main/java/net/jitse/npclib/listeners/PlayerListener.java +++ b/api/src/main/java/net/jitse/npclib/listeners/PlayerListener.java @@ -30,17 +30,8 @@ public class PlayerListener implements Listener { @EventHandler public void onPlayerQuit(PlayerQuitEvent event) { - onPlayerLeave(event.getPlayer()); - } - - @EventHandler(ignoreCancelled = true, priority = EventPriority.MONITOR) - public void onPlayerKick(PlayerKickEvent event) { - onPlayerLeave(event.getPlayer()); - } - - private void onPlayerLeave(Player player) { - for (NPCBase npc : NPCManager.getAllNPCs()) - npc.onLogout(player); + for (NPCBase npc : NPCManager.getAllNPCs()) + npc.onLogout(event.getPlayer()); } @EventHandler From b00850b19dbb43c68c9b175a9f1c2075c382221f Mon Sep 17 00:00:00 2001 From: A248 Date: Mon, 20 Apr 2020 15:16:24 -0400 Subject: [PATCH 2/3] Permit periodic task instead of PlayerMoveEvent through NPCLibOptions Adds NPCLibOptions as an optional parameter to NPCLib's constructor. The movement handling specified by the NPCLibOptions will determine whether to use the PlayerMoveEvent or a task. --- .../main/java/net/jitse/npclib/NPCLib.java | 19 ++++- .../java/net/jitse/npclib/NPCLibOptions.java | 77 +++++++++++++++++++ .../npclib/listeners/HandleMoveBase.java | 26 +++++++ .../listeners/PeriodicMoveListener.java | 48 ++++++++++++ .../npclib/listeners/PlayerListener.java | 31 +------- .../listeners/PlayerMoveEventListener.java | 22 ++++++ 6 files changed, 192 insertions(+), 31 deletions(-) create mode 100644 api/src/main/java/net/jitse/npclib/NPCLibOptions.java create mode 100644 api/src/main/java/net/jitse/npclib/listeners/HandleMoveBase.java create mode 100644 api/src/main/java/net/jitse/npclib/listeners/PeriodicMoveListener.java create mode 100644 api/src/main/java/net/jitse/npclib/listeners/PlayerMoveEventListener.java diff --git a/api/src/main/java/net/jitse/npclib/NPCLib.java b/api/src/main/java/net/jitse/npclib/NPCLib.java index 32de577..12515c9 100644 --- a/api/src/main/java/net/jitse/npclib/NPCLib.java +++ b/api/src/main/java/net/jitse/npclib/NPCLib.java @@ -4,11 +4,14 @@ package net.jitse.npclib; +import net.jitse.npclib.NPCLibOptions.MovementHandling; import net.jitse.npclib.api.NPC; import net.jitse.npclib.api.utilities.Logger; import net.jitse.npclib.listeners.ChunkListener; import net.jitse.npclib.listeners.PacketListener; +import net.jitse.npclib.listeners.PeriodicMoveListener; import net.jitse.npclib.listeners.PlayerListener; +import net.jitse.npclib.listeners.PlayerMoveEventListener; import net.jitse.npclib.metrics.NPCLibMetrics; import org.bukkit.plugin.PluginManager; import org.bukkit.plugin.java.JavaPlugin; @@ -23,7 +26,7 @@ public final class NPCLib { private double autoHideDistance = 50.0; - public NPCLib(JavaPlugin plugin) { + private NPCLib(JavaPlugin plugin, MovementHandling moveHandling) { this.plugin = plugin; this.logger = new Logger("NPCLib"); @@ -49,6 +52,12 @@ public final class NPCLib { pluginManager.registerEvents(new PlayerListener(this), plugin); pluginManager.registerEvents(new ChunkListener(this), plugin); + if (moveHandling.usePme) { + pluginManager.registerEvents(new PlayerMoveEventListener(), plugin); + } else { + pluginManager.registerEvents(new PeriodicMoveListener(this, moveHandling.updateInterval), plugin); + } + // Boot the according packet listener. new PacketListener().start(this); @@ -59,6 +68,14 @@ public final class NPCLib { logger.info("Enabled for Minecraft " + versionName); } + public NPCLib(JavaPlugin plugin) { + this(plugin, MovementHandling.playerMoveEvent()); + } + + public NPCLib(JavaPlugin plugin, NPCLibOptions options) { + this(plugin, options.moveHandling); + } + /** * @return The JavaPlugin instance. */ diff --git a/api/src/main/java/net/jitse/npclib/NPCLibOptions.java b/api/src/main/java/net/jitse/npclib/NPCLibOptions.java new file mode 100644 index 0000000..3ca8445 --- /dev/null +++ b/api/src/main/java/net/jitse/npclib/NPCLibOptions.java @@ -0,0 +1,77 @@ +package net.jitse.npclib; + +/** + * Mutable preferences for library usage + * + * @author A248 + * + */ +public class NPCLibOptions { + + MovementHandling moveHandling; + + /** + * Creates the default options + * + */ + public NPCLibOptions() { + moveHandling = MovementHandling.playerMoveEvent(); + } + + /** + * Specifies the motion handling which will be used for the library.
+ * Programmers may choose between using the PlayerMoveEvent or + * a periodic task.
+ *
+ * Note that NPCLib will always use events such as the PlayerTeleportEvent + * and PlayerChangedWorldEvent in addition to the specified option. + * + * @param moveHandling the movement handling + * @return the same NPCLibOptions + */ + public NPCLibOptions setMovementHandling(MovementHandling moveHandling) { + this.moveHandling = moveHandling; + return this; + } + + /** + * Options relating to movement handling + * + * @author A248 + * + */ + public static class MovementHandling { + + final boolean usePme; + final long updateInterval; + + private MovementHandling(boolean usePme, long updateInterval) { + this.usePme = usePme; + this.updateInterval = updateInterval; + } + + /** + * Gets movement handling using the PlayerMoveEvent + * + * @return movement handling + */ + public static MovementHandling playerMoveEvent() { + return new MovementHandling(false, 0); + } + + /** + * Gets movement handling using a periodic update interval in ticks. + * + * @param updateInterval the update interval in ticks + * @return movement handling + */ + public static MovementHandling repeatingTask(long updateInterval) { + if (updateInterval <= 0) { + throw new IllegalArgumentException("Negative update interval"); + } + return new MovementHandling(true, updateInterval); + } + + } + +} diff --git a/api/src/main/java/net/jitse/npclib/listeners/HandleMoveBase.java b/api/src/main/java/net/jitse/npclib/listeners/HandleMoveBase.java new file mode 100644 index 0000000..11d375e --- /dev/null +++ b/api/src/main/java/net/jitse/npclib/listeners/HandleMoveBase.java @@ -0,0 +1,26 @@ +package net.jitse.npclib.listeners; + +import org.bukkit.entity.Player; + +import net.jitse.npclib.internal.NPCBase; +import net.jitse.npclib.internal.NPCManager; + +public class HandleMoveBase { + + void handleMove(Player player) { + for (NPCBase npc : NPCManager.getAllNPCs()) { + if (!npc.getShown().contains(player.getUniqueId())) { + continue; // NPC was never supposed to be shown to the player. + } + + if (!npc.isShown(player) && npc.inRangeOf(player) && npc.inViewOf(player)) { + // The player is in range and can see the NPC, auto-show it. + npc.show(player, true); + } else if (npc.isShown(player) && !npc.inRangeOf(player)) { + // The player is not in range of the NPC anymore, auto-hide it. + npc.hide(player, true); + } + } + } + +} diff --git a/api/src/main/java/net/jitse/npclib/listeners/PeriodicMoveListener.java b/api/src/main/java/net/jitse/npclib/listeners/PeriodicMoveListener.java new file mode 100644 index 0000000..66623e8 --- /dev/null +++ b/api/src/main/java/net/jitse/npclib/listeners/PeriodicMoveListener.java @@ -0,0 +1,48 @@ +package net.jitse.npclib.listeners; + +import java.util.HashMap; +import java.util.UUID; + +import org.bukkit.Bukkit; +import org.bukkit.entity.Player; +import org.bukkit.event.EventHandler; +import org.bukkit.event.Listener; +import org.bukkit.event.player.PlayerJoinEvent; +import org.bukkit.event.player.PlayerQuitEvent; +import org.bukkit.scheduler.BukkitTask; + +import net.jitse.npclib.NPCLib; + +public class PeriodicMoveListener extends HandleMoveBase implements Listener { + + private final NPCLib instance; + private final long updateInterval; + + private final HashMap tasks = new HashMap<>(); + + public PeriodicMoveListener(NPCLib instance, long updateInterval) { + this.instance = instance; + this.updateInterval = updateInterval; + } + + private void startTask(UUID uuid) { + // purposefully using UUIDs and not holding player references + tasks.put(uuid, Bukkit.getScheduler().runTaskTimer(instance.getPlugin(), () -> { + Player player = Bukkit.getPlayer(uuid); + if (player != null) { // safety check + handleMove(player); + } + }, 1L, updateInterval)); + } + + @EventHandler + public void onPlayerJoin(PlayerJoinEvent evt) { + startTask(evt.getPlayer().getUniqueId()); + } + + @EventHandler + public void onPlayerQuit(PlayerQuitEvent evt) { + tasks.remove(evt.getPlayer().getUniqueId()).cancel(); + } + +} diff --git a/api/src/main/java/net/jitse/npclib/listeners/PlayerListener.java b/api/src/main/java/net/jitse/npclib/listeners/PlayerListener.java index d39553e..d238d66 100755 --- a/api/src/main/java/net/jitse/npclib/listeners/PlayerListener.java +++ b/api/src/main/java/net/jitse/npclib/listeners/PlayerListener.java @@ -11,7 +11,6 @@ import org.bukkit.Location; import org.bukkit.World; import org.bukkit.entity.Player; import org.bukkit.event.EventHandler; -import org.bukkit.event.EventPriority; import org.bukkit.event.Listener; import org.bukkit.event.entity.PlayerDeathEvent; import org.bukkit.event.player.*; @@ -20,7 +19,7 @@ import org.bukkit.scheduler.BukkitRunnable; /** * @author Jitse Boonstra */ -public class PlayerListener implements Listener { +public class PlayerListener extends HandleMoveBase implements Listener { private final NPCLib instance; @@ -82,36 +81,8 @@ public class PlayerListener implements Listener { } } - @EventHandler - public void onPlayerMove(PlayerMoveEvent event) { - Location from = event.getFrom(); - Location to = event.getTo(); - // Only check movement when the player moves from one block to another. The event is called often - // as it is also called when the pitch or yaw change. This is worth it from a performance view. - if (to == null || from.getBlockX() != to.getBlockX() - || from.getBlockY() != to.getBlockY() - || from.getBlockZ() != to.getBlockZ()) - handleMove(event.getPlayer()); - } - @EventHandler public void onPlayerTeleport(PlayerTeleportEvent event) { handleMove(event.getPlayer()); } - - private void handleMove(Player player) { - for (NPCBase npc : NPCManager.getAllNPCs()) { - if (!npc.getShown().contains(player.getUniqueId())) { - continue; // NPC was never supposed to be shown to the player. - } - - if (!npc.isShown(player) && npc.inRangeOf(player) && npc.inViewOf(player)) { - // The player is in range and can see the NPC, auto-show it. - npc.show(player, true); - } else if (npc.isShown(player) && !npc.inRangeOf(player)) { - // The player is not in range of the NPC anymore, auto-hide it. - npc.hide(player, true); - } - } - } } diff --git a/api/src/main/java/net/jitse/npclib/listeners/PlayerMoveEventListener.java b/api/src/main/java/net/jitse/npclib/listeners/PlayerMoveEventListener.java new file mode 100644 index 0000000..a07aa10 --- /dev/null +++ b/api/src/main/java/net/jitse/npclib/listeners/PlayerMoveEventListener.java @@ -0,0 +1,22 @@ +package net.jitse.npclib.listeners; + +import org.bukkit.Location; +import org.bukkit.event.EventHandler; +import org.bukkit.event.Listener; +import org.bukkit.event.player.PlayerMoveEvent; + +public class PlayerMoveEventListener extends HandleMoveBase implements Listener { + + @EventHandler + public void onPlayerMove(PlayerMoveEvent event) { + Location from = event.getFrom(); + Location to = event.getTo(); + // Only check movement when the player moves from one block to another. The event is called often + // as it is also called when the pitch or yaw change. This is worth it from a performance view. + if (to == null || from.getBlockX() != to.getBlockX() + || from.getBlockY() != to.getBlockY() + || from.getBlockZ() != to.getBlockZ()) + handleMove(event.getPlayer()); + } + +} From b4ce4fbf68c3216434d66250fa5e729e185e970b Mon Sep 17 00:00:00 2001 From: A248 Date: Mon, 20 Apr 2020 16:02:58 -0400 Subject: [PATCH 3/3] Player#isOnline is not thread safe CraftBukkit 1.8.8 implementation checks whether Bukkit.getPlayer() != null. Bukkit.getPlayer(UUID) is itself not thread safe. --- .../main/java/net/jitse/npclib/listeners/PlayerListener.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/net/jitse/npclib/listeners/PlayerListener.java b/api/src/main/java/net/jitse/npclib/listeners/PlayerListener.java index d238d66..6a72aa2 100755 --- a/api/src/main/java/net/jitse/npclib/listeners/PlayerListener.java +++ b/api/src/main/java/net/jitse/npclib/listeners/PlayerListener.java @@ -64,7 +64,7 @@ public class PlayerListener extends HandleMoveBase implements Listener { this.cancel(); } } - }.runTaskTimerAsynchronously(instance.getPlugin(), 0, 1); + }.runTaskTimer(instance.getPlugin(), 0L, 1L); } }