Delete experiment (#1589)

* Try loading adjacent chunks, but too slow.

* NMS void delete

* Revert the extra chunk in IslandDeletion.

That was for another deletion approach.

* Added NMS abstraction.

* Debug

* Clearer code.

* Clarify exceptions

pom.xml

@@ -68,9 +68,10 @@
         <powermock.version>2.0.4</powermock.version>
         <mongodb.version>3.8.0</mongodb.version>
         <!-- More visible way to change dependency versions -->
-        <spigot.version>1.16.2-R0.1-SNAPSHOT</spigot.version>
-        <!-- Might differ from the last Spigot release for short periods of time -->
-        <paper.version>1.16.2-R0.1-SNAPSHOT</paper.version>
+        <spigot.version>1.16.4-R0.1-SNAPSHOT</spigot.version>
+        <!-- Might differ from the last Spigot release for short periods 
+            of time -->
+        <paper.version>1.16.4-R0.1-SNAPSHOT</paper.version>
         <bstats.version>1.7</bstats.version>
         <vault.version>1.7</vault.version>
         <placeholderapi.version>2.10.5</placeholderapi.version>
@@ -291,6 +292,12 @@
             <version>1.0.2</version>
             <scope>compile</scope>
         </dependency>
+        <dependency> <!-- Spigot (this includes Spigot API, Bukkit API, Craftbukkit and NMS) -->
+            <groupId>org.spigotmc</groupId>
+            <artifactId>spigot</artifactId>
+            <version>1.16.4-R0.1-SNAPSHOT</version>
+            <scope>provided</scope>
+        </dependency>
     </dependencies>
 
     <build>
@@ -445,7 +452,7 @@
                 <configuration>
                     <append>true</append>
                     <excludes>
-                        <!-- This is required to prevent Jacoco from adding
+                        <!-- This is required to prevent Jacoco from adding 
                             synthetic fields to a JavaBean class (causes errors in testing) -->
                         <exclude>**/*Names*</exclude>
                     </excludes>

src/main/java/world/bentobox/bentobox/nms/NMSAbstraction.java

@@ -0,0 +1,27 @@
+package world.bentobox.bentobox.nms;
+
+import org.bukkit.Bukkit;
+import org.bukkit.Chunk;
+import org.bukkit.Material;
+import org.bukkit.block.data.BlockData;
+import org.bukkit.craftbukkit.v1_16_R3.block.data.CraftBlockData;
+
+import net.minecraft.server.v1_16_R3.IBlockData;
+
+public interface NMSAbstraction {
+
+    static final IBlockData AIR = ((CraftBlockData) Bukkit.createBlockData(Material.AIR)).getState();
+
+    /**
+     * Update the low-level chunk information for the given block to the new block ID and data.  This
+     * change will not be propagated to clients until the chunk is refreshed to them.
+     * @param chunk - chunk to be changed
+     * @param x - x coordinate within chunk 0 - 15
+     * @param y - y coordinate within chunk 0 - world height, e.g. 255
+     * @param z - z coordinate within chunk 0 - 15
+     * @param blockData - block data to set the block
+     * @param applyPhysics - apply physics or not
+     */
+    public void setBlockInNativeChunk(Chunk chunk, int x, int y, int z, BlockData blockData, boolean applyPhysics);
+
+}

src/main/java/world/bentobox/bentobox/nms/fallback/NMSHandler.java

@@ -0,0 +1,20 @@
+package world.bentobox.bentobox.nms.fallback;
+
+import org.bukkit.Chunk;
+import org.bukkit.block.data.BlockData;
+
+import world.bentobox.bentobox.nms.NMSAbstraction;
+
+/**
+ * @author tastybento
+ *
+ */
+public class NMSHandler implements NMSAbstraction {
+
+    @Override
+    public void setBlockInNativeChunk(Chunk chunk, int x, int y, int z, BlockData blockData, boolean applyPhysics) {
+        chunk.getBlock(x, y, z).setBlockData(blockData, applyPhysics);
+    }
+
+
+}

src/main/java/world/bentobox/bentobox/nms/v1_16_R3/NMSHandler.java

@@ -0,0 +1,25 @@
+package world.bentobox.bentobox.nms.v1_16_R3;
+
+import org.bukkit.Chunk;
+import org.bukkit.block.data.BlockData;
+import org.bukkit.craftbukkit.v1_16_R3.CraftWorld;
+import org.bukkit.craftbukkit.v1_16_R3.block.data.CraftBlockData;
+
+import net.minecraft.server.v1_16_R3.BlockPosition;
+import world.bentobox.bentobox.nms.NMSAbstraction;
+
+public class NMSHandler implements NMSAbstraction {
+
+    @Override
+    public void setBlockInNativeChunk(Chunk chunk, int x, int y, int z, BlockData blockData, boolean applyPhysics) {
+        CraftBlockData craft = (CraftBlockData) blockData;
+        net.minecraft.server.v1_16_R3.World nmsWorld = ((CraftWorld) chunk.getWorld()).getHandle();
+        net.minecraft.server.v1_16_R3.Chunk nmsChunk = nmsWorld.getChunkAt(chunk.getX(), chunk.getZ());
+        BlockPosition bp = new BlockPosition((chunk.getX() << 4) + x, y, (chunk.getZ() << 4) + z);
+        // Setting the block to air before setting to another state prevents some console errors
+        nmsChunk.setType(bp, AIR, applyPhysics, true);
+        nmsChunk.setType(bp, craft.getState(), applyPhysics, true);
+    }
+
+
+}

src/main/java/world/bentobox/bentobox/util/DeleteIslandChunks.java

@@ -2,11 +2,12 @@ package world.bentobox.bentobox.util;
 
 import java.util.Arrays;
 import java.util.Random;
+import java.util.concurrent.CompletableFuture;
 
 import org.bukkit.Bukkit;
 import org.bukkit.Chunk;
-import org.bukkit.Material;
 import org.bukkit.World;
+import org.bukkit.World.Environment;
 import org.bukkit.entity.Entity;
 import org.bukkit.entity.Player;
 import org.bukkit.generator.ChunkGenerator;
@@ -20,6 +21,7 @@ import world.bentobox.bentobox.api.addons.GameModeAddon;
 import world.bentobox.bentobox.api.events.island.IslandEvent;
 import world.bentobox.bentobox.api.events.island.IslandEvent.Reason;
 import world.bentobox.bentobox.database.objects.IslandDeletion;
+import world.bentobox.bentobox.nms.NMSAbstraction;
 
 /**
  * Deletes islands chunk by chunk
@@ -33,55 +35,89 @@ public class DeleteIslandChunks {
     private BukkitTask task;
     private IslandDeletion di;
     private boolean inDelete;
+    private BentoBox plugin;
+    private NMSAbstraction nms;
 
     public DeleteIslandChunks(BentoBox plugin, IslandDeletion di) {
-        // Fire event
-        IslandEvent.builder().deletedIslandInfo(di).reason(Reason.DELETE_CHUNKS).build();
-
+        this.plugin = plugin;
         this.chunkX = di.getMinXChunk();
         this.chunkZ = di.getMinZChunk();
         this.di = di;
+        try {
+            this.nms = Util.getNMS();
+        } catch (Exception e) {
+            plugin.logError("Could not delete chunks because of NMS error");
+            return;
+        }
+        // Fire event
+        IslandEvent.builder().deletedIslandInfo(di).reason(Reason.DELETE_CHUNKS).build();
+        regenerateChunks();
+
+    }
+
+    private void regenerateChunks() {
         // Run through all chunks of the islands and regenerate them.
         task = Bukkit.getScheduler().runTaskTimer(plugin, () -> {
             if (inDelete) return;
             inDelete = true;
             for (int i = 0; i < plugin.getSettings().getDeleteSpeed(); i++) {
-                plugin.getIWM().getAddon(di.getWorld()).ifPresent(gm -> {
-                    // Overworld
-                    processChunk(gm, di.getWorld(), chunkX, chunkZ);
-                    // Nether
-                    if (plugin.getIWM().isNetherGenerate(di.getWorld()) && plugin.getIWM().isNetherIslands(di.getWorld())) {
-                        processChunk(gm, plugin.getIWM().getNetherWorld(di.getWorld()), chunkX, chunkZ);
-                    }
-                    // End
-                    if (plugin.getIWM().isEndGenerate(di.getWorld()) && plugin.getIWM().isEndIslands(di.getWorld())) {
-                        processChunk(gm, plugin.getIWM().getEndWorld(di.getWorld()), chunkX, chunkZ);
-                    }
-                    chunkZ++;
-                    if (chunkZ > di.getMaxZChunk()) {
-                        chunkZ = di.getMinZChunk();
-                        chunkX++;
-                        if (chunkX > di.getMaxXChunk()) {
-                            // We're done
-                            task.cancel();
-                            // Fire event
-                            IslandEvent.builder().deletedIslandInfo(di).reason(Reason.DELETED).build();
-                        }
-                    }
-                });
+                plugin.getIWM().getAddon(di.getWorld()).ifPresent(gm ->
+                // Overworld
+                processChunk(gm, Environment.NORMAL, chunkX, chunkZ).thenRun(() ->
+                // Nether
+                processChunk(gm, Environment.NETHER, chunkX, chunkZ).thenRun(() ->
+                // End
+                processChunk(gm, Environment.NETHER, chunkX, chunkZ).thenRun(() -> finish()))));
             }
-            inDelete = false;
         }, 0L, 20L);
+
     }
 
-    private void processChunk(GameModeAddon gm, World world, int x, int z) {
-        if (PaperLib.isChunkGenerated(world, x, z)) {
-            PaperLib.getChunkAtAsync(world, x, z).thenAccept(chunk -> regenerateChunk(gm, chunk));
+    private void finish() {
+        chunkZ++;
+        if (chunkZ > di.getMaxZChunk()) {
+            chunkZ = di.getMinZChunk();
+            chunkX++;
+            if (chunkX > di.getMaxXChunk()) {
+                // We're done
+                task.cancel();
+                // Fire event
+                IslandEvent.builder().deletedIslandInfo(di).reason(Reason.DELETED).build();
+            }
         }
     }
 
+    private CompletableFuture<Boolean> processChunk(GameModeAddon gm, Environment env, int x, int z) {
+        World world = di.getWorld();
+        switch (env) {
+        case NETHER:
+            // Nether
+            if (plugin.getIWM().isNetherGenerate(di.getWorld()) && plugin.getIWM().isNetherIslands(di.getWorld())) {
+                world = plugin.getIWM().getNetherWorld(di.getWorld());
+            } else {
+                return CompletableFuture.completedFuture(false);
+            }
+            break;
+        case THE_END:
+            // End
+            if (plugin.getIWM().isEndGenerate(di.getWorld()) && plugin.getIWM().isEndIslands(di.getWorld())) {
+                world = plugin.getIWM().getEndWorld(di.getWorld());
+            } else {
+                return CompletableFuture.completedFuture(false);
+            }
+            break;
+        default:
+            break;
+        }
+        if (PaperLib.isChunkGenerated(world, x, z)) {
+            PaperLib.getChunkAtAsync(world, x, z).thenAccept(chunk ->regenerateChunk(gm, chunk));
+
+            return CompletableFuture.completedFuture(true);
+        }
+        return CompletableFuture.completedFuture(false);
+    }
+
     private void regenerateChunk(GameModeAddon gm, Chunk chunk) {
-        boolean isLoaded = chunk.isLoaded();
         // Clear all inventories
         Arrays.stream(chunk.getTileEntities()).filter(te -> (te instanceof InventoryHolder))
         .filter(te -> di.inBounds(te.getLocation().getBlockX(), te.getLocation().getBlockZ()))
@@ -91,21 +127,25 @@ public class DeleteIslandChunks {
         ChunkGenerator cg = gm.getDefaultWorldGenerator(chunk.getWorld().getName(), "");
         // Will be null if use-own-generator is set to true
         if (cg != null) {
-            ChunkData cd = cg.generateChunkData(chunk.getWorld(), new Random(), chunk.getX(), chunk.getZ(), grid);
-            int baseX = chunk.getX() << 4;
-            int baseZ = chunk.getZ() << 4;
-            for (int x = 0; x < 16; x++) {
-                for (int z = 0; z < 16; z++) {
-                    if (di.inBounds(baseX + x, baseZ + z)) {                        
-                        for (int y = 0; y < chunk.getWorld().getMaxHeight(); y++) {
-                            // Note: setting block to air before setting it to something else stops a bug in the server
-                            // where it reports a "
-                            chunk.getBlock(x, y, z).setType(Material.AIR, false);
-                            chunk.getBlock(x, y, z).setBlockData(cd.getBlockData(x, y, z), false);
-                            // 3D biomes, 4 blocks separated
-                            if (x%4 == 0 && y%4 == 0 && z%4 == 0) {
-                                chunk.getBlock(x, y, z).setBiome(grid.getBiome(x, y, z));
-                            }
+            Bukkit.getScheduler().runTaskAsynchronously(plugin, () -> {
+                ChunkData cd = cg.generateChunkData(chunk.getWorld(), new Random(), chunk.getX(), chunk.getZ(), grid);
+                Bukkit.getScheduler().runTask(plugin, () -> createChunk(cd, chunk, grid));
+            });
+        }
+
+    }
+
+    private void createChunk(ChunkData cd, Chunk chunk, MyBiomeGrid grid) {
+        int baseX = chunk.getX() << 4;
+        int baseZ = chunk.getZ() << 4;
+        for (int x = 0; x < 16; x++) {
+            for (int z = 0; z < 16; z++) {
+                if (di.inBounds(baseX + x, baseZ + z)) {
+                    for (int y = 0; y < chunk.getWorld().getMaxHeight(); y++) {
+                        nms.setBlockInNativeChunk(chunk, x, y, z, cd.getBlockData(x, y, z), false);
+                        // 3D biomes, 4 blocks separated
+                        if (x%4 == 0 && y%4 == 0 && z%4 == 0) {
+                            chunk.getBlock(x, y, z).setBiome(grid.getBiome(x, y, z));
                         }
                     }
                 }
@@ -113,8 +153,6 @@ public class DeleteIslandChunks {
         }
         // Remove all entities in chunk, including any dropped items as a result of clearing the blocks above
         Arrays.stream(chunk.getEntities()).filter(e -> !(e instanceof Player) && di.inBounds(e.getLocation().getBlockX(), e.getLocation().getBlockZ())).forEach(Entity::remove);
-        if (!isLoaded) {
-            chunk.unload(true);
-        }
+        inDelete = false;
     }
 }

src/main/java/world/bentobox/bentobox/util/Util.java

@@ -1,5 +1,6 @@
 package world.bentobox.bentobox.util;
 
+import java.lang.reflect.InvocationTargetException;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
@@ -44,6 +45,7 @@ import io.papermc.lib.PaperLib;
 import io.papermc.lib.features.blockstatesnapshot.BlockStateSnapshotResult;
 import world.bentobox.bentobox.BentoBox;
 import world.bentobox.bentobox.api.user.User;
+import world.bentobox.bentobox.nms.NMSAbstraction;
 
 /**
  * A set of utility methods
@@ -656,4 +658,35 @@ public class Util {
         double maxHealth = player.getAttribute(Attribute.GENERIC_MAX_HEALTH).getBaseValue();
         player.setHealth(maxHealth);
     }
+
+    /**
+     * Checks what version the server is running and picks the appropriate NMS handler, or fallback
+     * @return an NMS accelerated class for this server, or a fallback Bukkit API based one
+     * @throws ClassNotFoundException - thrown if there is no fallback class - should never be thrown
+     * @throws SecurityException - thrown if security violation - should never be thrown
+     * @throws NoSuchMethodException - thrown if no constructor for NMS package
+     * @throws InvocationTargetException - should never be thrown
+     * @throws IllegalArgumentException - should never be thrown
+     * @throws IllegalAccessException - should never be thrown
+     * @throws InstantiationException - should never be thrown
+     */
+    public static NMSAbstraction getNMS() throws ClassNotFoundException, InstantiationException, IllegalAccessException, IllegalArgumentException, InvocationTargetException, NoSuchMethodException, SecurityException {
+        String serverPackageName = Bukkit.getServer().getClass().getPackage().getName();
+        String pluginPackageName = plugin.getClass().getPackage().getName();
+        String version = serverPackageName.substring(serverPackageName.lastIndexOf('.') + 1);
+        Class<?> clazz;
+        try {
+            clazz = Class.forName(pluginPackageName + ".nms." + version + ".NMSHandler");
+        } catch (Exception e) {
+            plugin.logWarning("No NMS Handler found for " + version + ", falling back to Bukkit API.");
+            clazz = Class.forName(pluginPackageName + ".nms.fallback.NMSHandler");
+        }
+        // Check if we have a NMSAbstraction implementing class at that location.
+        if (NMSAbstraction.class.isAssignableFrom(clazz)) {
+            return (NMSAbstraction) clazz.getConstructor().newInstance();
+        } else {
+            throw new IllegalStateException("Class " + clazz.getName() + " does not implement NMSAbstraction");
+        }
+    }
+
 }

src/main/resources/config.yml

@@ -157,7 +157,7 @@ island:
   # Smaller values will help reduce noticeable lag but will make deleting take longer.
   # A setting of 0 will leave island blocks (not recommended).
   # Added since 1.7.0.
-  delete-speed: 1
+  delete-speed: 100
   deletion:
     # Toggles whether islands, when players are resetting them, should be kept in the world or deleted.
     # * If set to 'true', whenever a player resets his island, his previous island will become unowned and won't be deleted from the world.