diff --git a/src/main/java/world/bentobox/bentobox/Settings.java b/src/main/java/world/bentobox/bentobox/Settings.java index f4213ba6d..a0e31b21f 100644 --- a/src/main/java/world/bentobox/bentobox/Settings.java +++ b/src/main/java/world/bentobox/bentobox/Settings.java @@ -544,6 +544,16 @@ public class Settings implements ConfigObject { @ConfigEntry(path = "island.obsidian-scooping-lava-tip-duration", since = "3.14.0") private int obsidianScoopingLavaTipDuration = 5; + @ConfigComment("Maximum number of history (log) entries kept per island. The oldest entries") + @ConfigComment("are dropped first once the cap is reached, so long-lived islands do not grow") + @ConfigComment("without bound in memory and in the database.") + @ConfigComment("Set to 0 (or a negative value) for unlimited history. This is the default so") + @ConfigComment("existing servers keep their current behaviour.") + @ConfigComment("Note: capping can undercount the historical-members placeholder once old") + @ConfigComment("JOINED entries are dropped off the front.") + @ConfigEntry(path = "island.history.max-entries", since = "3.22.3") + private int islandHistoryMaxEntries = 0; + /* WORLD */ @ConfigComment("Vanilla structures disabled by default in EVERY BentoBox game mode world") @ConfigComment("(overworld, nether and end). List the structure keys to stop generating and to") @@ -1835,6 +1845,26 @@ public void setObsidianScoopingLavaTipDuration(int obsidianScoopingLavaTipDurati this.obsidianScoopingLavaTipDuration = obsidianScoopingLavaTipDuration; } + /** + * Gets the maximum number of history entries kept per island. + * + * @return the cap; 0 or less means unlimited + * @since 3.22.3 + */ + public int getIslandHistoryMaxEntries() { + return islandHistoryMaxEntries; + } + + /** + * Sets the maximum number of history entries kept per island. + * + * @param islandHistoryMaxEntries the cap; 0 or less means unlimited + * @since 3.22.3 + */ + public void setIslandHistoryMaxEntries(int islandHistoryMaxEntries) { + this.islandHistoryMaxEntries = islandHistoryMaxEntries; + } + /** * @return the islandNumber * @since 2.0.0 diff --git a/src/main/java/world/bentobox/bentobox/api/commands/DelayedTeleportCommand.java b/src/main/java/world/bentobox/bentobox/api/commands/DelayedTeleportCommand.java index 7d55f9d3b..fbc2190e0 100644 --- a/src/main/java/world/bentobox/bentobox/api/commands/DelayedTeleportCommand.java +++ b/src/main/java/world/bentobox/bentobox/api/commands/DelayedTeleportCommand.java @@ -10,6 +10,7 @@ import org.bukkit.event.EventPriority; import org.bukkit.event.Listener; import org.bukkit.event.player.PlayerMoveEvent; +import org.bukkit.event.player.PlayerQuitEvent; import org.bukkit.event.player.PlayerTeleportEvent; import org.bukkit.scheduler.BukkitTask; @@ -95,6 +96,20 @@ public void onPlayerTeleport(PlayerTeleportEvent e) { } + /** + * Cancels and removes any pending delayed command when the player quits, + * otherwise the entry would remain in the monitor map forever. + * + * @param e Player quit event + */ + @EventHandler(priority = EventPriority.NORMAL) + public void onPlayerQuit(PlayerQuitEvent e) { + DelayedCommand delayed = toBeMonitored.remove(e.getPlayer().getUniqueId()); + if (delayed != null) { + delayed.task().cancel(); + } + } + /** * Top level command * @param addon - addon creating the command diff --git a/src/main/java/world/bentobox/bentobox/api/commands/admin/blueprints/AdminBlueprintCommand.java b/src/main/java/world/bentobox/bentobox/api/commands/admin/blueprints/AdminBlueprintCommand.java index 24c966cf8..fef66801f 100644 --- a/src/main/java/world/bentobox/bentobox/api/commands/admin/blueprints/AdminBlueprintCommand.java +++ b/src/main/java/world/bentobox/bentobox/api/commands/admin/blueprints/AdminBlueprintCommand.java @@ -9,6 +9,9 @@ import org.bukkit.Bukkit; import org.bukkit.Color; import org.bukkit.Particle; +import org.bukkit.event.EventHandler; +import org.bukkit.event.Listener; +import org.bukkit.event.player.PlayerQuitEvent; import world.bentobox.bentobox.api.commands.CompositeCommand; import world.bentobox.bentobox.api.commands.ConfirmableCommand; @@ -18,7 +21,7 @@ import world.bentobox.bentobox.managers.BlueprintsManager; import world.bentobox.bentobox.panels.BlueprintManagementPanel; -public class AdminBlueprintCommand extends ConfirmableCommand { +public class AdminBlueprintCommand extends ConfirmableCommand implements Listener { // Clipboards private Map clipboards; @@ -39,6 +42,7 @@ public void setup() { clipboards = new HashMap<>(); displayClipboards = new HashMap<>(); + Bukkit.getPluginManager().registerEvents(this, getPlugin()); // Sub commands new AdminBlueprintLoadCommand(this); @@ -64,6 +68,17 @@ protected Map getClipboards() { return clipboards; } + /** + * Releases the quitting player's clipboard. A clipboard can hold a full copied + * island (tens of thousands of block objects), so it must not outlive the player. + * + * @param e Player quit event + */ + @EventHandler + public void onPlayerQuit(PlayerQuitEvent e) { + clipboards.remove(e.getPlayer().getUniqueId()); + } + /** * This method shows clipboard for requested user. diff --git a/src/main/java/world/bentobox/bentobox/api/flags/FlagListener.java b/src/main/java/world/bentobox/bentobox/api/flags/FlagListener.java index 4ea0736cc..16e024179 100644 --- a/src/main/java/world/bentobox/bentobox/api/flags/FlagListener.java +++ b/src/main/java/world/bentobox/bentobox/api/flags/FlagListener.java @@ -1,7 +1,9 @@ package world.bentobox.bentobox.api.flags; +import java.util.Map; import java.util.Optional; import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; import org.bukkit.Bukkit; import org.bukkit.Location; @@ -34,6 +36,16 @@ public abstract class FlagListener implements Listener { private static final String WHY = "Why: "; + /** + * Cache of per-world why-debug metadata keys to avoid rebuilding the string + * on every protection check. + */ + private static final Map WHY_DEBUG_KEYS = new ConcurrentHashMap<>(); + + private static String whyDebugKey(String worldName) { + return worldName == null ? "null_why_debug" : WHY_DEBUG_KEYS.computeIfAbsent(worldName, n -> n + "_why_debug"); + } + /** * Reason for why flag was allowed or disallowed * Used by admins for debugging player actions @@ -150,7 +162,7 @@ public boolean checkIsland(@NonNull Event e, @Nullable Player player, @Nullable * @return true if the check is okay, false if it was disallowed */ public boolean checkIsland(@NonNull Event e, @Nullable Player player, @Nullable Location loc, @NonNull Flag flag, boolean silent) { - + // Set user user = player == null ? null : User.getInstance(player); if (loc == null) { @@ -168,20 +180,19 @@ public boolean checkIsland(@NonNull Event e, @Nullable Player player, @Nullable // Get the island and if present Optional island = getIslands().getProtectedIslandAt(loc); - + // Handle Settings Flag if (flag.getType().equals(Flag.Type.SETTING)) { return processSetting(flag, island, e, loc); } // Protection flag - // Ops or "bypass everywhere" moderators can do anything unless they have switched it off - if (hasBypassEverywhere(loc, flag)) { - report(user, e, loc, flag, user.isOp() ? Why.OP : Why.BYPASS_EVERYWHERE); - return true; - } // Check if the island is deleted - if so, then nothing is allowed by default if (isDeletedIsland(island)) { + // Ops or "bypass everywhere" moderators can do anything unless they have switched it off + if (checkBypassEverywhere(e, loc, flag)) { + return true; + } report(user, e, loc, flag, Why.ISLAND_DELETED); noGo(e, flag, silent, WORLD_PROTECTED); return false; @@ -203,19 +214,39 @@ public boolean checkIsland(@NonNull Event e, @Nullable Player player, @Nullable report(user, e, loc, flag, Why.ALLOWED_IN_WORLD); return true; } + // Ops or "bypass everywhere" moderators can do anything unless they have switched it off + if (checkBypassEverywhere(e, loc, flag)) { + return true; + } report(user, e, loc, flag, Why.NOT_ALLOWED_IN_WORLD); noGo(e, flag, silent, WORLD_PROTECTED); return false; } + /** + * Checks the bypass-everywhere permissions and reports if they apply. Only called on + * the deny path so the common allow path skips the permission lookups entirely. + * @return true if the user may bypass protection everywhere + */ + private boolean checkBypassEverywhere(@NonNull Event e, @NonNull Location loc, @NonNull Flag flag) { + if (hasBypassEverywhere(loc, flag)) { + report(user, e, loc, flag, user.isOp() ? Why.OP : Why.BYPASS_EVERYWHERE); + return true; + } + return false; + } + private boolean isDeletedIsland(Optional island) { return island.isPresent() && (island.get().isDeleted() || island.get().isDeletable()); } private boolean hasBypassEverywhere(Location loc, Flag flag) { - return !user.getMetaData(AdminSwitchCommand.META_TAG).map(MetaDataValue::asBoolean).orElse(false) - && (user.hasPermission(getIWM().getPermissionPrefix(loc.getWorld()) + "mod.bypassprotect") - || user.hasPermission(getIWM().getPermissionPrefix(loc.getWorld()) + "mod.bypass." + flag.getID() + ".everywhere")); + if (user.getMetaData(AdminSwitchCommand.META_TAG).map(MetaDataValue::asBoolean).orElse(false)) { + return false; + } + String prefix = getIWM().getPermissionPrefix(loc.getWorld()); + return user.hasPermission(prefix + "mod.bypassprotect") + || user.hasPermission(prefix + "mod.bypass." + flag.getID() + ".everywhere"); } private boolean processBypass(@NonNull Flag flag, Island island, @NonNull Event e, @NonNull Location loc, boolean silent) { @@ -224,7 +255,12 @@ private boolean processBypass(@NonNull Flag flag, Island island, @NonNull Event if (island.isAllowed(user, flag)) { report(user, e, loc, flag, Why.RANK_ALLOWED); return true; - } else if (!user.getMetaData(AdminSwitchCommand.META_TAG).map(MetaDataValue::asBoolean).orElse(false) + } + // Ops or "bypass everywhere" moderators can do anything unless they have switched it off + if (checkBypassEverywhere(e, loc, flag)) { + return true; + } + if (!user.getMetaData(AdminSwitchCommand.META_TAG).map(MetaDataValue::asBoolean).orElse(false) && (user.hasPermission(getIWM().getPermissionPrefix(loc.getWorld()) + "mod.bypass." + flag.getID() + ".island"))) { report(user, e, loc, flag, Why.BYPASS_ISLAND); return true; @@ -239,6 +275,10 @@ private boolean processWorldSetting(@NonNull Flag flag, @NonNull Location loc, @ report(user, e, loc, flag, Why.ALLOWED_IN_WORLD); return true; } + // Ops or "bypass everywhere" moderators can do anything unless they have switched it off + if (checkBypassEverywhere(e, loc, flag)) { + return true; + } report(user, e, loc, flag, Why.NOT_ALLOWED_IN_WORLD); noGo(e, flag, silent, WORLD_PROTECTED); return false; @@ -247,11 +287,13 @@ private boolean processWorldSetting(@NonNull Flag flag, @NonNull Location loc, @ private boolean processSetting(@NonNull Flag flag, Optional island, @NonNull Event e, @NonNull Location loc) { // If the island exists, return the setting, otherwise return the default setting for this flag if (island.isPresent()) { - report(user, e, loc, flag, island.map(x -> x.isAllowed(flag)).orElse(false) ? Why.SETTING_ALLOWED_ON_ISLAND : Why.SETTING_NOT_ALLOWED_ON_ISLAND); - } else { - report(user, e, loc, flag, flag.isSetForWorld(loc.getWorld()) ? Why.SETTING_ALLOWED_IN_WORLD : Why.SETTING_NOT_ALLOWED_IN_WORLD); + boolean allowed = island.get().isAllowed(flag); + report(user, e, loc, flag, allowed ? Why.SETTING_ALLOWED_ON_ISLAND : Why.SETTING_NOT_ALLOWED_ON_ISLAND); + return allowed; } - return island.map(x -> x.isAllowed(flag)).orElseGet(() -> flag.isSetForWorld(loc.getWorld())); + boolean allowed = flag.isSetForWorld(loc.getWorld()); + report(user, e, loc, flag, allowed ? Why.SETTING_ALLOWED_IN_WORLD : Why.SETTING_NOT_ALLOWED_IN_WORLD); + return allowed; } /** @@ -264,7 +306,12 @@ private boolean processSetting(@NonNull Flag flag, Optional island, @Non */ protected void report(@Nullable User user, @NonNull Event e, @NonNull Location loc, @NonNull Flag flag, @NonNull Why why) { // A quick way to debug flag listener unit tests is to add this line here: System.out.println(why.name()); NOSONAR - if (user != null && user.isPlayer() && user.getPlayer().getMetadata(loc.getWorld().getName() + "_why_debug").stream() + if (user == null || !user.isPlayer()) { + return; + } + // hasMetadata is a cheap map hit - avoids the metadata list + stream allocations on every check + String whyDebugKey = whyDebugKey(loc.getWorld().getName()); + if (user.getPlayer().hasMetadata(whyDebugKey) && user.getPlayer().getMetadata(whyDebugKey).stream() .filter(p -> p.getOwningPlugin().equals(getPlugin())).findFirst().map(MetadataValue::asBoolean).orElse(false)) { String whyEvent = WHY + e.getEventName() + " in world " + loc.getWorld().getName() + " at " + Util.xyz(loc.toVector()); String whyBypass = WHY + user.getName() + " " + flag.getID() + " - " + why.name(); @@ -327,8 +374,9 @@ public void report(@Nullable Addon addon, @NonNull Location loc, @NonNull String String prefix = addon != null ? "[" + addon.getDescription().getName() + "] " : ""; String whyMessage = WHY + prefix + message + " - " + reason.name() + " in world " + loc.getWorld().getName() + " at " + Util.xyz(loc.toVector()); + String whyDebugKey = whyDebugKey(loc.getWorld().getName()); Bukkit.getOnlinePlayers().stream() - .filter(p -> p.getMetadata(loc.getWorld().getName() + "_why_debug").stream() + .filter(p -> p.hasMetadata(whyDebugKey) && p.getMetadata(whyDebugKey).stream() .filter(m -> m.getOwningPlugin().equals(getPlugin())) .findFirst().map(MetadataValue::asBoolean).orElse(false)) .forEach(p -> { diff --git a/src/main/java/world/bentobox/bentobox/api/user/User.java b/src/main/java/world/bentobox/bentobox/api/user/User.java index 430da90ac..bc5165b20 100644 --- a/src/main/java/world/bentobox/bentobox/api/user/User.java +++ b/src/main/java/world/bentobox/bentobox/api/user/User.java @@ -79,10 +79,12 @@ public class User implements MetaDataAble { // Used for particle validation private static final Map> VALIDATION_CHECK; + // Resolved once - the enum lookup chain is too costly to run per spawned particle + private static final Particle DUST_PARTICLE = Enums.getIfPresent(Particle.class, "DUST") + .or(Enums.getIfPresent(Particle.class, "REDSTONE").or(Particle.FLAME)); static { Map> v = new EnumMap<>(Particle.class); - v.put(Enums.getIfPresent(Particle.class, "DUST") - .or(Enums.getIfPresent(Particle.class, "REDSTONE").or(Particle.FLAME)), Particle.DustOptions.class); + v.put(DUST_PARTICLE, Particle.DustOptions.class); if (Enums.getIfPresent(Particle.class, "ITEM").isPresent()) { // 1.20.6 Particles v.put(Particle.ITEM, ItemStack.class); @@ -110,7 +112,7 @@ public static void clearUsers() { /** * Gets an instance of User from a CommandSender - * + * * @param sender - command sender, e.g. console * @return user - user */ @@ -125,7 +127,7 @@ public static User getInstance(@NonNull CommandSender sender) { /** * Gets an instance of User from a Player object. - * + * * @param player - the player * @return user - user */ @@ -140,7 +142,7 @@ public static User getInstance(@NonNull Player player) { /** * Gets an instance of User from a UUID. This will always return a user object. * If the player is offline then the getPlayer value will be null. - * + * * @param uuid - UUID * @return user - user */ @@ -155,7 +157,7 @@ public static User getInstance(@NonNull UUID uuid) { /** * Gets an instance of User from an OfflinePlayer - * + * * @param offlinePlayer offline Player * @return user * @since 1.3.0 @@ -170,7 +172,7 @@ public static User getInstance(@NonNull OfflinePlayer offlinePlayer) { /** * Removes this player from the User cache and player manager cache - * + * * @param player the player */ public static void removePlayer(Player player) { @@ -222,7 +224,7 @@ private User(UUID playerUUID) { /** * Used for testing - * + * * @param p - plugin */ public static void setPlugin(BentoBox p) { @@ -235,7 +237,7 @@ public Set getEffectivePermissions() { /** * Get the user's inventory - * + * * @return player's inventory */ @NonNull @@ -245,7 +247,7 @@ public PlayerInventory getInventory() { /** * Get the user's location - * + * * @return location */ @NonNull @@ -257,7 +259,7 @@ public Location getLocation() { /** * Get the user's name - * + * * @return player's name */ @NonNull @@ -267,7 +269,7 @@ public String getName() { /** * Get the user's display name - * + * * @return player's display name if the player is online otherwise just their * name * @since 1.22.1 @@ -280,7 +282,7 @@ public String getDisplayName() { /** * Get the user's display name as a text Component - * + * * @return player's display name if the player is online otherwise just their * name * @since 3.4.0 @@ -291,7 +293,7 @@ public Component displayName() { /** * Check if the User is a player before calling this method. {@link #isPlayer()} - * + * * @return the player */ @NonNull @@ -308,7 +310,7 @@ public boolean isPlayer() { /** * Use {@link #isOfflinePlayer()} before calling this method - * + * * @return the offline player * @since 1.3.0 */ @@ -345,7 +347,7 @@ public boolean hasPermission(@Nullable String permission) { /** * Removes permission from user - * + * * @param name - Name of the permission to remove * @return true if successful * @since 1.5.0 @@ -366,7 +368,7 @@ public boolean removePerm(String name) { /** * Add a permission to user - * + * * @param name - Name of the permission to attach * @return The PermissionAttachment that was just created * @since 1.5.0 @@ -382,7 +384,7 @@ public boolean isOnline() { /** * Checks if user is Op - * + * * @return true if user is Op */ public boolean isOp() { @@ -399,7 +401,7 @@ public boolean isOp() { * Get the maximum value of a numerical permission setting. If a player is given * an explicit negative number then this is treated as "unlimited" and returned * immediately. - * + * * @param permissionPrefix the start of the perm, e.g., * {@code plugin.mypermission} * @param defaultValue the default value; the result may be higher or lower @@ -467,7 +469,7 @@ private int iteratePerms(List permissions, String permPrefix, int defaul /** * Gets a translation for a specific world - * + * * @param world - world of translation * @param reference - reference found in a locale file * @param variables - variables to insert into translated string. Variables go @@ -488,7 +490,7 @@ public String getTranslation(World world, String reference, String... variables) * Gets a translation of this reference for this user with colors converted. * Translations may be overridden by Addons by using the same reference prefixed * by the addon name (from the Addon Description) in lower case. - * + * * @param reference - reference found in a locale file * @param variables - variables to insert into translated string. Variables go * in pairs, for example "[name]", "tastybento" @@ -596,7 +598,7 @@ static String computeLegacy(String raw) { * Gets a translation of this reference for this user without colors translated. * Translations may be overridden by Addons by using the same reference prefixed * by the addon name (from the Addon Description) in lower case. - * + * * @param reference - reference found in a locale file * @param variables - variables to insert into translated string. Variables go * in pairs, for example "[name]", "tastybento" @@ -704,7 +706,7 @@ private String replaceVars(String reference, String[] variables) { /** * Gets a translation of this reference for this user. - * + * * @param reference - reference found in a locale file * @param variables - variables to insert into translated string. Variables go * in pairs, for example "[name]", "tastybento" @@ -718,7 +720,7 @@ public String getTranslationOrNothing(String reference, String... variables) { /** * Send a message to sender if message is not empty. - * + * * @param reference - language file reference * @param variables - CharSequence target, replacement pairs */ @@ -967,7 +969,7 @@ public Component getTranslationAsComponent(@NonNull String reference, @NonNull S /** * Sends a message to sender if message is not empty and if the same wasn't sent * within the previous Notifier.NOTIFICATION_DELAY seconds. - * + * * @param reference - language file reference * @param variables - CharSequence target, replacement pairs * @@ -983,7 +985,7 @@ public void notify(String reference, String... variables) { /** * Sends a message to sender if message is not empty and if the same wasn't sent * within the previous Notifier.NOTIFICATION_DELAY seconds. - * + * * @param world - the world the translation should come from * @param reference - language file reference * @param variables - CharSequence target, replacement pairs @@ -1000,7 +1002,7 @@ public void notify(World world, String reference, String... variables) { /** * Sets the user's game mode - * + * * @param mode - GameMode */ public void setGameMode(GameMode mode) { @@ -1012,7 +1014,7 @@ public void setGameMode(GameMode mode) { /** * Teleports user to this location. If the user is in a vehicle, they will exit * first. - * + * * @param location - the location */ public void teleport(Location location) { @@ -1023,7 +1025,7 @@ public void teleport(Location location) { /** * Gets the current world this entity resides in - * + * * @return World - world */ @NonNull @@ -1041,7 +1043,7 @@ public void closeInventory() { /** * Get the user's locale - * + * * @return Locale */ public Locale getLocale() { @@ -1078,7 +1080,7 @@ public void updateInventory() { /** * Performs a command as the player - * + * * @param command - command to execute * @return true if the command was successful, otherwise false */ @@ -1097,7 +1099,7 @@ public boolean performCommand(String command) { /** * Checks if a user is in one of the game worlds - * + * * @return true if user is, false if not */ public boolean inWorld() { @@ -1107,7 +1109,7 @@ public boolean inWorld() { /** * Spawn particles to the player. They are only displayed if they are within the * server's view distance. - * + * * @param particle Particle to display. * @param dustOptions Particle.DustOptions for the particle to display. * @param x X coordinate of the particle to display. @@ -1126,11 +1128,19 @@ public void spawnParticle(Particle particle, @Nullable Object dustOptions, doubl + " must be provided when using Particle." + particle + " as particle."); } - // Check if this particle is beyond the viewing distance of the server - if (this.player != null && this.player.getLocation().toVector().distanceSquared(new Vector(x, y, - z)) < (Bukkit.getServer().getViewDistance() * 256 * Bukkit.getServer().getViewDistance())) { - if (particle.equals(Enums.getIfPresent(Particle.class, "DUST") - .or(Enums.getIfPresent(Particle.class, "REDSTONE").or(Particle.FLAME)))) { + // Check if this particle is beyond the viewing distance of the server. + // Plain coordinate math can run hundreds of times per tick when + // range/clipboard displays are active, so avoid vector allocations. + if (this.player == null) { + return; + } + Location l = this.player.getLocation(); + double dx = l.getX() - x; + double dy = l.getY() - y; + double dz = l.getZ() - z; + if (dx * dx + dy * dy + dz * dz < (Bukkit.getServer().getViewDistance() * 256 + * Bukkit.getServer().getViewDistance())) { + if (particle.equals(DUST_PARTICLE)) { player.spawnParticle(particle, x, y, z, 1, 0, 0, 0, 1, dustOptions); } else if (dustOptions != null) { player.spawnParticle(particle, x, y, z, 1, dustOptions); @@ -1145,7 +1155,7 @@ public void spawnParticle(Particle particle, @Nullable Object dustOptions, doubl /** * Spawn particles to the player. They are only displayed if they are within the * server's view distance. Compatibility method for older usages. - * + * * @param particle Particle to display. * @param dustOptions Particle.DustOptions for the particle to display. * @param x X coordinate of the particle to display. @@ -1159,7 +1169,7 @@ public void spawnParticle(Particle particle, Particle.DustOptions dustOptions, d /** * Spawn particles to the player. They are only displayed if they are within the * server's view distance. - * + * * @param particle Particle to display. * @param dustOptions Particle.DustOptions for the particle to display. * @param x X coordinate of the particle to display. @@ -1172,7 +1182,7 @@ public void spawnParticle(Particle particle, Particle.DustOptions dustOptions, i /* * (non-Javadoc) - * + * * @see java.lang.Object#hashCode() */ @Override @@ -1185,7 +1195,7 @@ public int hashCode() { /* * (non-Javadoc) - * + * * @see java.lang.Object#equals(java.lang.Object) */ @Override @@ -1207,7 +1217,7 @@ public boolean equals(Object obj) { /** * Set the addon context when a command is executed - * + * * @param addon - the addon executing the command */ public void setAddon(Addon addon) { @@ -1216,7 +1226,7 @@ public void setAddon(Addon addon) { /** * Get all the metadata for this user - * + * * @return the metaData * @since 1.15.4 */ diff --git a/src/main/java/world/bentobox/bentobox/blueprints/BlueprintClipboard.java b/src/main/java/world/bentobox/bentobox/blueprints/BlueprintClipboard.java index 2d6bb3c9d..06238f7e6 100644 --- a/src/main/java/world/bentobox/bentobox/blueprints/BlueprintClipboard.java +++ b/src/main/java/world/bentobox/bentobox/blueprints/BlueprintClipboard.java @@ -8,6 +8,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.stream.Collectors; import org.bukkit.Bukkit; import org.bukkit.Location; @@ -160,14 +161,16 @@ private void copyAsync(World world, User user, List vectorsToCopy, int s } copying = true; NamespacedKey key = new NamespacedKey(BentoBox.getInstance(), "associatedDisplayEntity"); + // Index the world's entities by block position once per tick instead of scanning + // the full entity list for every copied block + Map> entitiesByBlock = world.getEntities().stream() + .filter(Objects::nonNull) + .filter(e -> !(e instanceof Player)) + .filter(e -> !e.getPersistentDataContainer().has(key, PersistentDataType.STRING)) // Do not copy hidden display entities + .collect(Collectors.groupingBy(e -> new Vector(e.getLocation().getBlockX(), + e.getLocation().getBlockY(), e.getLocation().getBlockZ()))); vectorsToCopy.stream().skip(index).limit(speed).forEach(v -> { - List ents = world.getEntities().stream() - .filter(Objects::nonNull) - .filter(e -> !(e instanceof Player)) - .filter(e -> !e.getPersistentDataContainer().has(key, PersistentDataType.STRING)) // Do not copy hidden display entities - .filter(e -> new Vector(e.getLocation().getBlockX(), e.getLocation().getBlockY(), - e.getLocation().getBlockZ()).equals(v)) - .toList(); + List ents = entitiesByBlock.getOrDefault(v, List.of()); if (copyBlock(v.toLocation(world), copyAir, copyBiome, ents, noWater)) { count++; } diff --git a/src/main/java/world/bentobox/bentobox/database/json/adapters/PairTypeAdapter.java b/src/main/java/world/bentobox/bentobox/database/json/adapters/PairTypeAdapter.java index ae36aa815..d79bad9c5 100644 --- a/src/main/java/world/bentobox/bentobox/database/json/adapters/PairTypeAdapter.java +++ b/src/main/java/world/bentobox/bentobox/database/json/adapters/PairTypeAdapter.java @@ -11,6 +11,9 @@ import world.bentobox.bentobox.util.Pair; public class PairTypeAdapter extends TypeAdapter> { + // Gson construction is expensive and this adapter runs per field on every database load + private static final Gson GSON = new Gson(); + private final Type xType; private final Type zType; @@ -23,10 +26,9 @@ public PairTypeAdapter(Type xType, Type zType) { public void write(JsonWriter out, Pair pair) throws IOException { out.beginObject(); out.name("x"); - Gson gson = new Gson(); - gson.toJson(pair.getKey(), xType, out); + GSON.toJson(pair.getKey(), xType, out); out.name("z"); - gson.toJson(pair.getValue(), zType, out); + GSON.toJson(pair.getValue(), zType, out); out.endObject(); } @@ -39,9 +41,9 @@ public Pair read(JsonReader in) throws IOException { while (in.hasNext()) { String name = in.nextName(); if (name.equals("x")) { - x = new Gson().fromJson(in, xType); + x = GSON.fromJson(in, xType); } else if (name.equals("z")) { - z = new Gson().fromJson(in, zType); + z = GSON.fromJson(in, zType); } } in.endObject(); diff --git a/src/main/java/world/bentobox/bentobox/database/objects/Island.java b/src/main/java/world/bentobox/bentobox/database/objects/Island.java index 3cdd4ed46..d0a44f959 100644 --- a/src/main/java/world/bentobox/bentobox/database/objects/Island.java +++ b/src/main/java/world/bentobox/bentobox/database/objects/Island.java @@ -186,6 +186,7 @@ public class Island implements DataObject, MetaDataAble { private Map flags = new HashMap<>(); //// Island History //// + @Adapter(LogEntryListAdapter.class) @Expose private List history = new LinkedList<>(); @@ -404,7 +405,14 @@ public long getCreatedDate() { * @return flag value */ public int getFlag(@NonNull Flag flag) { - return flags.computeIfAbsent(flag.getID(), k -> flag.getDefaultRank()); + // Plain get first - computeIfAbsent allocates a capturing lambda on every + // call and this runs on every protection check + Integer rank = flags.get(flag.getID()); + if (rank == null) { + rank = flag.getDefaultRank(); + flags.put(flag.getID(), rank); + } + return rank; } /** @@ -487,7 +495,7 @@ public ImmutableSet getMemberSet() { * @return the minProtectedX */ public int getMinProtectedX() { - return Math.max(getMinX(), getProtectionCenter().getBlockX() - this.getProtectionRange()); + return Math.max(getMinX(), rawProtectionCenter().getBlockX() - this.getProtectionRange()); } /** @@ -498,7 +506,7 @@ public int getMinProtectedX() { * @since 1.5.2 */ public int getMaxProtectedX() { - return Math.min(getMaxX(), getProtectionCenter().getBlockX() + this.getProtectionRange()); + return Math.min(getMaxX(), rawProtectionCenter().getBlockX() + this.getProtectionRange()); } /** @@ -508,7 +516,18 @@ public int getMaxProtectedX() { * @return the minProtectedZ */ public int getMinProtectedZ() { - return Math.max(getMinZ(), getProtectionCenter().getBlockZ() - this.getProtectionRange()); + return Math.max(getMinZ(), rawProtectionCenter().getBlockZ() - this.getProtectionRange()); + } + + /** + * Read-only access to the protection center without the defensive clone that + * {@link #getProtectionCenter()} makes. For internal coordinate reads only - + * callers must not mutate the returned location. + */ + @NonNull + private Location rawProtectionCenter() { + return location == null ? Objects.requireNonNull(center, "Island getCenter requires a non-null center") + : location; } /** @@ -519,7 +538,7 @@ public int getMinProtectedZ() { * @since 1.5.2 */ public int getMaxProtectedZ() { - return Math.min(getMaxZ(), getProtectionCenter().getBlockZ() + this.getProtectionRange()); + return Math.min(getMaxZ(), rawProtectionCenter().getBlockZ() + this.getProtectionRange()); } /** @@ -606,8 +625,13 @@ public boolean isUnowned() { * @see #getRange() */ public int getProtectionRange() { - return Math.min(this.getRange(), - getRawProtectionRange() + this.getBonusRanges().stream().mapToInt(BonusRangeRecord::getRange).sum()); + // Plain loop - this is called several times per protection event and the + // stream pipeline allocates even when there are no bonus ranges + int bonus = 0; + for (BonusRangeRecord r : getBonusRanges()) { + bonus += r.getRange(); + } + return Math.min(this.getRange(), getRawProtectionRange() + bonus); } /** @@ -941,14 +965,23 @@ public boolean isSpawn() { * {@code false} otherwise. */ public boolean onIsland(@NonNull Location target) { - return Util.sameWorld(this.world, target.getWorld()) - && (target.getWorld().getEnvironment().equals(Environment.NORMAL) - || this.getPlugin().getIWM().isIslandNether(target.getWorld()) - || this.getPlugin().getIWM().isIslandEnd(target.getWorld())) - && target.getBlockX() >= this.getMinProtectedX() - && target.getBlockX() < (this.getMinProtectedX() + this.getProtectionRange() * 2) - && target.getBlockZ() >= this.getMinProtectedZ() - && target.getBlockZ() < (this.getMinProtectedZ() + this.getProtectionRange() * 2); + // Cheapest checks first: int coordinate compares before any world resolution + int x = target.getBlockX(); + int minProtectedX = this.getMinProtectedX(); + int protectedSide = this.getProtectionRange() * 2; + if (x < minProtectedX || x >= minProtectedX + protectedSide) { + return false; + } + int z = target.getBlockZ(); + int minProtectedZ = this.getMinProtectedZ(); + if (z < minProtectedZ || z >= minProtectedZ + protectedSide) { + return false; + } + World targetWorld = target.getWorld(); + return Util.sameWorld(this.world, targetWorld) + && (targetWorld.getEnvironment() == Environment.NORMAL + || this.getPlugin().getIWM().isIslandNether(targetWorld) + || this.getPlugin().getIWM().isIslandEnd(targetWorld)); } /** @@ -1437,11 +1470,20 @@ public List getHistory() { /** * Adds a {@link LogEntry} to the history of this island. - * + * History is capped at the {@code island.history.max-entries} config setting; the + * oldest entry is dropped when the cap is reached so long-lived islands do not grow + * without bound in memory and in the database. A cap of 0 or less means unlimited. + * * @param logEntry the LogEntry to add. */ public void log(LogEntry logEntry) { history.add(logEntry); + int max = BentoBox.getInstance().getSettings().getIslandHistoryMaxEntries(); + if (max > 0) { + while (history.size() > max) { + history.remove(0); + } + } setChanged(); } diff --git a/src/main/java/world/bentobox/bentobox/database/objects/IslandDeletion.java b/src/main/java/world/bentobox/bentobox/database/objects/IslandDeletion.java index 2b444a5e6..a72d45619 100644 --- a/src/main/java/world/bentobox/bentobox/database/objects/IslandDeletion.java +++ b/src/main/java/world/bentobox/bentobox/database/objects/IslandDeletion.java @@ -218,7 +218,8 @@ public void setUniqueId(String uniqueId) { } public boolean inBounds(int x, int z) { - return box.contains(new Vector(x, 0, z)); + // Called per block during deletion scans - avoid allocating a vector + return box.contains(x, 0, z); } /** diff --git a/src/main/java/world/bentobox/bentobox/listeners/PrimaryIslandListener.java b/src/main/java/world/bentobox/bentobox/listeners/PrimaryIslandListener.java index ce55ddfcd..aeb6792ed 100644 --- a/src/main/java/world/bentobox/bentobox/listeners/PrimaryIslandListener.java +++ b/src/main/java/world/bentobox/bentobox/listeners/PrimaryIslandListener.java @@ -36,8 +36,13 @@ public void onPlayerJoin(final PlayerJoinEvent event) { @EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true) public void onPlayerMove(final PlayerMoveEvent event) { - if (!event.getFrom().toVector().equals(event.getTo().toVector())) { - setIsland(event.getPlayer(), event.getTo()); + // Island bounds are block-based, so only a block change can alter the result. + // Compare raw coordinates to avoid allocating vectors on every move event. + Location from = event.getFrom(); + Location to = event.getTo(); + if (from.getBlockX() != to.getBlockX() || from.getBlockY() != to.getBlockY() + || from.getBlockZ() != to.getBlockZ()) { + setIsland(event.getPlayer(), to); } } diff --git a/src/main/java/world/bentobox/bentobox/listeners/StandardSpawnProtectionListener.java b/src/main/java/world/bentobox/bentobox/listeners/StandardSpawnProtectionListener.java index 1293a8583..b315bb83a 100644 --- a/src/main/java/world/bentobox/bentobox/listeners/StandardSpawnProtectionListener.java +++ b/src/main/java/world/bentobox/bentobox/listeners/StandardSpawnProtectionListener.java @@ -10,7 +10,7 @@ import org.bukkit.event.block.BlockPlaceEvent; import org.bukkit.event.entity.EntityExplodeEvent; import org.bukkit.event.player.PlayerBucketEmptyEvent; -import org.bukkit.util.Vector; +import org.bukkit.util.NumberConversions; import org.eclipse.jdt.annotation.NonNull; import world.bentobox.bentobox.BentoBox; @@ -123,13 +123,15 @@ private boolean atSpawn(@NonNull Location location) { // If end portals are active, there is no common spawn return false; } - Vector p = location.toVector().multiply(new Vector(1, 0, 1)); - Vector spawn = location.getWorld().getSpawnLocation().toVector().multiply(new Vector(1, 0, 1)); int radius = env == World.Environment.THE_END ? plugin.getIWM().getEndSpawnRadius(gameWorld) : plugin.getIWM().getNetherSpawnRadius(gameWorld); - Vector diff = p.subtract(spawn); - return Math.abs(diff.getBlockX()) <= radius && Math.abs(diff.getBlockZ()) <= radius; + // Plain coordinate math runs per block event, so it avoid vector allocations. + // floor() keeps the exact semantics of the previous Vector#getBlockX comparison. + Location spawn = location.getWorld().getSpawnLocation(); + int diffX = NumberConversions.floor(location.getX() - spawn.getX()); + int diffZ = NumberConversions.floor(location.getZ() - spawn.getZ()); + return Math.abs(diffX) <= radius && Math.abs(diffZ) <= radius; } /** diff --git a/src/main/java/world/bentobox/bentobox/listeners/flags/protection/LockAndBanListener.java b/src/main/java/world/bentobox/bentobox/listeners/flags/protection/LockAndBanListener.java index 3dda116f7..278cd6ee2 100644 --- a/src/main/java/world/bentobox/bentobox/listeners/flags/protection/LockAndBanListener.java +++ b/src/main/java/world/bentobox/bentobox/listeners/flags/protection/LockAndBanListener.java @@ -1,12 +1,14 @@ package world.bentobox.bentobox.listeners.flags.protection; import java.util.HashSet; +import java.util.Optional; import java.util.Set; import java.util.UUID; import org.bukkit.Bukkit; import org.bukkit.Location; import org.bukkit.Sound; +import org.bukkit.entity.Entity; import org.bukkit.entity.Player; import org.bukkit.event.EventHandler; import org.bukkit.event.EventPriority; @@ -110,15 +112,23 @@ public void onVehicleMove(VehicleMoveEvent e) { if (e.getFrom().getBlockX() - e.getTo().getBlockX() == 0 && e.getFrom().getBlockZ() - e.getTo().getBlockZ() == 0) { return; } - // For each Player in the vehicle - e.getVehicle().getPassengers().stream().filter(Player.class::isInstance).map(Player.class::cast).forEach(p -> { - if (!checkAndNotify(p, e.getTo()).isAllowed()) { - p.leaveVehicle(); - p.teleport(e.getFrom()); - e.getVehicle().getWorld().playSound(e.getFrom(), Sound.BLOCK_ANVIL_HIT, 1F, 1F); - eject(p); + // For each Player in the vehicle. Resolve the island lazily on the first Player + // passenger so unmanned/mob-ridden vehicles pay no grid lookup, and reuse it for + // any further player passengers (the rare multi-player case). + Optional island = null; + for (Entity passenger : e.getVehicle().getPassengers()) { + if (passenger instanceof Player p) { + if (island == null) { + island = getIslands().getProtectedIslandAt(e.getTo()); + } + if (!checkAndNotify(p, e.getTo(), island).isAllowed()) { + p.leaveVehicle(); + p.teleport(e.getFrom()); + e.getVehicle().getWorld().playSound(e.getFrom(), Sound.BLOCK_ANVIL_HIT, 1F, 1F); + eject(p); + } } - }); + } } // Login check @@ -144,6 +154,19 @@ public void onPlayerQuit(PlayerQuitEvent e) { * @return CheckResult LOCKED, BANNED or OPEN. If an island is locked, that will take priority over banned */ private CheckResult check(@NonNull Player player, Location loc) + { + return check(player, loc, this.getIslands().getProtectedIslandAt(loc)); + } + + /** + * Check if a player is banned or the island is locked, using an already-resolved island + * to avoid repeated island lookups for the same location. + * @param player - player + * @param loc - location to check + * @param island - island at the location, if any + * @return CheckResult LOCKED, BANNED or OPEN. If an island is locked, that will take priority over banned + */ + private CheckResult check(@NonNull Player player, Location loc, Optional island) { // Ops or NPC's are allowed everywhere if (player.isOp() || player.hasMetadata("NPC")) @@ -152,7 +175,7 @@ private CheckResult check(@NonNull Player player, Location loc) } // See if the island is locked to non-members or player is banned - return this.getIslands().getProtectedIslandAt(loc). + return island. map(is -> { if (is.isBanned(player.getUniqueId())) @@ -181,7 +204,19 @@ private CheckResult check(@NonNull Player player, Location loc) */ private CheckResult checkAndNotify(@NonNull Player player, Location loc) { - CheckResult result = this.check(player, loc); + return checkAndNotify(player, loc, this.getIslands().getProtectedIslandAt(loc)); + } + + /** + * Same as {@link #checkAndNotify(Player, Location)} but reuses an already-resolved island. + * @param player - player + * @param loc - location to check + * @param island - island at the location, if any + * @return CheckResult + */ + private CheckResult checkAndNotify(@NonNull Player player, Location loc, Optional island) + { + CheckResult result = this.check(player, loc, island); if (result == CheckResult.OPEN) { // Player is in an open area, clear notification state notifiedPlayers.remove(player.getUniqueId()); @@ -195,7 +230,7 @@ private CheckResult checkAndNotify(@NonNull Player player, Location loc) User.getInstance(player).notify("protection.locked-island-bypass"); } } - notifyIfDeletable(player, loc); + notifyIfDeletable(player, island); return result; } @@ -208,13 +243,12 @@ private CheckResult checkAndNotify(@NonNull Player player, Location loc) *

Fires at most once per entry, using the same "move out to reset" * pattern as the lock notification. */ - private void notifyIfDeletable(@NonNull Player player, Location loc) { + private void notifyIfDeletable(@NonNull Player player, Optional island) { if (!player.isOp()) { deletableNotified.remove(player.getUniqueId()); return; } - boolean deletable = getIslands().getProtectedIslandAt(loc) - .map(Island::isDeletable).orElse(false); + boolean deletable = island.map(Island::isDeletable).orElse(false); if (deletable) { if (deletableNotified.add(player.getUniqueId())) { User.getInstance(player).notify("protection.deletable-island-admin"); diff --git a/src/main/java/world/bentobox/bentobox/listeners/flags/settings/MobSpawnListener.java b/src/main/java/world/bentobox/bentobox/listeners/flags/settings/MobSpawnListener.java index 06a9ce38c..4e1173def 100644 --- a/src/main/java/world/bentobox/bentobox/listeners/flags/settings/MobSpawnListener.java +++ b/src/main/java/world/bentobox/bentobox/listeners/flags/settings/MobSpawnListener.java @@ -96,8 +96,10 @@ public void onRaidFinishEvent(RaidFinishEvent event) */ void onMobSpawn(CreatureSpawnEvent e) { + // Entity#getLocation allocates a new Location on every call, so fetch it once + Location location = e.getLocation(); // If not in the right world, or spawning is not natural return - if (!this.getIWM().inWorld(e.getEntity().getLocation())) + if (!this.getIWM().inWorld(location)) { return; } @@ -109,7 +111,7 @@ void onMobSpawn(CreatureSpawnEvent e) RAID, REINFORCEMENTS, SILVERFISH_BLOCK, TRAP, VILLAGE_DEFENSE, VILLAGE_INVASION -> { boolean cancelNatural = this.shouldCancel(e.getEntity(), - e.getLocation(), + location, Flags.ANIMAL_NATURAL_SPAWN, Flags.MONSTER_NATURAL_SPAWN); e.setCancelled(cancelNatural); @@ -118,7 +120,7 @@ void onMobSpawn(CreatureSpawnEvent e) case SPAWNER, TRIAL_SPAWNER -> { boolean cancelSpawners = this.shouldCancel(e.getEntity(), - e.getLocation(), + location, Flags.ANIMAL_SPAWNERS_SPAWN, Flags.MONSTER_SPAWNERS_SPAWN); e.setCancelled(cancelSpawners); diff --git a/src/main/java/world/bentobox/bentobox/listeners/flags/settings/PVPListener.java b/src/main/java/world/bentobox/bentobox/listeners/flags/settings/PVPListener.java index 9db84910b..911c11323 100644 --- a/src/main/java/world/bentobox/bentobox/listeners/flags/settings/PVPListener.java +++ b/src/main/java/world/bentobox/bentobox/listeners/flags/settings/PVPListener.java @@ -145,8 +145,9 @@ public void onSplashPotionSplash(final PotionSplashEvent e) { return; } // Run through affected entities and cancel the splash for protected players + Flag flag = getFlag(e.getEntity().getWorld()); for (LivingEntity le : e.getAffectedEntities()) { - if (!le.getUniqueId().equals(user.getUniqueId()) && blockPVP(user, le, e, getFlag(e.getEntity().getWorld()))) { + if (!le.getUniqueId().equals(user.getUniqueId()) && blockPVP(user, le, e, flag)) { e.setIntensity(le, 0); } } @@ -222,7 +223,8 @@ public void onLingeringPotionDamage(AreaEffectCloudApplyEvent e) { if (thrownPotions.containsKey(e.getEntity().getEntityId())) { User user = User.getInstance(thrownPotions.get(e.getEntity().getEntityId())); // Run through affected entities and delete them if they are safe - e.getAffectedEntities().removeIf(le -> !le.getUniqueId().equals(user.getUniqueId()) && blockPVP(user, le, e, getFlag(e.getEntity().getWorld()))); + Flag flag = getFlag(e.getEntity().getWorld()); + e.getAffectedEntities().removeIf(le -> !le.getUniqueId().equals(user.getUniqueId()) && blockPVP(user, le, e, flag)); } } diff --git a/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/CleanSuperFlatListener.java b/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/CleanSuperFlatListener.java index 3d91e2586..29a22a4f3 100644 --- a/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/CleanSuperFlatListener.java +++ b/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/CleanSuperFlatListener.java @@ -1,6 +1,8 @@ package world.bentobox.bentobox.listeners.flags.worldsettings; +import java.util.HashMap; import java.util.LinkedList; +import java.util.Map; import java.util.Queue; import org.bukkit.Bukkit; @@ -11,7 +13,6 @@ import org.bukkit.event.EventHandler; import org.bukkit.event.EventPriority; import org.bukkit.event.world.ChunkLoadEvent; -import org.bukkit.generator.ChunkGenerator; import org.bukkit.scheduler.BukkitTask; import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; @@ -55,8 +56,16 @@ public class CleanSuperFlatListener extends FlagListener { private WorldRegenerator regenerator; + /** + * Per-world cache of whether a default world generator exists, so the addon + * lookup does not run for every superflat chunk load. + */ + private final Map hasGenerator = new HashMap<>(); + @EventHandler(priority = EventPriority.LOW, ignoreCancelled = true) public void onBentoBoxReady(BentoBoxReadyEvent e) { + // Clear the generator cache so a reload picks up newly assigned/registered generators. + hasGenerator.clear(); this.regenerator = Util.getRegenerator(); if (regenerator == null) { plugin.logError("Could not start CleanSuperFlat because of NMS error"); @@ -75,9 +84,10 @@ public void onChunkLoad(ChunkLoadEvent e) return; } - ChunkGenerator cg = plugin.getAddonsManager().getDefaultWorldGenerator(world.getName(), ""); - - if (cg == null) + boolean cg = hasGenerator.computeIfAbsent(world.getName(), + name -> plugin.getAddonsManager().getDefaultWorldGenerator(name, "") != null); + + if (!cg) { Flags.CLEAN_SUPER_FLAT.setSetting(world, false); diff --git a/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/EnterExitListener.java b/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/EnterExitListener.java index 9f82fdc9a..df78492e2 100644 --- a/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/EnterExitListener.java +++ b/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/EnterExitListener.java @@ -7,7 +7,6 @@ import org.bukkit.event.EventPriority; import org.bukkit.event.player.PlayerMoveEvent; import org.bukkit.event.player.PlayerTeleportEvent; -import org.bukkit.util.Vector; import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; @@ -24,7 +23,6 @@ */ public class EnterExitListener extends FlagListener { - private static final Vector XZ = new Vector(1,0,1); private static final String ISLAND_MESSAGE = "protection.flags.ENTER_EXIT_MESSAGES.island"; @EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true) @@ -39,9 +37,11 @@ public void onTeleport(PlayerTeleportEvent e) { private void handleEnterExit(@NonNull User user, @NonNull Location from, @Nullable Location to, @NonNull PlayerMoveEvent e) { - // Only process if there is a change in X or Z coords + // Only process if there is a change in X or Z coords. Island protection bounds + // are block-based, so same-block moves cannot change the result; comparing block + // coordinates avoids allocating vectors on every move event. if (from.getWorld() != null && to != null && from.getWorld().equals(to.getWorld()) - && from.toVector().multiply(XZ).equals(to.toVector().multiply(XZ))) { + && from.getBlockX() == to.getBlockX() && from.getBlockZ() == to.getBlockZ()) { return; } diff --git a/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/GeoLimitMobsListener.java b/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/GeoLimitMobsListener.java index bb57f11c2..2a2b08fb3 100644 --- a/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/GeoLimitMobsListener.java +++ b/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/GeoLimitMobsListener.java @@ -1,9 +1,11 @@ package world.bentobox.bentobox.listeners.flags.worldsettings; +import java.util.Iterator; import java.util.Map; import java.util.WeakHashMap; import org.bukkit.Bukkit; +import org.bukkit.Location; import org.bukkit.entity.Entity; import org.bukkit.entity.Projectile; import org.bukkit.event.EventHandler; @@ -32,13 +34,20 @@ public class GeoLimitMobsListener extends FlagListener { */ @EventHandler(priority = EventPriority.LOW, ignoreCancelled = true) public void onPluginReady(BentoBoxReadyEvent event) { - // Kick off the task to remove entities that go outside island boundaries + // Kick off the task to remove entities that go outside island boundaries. + // This runs every second, so avoid stream allocations Bukkit.getScheduler().runTaskTimer(getPlugin(), () -> { - mobSpawnTracker.entrySet().stream() - .filter(e -> !e.getValue().onIsland(e.getKey().getLocation())) - .map(Map.Entry::getKey) - .forEach(Entity::remove); - mobSpawnTracker.keySet().removeIf(e -> e == null || e.isDead()); + Iterator> it = mobSpawnTracker.entrySet().iterator(); + while (it.hasNext()) { + Map.Entry entry = it.next(); + Entity mob = entry.getKey(); + if (mob == null || mob.isDead()) { + it.remove(); + } else if (!entry.getValue().onIsland(mob.getLocation())) { + mob.remove(); + it.remove(); + } + } }, 20L, 20L); } @@ -48,9 +57,11 @@ public void onPluginReady(BentoBoxReadyEvent event) { */ @EventHandler(priority = EventPriority.LOW, ignoreCancelled = true) public void onMobSpawn(CreatureSpawnEvent e) { - if (getIWM().inWorld(e.getLocation()) - && getIWM().getGeoLimitSettings(e.getLocation().getWorld()).contains(e.getEntityType().name())) { - getIslands().getIslandAt(e.getLocation()).ifPresent(i -> mobSpawnTracker.put(e.getEntity(), i)); + // Entity#getLocation allocates a new Location on every call, so fetch it once + Location location = e.getLocation(); + if (getIWM().inWorld(location) + && getIWM().getGeoLimitSettings(location.getWorld()).contains(e.getEntityType().name())) { + getIslands().getIslandAt(location).ifPresent(i -> mobSpawnTracker.put(e.getEntity(), i)); } } @@ -68,9 +79,11 @@ public void onMobDeath(final EntityDeathEvent e) { */ @EventHandler(priority = EventPriority.LOW, ignoreCancelled = true) public void onProjectileLaunch(final ProjectileLaunchEvent e) { - if (getIWM().inWorld(e.getEntity().getLocation()) - && getIWM().getGeoLimitSettings(e.getEntity().getLocation().getWorld()).contains(e.getEntityType().name())) { - getIslands().getIslandAt(e.getEntity().getLocation()).ifPresent(i -> mobSpawnTracker.put(e.getEntity(), i)); + // Entity#getLocation allocates a new Location on every call, so fetch it once + Location location = e.getEntity().getLocation(); + if (getIWM().inWorld(location) + && getIWM().getGeoLimitSettings(location.getWorld()).contains(e.getEntityType().name())) { + getIslands().getIslandAt(location).ifPresent(i -> mobSpawnTracker.put(e.getEntity(), i)); } } diff --git a/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/InvincibleVisitorsListener.java b/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/InvincibleVisitorsListener.java index fb3cad9f2..f88941ea4 100644 --- a/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/InvincibleVisitorsListener.java +++ b/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/InvincibleVisitorsListener.java @@ -3,6 +3,7 @@ import java.util.Arrays; import java.util.Comparator; import java.util.Objects; +import java.util.Optional; import org.bukkit.Bukkit; import org.bukkit.Material; @@ -28,6 +29,7 @@ import world.bentobox.bentobox.api.panels.builders.PanelBuilder; import world.bentobox.bentobox.api.panels.builders.PanelItemBuilder; import world.bentobox.bentobox.api.user.User; +import world.bentobox.bentobox.database.objects.Island; import world.bentobox.bentobox.util.Util; import world.bentobox.bentobox.util.teleport.SafeSpotTeleport; @@ -156,13 +158,14 @@ public void onVisitorGetDamage(EntityDamageEvent e) { e.setCancelled(true); // Handle the void - teleport player back to island in a safe spot if(e.getCause().equals(DamageCause.VOID)) { - if (getIslands().getIslandAt(p.getLocation()).isPresent()) { - getIslands().getIslandAt(p.getLocation()).ifPresent(island -> + // Single lookup - getIslandAt walks the grid and getLocation copies, no need to do either twice + Optional island = getIslands().getIslandAt(p.getLocation()); + if (island.isPresent()) { // Teleport new SafeSpotTeleport.Builder(getPlugin()) .entity(p) - .location(island.getProtectionCenter().toVector().toLocation(p.getWorld())) - .build()); + .location(island.get().getProtectionCenter().toVector().toLocation(p.getWorld())) + .build(); } else if (getIslands().hasIsland(p.getWorld(), p.getUniqueId())) { // No island in this location - if the player has an island try to teleport them back getIslands().homeTeleportAsync(p.getWorld(), p); diff --git a/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/LimitMobsListener.java b/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/LimitMobsListener.java index f8a131280..57101a389 100644 --- a/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/LimitMobsListener.java +++ b/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/LimitMobsListener.java @@ -1,5 +1,6 @@ package world.bentobox.bentobox.listeners.flags.worldsettings; +import org.bukkit.Location; import org.bukkit.entity.EntityType; import org.bukkit.event.EventHandler; import org.bukkit.event.EventPriority; @@ -28,7 +29,9 @@ public void onMobSpawn(CreatureSpawnEvent e) { } private void check(CreatureSpawnEvent e, EntityType type) { - if (getIWM().inWorld(e.getLocation()) && getIWM().getMobLimitSettings(e.getLocation().getWorld()).contains(type.name())) { + // Entity#getLocation allocates a new Location on every call, so fetch it once + Location location = e.getLocation(); + if (getIWM().inWorld(location) && getIWM().getMobLimitSettings(location.getWorld()).contains(type.name())) { e.setCancelled(true); } } diff --git a/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/LiquidsFlowingOutListener.java b/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/LiquidsFlowingOutListener.java index bf6b90835..702315c9b 100644 --- a/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/LiquidsFlowingOutListener.java +++ b/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/LiquidsFlowingOutListener.java @@ -26,20 +26,20 @@ public void onLiquidFlow(BlockFromToEvent e) { Block from = e.getBlock(); Block to = e.getToBlock(); - if (!getIWM().inWorld(from.getLocation()) || Flags.LIQUIDS_FLOWING_OUT.isSetForWorld(from.getWorld())) { - // We do not want to run any check if this is not the right world or if it is allowed. + // https://github.com/BentoBoxWorld/BentoBox/issues/511#issuecomment-460040287 + if (to.getY() != from.getY()) { + // We do not run any checks if this is a vertical flow - would be too much resource consuming. return; } - // https://github.com/BentoBoxWorld/BentoBox/issues/511#issuecomment-460040287 - // Time to do some maths! We've got the vector FromTo, let's check if its y coordinate is different from zero. - if (to.getLocation().toVector().subtract(from.getLocation().toVector()).getY() != 0) { - // We do not run any checks if this is a vertical flow - would be too much resource consuming. + Location fromLocation = from.getLocation(); + if (!getIWM().inWorld(fromLocation) || Flags.LIQUIDS_FLOWING_OUT.isSetForWorld(from.getWorld())) { + // We do not want to run any check if this is not the right world or if it is allowed. return; } // Only prevent if it is flowing into the area between islands or into another island. - Optional fromIsland = getIslands().getProtectedIslandAt(from.getLocation()); + Optional fromIsland = getIslands().getProtectedIslandAt(fromLocation); Optional toIsland = getIslands().getProtectedIslandAt(to.getLocation()); if (toIsland.isEmpty() || (fromIsland.isPresent() && !fromIsland.equals(toIsland))) { e.setCancelled(true); diff --git a/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/NaturalSpawningOutsideRangeListener.java b/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/NaturalSpawningOutsideRangeListener.java index ade03ed48..18f740a2b 100644 --- a/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/NaturalSpawningOutsideRangeListener.java +++ b/src/main/java/world/bentobox/bentobox/listeners/flags/worldsettings/NaturalSpawningOutsideRangeListener.java @@ -1,5 +1,6 @@ package world.bentobox.bentobox.listeners.flags.worldsettings; +import org.bukkit.Location; import org.bukkit.event.EventHandler; import org.bukkit.event.EventPriority; import org.bukkit.event.entity.CreatureSpawnEvent; @@ -17,13 +18,15 @@ public class NaturalSpawningOutsideRangeListener extends FlagListener { @EventHandler(priority = EventPriority.LOWEST) public void onCreatureSpawn(CreatureSpawnEvent e) { - if (!getIWM().inWorld(e.getLocation()) || Flags.NATURAL_SPAWNING_OUTSIDE_RANGE.isSetForWorld(e.getLocation().getWorld())) { + // Entity#getLocation allocates a new Location on every call, so fetch it once + Location location = e.getLocation(); + if (!getIWM().inWorld(location) || Flags.NATURAL_SPAWNING_OUTSIDE_RANGE.isSetForWorld(location.getWorld())) { // We do not want to run any check if this is not the right world or if it is allowed. return; } // If it is a natural spawn and there is no protected island at the location, block the spawn. - if (e.getSpawnReason() == CreatureSpawnEvent.SpawnReason.NATURAL && getIslands().getProtectedIslandAt(e.getLocation()).isEmpty()) { + if (e.getSpawnReason() == CreatureSpawnEvent.SpawnReason.NATURAL && getIslands().getProtectedIslandAt(location).isEmpty()) { e.setCancelled(true); } } diff --git a/src/main/java/world/bentobox/bentobox/managers/island/IslandCache.java b/src/main/java/world/bentobox/bentobox/managers/island/IslandCache.java index 3d78d214e..871ce6e94 100644 --- a/src/main/java/world/bentobox/bentobox/managers/island/IslandCache.java +++ b/src/main/java/world/bentobox/bentobox/managers/island/IslandCache.java @@ -265,10 +265,11 @@ public void setPrimaryIsland(@NonNull UUID uuid, @NonNull Island island) { */ public boolean isIslandAt(@NonNull Location location) { World w = Util.getWorld(location.getWorld()); - if (w == null || !grids.containsKey(w)) { + if (w == null) { return false; } - return grids.get(w).isIslandAt(location.getBlockX(), location.getBlockZ()); + IslandGrid grid = grids.get(w); + return grid != null && grid.isIslandAt(location.getBlockX(), location.getBlockZ()); } /** @@ -281,10 +282,11 @@ public boolean isIslandAt(@NonNull Location location) { @Nullable public Island getIslandAt(@NonNull Location location) { World w = Util.getWorld(location.getWorld()); - if (w == null || !grids.containsKey(w)) { + if (w == null) { return null; } - return grids.get(w).getIslandAt(location.getBlockX(), location.getBlockZ()); + IslandGrid grid = grids.get(w); + return grid == null ? null : grid.getIslandAt(location.getBlockX(), location.getBlockZ()); } /** diff --git a/src/main/java/world/bentobox/bentobox/util/Util.java b/src/main/java/world/bentobox/bentobox/util/Util.java index 0de0cfd32..78f80b6f5 100644 --- a/src/main/java/world/bentobox/bentobox/util/Util.java +++ b/src/main/java/world/bentobox/bentobox/util/Util.java @@ -8,10 +8,12 @@ import java.util.Date; import java.util.Enumeration; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.UUID; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; import java.util.jar.JarEntry; import java.util.jar.JarFile; @@ -332,17 +334,31 @@ public static boolean sameWorld(World world, World world2) { // matches nothing return false; } + // Same instance is the overwhelmingly common case on the event path + if (world == world2) { + return true; + } return stripName(world).equals(stripName(world2)); } + /** + * Cache of world name -> name with any _nether/_the_end suffix stripped. + * The mapping is a pure function of the name, so it never needs invalidation. + */ + private static final Map STRIPPED_NAMES = new ConcurrentHashMap<>(); + private static String stripName(World world) { - if (world.getName().endsWith(NETHER)) { - return world.getName().substring(0, world.getName().length() - NETHER.length()); + return STRIPPED_NAMES.computeIfAbsent(world.getName(), Util::stripSuffix); + } + + private static String stripSuffix(String name) { + if (name.endsWith(NETHER)) { + return name.substring(0, name.length() - NETHER.length()); } - if (world.getName().endsWith(THE_END)) { - return world.getName().substring(0, world.getName().length() - THE_END.length()); + if (name.endsWith(THE_END)) { + return name.substring(0, name.length() - THE_END.length()); } - return world.getName(); + return name; } /** @@ -350,12 +366,23 @@ private static String stripName(World world) { * @param world - world * @return over world or null if world is null or a world cannot be found */ + /** + * Cache of world name -> overworld name (suffixes replaced). Pure string + * function, so entries never go stale. The World itself is still resolved + * live via {@link Bukkit#getWorld(String)}. + */ + private static final Map OVERWORLD_NAMES = new ConcurrentHashMap<>(); + @Nullable public static World getWorld(@Nullable World world) { if (world == null) { return null; } - return world.getEnvironment().equals(Environment.NORMAL) ? world : Bukkit.getWorld(world.getName().replace(NETHER, "").replace(THE_END, "")); + if (world.getEnvironment() == Environment.NORMAL) { + return world; + } + return Bukkit.getWorld(OVERWORLD_NAMES.computeIfAbsent(world.getName(), + n -> n.replace(NETHER, "").replace(THE_END, ""))); } /** diff --git a/src/main/java/world/bentobox/bentobox/util/heads/HeadGetter.java b/src/main/java/world/bentobox/bentobox/util/heads/HeadGetter.java index 661dbee6d..8f641bdee 100644 --- a/src/main/java/world/bentobox/bentobox/util/heads/HeadGetter.java +++ b/src/main/java/world/bentobox/bentobox/util/heads/HeadGetter.java @@ -31,7 +31,7 @@ /** * This class manages getting player heads for requester. - * + * * @author tastybento, BONNe1704 */ public class HeadGetter { @@ -53,6 +53,9 @@ public class HeadGetter { private static final String TEXTURES = "textures"; + // Gson construction is expensive, don't rebuild it per head lookup + private static final Gson GSON = new Gson(); + private static final String NAME = "name"; /** @@ -151,7 +154,7 @@ public static void getHead(PanelItem panelItem, HeadRequester requester) { /** * This method allows to add HeadCache object into local cache. It will provide * addons to use HeadGetter cache directly. - * + * * @param cache Cache object that need to be added into local cache. * @since 1.14.1 */ @@ -290,7 +293,7 @@ private void runPlayerHeadGetter() { /** * This method gets and returns userId from mojang web API based on user name. - * + * * @param name user which Id must be returned. * @return String value for user Id. * @since 1.14.1 @@ -299,10 +302,8 @@ private static UUID getUserIdFromName(String name) { UUID userId; try { - Gson gsonReader = new Gson(); - // Get mojang user-id from given nickname - JsonObject jsonObject = gsonReader.fromJson( + JsonObject jsonObject = GSON.fromJson( HeadGetter.getURLContent("https://api.mojang.com/users/profiles/minecraft/" + name), JsonObject.class); /* @@ -335,10 +336,8 @@ private static UUID getUserIdFromName(String name) { */ private static @Nullable String getTextureFromUUID(UUID userId) { try { - Gson gsonReader = new Gson(); - // Get user encoded texture value. - JsonObject jsonObject = gsonReader.fromJson( + JsonObject jsonObject = GSON.fromJson( HeadGetter.getURLContent( "https://sessionserver.mojang.com/session/minecraft/profile/" + userId.toString()), JsonObject.class); @@ -379,13 +378,11 @@ private static UUID getUserIdFromName(String name) { */ private static @NonNull Pair getTextureFromName(String userName, @Nullable UUID userId) { try { - Gson gsonReader = new Gson(); - // Get user encoded texture value. // mc-heads returns correct skin with providing just a name, unlike mojang api, // which // requires UUID. - JsonObject jsonObject = gsonReader.fromJson(HeadGetter.getURLContent( + JsonObject jsonObject = GSON.fromJson(HeadGetter.getURLContent( "https://mc-heads.net/minecraft/profile/" + (userId == null ? userName : userId.toString())), JsonObject.class); @@ -449,7 +446,7 @@ private static URL getSkinURLFromBase64(String base64) { */ try { String decoded = new String(Base64.getDecoder().decode(base64)); - JsonObject json = new Gson().fromJson(decoded, JsonObject.class); + JsonObject json = GSON.fromJson(decoded, JsonObject.class); String url = json.getAsJsonObject(TEXTURES).getAsJsonObject("SKIN").get("url").getAsString(); return new URI(url).toURL(); } catch (Exception e) { diff --git a/src/main/java/world/bentobox/bentobox/util/teleport/ClosestSafeSpotTeleport.java b/src/main/java/world/bentobox/bentobox/util/teleport/ClosestSafeSpotTeleport.java index fc3eb3e4d..27f8795a8 100644 --- a/src/main/java/world/bentobox/bentobox/util/teleport/ClosestSafeSpotTeleport.java +++ b/src/main/java/world/bentobox/bentobox/util/teleport/ClosestSafeSpotTeleport.java @@ -345,10 +345,12 @@ void scanAndPopulateBlockQueue(ChunkSnapshot chunkSnapshot) for (int y = Math.max(minY, startY - this.range); y < Math.min(maxY, startY + this.range); y++) { - Vector positionVector = new Vector(chunkX + x, y, chunkZ + z); - if (this.boundingBox.contains(positionVector)) + // Raw-coordinate bounds check first and only allocate a vector for + // positions actually inside the search area + if (this.boundingBox.contains(chunkX + x, y, chunkZ + z)) { // Process positions that are inside bounding box of search area. + Vector positionVector = new Vector(chunkX + x, y, chunkZ + z); PositionData positionData = new PositionData( positionVector, diff --git a/src/test/java/world/bentobox/bentobox/api/flags/FlagListenerTest.java b/src/test/java/world/bentobox/bentobox/api/flags/FlagListenerTest.java index d21ca06d5..7d2899666 100644 --- a/src/test/java/world/bentobox/bentobox/api/flags/FlagListenerTest.java +++ b/src/test/java/world/bentobox/bentobox/api/flags/FlagListenerTest.java @@ -44,6 +44,7 @@ public void setUp() throws Exception { when(location.toVector()).thenReturn(new Vector(10, 64, 20)); // Enable why-debug on mockPlayer for this world, issuer is the same player (uuid) + when(mockPlayer.hasMetadata("bskyblock_world_why_debug")).thenReturn(true); when(mockPlayer.getMetadata("bskyblock_world_why_debug")) .thenReturn(Collections.singletonList(new FixedMetadataValue(plugin, true))); when(mockPlayer.getMetadata("bskyblock_world_why_debug_issuer")) diff --git a/src/test/java/world/bentobox/bentobox/listeners/StandardSpawnProtectionListenerTest.java b/src/test/java/world/bentobox/bentobox/listeners/StandardSpawnProtectionListenerTest.java index d119f377e..a80770f4e 100644 --- a/src/test/java/world/bentobox/bentobox/listeners/StandardSpawnProtectionListenerTest.java +++ b/src/test/java/world/bentobox/bentobox/listeners/StandardSpawnProtectionListenerTest.java @@ -81,6 +81,8 @@ public void setUp() throws Exception { mockedUtil.when(() -> Util.getWorld(any())).thenReturn(world); // Location when(location.toVector()).thenReturn(new Vector(5,5,5)); + when(location.getX()).thenReturn(5D); + when(location.getZ()).thenReturn(5D); when(location.getWorld()).thenReturn(nether); when(spawnLocation.toVector()).thenReturn(new Vector(0,0,0)); when(spawnLocation.getWorld()).thenReturn(nether); @@ -286,11 +288,7 @@ void testOnExplosion() { blockList.add(block); blockList.add(block); // Make some inside and outside spawn - when(location.toVector()).thenReturn(new Vector(0,0,0), - new Vector(0,0,0), - new Vector(0,0,0), - new Vector(0,0,0), - new Vector(10000,0,0)); + when(location.getX()).thenReturn(0D, 0D, 0D, 0D, 10000D); EntityExplodeEvent e = getExplodeEvent(mockPlayer, location, blockList); ssp.onExplosion(e); // 4 blocks inside the spawn should be removed, leaving one @@ -311,11 +309,7 @@ void testOnExplosionNoProtection() { blockList.add(block); blockList.add(block); // Make some inside and outside spawn - when(location.toVector()).thenReturn(new Vector(0,0,0), - new Vector(0,0,0), - new Vector(0,0,0), - new Vector(0,0,0), - new Vector(10000,0,0)); + when(location.getX()).thenReturn(0D, 0D, 0D, 0D, 10000D); EntityExplodeEvent e = getExplodeEvent(mockPlayer, location, blockList); ssp.onExplosion(e); // No blocks should be removed @@ -344,11 +338,7 @@ void testOnExplosionInStandardEndWorldNoNPE() { blockList.add(block); blockList.add(block); // Make some inside and outside spawn - when(location.toVector()).thenReturn(new Vector(0, 0, 0), - new Vector(0, 0, 0), - new Vector(0, 0, 0), - new Vector(0, 0, 0), - new Vector(10000, 0, 0)); + when(location.getX()).thenReturn(0D, 0D, 0D, 0D, 10000D); EntityExplodeEvent e = getExplodeEvent(mockPlayer, location, blockList); // Should not throw NullPointerException ssp.onExplosion(e); diff --git a/src/test/java/world/bentobox/bentobox/listeners/flags/protection/LockAndBanListenerTest.java b/src/test/java/world/bentobox/bentobox/listeners/flags/protection/LockAndBanListenerTest.java index 8efdd181c..081af79ad 100644 --- a/src/test/java/world/bentobox/bentobox/listeners/flags/protection/LockAndBanListenerTest.java +++ b/src/test/java/world/bentobox/bentobox/listeners/flags/protection/LockAndBanListenerTest.java @@ -263,6 +263,39 @@ void testVerticalVehicleMoveOnly() { verify(im, never()).getProtectedIslandAt(any()); } + @Test + void testVehicleMoveNoPlayerPassengersSkipsIslandLookup() { + // A vehicle carrying only non-player entities (e.g. a mob-ridden horse or an + // empty minecart) must not trigger an island lookup when it crosses a boundary. + Vehicle vehicle = mock(Vehicle.class); + Entity mob = mock(Entity.class); // not a Player + List passengers = new ArrayList<>(); + passengers.add(mob); + when(vehicle.getPassengers()).thenReturn(passengers); + // Horizontal move across a block boundary + listener.onVehicleMove(new VehicleMoveEvent(vehicle, outside, inside)); + // Lazy lookup: no player passenger, so the grid is never queried + verify(im, never()).getProtectedIslandAt(any()); + } + + @Test + void testVehicleMoveResolvesIslandOnceForMultiplePlayers() { + // Two players in one vehicle should share a single island lookup, not one each. + when(mockPlayer.getUniqueId()).thenReturn(uuid); + Vehicle vehicle = mock(Vehicle.class); + Player player2 = mock(Player.class); + when(player2.isOp()).thenReturn(false); + when(player2.hasPermission(anyString())).thenReturn(false); + List passengers = new ArrayList<>(); + passengers.add(mockPlayer); + passengers.add(player2); + when(vehicle.getPassengers()).thenReturn(passengers); + // Horizontal move across a block boundary into an open island + listener.onVehicleMove(new VehicleMoveEvent(vehicle, outside, inside)); + // Resolved lazily on the first player and reused for the second + verify(im, org.mockito.Mockito.times(1)).getProtectedIslandAt(inside); + } + @Test void testPlayerMoveIntoBannedIsland() { // Make player diff --git a/src/test/java/world/bentobox/bentobox/listeners/flags/worldsettings/LiquidsFlowingOutListenerTest.java b/src/test/java/world/bentobox/bentobox/listeners/flags/worldsettings/LiquidsFlowingOutListenerTest.java index 89f9b126c..4ea35f3f2 100644 --- a/src/test/java/world/bentobox/bentobox/listeners/flags/worldsettings/LiquidsFlowingOutListenerTest.java +++ b/src/test/java/world/bentobox/bentobox/listeners/flags/worldsettings/LiquidsFlowingOutListenerTest.java @@ -120,8 +120,9 @@ void testFlagIsAllowed() { @Test void testLiquidFlowsVertically() { // "To" is at (1,0,0) - // Set "from" at (1,1,0) so that the vector's y coordinate != 0, which means the liquid flows vertically. + // Set "from" at (1,1,0) so that the y coordinates differ, which means the liquid flows vertically. when(from.getLocation()).thenReturn(new Location(world, 1, 1, 0)); + when(from.getY()).thenReturn(1); // Run new LiquidsFlowingOutListener().onLiquidFlow(event); diff --git a/src/test/java/world/bentobox/bentobox/listeners/flags/worldsettings/VisitorKeepInventoryListenerTest.java b/src/test/java/world/bentobox/bentobox/listeners/flags/worldsettings/VisitorKeepInventoryListenerTest.java index ecc9103d7..4762c6931 100644 --- a/src/test/java/world/bentobox/bentobox/listeners/flags/worldsettings/VisitorKeepInventoryListenerTest.java +++ b/src/test/java/world/bentobox/bentobox/listeners/flags/worldsettings/VisitorKeepInventoryListenerTest.java @@ -62,6 +62,7 @@ public void setUp() throws Exception { when(location.getWorld()).thenReturn(world); when(location.toVector()).thenReturn(new Vector(1,2,3)); // Turn on why for player + when(mockPlayer.hasMetadata("bskyblock_world_why_debug")).thenReturn(true); when(mockPlayer.getMetadata("bskyblock_world_why_debug")).thenReturn(Collections.singletonList(new FixedMetadataValue(plugin, true))); when(mockPlayer.getMetadata("bskyblock_world_why_debug_issuer")).thenReturn(Collections.singletonList(new FixedMetadataValue(plugin, uuid.toString()))); User.getInstance(mockPlayer);