diff --git a/pom.xml b/pom.xml index 2096cd20b..c52bfe3a4 100644 --- a/pom.xml +++ b/pom.xml @@ -75,7 +75,7 @@ -LOCAL - 3.8.0 + 3.8.1 bentobox-world https://sonarcloud.io ${project.basedir}/lib diff --git a/src/main/java/world/bentobox/bentobox/blueprints/BlueprintClipboard.java b/src/main/java/world/bentobox/bentobox/blueprints/BlueprintClipboard.java index e42057ad0..4d7e093fc 100644 --- a/src/main/java/world/bentobox/bentobox/blueprints/BlueprintClipboard.java +++ b/src/main/java/world/bentobox/bentobox/blueprints/BlueprintClipboard.java @@ -58,7 +58,6 @@ public class BlueprintClipboard { /** * Used to filter out hidden DisplayEntity armor stands when copying */ - private NamespacedKey key; private @Nullable Blueprint blueprint; private @Nullable Location pos1; private @Nullable Location pos2; @@ -84,7 +83,6 @@ public class BlueprintClipboard { public BlueprintClipboard(@NonNull Blueprint blueprint) { this(); this.blueprint = blueprint; - this.key = new NamespacedKey(BentoBox.getInstance(), "associatedDisplayEntity"); } public BlueprintClipboard() { @@ -165,6 +163,7 @@ private void copyAsync(World world, User user, List vectorsToCopy, int s return; } copying = true; + NamespacedKey key = new NamespacedKey(BentoBox.getInstance(), "associatedDisplayEntity"); vectorsToCopy.stream().skip(index).limit(speed).forEach(v -> { List ents = world.getEntities().stream() .filter(Objects::nonNull) diff --git a/src/main/java/world/bentobox/bentobox/managers/island/DefaultNewIslandLocationStrategy.java b/src/main/java/world/bentobox/bentobox/managers/island/DefaultNewIslandLocationStrategy.java index fc5ecab3b..ff89b5865 100644 --- a/src/main/java/world/bentobox/bentobox/managers/island/DefaultNewIslandLocationStrategy.java +++ b/src/main/java/world/bentobox/bentobox/managers/island/DefaultNewIslandLocationStrategy.java @@ -15,76 +15,117 @@ import world.bentobox.bentobox.util.Util; /** - * The default strategy for generating locations for island + * The default strategy for generating locations for new islands. + * This strategy finds the next available island spot by searching in an outward square spiral pattern + * from the last known island location or the world's starting coordinates. + *

+ * If you wish to create an alternative strategy, you must implement the {@link NewIslandLocationStrategy} interface. + * Your implementation should be robust and consider the following: + *

+ * The methods in this class, like {@link #isIsland(Location)}, can be useful helpers for custom implementations. + *

+ * * @author tastybento, leonardochaia * @since 1.8.0 - * */ public class DefaultNewIslandLocationStrategy implements NewIslandLocationStrategy { /** - * The amount times to tolerate island check returning blocks without known - * island. + * The maximum number of times to tolerate finding non-BentoBox blocks in a potential island spot + * before giving up. This acts as a safeguard against infinite loops when searching for a free location + * in a world that was not generated by BentoBox or has been heavily modified. */ protected static final Integer MAX_UNOWNED_ISLANDS = 20; + /** + * Represents the result of checking a potential island location. + */ protected enum Result { - ISLAND_FOUND, BLOCKS_IN_AREA, FREE + /** + * A BentoBox island already exists at this location. + */ + ISLAND_FOUND, + /** + * No BentoBox island exists, but there are other blocks in the area. + * This spot is considered occupied. + */ + BLOCKS_IN_AREA, + /** + * The location is free and suitable for a new island. + */ + FREE } protected final BentoBox plugin = BentoBox.getInstance(); @Override public Location getNextLocation(World world) { + // Get the last known island location from the database. Location last = plugin.getIslands().getLast(world); if (last == null) { + // If no island has been created yet, start from the configured offset. last = new Location(world, (double) plugin.getIWM().getIslandXOffset(world) + plugin.getIWM().getIslandStartX(world), plugin.getIWM().getIslandHeight(world), (double) plugin.getIWM().getIslandZOffset(world) + plugin.getIWM().getIslandStartZ(world)); } - // Find a free spot + // Find a free spot by spiraling outwards. Map result = new EnumMap<>(Result.class); - // Check center + // Check the starting location. Result r = isIsland(last); + // Loop until a FREE spot is found or we hit the tolerance limit for unowned islands. while (!r.equals(Result.FREE) && result.getOrDefault(Result.BLOCKS_IN_AREA, 0) < MAX_UNOWNED_ISLANDS) { + // Move to the next location in the spiral. nextGridLocation(last); + // Count the result of the previous check. result.put(r, result.getOrDefault(r, 0) + 1); + // Check the new location. r = isIsland(last); } if (!r.equals(Result.FREE)) { - // We could not find a free spot within the limit required. It's likely this - // world is not empty + // We could not find a free spot within the required limit. + // This likely means the world is not empty or has many existing structures. plugin.logError("Could not find a free spot for islands! Is this world empty?"); plugin.logError("Blocks around center locations: " + result.getOrDefault(Result.BLOCKS_IN_AREA, 0) + " max " + MAX_UNOWNED_ISLANDS); plugin.logError("Known islands: " + result.getOrDefault(Result.ISLAND_FOUND, 0) + " max unlimited."); return null; } + // A free spot was found. Save it as the last location for the next search. plugin.getIslands().setLast(last); return last; } /** - * Checks if there is an island or blocks at this location + * Checks if a given location is free for a new island. + * It checks for existing BentoBox islands, islands pending deletion, and any other blocks in the area. * - * @param location - the location - * @return Result enum indicated what was found or not found + * @param location The center location of the potential island spot. + * @return {@link Result} enum indicating what was found. */ protected Result isIsland(Location location) { - // Quick check + // Quick check using the island grid cache. if (plugin.getIslands().isIslandAt(location)) { return Result.ISLAND_FOUND; } World world = location.getWorld(); - // Check 4 corners + // Check the four corners of the island protection area to be more thorough. int dist = plugin.getIWM().getIslandDistance(location.getWorld()); Set locs = new HashSet<>(); locs.add(location); + // Define the corners of the island's bounding box. locs.add(new Location(world, location.getX() - dist, 0, location.getZ() - dist)); locs.add(new Location(world, location.getX() - dist, 0, location.getZ() + dist - 1)); locs.add(new Location(world, location.getX() + dist - 1, 0, location.getZ() - dist)); @@ -92,59 +133,74 @@ protected Result isIsland(Location location) { boolean generated = false; for (Location l : locs) { + // Check if an island exists or is being deleted at any of the check points. if (plugin.getIslands().getIslandAt(l).isPresent() || plugin.getIslandDeletionManager().inDeletion(l)) { return Result.ISLAND_FOUND; } + // Check if the chunk is generated. An ungenerated chunk is considered free. if (Util.isChunkGenerated(l)) generated = true; } - // If chunk has not been generated yet, then it's not occupied + // If no chunks in the area have been generated yet, then it's definitely not occupied. if (!generated) { return Result.FREE; } - // Block check + // If configured, check for any non-air/non-water blocks in the immediate vicinity of the center. + // This is to detect pre-existing structures in imported worlds. if (plugin.getIWM().isCheckForBlocks(world) && !plugin.getIWM().isUseOwnGenerator(world) && Arrays.stream(BlockFace.values()).anyMatch(bf -> !location.getBlock().getRelative(bf).isEmpty() && !location.getBlock().getRelative(bf).getType().equals(Material.WATER))) { - // Block found + // A block was found. Create a temporary, reserved island object at this location + // to mark it as occupied in the database and prevent it from being checked again. plugin.getIslands().createIsland(location); return Result.BLOCKS_IN_AREA; } + // The location is free. return Result.FREE; } /** - * Finds the next free island spot based off the last known island Uses - * island_distance setting from the config file Builds up in a grid fashion + * Calculates the next island location in an outward square spiral pattern. + * This method modifies the provided {@link Location} object in-place. + * The algorithm works by moving along the edges of increasingly larger squares + * centered at the origin (0,0). The distance 'd' determines the step size. * - * @param lastIsland - last island location - * @return Location of next free island + * @param lastIsland The location of the last island, which will be modified to the next location. + * @return The same Location object, now set to the next grid position. */ private Location nextGridLocation(final Location lastIsland) { int x = lastIsland.getBlockX(); int z = lastIsland.getBlockZ(); + // The distance between island centers is twice the protection distance. int d = plugin.getIWM().getIslandDistance(lastIsland.getWorld()) * 2; if (x < z) { if (-1 * x < z) { + // Move right (positive X) lastIsland.setX(lastIsland.getX() + d); return lastIsland; } + // Move up (positive Z) lastIsland.setZ(lastIsland.getZ() + d); return lastIsland; } if (x > z) { if (-1 * x >= z) { + // Move left (negative X) lastIsland.setX(lastIsland.getX() - d); return lastIsland; } + // Move down (negative Z) lastIsland.setZ(lastIsland.getZ() - d); return lastIsland; } + // This condition handles the corner case where x == z. if (x <= 0) { + // Move up (positive Z) from the bottom-left corner. lastIsland.setZ(lastIsland.getZ() + d); return lastIsland; } + // Move down (negative Z) from the top-right corner. lastIsland.setZ(lastIsland.getZ() - d); return lastIsland; } diff --git a/src/main/java/world/bentobox/bentobox/managers/island/NewIsland.java b/src/main/java/world/bentobox/bentobox/managers/island/NewIsland.java index 58f635bd4..6adf59f37 100644 --- a/src/main/java/world/bentobox/bentobox/managers/island/NewIsland.java +++ b/src/main/java/world/bentobox/bentobox/managers/island/NewIsland.java @@ -24,10 +24,21 @@ import world.bentobox.bentobox.managers.IslandsManager; /** - * Create and paste a new island + * Handles the creation and pasting of a new island for a player. + * This class uses a builder pattern for flexible construction. + * + *

+ * The process involves: + *

    + *
  • Determining the next available location for the island
  • + *
  • Cleaning up user data (e.g., deaths, permissions)
  • + *
  • Firing relevant events for plugins/addons
  • + *
  • Pasting the blueprint or skipping paste if requested
  • + *
  • Setting up metrics and logging
  • + *
+ *

* * @author tastybento - * */ public class NewIsland { private final BentoBox plugin; @@ -41,6 +52,13 @@ public class NewIsland { private NewIslandLocationStrategy locationStrategy; + /** + * Constructs a new island using the provided builder. + * Fires a PRECREATE event before proceeding. + * + * @param builder Builder containing all required parameters + * @throws IOException if insufficient parameters or event is cancelled + */ public NewIsland(Builder builder) throws IOException { plugin = BentoBox.getInstance(); this.user = builder.user2; @@ -51,14 +69,15 @@ public NewIsland(Builder builder) throws IOException { this.addon = builder.addon2; this.locationStrategy = builder.locationStrategy2; + // Use default location strategy if none provided if (this.locationStrategy == null) { this.locationStrategy = new DefaultNewIslandLocationStrategy(); } - // Fire pre-create event + // Fire pre-create event to allow cancellation or modification IslandBaseEvent event = IslandEvent.builder().involvedPlayer(user.getUniqueId()).reason(Reason.PRECREATE) .build(); if (event.getNewEvent().map(IslandBaseEvent::isCancelled).orElse(event.isCancelled())) { - // Do nothing + // Event was cancelled, abort creation return; } newIsland(builder.oldIsland2); @@ -81,9 +100,8 @@ public static Builder builder() { } /** - * Build a new island for a player - * - * @author tastybento + * Builder for NewIsland. + * Allows flexible construction and validation of required parameters. */ public static class Builder { private Island oldIsland2; @@ -95,22 +113,29 @@ public static class Builder { private GameModeAddon addon2; private NewIslandLocationStrategy locationStrategy2; + /** + * Sets the old island to be replaced. + * Also sets the world to the old island's world. + */ public Builder oldIsland(Island oldIsland) { this.oldIsland2 = oldIsland; this.world2 = oldIsland.getWorld(); return this; } + /** + * Sets the player for whom the island is being created. + */ public Builder player(User player) { this.user2 = player; return this; } /** - * Sets the reason + * Sets the reason for island creation. + * Only CREATE or RESET are allowed. * - * @param reason reason, can only be {@link Reason#CREATE} or - * {@link Reason#RESET}. + * @param reason reason, can only be {@link Reason#CREATE} or {@link Reason#RESET}. */ public Builder reason(Reason reason) { if (!reason.equals(Reason.CREATE) && !reason.equals(Reason.RESET)) { @@ -121,9 +146,7 @@ public Builder reason(Reason reason) { } /** - * Set the addon - * - * @param addon a game mode addon + * Sets the game mode addon and its world. */ public Builder addon(GameModeAddon addon) { this.addon2 = addon; @@ -132,7 +155,7 @@ public Builder addon(GameModeAddon addon) { } /** - * No blocks will be pasted + * Indicates that no blocks should be pasted for the island. */ public Builder noPaste() { this.noPaste2 = true; @@ -140,7 +163,7 @@ public Builder noPaste() { } /** - * @param name - name of Blueprint bundle + * Sets the name of the blueprint bundle to use for the island. */ public Builder name(String name) { this.name2 = name; @@ -148,6 +171,8 @@ public Builder name(String name) { } /** + * Sets the location strategy for finding the next island location. + * * @param strategy - the location strategy to use * @since 1.8.0 */ @@ -157,8 +182,10 @@ public Builder locationStrategy(NewIslandLocationStrategy strategy) { } /** + * Builds the island. + * * @return Island - * @throws IOException - if there are insufficient parameters, i.e., no user + * @throws IOException if insufficient parameters (e.g., no user) */ public Island build() throws IOException { if (user2 != null) { @@ -170,63 +197,64 @@ public Island build() throws IOException { } /** - * Makes an island. + * Creates a new island for the user. + * Handles location finding, user cleanup, event firing, blueprint pasting, and logging. * * @param oldIsland old island that is being replaced, if any - * @throws IOException - if an island cannot be made. Message is the tag to show - * the user. + * @throws IOException if an island cannot be made. Message is the tag to show the user. */ public void newIsland(Island oldIsland) throws IOException { - // Find the new island location + // Find the new island location, either reserved or next available Location next = checkReservedIsland(); if (next == null) { next = this.makeNextIsland(); } - // Clean up the user + // Clean up user data before moving to new island cleanUpUser(next); - // Fire event + // Fire event for plugins/addons to react or cancel IslandBaseEvent event = IslandEvent.builder().involvedPlayer(user.getUniqueId()).reason(reason).island(island) .location(island.getCenter()) .blueprintBundle(plugin.getBlueprintsManager().getBlueprintBundles(addon).get(name)) .oldIsland(oldIsland).build(); if (event.getNewEvent().map(IslandBaseEvent::isCancelled).orElse(event.isCancelled())) { - // Do nothing + // Event was cancelled, abort creation return; } event = event.getNewEvent().orElse(event); - // Get the new BlueprintBundle if it was changed + // Get the new BlueprintBundle if it was changed by the event switch (reason) { case CREATE -> name = ((IslandCreateEvent) event).getBlueprintBundle().getUniqueId(); case RESET -> name = ((IslandResetEvent) event).getBlueprintBundle().getUniqueId(); default -> { - // Do nothing of other cases + // Do nothing for other cases } } // Set the player's primary island plugin.getIslands().setPrimaryIsland(user.getUniqueId(), island); - // Run task to run after creating the island in one tick if island is not being - // pasted + // Run post-creation tasks after creating the island if (noPaste) { + // If noPaste is true, skip blueprint paste and run post-creation immediately Bukkit.getScheduler().runTask(plugin, () -> postCreationTask(oldIsland)); } else { - // Find out how far away the player is from the new island + // Determine if NMS (native Minecraft server) paste is needed based on player state boolean useNMS = user.isOfflinePlayer() || !user.getWorld().equals(island.getWorld()) || (user.getLocation().distance(island.getCenter()) >= Bukkit.getViewDistance() * 16D); - // Create islands, then run task + // Paste the blueprint, then run post-creation tasks plugin.getBlueprintsManager().paste(addon, island, name, () -> postCreationTask(oldIsland), useNMS); } - // Set default settings + // Set default island flags/settings island.setFlagsDefaults(); - // Register metrics + // Register metrics for island creation plugin.getMetrics().ifPresent(BStats::increaseIslandsCreatedCount); - // Add history record + // Add history record for island creation island.log(new LogEntry.Builder(LogType.JOINED).data(user.getUniqueId().toString(), "owner").build()); - // Save island + // Save island to database IslandsManager.updateIsland(island); } /** - * Tasks to run after the new island has been created + * Tasks to run after the new island has been created. + * Handles spawn point setup, player teleportation, and cleanup. * * @param oldIsland - old island that will be deleted */ @@ -235,38 +263,39 @@ private void postCreationTask(Island oldIsland) { if (island.getSpawnPoint(Environment.NORMAL) != null) { plugin.getIslands().setHomeLocation(user, island.getSpawnPoint(Environment.NORMAL)); } - // Stop the player from falling or moving if they are + // If player is online, handle teleportation and movement if (user.isOnline()) { if (reason.equals(Reason.RESET) || (reason.equals(Reason.CREATE) && plugin.getIWM().isTeleportPlayerToIslandUponIslandCreation(world))) { + // Stop the player from falling or moving user.getPlayer().setVelocity(new Vector(0, 0, 0)); user.getPlayer().setFallDistance(0F); - // Teleport player after this island is built + // Teleport player after island is built, then tidy up plugin.getIslands().homeTeleportAsync(world, user.getPlayer(), true).thenRun(() -> tidyUp(oldIsland)); return; } else { - // let's send him a message so that he knows he can teleport to his island! + // Notify player they can teleport to their island user.sendMessage("commands.island.create.you-can-teleport-to-your-island"); } } else { - // Remove the player again to completely clear the data + // If player is offline, remove player data to clear cache User.removePlayer(user.getPlayer()); } tidyUp(oldIsland); } /** - * Cleans up a user before moving them to a new island. Resets deaths. Checks - * range permissions and saves the player to the database. + * Cleans up a user before moving them to a new island. + * Resets deaths and checks range permissions. * * @param loc - the new island location */ private void cleanUpUser(Location loc) { - // Reset deaths + // Reset deaths if configured if (plugin.getIWM().isDeathsResetOnNewIsland(world)) { plugin.getPlayers().setDeaths(world, user.getUniqueId(), 0); } - // Check if owner has a different range permission than the island size + // Set protection range based on user's permission, if different from default island.setProtectionRange(user.getPermissionValue( plugin.getIWM().getAddon(island.getWorld()).map(GameModeAddon::getPermissionPrefix).orElse("") + "island.range", @@ -274,21 +303,20 @@ private void cleanUpUser(Location loc) { } /** - * Get the next island location and add it to the island grid + * Finds the next available location for a new island and adds it to the grid. * * @return location of new island - * @throws IOException - if there are no unoccupied spots or the island could - * not be added to the grid + * @throws IOException if no unoccupied spots or island cannot be added to grid */ private Location makeNextIsland() throws IOException { - // If the reservation fails, then we need to make a new island anyway - Location next = this.locationStrategy.getNextLocation(world); + // Use location strategy to find next available spot + Location next = this.locationStrategy.getNextLocation(world, user); if (next == null) { plugin.logError("Failed to make island - no unoccupied spot found."); plugin.logError("If the world was imported, try multiple times until all unowned islands are known."); throw new IOException("commands.island.create.cannot-create-island"); } - // Add to the grid + // Add island to grid island = plugin.getIslands().createIsland(next, user.getUniqueId()); if (island == null) { plugin.logError("Failed to make island! Island could not be added to the grid."); @@ -298,13 +326,14 @@ private Location makeNextIsland() throws IOException { } /** - * Get the reserved island location + * Checks if the user has a reserved island location. + * If so, clears the reservation and returns the location. * * @return reserved island location, or null if none found */ private Location checkReservedIsland() { if (plugin.getIslands().hasIsland(world, user)) { - // Island exists, it just needs pasting + // Island exists, just needs pasting island = plugin.getIslands().getIsland(world, user); if (island != null && island.isReserved()) { Location l = island.getCenter(); @@ -316,14 +345,19 @@ private Location checkReservedIsland() { return null; } + /** + * Cleans up after island creation. + * Deletes old island and fires exit event. + * + * @param oldIsland the old island to delete + */ private void tidyUp(Island oldIsland) { - // Delete old island + // Delete old island if present if (oldIsland != null) { - // Delete the old island plugin.getIslands().deleteIsland(oldIsland, true, user.getUniqueId()); } - // Fire exit event + // Fire exit event for plugins/addons IslandEvent.builder().involvedPlayer(user.getUniqueId()) .reason(reason == Reason.RESET ? Reason.RESETTED : Reason.CREATED).island(island) .location(island.getCenter()).oldIsland(oldIsland).build(); diff --git a/src/main/java/world/bentobox/bentobox/managers/island/NewIslandLocationStrategy.java b/src/main/java/world/bentobox/bentobox/managers/island/NewIslandLocationStrategy.java index 462323ac9..18ec6ddc4 100644 --- a/src/main/java/world/bentobox/bentobox/managers/island/NewIslandLocationStrategy.java +++ b/src/main/java/world/bentobox/bentobox/managers/island/NewIslandLocationStrategy.java @@ -3,6 +3,8 @@ import org.bukkit.Location; import org.bukkit.World; +import world.bentobox.bentobox.api.user.User; + /** * Determines the locations for new islands * @author tastybento, leonardochaia @@ -10,5 +12,24 @@ * */ public interface NewIslandLocationStrategy { + + /** + * Provide the next location for an island based on the world + * @param world world + * @return location of island + */ Location getNextLocation(World world); + + /** + * Provide the next location for an island based on the world and the user doing the request. + * This allows the user to have an effect on the location, if required. Default location is the same + * as {@link #getNextLocation(World)} + * @param world - world + * @param user - user + * @return location of island + * @since 3.8.1 + */ + default Location getNextLocation(World world, User user) { + return this.getNextLocation(world); + } } diff --git a/src/main/resources/locales/en-US.yml b/src/main/resources/locales/en-US.yml index 244b46e03..252f6890d 100644 --- a/src/main/resources/locales/en-US.yml +++ b/src/main/resources/locales/en-US.yml @@ -984,8 +984,8 @@ protection: BOAT: name: Boats description: |- - Toggle placing, breaking and - entering into boats. + &a Toggle placing, breaking and + &a entering into boats. hint: No boat interaction allowed BOOKSHELF: name: Bookshelves @@ -999,14 +999,14 @@ protection: hint: Block breaking disabled BREAK_SPAWNERS: description: |- - Toggle spawners breaking. - Overrides the Break Blocks flag. + &a Toggle spawners breaking. + &a Overrides the Break Blocks flag. name: Break spawners hint: Spawner breaking disabled BREAK_HOPPERS: description: |- - Toggle hoppers breaking. - Overrides the Break Blocks flag. + &a Toggle hoppers breaking. + &a Overrides the Break Blocks flag. name: Break hoppers hint: Hoppers breaking disabled BREEDING: