Finished fix for #2636 and fixed a threading issue with BlockStates

CHANGELOG.md

@@ -34,6 +34,8 @@
 * Fixed elevator floor order
 * Fixed #2449
 * Fixed #2511
+* Fixed #2636
+* Fixed a threading issue related to BlockStates and persistent data
 
 ## Release Candidate 19 (11 Jan 2021)
 

src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/BlockListener.java

@@ -10,6 +10,7 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.annotation.ParametersAreNonnullByDefault;
 
+import org.bukkit.Bukkit;
 import org.bukkit.Material;
 import org.bukkit.block.Block;
 import org.bukkit.block.BlockFace;
@@ -50,7 +51,10 @@ import me.mrCookieSlime.Slimefun.api.Slimefun;
  */
 public class BlockListener implements Listener {
 
+    private final SlimefunPlugin plugin;
+
     public BlockListener(@Nonnull SlimefunPlugin plugin) {
+        this.plugin = plugin;
         plugin.getServer().getPluginManager().registerEvents(this, plugin);
     }
 
@@ -63,19 +67,26 @@ public class BlockListener implements Listener {
          */
         Block block = e.getBlock();
 
+        // Fixes #2636
         if (e.getBlockReplacedState().getType().isAir()) {
             SlimefunItem sfItem = BlockStorage.check(block);
 
             if (sfItem != null) {
                 /*
-                 * Temp fix for #2636
-                 * for (ItemStack item : sfItem.getDrops()) {
-                 * if (item != null && !item.getType().isAir()) {
-                 * block.getWorld().dropItemNaturally(block.getLocation(), item);
-                 * }
-                 * }
+                 * We can move the TickerTask synchronization to an async task to
+                 * avoid blocking the main Thread here.
                  */
-                BlockStorage.clearBlockInfo(block);
+                Bukkit.getScheduler().runTaskAsynchronously(plugin, () -> {
+                    if (!SlimefunPlugin.getTickerTask().isDeletedSoon(block.getLocation())) {
+                        for (ItemStack item : sfItem.getDrops()) {
+                            if (item != null && !item.getType().isAir()) {
+                                SlimefunPlugin.runSync(() -> block.getWorld().dropItemNaturally(block.getLocation(), item));
+                            }
+                        }
+
+                        BlockStorage.clearBlockInfo(block);
+                    }
+                });
             }
         } else if (BlockStorage.hasBlockInfo(e.getBlock())) {
             e.setCancelled(true);
@@ -119,7 +130,7 @@ public class BlockListener implements Listener {
         int fortune = getBonusDropsWithFortune(item, e.getBlock());
         List<ItemStack> drops = new ArrayList<>();
 
-        if (item.getType() != Material.AIR) {
+        if (!item.getType().isAir()) {
             callToolHandler(e, item, fortune, drops);
         }
 
@@ -159,9 +170,13 @@ public class BlockListener implements Listener {
             SlimefunBlockHandler blockHandler = SlimefunPlugin.getRegistry().getBlockHandlers().get(sfItem.getId());
 
             if (blockHandler != null) {
-                if (!blockHandler.onBreak(e.getPlayer(), e.getBlock(), sfItem, UnregisterReason.PLAYER_BREAK)) {
-                    e.setCancelled(true);
-                    return;
+                try {
+                    if (!blockHandler.onBreak(e.getPlayer(), e.getBlock(), sfItem, UnregisterReason.PLAYER_BREAK)) {
+                        e.setCancelled(true);
+                        return;
+                    }
+                } catch (Exception | LinkageError x) {
+                    sfItem.error("Something went wrong while triggering a BlockHandler", x);
                 }
             } else {
                 sfItem.callItemHandler(BlockBreakHandler.class, handler -> handler.onBlockBreak(e, item, fortune, drops));

src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/BlockPhysicsListener.java

@@ -94,7 +94,7 @@ public class BlockPhysicsListener implements Listener {
                 Location loc = block.getLocation();
 
                 // Fixes #2496 - Make sure it is not a moving block
-                if (SlimefunPlugin.getTickerTask().isReserved(loc)) {
+                if (SlimefunPlugin.getTickerTask().isOccupiedSoon(loc)) {
                     e.setCancelled(true);
                 }
             }

src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/NetherIceResource.java

@@ -3,8 +3,16 @@ package io.github.thebusybiscuit.slimefun4.implementation.resources;
 import org.bukkit.World.Environment;
 import org.bukkit.block.Biome;
 
+import io.github.thebusybiscuit.slimefun4.api.geo.GEOResource;
 import io.github.thebusybiscuit.slimefun4.implementation.SlimefunItems;
 
+/**
+ * A {@link GEOResource} which consists of nether ice.
+ * It can only be found in the nether.
+ * 
+ * @author TheBusyBiscuit
+ *
+ */
 class NetherIceResource extends SlimefunResource {
 
     NetherIceResource() {

src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/OilResource.java

@@ -3,8 +3,21 @@ package io.github.thebusybiscuit.slimefun4.implementation.resources;
 import org.bukkit.World.Environment;
 import org.bukkit.block.Biome;
 
+import io.github.thebusybiscuit.slimefun4.api.geo.GEOResource;
 import io.github.thebusybiscuit.slimefun4.implementation.SlimefunItems;
+import io.github.thebusybiscuit.slimefun4.implementation.items.geo.GEOMiner;
+import io.github.thebusybiscuit.slimefun4.implementation.items.geo.OilPump;
 
+/**
+ * A {@link GEOResource} which consists of buckets of Oil.
+ * It cannot be obtained via a {@link GEOMiner} but instead requires
+ * and {@link OilPump}.
+ * 
+ * @author TheBusyBiscuit
+ *
+ * @see OilPump
+ *
+ */
 class OilResource extends SlimefunResource {
 
     OilResource() {

src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/SaltResource.java

@@ -3,8 +3,15 @@ package io.github.thebusybiscuit.slimefun4.implementation.resources;
 import org.bukkit.World.Environment;
 import org.bukkit.block.Biome;
 
+import io.github.thebusybiscuit.slimefun4.api.geo.GEOResource;
 import io.github.thebusybiscuit.slimefun4.implementation.SlimefunItems;
 
+/**
+ * A {@link GEOResource} which consists of Salt.
+ * 
+ * @author TheBusyBiscuit
+ *
+ */
 class SaltResource extends SlimefunResource {
 
     SaltResource() {

src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/SlimefunResource.java

@@ -10,6 +10,17 @@ import org.bukkit.inventory.ItemStack;
 import io.github.thebusybiscuit.slimefun4.api.geo.GEOResource;
 import io.github.thebusybiscuit.slimefun4.implementation.SlimefunPlugin;
 
+/**
+ * This is an abstract parent class for any {@link GEOResource}
+ * that is added by Slimefun itself. It is package-private, therefore
+ * only classes inside this package can access it.
+ * <p>
+ * It just provides a bit of a convenience for us to reduce redundancies
+ * in our {@link GEOResource} implementations.
+ * 
+ * @author TheBusyBiscuit
+ *
+ */
 abstract class SlimefunResource implements GEOResource {
 
     private final NamespacedKey key;

src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/UraniumResource.java

@@ -3,8 +3,15 @@ package io.github.thebusybiscuit.slimefun4.implementation.resources;
 import org.bukkit.World.Environment;
 import org.bukkit.block.Biome;
 
+import io.github.thebusybiscuit.slimefun4.api.geo.GEOResource;
 import io.github.thebusybiscuit.slimefun4.implementation.SlimefunItems;
 
+/**
+ * A {@link GEOResource} which consists of small chunks of Uranium.
+ * 
+ * @author TheBusyBiscuit
+ *
+ */
 class UraniumResource extends SlimefunResource {
 
     UraniumResource() {

src/main/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/TickerTask.java

@@ -252,10 +252,26 @@ public class TickerTask implements Runnable {
      * 
      * @return Whether this {@link Location} has been reserved and will be filled upon the next tick
      */
-    public boolean isReserved(@Nonnull Location l) {
+    public boolean isOccupiedSoon(@Nonnull Location l) {
+        Validate.notNull(l, "Null is not a valid Location!");
+
         return movingQueue.containsValue(l);
     }
 
+    /**
+     * This method checks if a given {@link Location} will be deleted on the next tick.
+     * 
+     * @param l
+     *            The {@link Location} to check
+     * 
+     * @return Whether this {@link Location} will be deleted on the next tick
+     */
+    public boolean isDeletedSoon(@Nonnull Location l) {
+        Validate.notNull(l, "Null is not a valid Location!");
+
+        return deletionQueue.containsKey(l);
+    }
+
     /**
      * This returns the delay between ticks
      * 

src/main/java/me/mrCookieSlime/Slimefun/Objects/SlimefunItem/SlimefunItem.java

@@ -151,7 +151,7 @@ public class SlimefunItem implements Placeable {
      *            The result of crafting this item
      */
     @ParametersAreNonnullByDefault
-    public SlimefunItem(Category category, SlimefunItemStack item, RecipeType recipeType, ItemStack[] recipe, ItemStack recipeOutput) {
+    public SlimefunItem(Category category, SlimefunItemStack item, RecipeType recipeType, ItemStack[] recipe, @Nullable ItemStack recipeOutput) {
         Validate.notNull(category, "'category' is not allowed to be null!");
         Validate.notNull(item, "'item' is not allowed to be null!");
         Validate.notNull(recipeType, "'recipeType' is not allowed to be null!");
@@ -263,10 +263,21 @@ public class SlimefunItem implements Placeable {
      * @return The linked {@link Research} or null
      */
     @Nullable
-    public Research getResearch() {
+    public final Research getResearch() {
         return research;
     }
 
+    /**
+     * This returns whether this {@link SlimefunItem} has a {@link Research}
+     * assigned to it.
+     * It is equivalent to a null check performed on {@link #getResearch()}.
+     * 
+     * @return Whether this {@link SlimefunItem} has a {@link Research}
+     */
+    public final boolean hasResearch() {
+        return research != null;
+    }
+
     /**
      * This returns a {@link Set} containing all instances of {@link ItemSetting} for this {@link SlimefunItem}.
      * 
@@ -490,7 +501,7 @@ public class SlimefunItem implements Placeable {
         if (itemStackTemplate.getAmount() != 1) {
             // @formatter:off
             warn("This item has an illegal stack size: " + itemStackTemplate.getAmount()
-                + "An Item size of 1 is recommended. Please inform the autho of " + addon.getName()
+                + ". An Item size of 1 is recommended. Please inform the author(s) of " + addon.getName()
                 + " to fix this. Crafting Results with amounts of higher should be handled"
                 + " via the recipeOutput parameter!");
             // @formatter:on

src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java

@@ -673,7 +673,8 @@ public class BlockStorage {
 
     @Nullable
     public static String checkID(@Nonnull Block b) {
-        if (SlimefunPlugin.getBlockDataService().isTileEntity(b.getType())) {
+        // Only access the BlockState when on the main thread
+        if (Bukkit.isPrimaryThread() && SlimefunPlugin.getBlockDataService().isTileEntity(b.getType())) {
             Optional<String> blockData = SlimefunPlugin.getBlockDataService().getBlockData(b);
 
             if (blockData.isPresent()) {