diff --git a/CHANGELOG.md b/CHANGELOG.md index 078db1883..e7be2c525 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/BlockListener.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/BlockListener.java index b14d02d8e..82a322bee 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/BlockListener.java +++ b/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 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)); diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/BlockPhysicsListener.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/BlockPhysicsListener.java index fd4bbd44e..bd3b8f760 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/BlockPhysicsListener.java +++ b/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); } } diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/NetherIceResource.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/NetherIceResource.java index eef7c7e85..3d7e79608 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/NetherIceResource.java +++ b/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() { diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/OilResource.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/OilResource.java index 551af133b..c45cc768d 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/OilResource.java +++ b/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() { diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/SaltResource.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/SaltResource.java index a75ad23c5..532bfa339 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/SaltResource.java +++ b/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() { diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/SlimefunResource.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/SlimefunResource.java index fd10467fb..03953e0ac 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/SlimefunResource.java +++ b/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. + *

+ * 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; diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/UraniumResource.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/UraniumResource.java index 3b7f3584f..831d8ffdd 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/resources/UraniumResource.java +++ b/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() { diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/TickerTask.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/TickerTask.java index 5b86f5e55..aee8512fe 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/TickerTask.java +++ b/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 * diff --git a/src/main/java/me/mrCookieSlime/Slimefun/Objects/SlimefunItem/SlimefunItem.java b/src/main/java/me/mrCookieSlime/Slimefun/Objects/SlimefunItem/SlimefunItem.java index 6c82dd51a..5cca61a0d 100644 --- a/src/main/java/me/mrCookieSlime/Slimefun/Objects/SlimefunItem/SlimefunItem.java +++ b/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 diff --git a/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java b/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java index 07a3a1a70..8870e9534 100644 --- a/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java +++ b/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 blockData = SlimefunPlugin.getBlockDataService().getBlockData(b); if (blockData.isPresent()) {