From 4ab638814f7f565699bc5fcd8e0355ca55cb1803 Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Sat, 6 Jan 2024 15:04:13 +0000 Subject: [PATCH 01/20] Update dough (#4079) --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 8fa8a7be1d..65fd79d941 100644 --- a/pom.xml +++ b/pom.xml @@ -355,7 +355,7 @@ com.github.baked-libs.dough dough-api - da42c2f268 + c4231a4d1a compile From 158c6eea211cb608d9ae5b4f2d7ea7ee45c81548 Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Sun, 7 Jan 2024 09:19:51 +0000 Subject: [PATCH 02/20] Fix backpack IDs not incrementing (#4081) --- .../slimefun4/api/player/PlayerProfile.java | 8 +++++--- .../api/profiles/TestPlayerBackpacks.java | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java index c51888a1d0..96290f4849 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java @@ -91,7 +91,10 @@ protected PlayerProfile(@Nonnull OfflinePlayer p, PlayerData data) { * Only intended for internal usage. * * @return The {@link Config} associated with this {@link PlayerProfile} + * + * @deprecated Look at {@link PlayerProfile#getPlayerData()} instead for reading data. */ + @Deprecated public @Nonnull Config getConfig() { return configFile; } @@ -245,10 +248,9 @@ public final void markDirty() { } public @Nonnull PlayerBackpack createBackpack(int size) { - IntStream stream = IntStream.iterate(0, i -> i + 1).filter(i -> !configFile.contains("backpacks." + i + ".size")); - int id = stream.findFirst().getAsInt(); + int nextId = this.data.getBackpacks().size(); // Size is not 0 indexed so next ID can just be the current size - PlayerBackpack backpack = PlayerBackpack.newBackpack(this.ownerId, id, size); + PlayerBackpack backpack = PlayerBackpack.newBackpack(this.ownerId, nextId, size); this.data.addBackpack(backpack); markDirty(); diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/api/profiles/TestPlayerBackpacks.java b/src/test/java/io/github/thebusybiscuit/slimefun4/api/profiles/TestPlayerBackpacks.java index 07f69761e0..90d475d479 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/api/profiles/TestPlayerBackpacks.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/api/profiles/TestPlayerBackpacks.java @@ -47,6 +47,21 @@ void testCreateBackpack() throws InterruptedException { Assertions.assertEquals(18, backpack.getInventory().getSize()); } + @Test + @DisplayName("Test creating a new backpack will increment the id") + void testCreateBackpackIncrementsId() throws InterruptedException { + Player player = server.addPlayer(); + PlayerProfile profile = TestUtilities.awaitProfile(player); + + PlayerBackpack backpackOne = profile.createBackpack(18); + PlayerBackpack backpackTwo = profile.createBackpack(18); + PlayerBackpack backpackThree = profile.createBackpack(18); + + Assertions.assertEquals(0, backpackOne.getId()); + Assertions.assertEquals(1, backpackTwo.getId()); + Assertions.assertEquals(2, backpackThree.getId()); + } + @Test @DisplayName("Test upgrading the backpack size") void testChangeSize() throws InterruptedException { From 92e6dc1d17f9479195cf24bff199715af0ad964b Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Thu, 11 Jan 2024 16:57:41 +0000 Subject: [PATCH 03/20] Cleanup disabled sensitive block code --- .../slimefun4/implementation/listeners/BlockListener.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) 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 c214c33a2c..5fdc720076 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 @@ -178,7 +178,7 @@ public void onBlockBreak(BlockBreakEvent e) { dropItems(e, drops); // Checks for vanilla sensitive blocks everywhere - checkForSensitiveBlocks(e.getBlock(), 0, e.isDropItems()); + // checkForSensitiveBlocks(e.getBlock(), 0, e.isDropItems()); } } @@ -306,8 +306,7 @@ private void checkForSensitiveBlockAbove(Player player, Block block, ItemStack i // Disabled for now due to #4069 - Servers crashing due to this check // There is additionally a second bug with `getMaxChainedNeighborUpdates` not existing in 1.17 @ParametersAreNonnullByDefault - private void checkForSensitiveBlocks(Block block, Integer count, boolean isDropItems) { - /* + private void checkForSensitiveBlocks(Block block, int count, boolean isDropItems) { if (count >= Bukkit.getServer().getMaxChainedNeighborUpdates()) { return; } @@ -329,7 +328,6 @@ private void checkForSensitiveBlocks(Block block, Integer count, boolean isDropI // Set the BlockData back: this makes it so containers and spawners drop correctly. This is a hacky fix. block.setBlockData(state.getBlockData(), false); state.update(true, false); - */ } /** From 81bc94226c87de0e8a0bb144a5a3ee7b98a6fb6f Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Thu, 11 Jan 2024 16:58:05 +0000 Subject: [PATCH 04/20] Move nonnull annotations to stop causing errors in vscode --- .../thebusybiscuit/slimefun4/utils/biomes/BiomeMap.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/utils/biomes/BiomeMap.java b/src/main/java/io/github/thebusybiscuit/slimefun4/utils/biomes/BiomeMap.java index 415e16aefd..872972c687 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/utils/biomes/BiomeMap.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/utils/biomes/BiomeMap.java @@ -140,16 +140,18 @@ public String toString() { return "BiomeMap " + dataMap.toString(); } + @Nonnull @ParametersAreNonnullByDefault - public static @Nonnull BiomeMap fromJson(NamespacedKey key, String json, BiomeDataConverter valueConverter) throws BiomeMapException { + public static BiomeMap fromJson(NamespacedKey key, String json, BiomeDataConverter valueConverter) throws BiomeMapException { // All parameters are validated by the Parser. BiomeMapParser parser = new BiomeMapParser<>(key, valueConverter); parser.read(json); return parser.buildBiomeMap(); } + @Nonnull @ParametersAreNonnullByDefault - public static @Nonnull BiomeMap fromJson(NamespacedKey key, String json, BiomeDataConverter valueConverter, boolean isLenient) throws BiomeMapException { + public static BiomeMap fromJson(NamespacedKey key, String json, BiomeDataConverter valueConverter, boolean isLenient) throws BiomeMapException { // All parameters are validated by the Parser. BiomeMapParser parser = new BiomeMapParser<>(key, valueConverter); parser.setLenient(isLenient); @@ -157,8 +159,9 @@ public String toString() { return parser.buildBiomeMap(); } + @Nonnull @ParametersAreNonnullByDefault - public static @Nonnull BiomeMap fromResource(NamespacedKey key, JavaPlugin plugin, String path, BiomeDataConverter valueConverter) throws BiomeMapException { + public static BiomeMap fromResource(NamespacedKey key, JavaPlugin plugin, String path, BiomeDataConverter valueConverter) throws BiomeMapException { Validate.notNull(key, "The key shall not be null."); Validate.notNull(plugin, "The plugin shall not be null."); Validate.notNull(path, "The path should not be null!"); From a78cba0711ae4613ba54979a0e87a5a270a3700a Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Sat, 13 Jan 2024 04:38:06 +0000 Subject: [PATCH 05/20] Fixes #4087 When a block is broken we don't want to allow users to still use the inventory. This allows for them to pull items out along with blocks being dropped (if the block implements that). This fix is in 2 parts, the main part is just preventing the inventory from being opened if the block is queued for deletion. The second smaller part is just closing the inventory for all viewers on break. We would do this during cleanup but that was a tick later, we want it done in this tick and to prevent opening before the cleaning up is ran. --- .../listeners/BlockListener.java | 9 ++ .../SlimefunItemInteractListener.java | 7 + .../interfaces/InventoryBlock.java | 6 +- .../TestSlimefunItemInteractListener.java | 137 ++++++++++++++++++ .../slimefun4/test/TestUtilities.java | 32 ++++ 5 files changed, 190 insertions(+), 1 deletion(-) create mode 100644 src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunItemInteractListener.java 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 5fdc720076..1d7a75df25 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 @@ -18,6 +18,7 @@ import org.bukkit.block.BlockState; import org.bukkit.block.data.BlockData; import org.bukkit.enchantments.Enchantment; +import org.bukkit.entity.HumanEntity; import org.bukkit.entity.Player; import org.bukkit.event.EventHandler; import org.bukkit.event.EventPriority; @@ -220,6 +221,14 @@ private void callBlockHandler(BlockBreakEvent e, ItemStack item, List } drops.addAll(sfItem.getDrops()); + // Partial fix for #4087 - We don't want the inventory to be usable post break, close it for anyone still inside + // The main fix is in SlimefunItemInteractListener preventing opening to begin with + // Close the inventory for all viewers of this block + // TODO(future): Remove this check when MockBukkit supports viewers + if (!Slimefun.instance().isUnitTest()) { + BlockStorage.getInventory(e.getBlock()).toInventory().getViewers().forEach(HumanEntity::closeInventory); + } + // Remove the block data BlockStorage.clearBlockInfo(e.getBlock()); } } diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/SlimefunItemInteractListener.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/SlimefunItemInteractListener.java index a9931b2cc6..195f6ac0c5 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/SlimefunItemInteractListener.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/SlimefunItemInteractListener.java @@ -57,6 +57,13 @@ public void onRightClick(PlayerInteractEvent e) { return; } + // Fixes #4087 - Prevents players from interacting with a block that is about to be deleted + // We especially don't want to open inventories as that can cause duplication + if (Slimefun.getTickerTask().isDeletedSoon(e.getClickedBlock().getLocation())) { + e.setCancelled(true); + return; + } + // Fire our custom Event PlayerRightClickEvent event = new PlayerRightClickEvent(e); Bukkit.getPluginManager().callEvent(event); diff --git a/src/main/java/me/mrCookieSlime/Slimefun/Objects/SlimefunItem/interfaces/InventoryBlock.java b/src/main/java/me/mrCookieSlime/Slimefun/Objects/SlimefunItem/interfaces/InventoryBlock.java index 199b60caeb..9c368d67ad 100644 --- a/src/main/java/me/mrCookieSlime/Slimefun/Objects/SlimefunItem/interfaces/InventoryBlock.java +++ b/src/main/java/me/mrCookieSlime/Slimefun/Objects/SlimefunItem/interfaces/InventoryBlock.java @@ -64,7 +64,11 @@ public boolean canOpen(Block b, Player p) { if (p.hasPermission("slimefun.inventory.bypass")) { return true; } else { - return item.canUse(p, false) && Slimefun.getProtectionManager().hasPermission(p, b.getLocation(), Interaction.INTERACT_BLOCK); + return item.canUse(p, false) && ( + // Protection manager doesn't exist in unit tests + Slimefun.instance().isUnitTest() + || Slimefun.getProtectionManager().hasPermission(p, b.getLocation(), Interaction.INTERACT_BLOCK) + ); } } }; diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunItemInteractListener.java b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunItemInteractListener.java new file mode 100644 index 0000000000..e403243393 --- /dev/null +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunItemInteractListener.java @@ -0,0 +1,137 @@ +package io.github.thebusybiscuit.slimefun4.implementation.listeners; + +import io.github.thebusybiscuit.slimefun4.api.events.SlimefunBlockBreakEvent; +import org.bukkit.World; +import org.bukkit.block.Block; +import org.bukkit.block.BlockFace; +import org.bukkit.entity.Player; +import org.bukkit.event.Event.Result; +import org.bukkit.event.block.Action; +import org.bukkit.event.block.BlockBreakEvent; +import org.bukkit.event.player.PlayerInteractEvent; +import org.bukkit.inventory.EquipmentSlot; +import org.bukkit.inventory.ItemStack; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import io.github.thebusybiscuit.slimefun4.api.events.PlayerRightClickEvent; +import io.github.thebusybiscuit.slimefun4.api.items.SlimefunItem; +import io.github.thebusybiscuit.slimefun4.api.recipes.RecipeType; +import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; +import io.github.thebusybiscuit.slimefun4.implementation.SlimefunItems; +import io.github.thebusybiscuit.slimefun4.implementation.items.electric.machines.ElectricFurnace; +import io.github.thebusybiscuit.slimefun4.test.TestUtilities; +import me.mrCookieSlime.Slimefun.api.BlockStorage; +import me.mrCookieSlime.Slimefun.api.inventory.BlockMenuPreset; +import be.seeseemelk.mockbukkit.MockBukkit; +import be.seeseemelk.mockbukkit.ServerMock; + +class TestSlimefunItemInteractListener { + + private static ServerMock server; + private static Slimefun plugin; + private static SlimefunItem slimefunItem; + + @BeforeAll + public static void load() { + server = MockBukkit.mock(); + plugin = MockBukkit.load(Slimefun.class); + + // Register block listener (for place + break) and our interact listener for inventory handling + new BlockListener(plugin); + new SlimefunItemInteractListener(plugin); + + // Enable tickers so the electric furnace can be registered + Slimefun.getCfg().setValue("URID.enable-tickers", true); + + slimefunItem = new ElectricFurnace( + TestUtilities.getItemGroup(plugin, "test"), SlimefunItems.ELECTRIC_FURNACE, RecipeType.NULL, new ItemStack[]{} + ) + .setCapacity(100) + .setEnergyConsumption(10) + .setProcessingSpeed(1); + slimefunItem.register(plugin); + } + + @AfterAll + public static void unload() { + MockBukkit.unmock(); + } + + @AfterEach + public void afterEach() { + server.getPluginManager().clearEvents(); + } + + // Test for dupe bug - issue #4087 + @Test + void testCannotOpenInvOfBrokenBlock() { + // Place down an electric furnace + Player player = server.addPlayer(); + ItemStack itemStack = slimefunItem.getItem(); + player.getInventory().setItemInMainHand(itemStack); + + // Create a world and place the block + World world = TestUtilities.createWorld(server); + Block block = TestUtilities.placeSlimefunBlock(server, itemStack, world, player); + + // Right click on the block + PlayerInteractEvent playerInteractEvent = new PlayerInteractEvent( + player, Action.RIGHT_CLICK_BLOCK, itemStack, block, BlockFace.UP, EquipmentSlot.HAND + ); + + server.getPluginManager().callEvent(playerInteractEvent); + server.getPluginManager().assertEventFired(PlayerInteractEvent.class, e -> { + // We cancel the event on inventory open + Assertions.assertSame(e.useInteractedBlock(), Result.DENY); + return true; + }); + + // Assert our right click event fired and the block usage was not denied + server.getPluginManager().assertEventFired(PlayerRightClickEvent.class, e -> { + Assertions.assertNotSame(e.useBlock(), Result.DENY); + return true; + }); + + // Assert we do have an inventory which would be opened + // TODO: Create an event for open inventory so this isn't guess work + Assertions.assertTrue(BlockMenuPreset.isInventory(slimefunItem.getId())); + Assertions.assertTrue(BlockStorage.getStorage(block.getWorld()).hasInventory(block.getLocation())); + // TODO(future): Check viewers - MockBukkit does not implement this today + + // Break the block + BlockBreakEvent blockBreakEvent = new BlockBreakEvent(block, player); + server.getPluginManager().callEvent(blockBreakEvent); + server.getPluginManager().assertEventFired(SlimefunBlockBreakEvent.class, e -> { + Assertions.assertEquals(slimefunItem.getId(), e.getSlimefunItem().getId()); + return true; + }); + + // Assert the block is queued for removal + Assertions.assertTrue(Slimefun.getTickerTask().isDeletedSoon(block.getLocation())); + + // Clear event queue since we'll be running duplicate events + server.getPluginManager().clearEvents(); + + // Right click on the block again now that it's broken + PlayerInteractEvent secondPlayerInteractEvent = new PlayerInteractEvent( + player, Action.RIGHT_CLICK_BLOCK, itemStack, block, BlockFace.UP, EquipmentSlot.HAND + ); + + server.getPluginManager().callEvent(secondPlayerInteractEvent); + server.getPluginManager().assertEventFired(PlayerInteractEvent.class, e -> { + // We cancelled the event due to the block being removed + Assertions.assertSame(e.useInteractedBlock(), Result.DENY); + return true; + }); + + // Assert our right click event was not fired due to the block being broken + Assertions.assertThrows( + AssertionError.class, + () -> server.getPluginManager().assertEventFired(PlayerRightClickEvent.class, e -> true) + ); + } +} diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/test/TestUtilities.java b/src/test/java/io/github/thebusybiscuit/slimefun4/test/TestUtilities.java index 56f517da20..d5f5b134c4 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/test/TestUtilities.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/test/TestUtilities.java @@ -10,17 +10,26 @@ import javax.annotation.Nonnull; import javax.annotation.ParametersAreNonnullByDefault; +import org.bukkit.Location; import org.bukkit.Material; import org.bukkit.NamespacedKey; import org.bukkit.OfflinePlayer; +import org.bukkit.World; +import org.bukkit.block.Block; +import org.bukkit.entity.Player; +import org.bukkit.event.block.BlockPlaceEvent; import org.bukkit.event.inventory.InventoryType; +import org.bukkit.inventory.EquipmentSlot; import org.bukkit.inventory.Inventory; import org.bukkit.inventory.ItemStack; import org.bukkit.plugin.Plugin; import org.junit.jupiter.api.Assertions; import org.mockito.Mockito; +import be.seeseemelk.mockbukkit.ServerMock; +import be.seeseemelk.mockbukkit.block.BlockMock; import io.github.bakedlibs.dough.items.CustomItemStack; +import io.github.thebusybiscuit.slimefun4.api.events.SlimefunBlockPlaceEvent; import io.github.thebusybiscuit.slimefun4.api.items.ItemGroup; import io.github.thebusybiscuit.slimefun4.api.items.SlimefunItem; import io.github.thebusybiscuit.slimefun4.api.player.PlayerProfile; @@ -28,6 +37,7 @@ import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; import io.github.thebusybiscuit.slimefun4.implementation.items.VanillaItem; import io.github.thebusybiscuit.slimefun4.test.mocks.MockSlimefunItem; +import me.mrCookieSlime.Slimefun.api.BlockStorage; public final class TestUtilities { @@ -89,4 +99,26 @@ private TestUtilities() {} public static @Nonnull int randomInt(int upperBound) { return random.nextInt(upperBound); } + + public static World createWorld(ServerMock server) { + World world = server.addSimpleWorld("world_" + randomInt()); + Slimefun.getRegistry().getWorlds().put(world.getName(), new BlockStorage(world)); + return world; + } + + public static Block placeSlimefunBlock(ServerMock server, ItemStack item, World world, Player player) { + int x = TestUtilities.randomInt(); + int z = TestUtilities.randomInt(); + Block block = new BlockMock(item.getType(), new Location(world, x, 0, z)); + Block blockAgainst = new BlockMock(Material.GRASS, new Location(world, x, 1, z)); + + BlockPlaceEvent blockPlaceEvent = new BlockPlaceEvent( + block, block.getState(), blockAgainst, item, player, true, EquipmentSlot.HAND + ); + + server.getPluginManager().callEvent(blockPlaceEvent); + server.getPluginManager().assertEventFired(SlimefunBlockPlaceEvent.class, e -> true); + + return block; + } } From 86533a8ec914af81bc8ba7ec4cfa7082b293e2dc Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Sat, 13 Jan 2024 16:37:57 +0000 Subject: [PATCH 06/20] Fixes NPE when interacting with air (#4089) --- .../implementation/listeners/SlimefunItemInteractListener.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/SlimefunItemInteractListener.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/SlimefunItemInteractListener.java index 195f6ac0c5..fd776e1baa 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/SlimefunItemInteractListener.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/SlimefunItemInteractListener.java @@ -59,7 +59,7 @@ public void onRightClick(PlayerInteractEvent e) { // Fixes #4087 - Prevents players from interacting with a block that is about to be deleted // We especially don't want to open inventories as that can cause duplication - if (Slimefun.getTickerTask().isDeletedSoon(e.getClickedBlock().getLocation())) { + if (e.getClickedBlock() != null && Slimefun.getTickerTask().isDeletedSoon(e.getClickedBlock().getLocation())) { e.setCancelled(true); return; } From 1fb27b93278257b6c8a4178a4f09af92c54373a0 Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Sun, 14 Jan 2024 11:02:31 +0000 Subject: [PATCH 07/20] Fix another bug with interaction fix and add more interaction tests (#4090) --- .../listeners/BlockListener.java | 6 +- .../TestSlimefunItemInteractListener.java | 98 +++++++++++++++++-- 2 files changed, 93 insertions(+), 11 deletions(-) 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 1d7a75df25..b4dc7c38ae 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 @@ -42,6 +42,7 @@ import io.github.thebusybiscuit.slimefun4.utils.tags.SlimefunTag; import me.mrCookieSlime.Slimefun.api.BlockStorage; +import me.mrCookieSlime.Slimefun.api.inventory.BlockMenu; /** * The {@link BlockListener} is responsible for listening to the {@link BlockPlaceEvent} @@ -224,9 +225,10 @@ private void callBlockHandler(BlockBreakEvent e, ItemStack item, List // Partial fix for #4087 - We don't want the inventory to be usable post break, close it for anyone still inside // The main fix is in SlimefunItemInteractListener preventing opening to begin with // Close the inventory for all viewers of this block + BlockMenu inventory = BlockStorage.getInventory(e.getBlock()); // TODO(future): Remove this check when MockBukkit supports viewers - if (!Slimefun.instance().isUnitTest()) { - BlockStorage.getInventory(e.getBlock()).toInventory().getViewers().forEach(HumanEntity::closeInventory); + if (inventory != null && !Slimefun.instance().isUnitTest()) { + inventory.toInventory().getViewers().forEach(HumanEntity::closeInventory); } // Remove the block data BlockStorage.clearBlockInfo(e.getBlock()); diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunItemInteractListener.java b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunItemInteractListener.java index e403243393..08b506e8c6 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunItemInteractListener.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunItemInteractListener.java @@ -1,6 +1,5 @@ package io.github.thebusybiscuit.slimefun4.implementation.listeners; -import io.github.thebusybiscuit.slimefun4.api.events.SlimefunBlockBreakEvent; import org.bukkit.World; import org.bukkit.block.Block; import org.bukkit.block.BlockFace; @@ -17,23 +16,34 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import io.github.bakedlibs.dough.common.ChatColors; import io.github.thebusybiscuit.slimefun4.api.events.PlayerRightClickEvent; +import io.github.thebusybiscuit.slimefun4.api.events.SlimefunBlockBreakEvent; +import io.github.thebusybiscuit.slimefun4.api.items.ItemGroup; import io.github.thebusybiscuit.slimefun4.api.items.SlimefunItem; import io.github.thebusybiscuit.slimefun4.api.recipes.RecipeType; import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; import io.github.thebusybiscuit.slimefun4.implementation.SlimefunItems; +import io.github.thebusybiscuit.slimefun4.implementation.items.electric.EnergyConnector; import io.github.thebusybiscuit.slimefun4.implementation.items.electric.machines.ElectricFurnace; +import io.github.thebusybiscuit.slimefun4.implementation.items.magical.staves.WindStaff; import io.github.thebusybiscuit.slimefun4.test.TestUtilities; import me.mrCookieSlime.Slimefun.api.BlockStorage; import me.mrCookieSlime.Slimefun.api.inventory.BlockMenuPreset; import be.seeseemelk.mockbukkit.MockBukkit; import be.seeseemelk.mockbukkit.ServerMock; +import be.seeseemelk.mockbukkit.entity.PlayerMock; class TestSlimefunItemInteractListener { private static ServerMock server; private static Slimefun plugin; - private static SlimefunItem slimefunItem; + // Block with inventory + private static SlimefunItem electricFurnace; + // Interactable block + private static SlimefunItem energyConnector; + // Interactable item + private static SlimefunItem windStaff; @BeforeAll public static void load() { @@ -47,13 +57,19 @@ public static void load() { // Enable tickers so the electric furnace can be registered Slimefun.getCfg().setValue("URID.enable-tickers", true); - slimefunItem = new ElectricFurnace( - TestUtilities.getItemGroup(plugin, "test"), SlimefunItems.ELECTRIC_FURNACE, RecipeType.NULL, new ItemStack[]{} - ) + ItemGroup testGroup = TestUtilities.getItemGroup(plugin, "test"); + + electricFurnace = new ElectricFurnace(testGroup, SlimefunItems.ELECTRIC_FURNACE, RecipeType.NULL, new ItemStack[]{}) .setCapacity(100) .setEnergyConsumption(10) .setProcessingSpeed(1); - slimefunItem.register(plugin); + electricFurnace.register(plugin); + + energyConnector = new EnergyConnector(testGroup, SlimefunItems.ENERGY_CONNECTOR, RecipeType.NULL, new ItemStack[9], null); + energyConnector.register(plugin); + + windStaff = new WindStaff(testGroup, SlimefunItems.STAFF_WIND, RecipeType.NULL, new ItemStack[9]); + windStaff.register(plugin); } @AfterAll @@ -71,7 +87,7 @@ public void afterEach() { void testCannotOpenInvOfBrokenBlock() { // Place down an electric furnace Player player = server.addPlayer(); - ItemStack itemStack = slimefunItem.getItem(); + ItemStack itemStack = electricFurnace.getItem(); player.getInventory().setItemInMainHand(itemStack); // Create a world and place the block @@ -98,7 +114,7 @@ void testCannotOpenInvOfBrokenBlock() { // Assert we do have an inventory which would be opened // TODO: Create an event for open inventory so this isn't guess work - Assertions.assertTrue(BlockMenuPreset.isInventory(slimefunItem.getId())); + Assertions.assertTrue(BlockMenuPreset.isInventory(electricFurnace.getId())); Assertions.assertTrue(BlockStorage.getStorage(block.getWorld()).hasInventory(block.getLocation())); // TODO(future): Check viewers - MockBukkit does not implement this today @@ -106,7 +122,7 @@ void testCannotOpenInvOfBrokenBlock() { BlockBreakEvent blockBreakEvent = new BlockBreakEvent(block, player); server.getPluginManager().callEvent(blockBreakEvent); server.getPluginManager().assertEventFired(SlimefunBlockBreakEvent.class, e -> { - Assertions.assertEquals(slimefunItem.getId(), e.getSlimefunItem().getId()); + Assertions.assertEquals(electricFurnace.getId(), e.getSlimefunItem().getId()); return true; }); @@ -134,4 +150,68 @@ void testCannotOpenInvOfBrokenBlock() { () -> server.getPluginManager().assertEventFired(PlayerRightClickEvent.class, e -> true) ); } + + @Test + void testRightClickItem() { + Player player = server.addPlayer(); + ItemStack itemStack = windStaff.getItem(); + player.getInventory().setItemInMainHand(itemStack); + + // Assert player is at full food level (wind staff reduces food level on usage) + Assertions.assertEquals(20, player.getFoodLevel()); + + // Right click the air + PlayerInteractEvent playerInteractEvent = new PlayerInteractEvent( + player, Action.RIGHT_CLICK_AIR, itemStack, null, BlockFace.UP, EquipmentSlot.HAND + ); + + server.getPluginManager().callEvent(playerInteractEvent); + server.getPluginManager().assertEventFired(PlayerInteractEvent.class, e -> { + // Assert our interaction was not cancelled + Assertions.assertNotSame(e.useItemInHand(), Result.DENY); + return true; + }); + + // Assert our right click event fired and the item usage was not denied + server.getPluginManager().assertEventFired(PlayerRightClickEvent.class, e -> { + Assertions.assertNotSame(e.useItem(), Result.DENY); + return true; + }); + + // Assert our food level is now 18 + Assertions.assertEquals(18, player.getFoodLevel()); + } + + @Test + void testRightClickInteractableBlock() { + // Place down an energy connector + PlayerMock player = server.addPlayer(); + ItemStack itemStack = energyConnector.getItem(); + player.getInventory().setItemInMainHand(itemStack); + + // Create a world and place the block + World world = TestUtilities.createWorld(server); + Block block = TestUtilities.placeSlimefunBlock(server, itemStack, world, player); + + // Right click on the block + PlayerInteractEvent playerInteractEvent = new PlayerInteractEvent( + player, Action.RIGHT_CLICK_BLOCK, itemStack, block, BlockFace.UP, EquipmentSlot.HAND + ); + + server.getPluginManager().callEvent(playerInteractEvent); + server.getPluginManager().assertEventFired(PlayerInteractEvent.class, e -> { + // Allow interaction of the block + Assertions.assertSame(e.useInteractedBlock(), Result.ALLOW); + return true; + }); + + // Assert our right click event fired and the block usage was not denied + server.getPluginManager().assertEventFired(PlayerRightClickEvent.class, e -> { + Assertions.assertNotSame(e.useBlock(), Result.DENY); + return true; + }); + + // Assert the message our energy connector sends + Assertions.assertEquals(ChatColors.color("&7Connected: " + "&4\u2718"), player.nextMessage()); + } } From eb4d23ec4559f8add0a95d831f1a35d044991076 Mon Sep 17 00:00:00 2001 From: J3fftw <44972470+J3fftw1@users.noreply.github.com> Date: Mon, 15 Jan 2024 14:40:43 +0100 Subject: [PATCH 08/20] Update MockBukkit to 1.20.4 along with existing tests (#4086) Co-authored-by: J3fftw1 <44972470+J3fftw1@users.noreply.github.com> Co-authored-by: Daniel Walsh Co-authored-by: Alessio Colombo <37039432+Sfiguz7@users.noreply.github.com> --- .gitignore | 2 +- docs/sop/update.md | 71 +++++++++++++++++++ pom.xml | 42 ++++++----- .../slimefun4/implementation/Slimefun.java | 36 +++------- .../slimefun4/utils/FileUtils.java | 24 +++++++ .../events/TestSlimefunBlockPlaceEvent.java | 10 +-- .../api/events/TestTalismanActivateEvent.java | 4 +- .../slimefun4/api/gps/TestWaypoints.java | 2 +- .../listeners/TestCargoNodeListener.java | 2 +- .../listeners/TestSlimefunGuideListener.java | 2 +- .../storage/backend/TestLegacyBackend.java | 4 +- .../slimefun4/test/TestUtilities.java | 2 +- 12 files changed, 144 insertions(+), 57 deletions(-) create mode 100644 docs/sop/update.md create mode 100644 src/main/java/io/github/thebusybiscuit/slimefun4/utils/FileUtils.java diff --git a/.gitignore b/.gitignore index f025c1e196..e6f9d7a43c 100644 --- a/.gitignore +++ b/.gitignore @@ -5,7 +5,7 @@ /.settings/ /.idea/ /.vscode/ -/data-store/ +/data-storage/ dependency-reduced-pom.xml diff --git a/docs/sop/update.md b/docs/sop/update.md new file mode 100644 index 0000000000..88470853b7 --- /dev/null +++ b/docs/sop/update.md @@ -0,0 +1,71 @@ +# Update Procedure + +Date: 2024-01-15 +Last updated: 2024-01-15 + +## Goal + +This SOP will go over updating Slimefun to the newest Minecraft version, most of this will only apply to major versions, but we have also seen minor versions break things. So please read through the whole SOP and make sure you do everything applicable. + +## Updating + +### Updating Bukkit/Spigot + +The first step is just updating Spigot in the pom.xml. This should only be done in 2 cases: +* There's a new major version (well, MC major - 1.19 -> 1.20 is a major) +* There was a change within MC or Bukkit/Spigot that broke the API + +To update the Spigot version, you will need to go to the `pom.xml` and find the `spigot.version` property, this will be within the `properties` property. Simply make this the MC version (e.g. `1.20` or in the case of minor `1.20.4`). + +Once updated, **make sure to run a build** to check for compilation failures with `mvn clean package -DskipTests=true`. We will go over the tests next. + +### Updating tests + +The next step is making sure our tests are still working correctly as is. This can be done by running `mvn test` and verifying that all tests pass correctly without any failures or errors. + +If there are any failures you will need to investigate these, it's best to run them one at a time, so you don't have the potential for cross-test contamination. If you find any issues with the tests, please fix them and make sure to add a comment to the PR explaining why the test was changed. + +If you need any help fixing tests feel free to join the [Discord](https://discord.gg/slimefun). + +Once all the tests are passed, check to see if there's a new version of [MockBukkit](https://github.com/MockBukkit/MockBukkit), this is the framework handling the Bukkit side of our tests. There very well may not be a new version, they usually lag updates a bit. If not, that's perfectly ok, just make sure to note it on the PR. + +### Testing in game + +The final and most important step is testing this in game. While I'd love for our tests to be perfect, they are not (especially if MockBukkit hasn't had an update yet). We need to ensure that everything is working in-game before we can ship a new version release. + +To do this, you will need to build the plugin with `mvn clean package` and then copy the jar from `target/` to your server's `plugins/` folder. Once you've done this, start the server. You will want to test various things but the things we always want covered are: +* Commands, verify running a few commands work + * `/sf versions` + * `/sf cheat` + * `/sf search` +* Items, verify you can use a few items (you can grab these from `/sf cheat`) + * Wind staff + * One of the talismans + * One of the backpacks +* Blocks, verify you can place, break and ensure they all work + * Ancient altar + * Ore washer + * Coal generator + +It is important to verify heads are still working (part of the energy network and the coal generator). If head skins are not loading, consider it as a bug: try figuring out what the issue is, and ask in the [Discord](https://discord.gg/slimefun) if you are not sure what the cause may be. + +Also make sure to verify that there are no errors in the console, any errors here should be investigated and fixed. + +If you find any issues, please fix them and make sure to add a comment to the PR explaining why the fix was needed. + +> **Note** +> An issue here usually means that we need to update Dough. If this is the case, please open a PR to Dough and then update the Dough version in the `pom.xml` to the new version. Once you've done this, make sure to run a build to verify everything is working correctly. + +### Final steps + +Once you've verified everything is working, you can go ahead and open the PR. We will get to this as soon as we can :) + +While the PR is open, make sure to verify the E2E tests are passing, and you should also verify the output of these. If the E2E tests look good then finally we will update these. + +#### Updating E2E tests + +**This is only needed in a major version** + +In the `e2e-testing.yml` file you will need to update the matrix strategy, please add the latest version of the old major (e.g. if 1.21 came out, add 1.20.x where x is the latest released version). If MC is requiring a new Java version make sure that is updated too in the `latest` version. + +Once updated, push and re-verify that the E2E tests are still passing. diff --git a/pom.xml b/pom.xml index 65fd79d941..46b0478251 100644 --- a/pom.xml +++ b/pom.xml @@ -26,7 +26,7 @@ 16 - 1.20 + 1.20.4 https://hub.spigotmc.org/javadocs/spigot/ @@ -334,13 +334,7 @@ - - - org.spigotmc - spigot-api - ${spigot.version}-R0.1-SNAPSHOT - provided - + @@ -355,7 +349,7 @@ com.github.baked-libs.dough dough-api - c4231a4d1a + a2364de77c compile @@ -398,10 +392,11 @@ 2.0.9 test + com.github.seeseemelk - MockBukkit-v1.18 - 2.0.0 + MockBukkit-v1.20 + 3.65.0 test @@ -513,12 +508,6 @@ - - com.mojang - authlib - 1.5.25 - provided - commons-lang @@ -526,5 +515,24 @@ 2.6 compile + + org.spigotmc + spigot-api + ${spigot.version}-R0.1-SNAPSHOT + provided + + + com.mojang + authlib + 6.0.52 + provided + + + + * + * + + + diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java index 8667eed682..28233ea741 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java @@ -13,7 +13,6 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; -import javax.annotation.ParametersAreNonnullByDefault; import io.github.thebusybiscuit.slimefun4.storage.Storage; import io.github.thebusybiscuit.slimefun4.storage.backend.legacy.LegacyStorage; @@ -29,7 +28,6 @@ import org.bukkit.plugin.Plugin; import org.bukkit.plugin.PluginDescriptionFile; import org.bukkit.plugin.java.JavaPlugin; -import org.bukkit.plugin.java.JavaPluginLoader; import org.bukkit.scheduler.BukkitTask; import io.github.bakedlibs.dough.config.Config; @@ -120,17 +118,16 @@ import io.github.thebusybiscuit.slimefun4.implementation.setup.PostSetup; import io.github.thebusybiscuit.slimefun4.implementation.setup.ResearchSetup; import io.github.thebusybiscuit.slimefun4.implementation.setup.SlimefunItemSetup; +import io.github.thebusybiscuit.slimefun4.implementation.tasks.SlimefunStartupTask; +import io.github.thebusybiscuit.slimefun4.implementation.tasks.TickerTask; import io.github.thebusybiscuit.slimefun4.implementation.tasks.armor.RadiationTask; import io.github.thebusybiscuit.slimefun4.implementation.tasks.armor.RainbowArmorTask; import io.github.thebusybiscuit.slimefun4.implementation.tasks.armor.SlimefunArmorTask; import io.github.thebusybiscuit.slimefun4.implementation.tasks.armor.SolarHelmetTask; -import io.github.thebusybiscuit.slimefun4.implementation.tasks.SlimefunStartupTask; -import io.github.thebusybiscuit.slimefun4.implementation.tasks.TickerTask; import io.github.thebusybiscuit.slimefun4.integrations.IntegrationsManager; import io.github.thebusybiscuit.slimefun4.utils.NumberUtils; import io.github.thebusybiscuit.slimefun4.utils.tags.SlimefunTag; import io.papermc.lib.PaperLib; - import me.mrCookieSlime.CSCoreLibPlugin.general.Inventory.MenuListener; import me.mrCookieSlime.Slimefun.api.BlockStorage; import me.mrCookieSlime.Slimefun.api.inventory.UniversalBlockMenu; @@ -141,7 +138,7 @@ * * @author TheBusyBiscuit */ -public final class Slimefun extends JavaPlugin implements SlimefunAddon { +public class Slimefun extends JavaPlugin implements SlimefunAddon { /** * This is the Java version we recommend server owners to use. @@ -209,30 +206,17 @@ public final class Slimefun extends JavaPlugin implements SlimefunAddon { private final SlimefunBowListener bowListener = new SlimefunBowListener(); /** - * Our default constructor for {@link Slimefun}. + * This constructor is invoked by Bukkit and within unit tests. + * Therefore we need to figure out if we're within unit tests or not. */ public Slimefun() { super(); - } - /** - * This constructor is invoked in Unit Test environments only. - * - * @param loader - * Our {@link JavaPluginLoader} - * @param description - * A {@link PluginDescriptionFile} - * @param dataFolder - * The data folder - * @param file - * A {@link File} for this {@link Plugin} - */ - @ParametersAreNonnullByDefault - public Slimefun(JavaPluginLoader loader, PluginDescriptionFile description, File dataFolder, File file) { - super(loader, description, dataFolder, file); - - // This is only invoked during a Unit Test - minecraftVersion = MinecraftVersion.UNIT_TEST; + // Check that we got loaded by MockBukkit rather than Bukkit's loader + // TODO: This is very much a hack and we can hopefully move to a more native way in the future + if (getClassLoader().getClass().getPackageName().startsWith("be.seeseemelk.mockbukkit")) { + minecraftVersion = MinecraftVersion.UNIT_TEST; + } } /** diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/utils/FileUtils.java b/src/main/java/io/github/thebusybiscuit/slimefun4/utils/FileUtils.java new file mode 100644 index 0000000000..f8441dc75e --- /dev/null +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/utils/FileUtils.java @@ -0,0 +1,24 @@ +package io.github.thebusybiscuit.slimefun4.utils; + +import java.io.File; + +public class FileUtils { + + public static boolean deleteDirectory(File folder) { + if (folder.isDirectory()) { + File[] files = folder.listFiles(); + + if (files != null) { + for (File file : files) { + // Recursive call to delete files and subfolders + if (!deleteDirectory(file)) { + return false; + } + } + } + } + + // Delete the folder itself + return folder.delete(); + } +} diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/api/events/TestSlimefunBlockPlaceEvent.java b/src/test/java/io/github/thebusybiscuit/slimefun4/api/events/TestSlimefunBlockPlaceEvent.java index f6d8d2868d..c33db57184 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/api/events/TestSlimefunBlockPlaceEvent.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/api/events/TestSlimefunBlockPlaceEvent.java @@ -65,7 +65,7 @@ void testEventIsFired() { int x = TestUtilities.randomInt(); int z = TestUtilities.randomInt(); Block block = new BlockMock(Material.GREEN_TERRACOTTA, new Location(world, x, 0, z)); - Block blockAgainst = new BlockMock(Material.GRASS, new Location(world, x, 1, z)); + Block blockAgainst = new BlockMock(Material.GRASS_BLOCK, new Location(world, x, 1, z)); Slimefun.getRegistry().getWorlds().put("my_world", new BlockStorage(world)); @@ -88,7 +88,7 @@ void testGetters() { int x = TestUtilities.randomInt(); int z = TestUtilities.randomInt(); Block block = new BlockMock(Material.GREEN_TERRACOTTA, new Location(world, x, 0, z)); - Block blockAgainst = new BlockMock(Material.GRASS, new Location(world, x, 1, z)); + Block blockAgainst = new BlockMock(Material.GRASS_BLOCK, new Location(world, x, 1, z)); Slimefun.getRegistry().getWorlds().put("my_world", new BlockStorage(world)); @@ -125,7 +125,7 @@ public void onBlockPlace(SlimefunBlockPlaceEvent event) { int x = TestUtilities.randomInt(); int z = TestUtilities.randomInt(); Block block = new BlockMock(Material.GREEN_TERRACOTTA, new Location(world, x, 0, z)); - Block blockAgainst = new BlockMock(Material.GRASS, new Location(world, x, 1, z)); + Block blockAgainst = new BlockMock(Material.GRASS_BLOCK, new Location(world, x, 1, z)); Slimefun.getRegistry().getWorlds().put("my_world", new BlockStorage(world)); @@ -153,7 +153,7 @@ void testBlockPlacementBeforeFullDeletion() { int x = TestUtilities.randomInt(); int z = TestUtilities.randomInt(); Block firstBlock = new BlockMock(Material.GREEN_TERRACOTTA, new Location(world, x, 0, z)); - Block firstBlockAgainst = new BlockMock(Material.GRASS, new Location(world, x, 1, z)); + Block firstBlockAgainst = new BlockMock(Material.GRASS_BLOCK, new Location(world, x, 1, z)); Slimefun.getRegistry().getWorlds().put("my_world", new BlockStorage(world)); @@ -176,7 +176,7 @@ void testBlockPlacementBeforeFullDeletion() { // Place second block in the same location Block secondBlock = new BlockMock(Material.GREEN_TERRACOTTA, new Location(world, x, 0, z)); - Block secondBlockAgainst = new BlockMock(Material.GRASS, new Location(world, x, 1, z)); + Block secondBlockAgainst = new BlockMock(Material.GRASS_BLOCK, new Location(world, x, 1, z)); BlockPlaceEvent secondBlockPlaceEvent = new BlockPlaceEvent( secondBlock, secondBlock.getState(), secondBlockAgainst, itemStack, player, true, EquipmentSlot.HAND diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/api/events/TestTalismanActivateEvent.java b/src/test/java/io/github/thebusybiscuit/slimefun4/api/events/TestTalismanActivateEvent.java index 83b49fa231..94b1947cf1 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/api/events/TestTalismanActivateEvent.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/api/events/TestTalismanActivateEvent.java @@ -55,9 +55,9 @@ void activateAnvilTalisman(boolean enderVariant, boolean inEnderChest) { ItemStack breakableItem = new ItemStack(Material.IRON_PICKAXE); if (inEnderChest) { - player.getEnderChest().addItem(talismanItem); + player.getEnderChest().setItem(9, talismanItem); } else { - player.getInventory().addItem(talismanItem); + player.getInventory().setItem(9, talismanItem); } player.getInventory().setItemInMainHand(breakableItem); diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/api/gps/TestWaypoints.java b/src/test/java/io/github/thebusybiscuit/slimefun4/api/gps/TestWaypoints.java index a0de64b14f..3b769c44a0 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/api/gps/TestWaypoints.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/api/gps/TestWaypoints.java @@ -3,7 +3,6 @@ import java.io.File; import java.io.IOException; -import org.apache.commons.io.FileUtils; import org.bukkit.entity.Player; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; @@ -15,6 +14,7 @@ import io.github.thebusybiscuit.slimefun4.api.player.PlayerProfile; import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; import io.github.thebusybiscuit.slimefun4.test.TestUtilities; +import io.github.thebusybiscuit.slimefun4.utils.FileUtils; import be.seeseemelk.mockbukkit.MockBukkit; import be.seeseemelk.mockbukkit.ServerMock; diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestCargoNodeListener.java b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestCargoNodeListener.java index 1288b34d7b..0951dce2ca 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestCargoNodeListener.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestCargoNodeListener.java @@ -84,7 +84,7 @@ void testGrassPlacement() { Location l = new Location(player.getWorld(), 300, 25, 1200); Block b = l.getBlock(); - b.setType(Material.GRASS); + b.setType(Material.GRASS_BLOCK); ItemGroup itemGroup = TestUtilities.getItemGroup(plugin, "cargo_test"); SlimefunItemStack item = new SlimefunItemStack("MOCK_CARGO_NODE_2", new CustomItemStack(Material.PLAYER_HEAD, "&4Cargo node!")); diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunGuideListener.java b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunGuideListener.java index 5c62c4090a..6b58f849b7 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunGuideListener.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunGuideListener.java @@ -45,7 +45,7 @@ void testFirstJoin(boolean hasPlayedBefore, boolean giveSlimefunGuide) { PlayerMock player = new PlayerMock(server, "CanIHazGuide"); if (hasPlayedBefore) { - player.setLastPlayed(System.currentTimeMillis()); + server.getPlayerList().setLastSeen(player.getUniqueId(), System.currentTimeMillis()); } PlayerJoinEvent event = new PlayerJoinEvent(player, "CanIHazGuide has joined and wants sum guide"); diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java b/src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java index 1859659999..c8e1916f54 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java @@ -5,8 +5,8 @@ import java.nio.file.Files; import java.util.UUID; -import org.apache.commons.io.FileUtils; import org.bukkit.Bukkit; +import org.bukkit.ChatColor; import org.bukkit.Location; import org.bukkit.NamespacedKey; import org.bukkit.OfflinePlayer; @@ -30,7 +30,7 @@ import io.github.thebusybiscuit.slimefun4.storage.backend.legacy.LegacyStorage; import io.github.thebusybiscuit.slimefun4.storage.data.PlayerData; import io.github.thebusybiscuit.slimefun4.test.TestUtilities; -import net.md_5.bungee.api.ChatColor; +import io.github.thebusybiscuit.slimefun4.utils.FileUtils; class TestLegacyBackend { diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/test/TestUtilities.java b/src/test/java/io/github/thebusybiscuit/slimefun4/test/TestUtilities.java index d5f5b134c4..edd6a458af 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/test/TestUtilities.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/test/TestUtilities.java @@ -110,7 +110,7 @@ public static Block placeSlimefunBlock(ServerMock server, ItemStack item, World int x = TestUtilities.randomInt(); int z = TestUtilities.randomInt(); Block block = new BlockMock(item.getType(), new Location(world, x, 0, z)); - Block blockAgainst = new BlockMock(Material.GRASS, new Location(world, x, 1, z)); + Block blockAgainst = new BlockMock(Material.GRASS_BLOCK, new Location(world, x, 1, z)); BlockPlaceEvent blockPlaceEvent = new BlockPlaceEvent( block, block.getState(), blockAgainst, item, player, true, EquipmentSlot.HAND From 6bc1b1f474cdfc1ddb4b637b67983912cb39ef42 Mon Sep 17 00:00:00 2001 From: J3fftw <44972470+J3fftw1@users.noreply.github.com> Date: Tue, 16 Jan 2024 13:08:14 +0100 Subject: [PATCH 09/20] fix items not being able to be placed on ancient altar (#4094) Co-authored-by: Daniel Walsh --- .../slimefun4/utils/ArmorStandUtils.java | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/utils/ArmorStandUtils.java b/src/main/java/io/github/thebusybiscuit/slimefun4/utils/ArmorStandUtils.java index 575446fb7a..033e3ca489 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/utils/ArmorStandUtils.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/utils/ArmorStandUtils.java @@ -2,6 +2,8 @@ import javax.annotation.Nonnull; +import io.github.thebusybiscuit.slimefun4.api.MinecraftVersion; +import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; import org.bukkit.Location; import org.bukkit.entity.ArmorStand; @@ -47,13 +49,22 @@ public class ArmorStandUtils { * @return The spawned {@link ArmorStand} */ public static @Nonnull ArmorStand spawnArmorStand(@Nonnull Location location) { - return location.getWorld().spawn(location, ArmorStand.class, armorStand -> { - armorStand.setVisible(false); - armorStand.setSilent(true); - armorStand.setMarker(true); - armorStand.setGravity(false); - armorStand.setBasePlate(false); - armorStand.setRemoveWhenFarAway(false); - }); + // 1.19 and below don't have the consumer method so flicker exists on these versions. + if (Slimefun.getMinecraftVersion().isBefore(MinecraftVersion.MINECRAFT_1_20)) { + ArmorStand armorStand = location.getWorld().spawn(location, ArmorStand.class); + setupArmorStand(armorStand); + return armorStand; + } + + return location.getWorld().spawn(location, ArmorStand.class, armorStand -> setupArmorStand(armorStand)); + } + + private static void setupArmorStand(ArmorStand armorStand) { + armorStand.setVisible(false); + armorStand.setSilent(true); + armorStand.setMarker(true); + armorStand.setGravity(false); + armorStand.setBasePlate(false); + armorStand.setRemoveWhenFarAway(false); } } From f7b42a1c620f7f89565039274780db7a19db7039 Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Wed, 17 Jan 2024 10:26:56 +0000 Subject: [PATCH 10/20] Update dough to fix item stacking issue (#4100) --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 46b0478251..a7b6c91df6 100644 --- a/pom.xml +++ b/pom.xml @@ -349,7 +349,7 @@ com.github.baked-libs.dough dough-api - a2364de77c + fcdbd45aa0 compile From 9fba3f6b057bca3dc947ad9b7d8de113fa66a49c Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 17 Jan 2024 17:16:25 +0100 Subject: [PATCH 11/20] [CI skip] Update dependency org.mockito:mockito-core to v5.9.0 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index a7b6c91df6..48e7d9db95 100644 --- a/pom.xml +++ b/pom.xml @@ -383,7 +383,7 @@ org.mockito mockito-core - 5.8.0 + 5.9.0 test From bcfbd3a598b512cec1fa36721ff33fea82c892b1 Mon Sep 17 00:00:00 2001 From: J3fftw <44972470+J3fftw1@users.noreply.github.com> Date: Wed, 17 Jan 2024 17:26:33 +0100 Subject: [PATCH 12/20] fix slimefun block turning into a vanilla block if there are viewers (#4101) --- .../slimefun4/implementation/listeners/BlockListener.java | 7 ++++--- .../listeners/TestSlimefunItemInteractListener.java | 7 ++++++- 2 files changed, 10 insertions(+), 4 deletions(-) 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 b4dc7c38ae..12937e45a4 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 @@ -226,9 +226,10 @@ private void callBlockHandler(BlockBreakEvent e, ItemStack item, List // The main fix is in SlimefunItemInteractListener preventing opening to begin with // Close the inventory for all viewers of this block BlockMenu inventory = BlockStorage.getInventory(e.getBlock()); - // TODO(future): Remove this check when MockBukkit supports viewers - if (inventory != null && !Slimefun.instance().isUnitTest()) { - inventory.toInventory().getViewers().forEach(HumanEntity::closeInventory); + if (inventory != null) { + for (HumanEntity human : new ArrayList<>(inventory.toInventory().getViewers())) { + human.closeInventory(); + } } // Remove the block data BlockStorage.clearBlockInfo(e.getBlock()); diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunItemInteractListener.java b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunItemInteractListener.java index 08b506e8c6..cc33e3750a 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunItemInteractListener.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestSlimefunItemInteractListener.java @@ -116,7 +116,9 @@ void testCannotOpenInvOfBrokenBlock() { // TODO: Create an event for open inventory so this isn't guess work Assertions.assertTrue(BlockMenuPreset.isInventory(electricFurnace.getId())); Assertions.assertTrue(BlockStorage.getStorage(block.getWorld()).hasInventory(block.getLocation())); - // TODO(future): Check viewers - MockBukkit does not implement this today + + // Assert player has the inventory open + Assertions.assertEquals(1, BlockStorage.getInventory(block).toInventory().getViewers().size()); // Break the block BlockBreakEvent blockBreakEvent = new BlockBreakEvent(block, player); @@ -129,6 +131,9 @@ void testCannotOpenInvOfBrokenBlock() { // Assert the block is queued for removal Assertions.assertTrue(Slimefun.getTickerTask().isDeletedSoon(block.getLocation())); + // Assert that the inventory was closed + Assertions.assertEquals(0, BlockStorage.getInventory(block).toInventory().getViewers().size()); + // Clear event queue since we'll be running duplicate events server.getPluginManager().clearEvents(); From bcdde8c2dcfb2e171425e7c031c0eee238b0a049 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Thu, 18 Jan 2024 15:06:33 +0100 Subject: [PATCH 13/20] [CI skip] Update actions/cache action to v4 --- .github/workflows/discord-webhook.yml | 2 +- .github/workflows/maven-compiler.yml | 2 +- .github/workflows/pull-request.yml | 2 +- .github/workflows/sonarcloud.yml | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/discord-webhook.yml b/.github/workflows/discord-webhook.yml index 3364a75bb1..b424d1c383 100644 --- a/.github/workflows/discord-webhook.yml +++ b/.github/workflows/discord-webhook.yml @@ -32,7 +32,7 @@ jobs: architecture: x64 - name: Cache Maven packages - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: ~/.m2 key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }} diff --git a/.github/workflows/maven-compiler.yml b/.github/workflows/maven-compiler.yml index 7529fdbd77..41f5aff546 100644 --- a/.github/workflows/maven-compiler.yml +++ b/.github/workflows/maven-compiler.yml @@ -36,7 +36,7 @@ jobs: architecture: x64 - name: Cache Maven packages - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: ~/.m2 key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }} diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index 89b9374dcf..ad2a33e080 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -30,7 +30,7 @@ jobs: architecture: x64 - name: Cache Maven packages - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: ~/.m2 key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }} diff --git a/.github/workflows/sonarcloud.yml b/.github/workflows/sonarcloud.yml index 6fe3823205..4d395db640 100644 --- a/.github/workflows/sonarcloud.yml +++ b/.github/workflows/sonarcloud.yml @@ -32,14 +32,14 @@ jobs: architecture: x64 - name: Cache SonarCloud packages - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: ~/.sonar/cache key: ${{ runner.os }}-sonar restore-keys: ${{ runner.os }}-sonar - name: Cache Maven packages - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: ~/.m2 key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }} From 7c917c396fcde3edaa59fdf22b7f4fcd15d00da5 Mon Sep 17 00:00:00 2001 From: ProfElements <43350117+ProfElements@users.noreply.github.com> Date: Thu, 18 Jan 2024 08:07:18 -0600 Subject: [PATCH 14/20] :sparkles: feat(api): Introduce SlimefunItemRegistryFinalizedEvent (#4099) Co-authored-by: JustAHuman-xD <65748158+JustAHuman-xD@users.noreply.github.com> Co-authored-by: Daniel Walsh --- .../SlimefunItemRegistryFinalizedEvent.java | 35 ++++++++++++++++ .../implementation/setup/PostSetup.java | 3 ++ .../TestSlimefunRegistryFinalizedEvent.java | 42 +++++++++++++++++++ 3 files changed, 80 insertions(+) create mode 100644 src/main/java/io/github/thebusybiscuit/slimefun4/api/events/SlimefunItemRegistryFinalizedEvent.java create mode 100644 src/test/java/io/github/thebusybiscuit/slimefun4/api/events/TestSlimefunRegistryFinalizedEvent.java diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/events/SlimefunItemRegistryFinalizedEvent.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/events/SlimefunItemRegistryFinalizedEvent.java new file mode 100644 index 0000000000..12943c82bf --- /dev/null +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/events/SlimefunItemRegistryFinalizedEvent.java @@ -0,0 +1,35 @@ +package io.github.thebusybiscuit.slimefun4.api.events; + +import javax.annotation.Nonnull; + +import org.bukkit.event.Event; +import org.bukkit.event.HandlerList; + +import io.github.thebusybiscuit.slimefun4.api.items.SlimefunItem; +import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; + + +/** + * This {@link Event} is fired after {@link Slimefun} finishes loading the + * {@link SlimefunItem} registry. We recommend listening to this event if you + * want to register recipes using items from other addons. + * + * @author ProfElements + */ +public class SlimefunItemRegistryFinalizedEvent extends Event { + + private static final HandlerList handlers = new HandlerList(); + + public SlimefunItemRegistryFinalizedEvent() {} + + @Nonnull + public static HandlerList getHandlerList() { + return handlers; + } + + @Nonnull + @Override + public HandlerList getHandlers() { + return getHandlerList(); + } +} diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/setup/PostSetup.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/setup/PostSetup.java index 13e7e7ef5b..ce8694bd31 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/setup/PostSetup.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/setup/PostSetup.java @@ -24,6 +24,7 @@ import com.google.gson.JsonElement; import com.google.gson.JsonObject; +import io.github.thebusybiscuit.slimefun4.api.events.SlimefunItemRegistryFinalizedEvent; import io.github.thebusybiscuit.slimefun4.api.items.SlimefunItem; import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; import io.github.thebusybiscuit.slimefun4.implementation.SlimefunItems; @@ -77,6 +78,8 @@ public static void loadItems() { } } + Bukkit.getPluginManager().callEvent(new SlimefunItemRegistryFinalizedEvent()); + loadOreGrinderRecipes(); loadSmelteryRecipes(); diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/api/events/TestSlimefunRegistryFinalizedEvent.java b/src/test/java/io/github/thebusybiscuit/slimefun4/api/events/TestSlimefunRegistryFinalizedEvent.java new file mode 100644 index 0000000000..bef63828c9 --- /dev/null +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/api/events/TestSlimefunRegistryFinalizedEvent.java @@ -0,0 +1,42 @@ +package io.github.thebusybiscuit.slimefun4.api.events; + +import org.junit.jupiter.api.Assertions; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import be.seeseemelk.mockbukkit.MockBukkit; +import be.seeseemelk.mockbukkit.ServerMock; +import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; +import io.github.thebusybiscuit.slimefun4.implementation.setup.PostSetup; + +class TestSlimefunRegistryFinalizedEvent { + + private static ServerMock server; + private static Slimefun plugin; + + @BeforeAll + public static void load() { + server = MockBukkit.mock(); + plugin = MockBukkit.load(Slimefun.class); + } + + @AfterAll + public static void unload() { + MockBukkit.unmock(); + } + + @Test + @DisplayName("Test that SlimefunRegistryFinalizedEvent is fired") + void testEventIsFired() { + // Make sure post setup does not throw + Assertions.assertDoesNotThrow(() -> PostSetup.loadItems()); + + // Make sure post setup sent the event + server.getPluginManager().assertEventFired(SlimefunItemRegistryFinalizedEvent.class, ignored -> true); + + server.getPluginManager().clearEvents(); + } +} From c95bcc927f9720f03f0016bafc24a76848e4f574 Mon Sep 17 00:00:00 2001 From: J3fftw <44972470+J3fftw1@users.noreply.github.com> Date: Fri, 19 Jan 2024 20:57:13 +0100 Subject: [PATCH 15/20] fix slimefun blocks turning into vanilla blocks for 1.20.1 and lower (#4105) --- .../slimefun4/utils/ArmorStandUtils.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/utils/ArmorStandUtils.java b/src/main/java/io/github/thebusybiscuit/slimefun4/utils/ArmorStandUtils.java index 033e3ca489..5635a040c2 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/utils/ArmorStandUtils.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/utils/ArmorStandUtils.java @@ -2,8 +2,7 @@ import javax.annotation.Nonnull; -import io.github.thebusybiscuit.slimefun4.api.MinecraftVersion; -import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; +import io.papermc.lib.PaperLib; import org.bukkit.Location; import org.bukkit.entity.ArmorStand; @@ -49,8 +48,12 @@ public class ArmorStandUtils { * @return The spawned {@link ArmorStand} */ public static @Nonnull ArmorStand spawnArmorStand(@Nonnull Location location) { - // 1.19 and below don't have the consumer method so flicker exists on these versions. - if (Slimefun.getMinecraftVersion().isBefore(MinecraftVersion.MINECRAFT_1_20)) { + // The consumer method was moved from World to RegionAccessor in 1.20.2 + // Due to this, we need to use a rubbish workaround to support 1.20.1 and below + // This causes flicker on these versions which sucks but not sure a better way around this right now. + if (PaperLib.getMinecraftVersion() <= 20 + && PaperLib.getMinecraftPatchVersion() < 2 + ) { ArmorStand armorStand = location.getWorld().spawn(location, ArmorStand.class); setupArmorStand(armorStand); return armorStand; From 31c7c4ead629bdb51226e358ed6be48b89477cc8 Mon Sep 17 00:00:00 2001 From: JustAHuman-xD <65748158+JustAHuman-xD@users.noreply.github.com> Date: Sat, 20 Jan 2024 15:10:17 -0600 Subject: [PATCH 16/20] Make SlimefunItem#getOptionalByItem static like intended (#4098) --- .../github/thebusybiscuit/slimefun4/api/items/SlimefunItem.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItem.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItem.java index 28dc680ff1..6400d8b925 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItem.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItem.java @@ -1199,7 +1199,7 @@ public final int hashCode() { * The {@link ItemStack} to check * @return The {@link Optional} {@link SlimefunItem} associated with this {@link ItemStack} if present, otherwise empty */ - public @Nonnull Optional getOptionalByItem(@Nullable ItemStack item) { + public static @Nonnull Optional getOptionalByItem(@Nullable ItemStack item) { return Optional.ofNullable(getByItem(item)); } } From cd3672c3f29dcb9d02d02cd1c80758c3badb6931 Mon Sep 17 00:00:00 2001 From: ybw0014 Date: Sun, 21 Jan 2024 09:49:07 -0500 Subject: [PATCH 17/20] fix: fix spawn in ArmorStandUtils (#4109) --- .../thebusybiscuit/slimefun4/utils/ArmorStandUtils.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/utils/ArmorStandUtils.java b/src/main/java/io/github/thebusybiscuit/slimefun4/utils/ArmorStandUtils.java index 5635a040c2..e2cf5ae10f 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/utils/ArmorStandUtils.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/utils/ArmorStandUtils.java @@ -21,6 +21,8 @@ */ public class ArmorStandUtils { + private ArmorStandUtils() {} + /** * Spawns an {@link ArmorStand} at the given {@link Location} with the given custom name *
@@ -51,15 +53,14 @@ public class ArmorStandUtils { // The consumer method was moved from World to RegionAccessor in 1.20.2 // Due to this, we need to use a rubbish workaround to support 1.20.1 and below // This causes flicker on these versions which sucks but not sure a better way around this right now. - if (PaperLib.getMinecraftVersion() <= 20 - && PaperLib.getMinecraftPatchVersion() < 2 - ) { + if (PaperLib.getMinecraftVersion() < 20 || + (PaperLib.getMinecraftVersion() == 20 && PaperLib.getMinecraftPatchVersion() < 2)) { ArmorStand armorStand = location.getWorld().spawn(location, ArmorStand.class); setupArmorStand(armorStand); return armorStand; } - return location.getWorld().spawn(location, ArmorStand.class, armorStand -> setupArmorStand(armorStand)); + return location.getWorld().spawn(location, ArmorStand.class, ArmorStandUtils::setupArmorStand); } private static void setupArmorStand(ArmorStand armorStand) { From da9c2ac4cc825a3a567cd0d5ac547941cb6c1822 Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Wed, 7 Feb 2024 09:26:09 +0000 Subject: [PATCH 18/20] Move PlayerProfile saving off the main thread (#4119) * Move PlayerProfile off main thread, add debugs and improve tab completion for debug Moved the PlayerProfile saving off the main thread, we generally load this off-thread but now we also save off-thread. I thought we were already doing this but apparently not, especially with our current YAML stuff this should definitely be done Also done a small change to ensure that we don't remove the PlayerProfile from memory if the player is still online. I don't think we ever had a reported issue from this but it's kinda weird behaviour Finally, added some debug logs to the saving logic, this can be enabled with `sf debug slimefun_player_profile_data`. Also added auto-complete to /sf debug because it's nice, this only works for Slimefun test cases rather than addons but that's fine. Mostly internal anyway * Update src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java --------- Co-authored-by: Alessio Colombo <37039432+Sfiguz7@users.noreply.github.com> --- .../slimefun4/api/player/PlayerProfile.java | 5 ++++- .../core/commands/SlimefunTabCompleter.java | 8 +++++++ .../slimefun4/core/debug/TestCase.java | 12 ++++++++++- .../core/services/AutoSavingService.java | 21 ++++++++++++++++--- 4 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java index 96290f4849..7185471363 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java @@ -8,7 +8,6 @@ import java.util.Set; import java.util.UUID; import java.util.function.Consumer; -import java.util.stream.IntStream; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -35,6 +34,8 @@ import io.github.thebusybiscuit.slimefun4.api.researches.Research; import io.github.thebusybiscuit.slimefun4.core.attributes.ProtectionType; import io.github.thebusybiscuit.slimefun4.core.attributes.ProtectiveArmor; +import io.github.thebusybiscuit.slimefun4.core.debug.Debug; +import io.github.thebusybiscuit.slimefun4.core.debug.TestCase; import io.github.thebusybiscuit.slimefun4.core.guide.GuideHistory; import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; import io.github.thebusybiscuit.slimefun4.implementation.items.armor.SlimefunArmorPiece; @@ -237,6 +238,7 @@ public void removeWaypoint(@Nonnull Waypoint waypoint) { * The profile can then be removed from RAM. */ public final void markForDeletion() { + Debug.log(TestCase.PLAYER_PROFILE_DATA, "Marking {} ({}) profile for deletion", name, ownerId); markedForDeletion = true; } @@ -244,6 +246,7 @@ public final void markForDeletion() { * Call this method if this Profile has unsaved changes. */ public final void markDirty() { + Debug.log(TestCase.PLAYER_PROFILE_DATA, "Marking {} ({}) profile as dirty", name, ownerId); dirty = true; } diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/core/commands/SlimefunTabCompleter.java b/src/main/java/io/github/thebusybiscuit/slimefun4/core/commands/SlimefunTabCompleter.java index 07ca70bf0c..50a665f308 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/core/commands/SlimefunTabCompleter.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/core/commands/SlimefunTabCompleter.java @@ -16,6 +16,7 @@ import io.github.thebusybiscuit.slimefun4.api.items.SlimefunItem; import io.github.thebusybiscuit.slimefun4.api.researches.Research; +import io.github.thebusybiscuit.slimefun4.core.debug.TestCase; import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; class SlimefunTabCompleter implements TabCompleter { @@ -33,6 +34,13 @@ public SlimefunTabCompleter(@Nonnull SlimefunCommand command) { public List onTabComplete(CommandSender sender, Command cmd, String label, String[] args) { if (args.length == 1) { return createReturnList(command.getSubCommandNames(), args[0]); + } else if (args.length == 2) { + if (args[0].equalsIgnoreCase("debug")) { + return createReturnList(TestCase.VALUES_LIST, args[1]); + } else { + // Returning null will make it fallback to the default arguments (all online players) + return null; + } } else if (args.length == 3) { if (args[0].equalsIgnoreCase("give")) { return createReturnList(getSlimefunItems(), args[2]); diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/core/debug/TestCase.java b/src/main/java/io/github/thebusybiscuit/slimefun4/core/debug/TestCase.java index 00b3bbf70c..dec31592d2 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/core/debug/TestCase.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/core/debug/TestCase.java @@ -1,5 +1,7 @@ package io.github.thebusybiscuit.slimefun4.core.debug; +import java.util.Arrays; +import java.util.List; import java.util.Locale; import javax.annotation.Nonnull; @@ -17,7 +19,15 @@ public enum TestCase { * being checked and why it is comparing IDs or meta. * This is helpful for us to check into why input nodes are taking a while for servers. */ - CARGO_INPUT_TESTING; + CARGO_INPUT_TESTING, + + /** + * Debug information regarding player profile loading, saving and handling. + * This is an area we're currently changing quite a bit and this will help ensure we're doing it safely + */ + PLAYER_PROFILE_DATA; + + public static final List VALUES_LIST = Arrays.stream(values()).map(TestCase::toString).toList(); TestCase() {} diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/core/services/AutoSavingService.java b/src/main/java/io/github/thebusybiscuit/slimefun4/core/services/AutoSavingService.java index a0455323f6..060ce0d772 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/core/services/AutoSavingService.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/core/services/AutoSavingService.java @@ -13,6 +13,8 @@ import org.bukkit.entity.Player; import io.github.thebusybiscuit.slimefun4.api.player.PlayerProfile; +import io.github.thebusybiscuit.slimefun4.core.debug.Debug; +import io.github.thebusybiscuit.slimefun4.core.debug.TestCase; import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; import me.mrCookieSlime.Slimefun.api.BlockStorage; @@ -39,9 +41,8 @@ public class AutoSavingService { public void start(@Nonnull Slimefun plugin, int interval) { this.interval = interval; - plugin.getServer().getScheduler().runTaskTimer(plugin, this::saveAllPlayers, 2000L, interval * 60L * 20L); + plugin.getServer().getScheduler().runTaskTimerAsynchronously(plugin, this::saveAllPlayers, 2000L, interval * 60L * 20L); plugin.getServer().getScheduler().runTaskTimerAsynchronously(plugin, this::saveAllBlocks, 2000L, interval * 60L * 20L); - } /** @@ -52,16 +53,30 @@ private void saveAllPlayers() { Iterator iterator = PlayerProfile.iterator(); int players = 0; + Debug.log(TestCase.PLAYER_PROFILE_DATA, "Saving all players data"); + while (iterator.hasNext()) { PlayerProfile profile = iterator.next(); if (profile.isDirty()) { players++; profile.save(); + + Debug.log(TestCase.PLAYER_PROFILE_DATA, "Saved data for {} ({})", + profile.getPlayer() != null ? profile.getPlayer().getName() : "Unknown", profile.getUUID() + ); } - if (profile.isMarkedForDeletion()) { + // Remove the PlayerProfile from memory if the player has left the server (marked from removal) + // and they're still not on the server + // At this point, we've already saved their profile so we can safely remove it + // without worry for having a data sync issue (e.g. data is changed but then we try to re-load older data) + if (profile.isMarkedForDeletion() && profile.getPlayer() == null) { iterator.remove(); + + Debug.log(TestCase.PLAYER_PROFILE_DATA, "Removed data from memory for {}", + profile.getUUID() + ); } } From 86fa6f890019eef8d474b485552dfb8f00a4415d Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Sat, 10 Feb 2024 02:09:44 +0000 Subject: [PATCH 19/20] Add update warning to /sf versions (#4096) --- pom.xml | 2 +- .../commands/subcommands/VersionsCommand.java | 18 ++++++++++++-- .../core/services/UpdaterService.java | 24 +++++++++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index 48e7d9db95..73c46273c6 100644 --- a/pom.xml +++ b/pom.xml @@ -349,7 +349,7 @@ com.github.baked-libs.dough dough-api - fcdbd45aa0 + 1108163a49 compile diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/core/commands/subcommands/VersionsCommand.java b/src/main/java/io/github/thebusybiscuit/slimefun4/core/commands/subcommands/VersionsCommand.java index da4b8d4e06..24abb7364c 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/core/commands/subcommands/VersionsCommand.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/core/commands/subcommands/VersionsCommand.java @@ -65,11 +65,25 @@ public void onExecute(@Nonnull CommandSender sender, @Nonnull String[] args) { .append(serverSoftware) .color(ChatColor.GREEN) .append(" " + Bukkit.getVersion() + '\n') - .color(ChatColor.DARK_GREEN) + .color(ChatColor.DARK_GREEN); + + builder .append("Slimefun ") .color(ChatColor.GREEN) - .append(Slimefun.getVersion() + '\n') + .append(Slimefun.getVersion()) .color(ChatColor.DARK_GREEN); + if (!Slimefun.getUpdater().isLatestVersion()) { + builder + .append(" (").color(ChatColor.GRAY) + .append("Update available").color(ChatColor.RED).event(new HoverEvent(HoverEvent.Action.SHOW_TEXT, new Text( + "Your Slimefun version is out of date!\n" + + "Please update to get the latest bug fixes and performance improvements.\n" + + "Please do not report any bugs without updating first." + ))) + .append(")").color(ChatColor.GRAY); + } + + builder.append("\n"); // @formatter:on if (Slimefun.getMetricsService().getVersion() != null) { diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/core/services/UpdaterService.java b/src/main/java/io/github/thebusybiscuit/slimefun4/core/services/UpdaterService.java index b556789a91..ac21007fce 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/core/services/UpdaterService.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/core/services/UpdaterService.java @@ -1,6 +1,7 @@ package io.github.thebusybiscuit.slimefun4.core.services; import java.io.File; +import java.util.concurrent.ExecutionException; import java.util.logging.Level; import javax.annotation.Nonnull; @@ -110,6 +111,29 @@ public int getBuildNumber() { return -1; } + public int getLatestVersion() { + if (updater != null && updater.getLatestVersion().isDone()) { + PrefixedVersion version; + try { + version = updater.getLatestVersion().get(); + return version.getVersionNumber(); + } catch (InterruptedException | ExecutionException e) { + return -1; + } + } + + return -1; + } + + public boolean isLatestVersion() { + if (getBuildNumber() == -1 || getLatestVersion() == -1) { + // We don't know if we're latest so just report we are + return true; + } + + return getBuildNumber() == getLatestVersion(); + } + /** * This will start the {@link UpdaterService} and check for updates. * If it can find an update it will automatically be installed. From 98bc59efc91d2b66396822dc0228b89ec27382b1 Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Sat, 10 Feb 2024 10:43:52 +0000 Subject: [PATCH 20/20] Fixes #4123 - Coal Generator will no longer be locked after researching (#4124) Due to a logic bug in the Legacy storage backend if there was a duplicate ID it would mark it as researched for the first, then see it researched already and remove it on the second. This was happening for the Coal Generator and Bio Reactor here. Both shared he same research ID 173. We're just doing this fix for now until we can move away from the legacy backend (work in progress). --- pom.xml | 13 +++++ .../storage/backend/legacy/LegacyStorage.java | 12 ++++- .../storage/backend/TestLegacyBackend.java | 49 ++++++++++++++++++- 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index 73c46273c6..411d3b015c 100644 --- a/pom.xml +++ b/pom.xml @@ -406,8 +406,21 @@ org.jetbrains annotations + + io.papermc.paper + paper-api + + + + + + io.papermc.paper + paper-api + 1.20.4-R0.1-20240205.114523-90 + test + diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/backend/legacy/LegacyStorage.java b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/backend/legacy/LegacyStorage.java index d7981a5466..59b0c82b96 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/backend/legacy/LegacyStorage.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/backend/legacy/LegacyStorage.java @@ -91,7 +91,17 @@ public void savePlayerData(@Nonnull UUID uuid, @Nonnull PlayerData data) { playerFile.setValue("researches." + research.getID(), true); // Remove the research if it's no longer researched - } else if (playerFile.contains("researches." + research.getID())) { + // ---- + // We have a duplicate ID (173) used for both Coal Gen and Bio Reactor + // If you researched the Goal Gen we would remove it on save if you didn't also have the Bio Reactor + // Due to the fact we would set it as researched (true in the branch above) on Coal Gen + // but then go into this branch and remove it if you didn't have Bio Reactor + // Sooooo we're gonna hack this for now while we move away from the Legacy Storage + // Let's make sure the user doesn't have _any_ research with this ID and _then_ remove it + } else if ( + playerFile.contains("researches." + research.getID()) + && !data.getResearches().stream().anyMatch((r) -> r.getID() == research.getID()) + ) { playerFile.setValue("researches." + research.getID(), null); } } diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java b/src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java index c8e1916f54..98653ee5b7 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java @@ -17,6 +17,7 @@ import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.meta.ItemMeta; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -52,8 +53,6 @@ public static void load() { // within the class isn't being fired (where ItemStack and other classes are registered) ConfigurationSerialization.registerClass(ItemStack.class); ConfigurationSerialization.registerClass(ItemMeta.class); - - setupResearches(); } @AfterAll @@ -62,9 +61,16 @@ public static void unload() throws IOException { FileUtils.deleteDirectory(new File("data-storage")); } + @AfterEach + public void cleanup() { + Slimefun.getRegistry().getResearches().clear(); + } + // Test simple loading and saving of player data @Test void testLoadingResearches() throws IOException { + setupResearches(); + // Create a player file which we can load UUID uuid = UUID.randomUUID(); File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); @@ -184,6 +190,8 @@ void testLoadingWaypoints() throws IOException { @Test void testSavingResearches() throws InterruptedException { + setupResearches(); + // Create a player file which we can load UUID uuid = UUID.randomUUID(); File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); @@ -279,6 +287,8 @@ void testSavingWaypoints() throws InterruptedException { // Test realistic situations @Test void testResearchChanges() throws InterruptedException { + setupResearches(); + UUID uuid = UUID.randomUUID(); File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); @@ -372,6 +382,41 @@ void testWaypointChanges() throws InterruptedException { Assertions.assertEquals(1, assertion.getWaypoints().size()); } + @Test + void testDuplicateResearchesDontGetUnResearched() throws InterruptedException { + // Create a player file which we can load + UUID uuid = UUID.randomUUID(); + File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); + + OfflinePlayer player = Bukkit.getOfflinePlayer(uuid); + PlayerProfile profile = TestUtilities.awaitProfile(player); + + // Setup initial research + NamespacedKey initialKey = new NamespacedKey(plugin, "test_1"); + Research initialResearch = new Research(initialKey, 1, "Test 1", 100); + initialResearch.register(); + + // Setup duplicate research + // Keep the ID as 1 but change name and key + NamespacedKey duplicateKey = new NamespacedKey(plugin, "test_2"); + Research duplicateResearch = new Research(duplicateKey, 1, "Test 2", 100); + duplicateResearch.register(); + + profile.setResearched(initialResearch, true); + + // Save the player data + LegacyStorage storage = new LegacyStorage(); + storage.savePlayerData(uuid, profile.getPlayerData()); + + // Assert the file exists and data is correct + Assertions.assertTrue(playerFile.exists()); + PlayerData assertion = storage.loadPlayerData(uuid); + // Will have both the initial and duplicate research + Assertions.assertEquals(2, assertion.getResearches().size()); + Assertions.assertTrue(assertion.getResearches().contains(initialResearch)); + Assertions.assertTrue(assertion.getResearches().contains(duplicateResearch)); + } + // Utils private static void setupResearches() { for (int i = 0; i < 10; i++) {