Merge pull request #2743 from BentoBoxWorld/develop

Release 3.8.1

Author: tastybento

pom.xml

@@ -75,7 +75,7 @@
         <!-- Do not change unless you want different name for local builds. -->
         <build.number>-LOCAL</build.number>
         <!-- This allows to change between versions. -->
-        <build.version>3.8.0</build.version>
+        <build.version>3.8.1</build.version>
         <sonar.organization>bentobox-world</sonar.organization>
         <sonar.host.url>https://sonarcloud.io</sonar.host.url>
         <server.jars>${project.basedir}/lib</server.jars>

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<Vector> vectorsToCopy, int s
                 return;
             }
             copying = true;
+            NamespacedKey key = new NamespacedKey(BentoBox.getInstance(), "associatedDisplayEntity");
             vectorsToCopy.stream().skip(index).limit(speed).forEach(v -> {
                 List<Entity> ents = world.getEntities().stream()
                         .filter(Objects::nonNull)

src/main/java/world/bentobox/bentobox/managers/island/DefaultNewIslandLocationStrategy.java

@@ -15,136 +15,192 @@
 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.
+ * <p>
+ * If you wish to create an alternative strategy, you must implement the {@link NewIslandLocationStrategy} interface.
+ * Your implementation should be robust and consider the following:
+ * <ul>
+ *     <li><b>Performance:</b> Finding a location can be a frequent operation. Your algorithm should be efficient.</li>
+ *     <li><b>Non-empty worlds:</b> The world may already contain structures. Your strategy should be able to
+ *     navigate around these or register them as "occupied" to avoid placing islands on top of them. This implementation
+ *     uses a tolerance limit ({@link #MAX_UNOWNED_ISLANDS}) to prevent infinite loops in heavily modified worlds.</li>
+ *     <li><b>Island distance:</b> Respect the island distance setting from the configuration to prevent islands from overlapping.</li>
+ *     <li><b>Concurrency:</b> While island creation is typically synchronized, consider any potential race conditions if your
+ *     strategy involves asynchronous operations.</li>
+ * </ul>
+ * The methods in this class, like {@link #isIsland(Location)}, can be useful helpers for custom implementations.
+ * </p>
+ *
  * @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, Integer> 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<Location> 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));
         locs.add(new Location(world, location.getX() + dist - 1, 0, location.getZ() + dist - 1));
 
         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;
     }