From 56178b0e0a0e3026e30fddd73efe668707277669 Mon Sep 17 00:00:00 2001 From: Brian Neumann-Fopiano Date: Tue, 13 Oct 2026 16:39:39 -0400 Subject: [PATCH] Fixed loot materialization - Container loot finishes on its owning thread before pending markers clear - Failed fills restore container contents and deferred native loot state - Failed materialization remains pending without refilling completed containers - Vanilla tables resolve from the current registry after reload - Studio loot previews follow the player scheduler - Tests cover loot rollback, retries, cancellation, and container classification --- .../arcane/iris/command/CommandStudio.java | 39 +- .../service/ModdedChunkUpdateService.java | 12 +- .../ModdedBlockResolutionContractTest.java | 39 ++ .../iris/modded/ModdedLootApplierTest.java | 203 +++++++- .../service/ModdedChunkUpdateServiceTest.java | 44 +- .../runtime/WorldChunkMaintenance.java | 11 +- .../platform/generation/EngineBukkitOps.java | 147 +++--- .../structure/object/WorldObjectPlacer.java | 6 +- .../iris/world/event/IrisLootEvent.java | 40 +- .../iris/world/loot/IrisVanillaLootTable.java | 11 +- .../WorldChunkMaintenanceRoutingTest.java | 5 + .../BukkitStorageClassificationTest.java | 54 +++ .../generation/EngineBukkitOpsLootTest.java | 443 ++++++++++++++---- .../world/loot/IrisVanillaLootTableTest.java | 76 +++ 14 files changed, 875 insertions(+), 255 deletions(-) create mode 100644 core/src/test/java/art/arcane/iris/platform/bukkit/BukkitStorageClassificationTest.java create mode 100644 core/src/test/java/art/arcane/iris/world/loot/IrisVanillaLootTableTest.java diff --git a/adapters/bukkit/plugin/src/main/java/art/arcane/iris/command/CommandStudio.java b/adapters/bukkit/plugin/src/main/java/art/arcane/iris/command/CommandStudio.java index 493df97fb..f5074e0c7 100644 --- a/adapters/bukkit/plugin/src/main/java/art/arcane/iris/command/CommandStudio.java +++ b/adapters/bukkit/plugin/src/main/java/art/arcane/iris/command/CommandStudio.java @@ -81,7 +81,6 @@ import org.bukkit.FluidCollisionMode; import org.bukkit.Location; import org.bukkit.World; import org.bukkit.entity.Player; -import org.bukkit.event.inventory.InventoryType; import org.bukkit.inventory.Inventory; import org.bukkit.util.Vector; @@ -309,23 +308,33 @@ public class CommandStudio implements DirectorExecutor { O ta = new O<>(); ta.set(-1); - var sender = sender(); - var player = player(); - var engine = engine(); + VolmitSender sender = sender(); + Player player = player(); + Engine engine = engine(); - ta.set(J.sr(() -> - { - if (!player.getOpenInventory().getType().equals(InventoryType.CHEST)) { + ta.set(J.sr(() -> { + if (!J.runEntity(player, () -> { + if (player.getOpenInventory().getTopInventory() != inv) { + J.csr(ta.get()); + sender.sendMessage(IrisLanguage.text(BukkitCommandMessages.COMMAND_STUDIO_OPENED_INVENTORY)); + return; + } + if (!add) { + inv.clear(); + } + Location location = player.getLocation(); + try { + EngineBukkitOps.addItems(engine, true, inv, tables, InventorySlotType.STORAGE, + location.getWorld(), location.getBlockX(), location.getBlockY(), location.getBlockZ()); + } catch (RuntimeException | Error failure) { + J.csr(ta.get()); + Iris.reportError(failure); + sender.sendMessage(IrisLanguage.text(BukkitCommandMessagesExtended.COMMAND_STUDIO_CANNOT_ADD_ITEMS_VIRTUAL_INVENTORY_BECAUSE, + MessageArgument.untrusted("value", String.valueOf(failure.getMessage())))); + } + }, 0, () -> J.csr(ta.get()))) { J.csr(ta.get()); - sender.sendMessage(IrisLanguage.text(BukkitCommandMessages.COMMAND_STUDIO_OPENED_INVENTORY)); - return; } - - if (!add) { - inv.clear(); - } - - EngineBukkitOps.addItems(engine, true, inv, tables, InventorySlotType.STORAGE, player.getWorld(), player.getLocation().getBlockX(), player.getLocation().getBlockY(), player.getLocation().getBlockZ()); }, fast ? 5 : 35)); sender().sendMessage(IrisLanguage.text(BukkitCommandMessagesExtended.COMMAND_STUDIO_OPENING_INVENTORY_NOW)); diff --git a/adapters/modded-common/src/main/java/art/arcane/iris/modded/service/ModdedChunkUpdateService.java b/adapters/modded-common/src/main/java/art/arcane/iris/modded/service/ModdedChunkUpdateService.java index 5fe28ac2d..13356e977 100644 --- a/adapters/modded-common/src/main/java/art/arcane/iris/modded/service/ModdedChunkUpdateService.java +++ b/adapters/modded-common/src/main/java/art/arcane/iris/modded/service/ModdedChunkUpdateService.java @@ -30,7 +30,6 @@ import art.arcane.iris.studio.view.PregeneratorJob; import art.arcane.iris.integration.Identifier; import art.arcane.iris.generation.cache.Cache; import art.arcane.iris.generation.runtime.Engine; -import art.arcane.iris.generation.runtime.GenerationFailures; import art.arcane.iris.generation.block.TileData; import art.arcane.iris.modded.IrisModdedChunkGenerator; import art.arcane.iris.modded.ModdedBlockResolution; @@ -382,16 +381,7 @@ public final class ModdedChunkUpdateService implements ModdedTickableService { if (!state.isStorageChest()) { return; } - try { - ModdedLootApplier.apply(engine, level, pos, state, chunk); - } catch (SavedBiomeUnavailableException unavailable) { - GenerationFailures.rethrowEngineFailure(unavailable); - IrisLogging.warnOnce("chunk-update-loot:" + level.name() + ":" + unavailable.getMessage(), - "Iris left the container at " + pos + " in " + level.name() + " without loot: " + unavailable.getMessage()); - } catch (Throwable e) { - GenerationFailures.rethrowEngineFailure(e); - IrisLogging.reportError(e); - } + ModdedLootApplier.apply(engine, level, pos, state, chunk); } else { NativeWorldMaintenance.refresh(level, pos); } diff --git a/adapters/modded-common/src/test/java/art/arcane/iris/modded/ModdedBlockResolutionContractTest.java b/adapters/modded-common/src/test/java/art/arcane/iris/modded/ModdedBlockResolutionContractTest.java index f096efb51..81dd2a00d 100644 --- a/adapters/modded-common/src/test/java/art/arcane/iris/modded/ModdedBlockResolutionContractTest.java +++ b/adapters/modded-common/src/test/java/art/arcane/iris/modded/ModdedBlockResolutionContractTest.java @@ -8,9 +8,12 @@ import art.arcane.volmlib.nativelib.terrain.NativeBlockState; import art.arcane.volmlib.util.hunk.Hunk; import net.minecraft.SharedConstants; +import net.minecraft.core.BlockPos; import net.minecraft.server.Bootstrap; +import net.minecraft.world.RandomizableContainer; import net.minecraft.world.level.block.Block; import net.minecraft.world.level.block.Blocks; +import net.minecraft.world.level.block.EntityBlock; import net.minecraft.world.level.block.SpeleothemBlock; import net.minecraft.world.level.block.state.properties.SpeleothemThickness; import org.junit.BeforeClass; @@ -62,6 +65,42 @@ public class ModdedBlockResolutionContractTest { assertEquals(Blocks.OAK_LOG, state.handle().getBlock()); } + @Test + public void copperChestVariantsReceiveIrisLootAndPostLoadUpdates() { + for (String key : new String[]{"copper_chest", "exposed_copper_chest", "weathered_copper_chest", + "oxidized_copper_chest", "waxed_copper_chest", "waxed_exposed_copper_chest", + "waxed_weathered_copper_chest", "waxed_oxidized_copper_chest"}) { + ModdedBlockState state = ModdedBlockResolution.BLOCKS.getOrNull("minecraft:" + key); + assertNotNull(key, state); + assertTrue(key, state.isStorage()); + assertTrue(key, state.isStorageChest()); + assertTrue(key, NativeBlockProperties.isUpdatable(state.handle())); + assertTrue(key, ((EntityBlock) state.handle().getBlock()).newBlockEntity(BlockPos.ZERO, state.handle()) + instanceof RandomizableContainer); + } + } + + @Test + public void lootContainerClassificationPreservesExistingContainerBoundaries() { + for (String key : new String[]{"chest", "trapped_chest", "barrel", "shulker_box", + "blue_shulker_box", "dispenser", "dropper", "hopper"}) { + ModdedBlockState state = ModdedBlockResolution.BLOCKS.getOrNull("minecraft:" + key); + assertNotNull(key, state); + assertTrue(key, state.isStorage()); + assertTrue(key, state.isStorageChest()); + } + for (Block block : new Block[]{Blocks.FURNACE, Blocks.BLAST_FURNACE, Blocks.SMOKER}) { + assertTrue(block.toString(), NativeBlockProperties.isStorage(block.defaultBlockState())); + assertFalse(block.toString(), NativeBlockProperties.isStorageChest(block.defaultBlockState())); + } + for (String key : new String[]{"stone", "air", "ender_chest", "copper_block"}) { + ModdedBlockState state = ModdedBlockResolution.BLOCKS.getOrNull("minecraft:" + key); + assertNotNull(key, state); + assertFalse(key, state.isStorage()); + assertFalse(key, state.isStorageChest()); + } + } + @Test public void cactusPlacementRequiresNativeCactusSupport() { assertTrue(NativeBlockProperties.canPlaceOnto(Blocks.CACTUS, Blocks.CACTUS)); diff --git a/adapters/modded-common/src/test/java/art/arcane/iris/modded/ModdedLootApplierTest.java b/adapters/modded-common/src/test/java/art/arcane/iris/modded/ModdedLootApplierTest.java index 2ceb3d812..261e935ab 100644 --- a/adapters/modded-common/src/test/java/art/arcane/iris/modded/ModdedLootApplierTest.java +++ b/adapters/modded-common/src/test/java/art/arcane/iris/modded/ModdedLootApplierTest.java @@ -1,33 +1,64 @@ package art.arcane.iris.modded; -import art.arcane.volmlib.nativelib.minecraft26_2.modded.NativeContainerLoot; - import art.arcane.iris.structure.placement.LootResolver; import art.arcane.iris.world.loot.IrisLootMode; +import art.arcane.volmlib.nativelib.minecraft26_2.modded.NativeContainerLoot; +import art.arcane.volmlib.nativelib.minecraft26_2.modded.NativeItemStack; +import art.arcane.volmlib.nativelib.terrain.NativeBlockPoint; +import art.arcane.volmlib.nativelib.terrain.NativeBlockState; +import art.arcane.volmlib.nativelib.terrain.NativeWorld; import art.arcane.volmlib.util.math.RNG; -import net.minecraft.resources.Identifier; -import net.minecraft.resources.ResourceKey; -import net.minecraft.core.BlockPos; -import net.minecraft.core.registries.Registries; -import net.minecraft.world.RandomizableContainer; -import net.minecraft.world.SimpleContainer; -import net.minecraft.world.item.ItemStack; -import net.minecraft.world.level.storage.loot.LootTable; -import org.junit.Test; - +import java.lang.reflect.Proxy; import java.util.ArrayList; import java.util.List; import java.util.Set; -import java.lang.reflect.Proxy; import java.util.concurrent.atomic.AtomicBoolean; +import net.minecraft.SharedConstants; +import net.minecraft.core.BlockPos; +import net.minecraft.core.component.DataComponentMap; +import net.minecraft.core.component.DataComponents; +import net.minecraft.core.registries.Registries; +import net.minecraft.resources.Identifier; +import net.minecraft.resources.ResourceKey; +import net.minecraft.server.Bootstrap; +import net.minecraft.server.level.ServerLevel; +import net.minecraft.world.CompoundContainer; +import net.minecraft.world.RandomizableContainer; +import net.minecraft.world.SimpleContainer; +import net.minecraft.world.entity.player.Player; +import net.minecraft.world.item.ItemStack; +import net.minecraft.world.item.Items; +import net.minecraft.world.level.block.Blocks; +import net.minecraft.world.level.block.ChestBlock; +import net.minecraft.world.level.block.entity.ChestBlockEntity; +import net.minecraft.world.level.block.state.BlockState; +import net.minecraft.world.level.block.state.properties.ChestType; +import net.minecraft.world.level.storage.loot.LootTable; +import org.junit.BeforeClass; +import org.junit.Test; +import org.mockito.MockedStatic; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; public class ModdedLootApplierTest { + @BeforeClass + public static void bootstrapMinecraftRegistries() { + SharedConstants.tryDetectVersion(); + Bootstrap.bootStrap(); + if (!Items.DIAMOND.builtInRegistryHolder().areComponentsBound()) { + Items.DIAMOND.builtInRegistryHolder().bindComponents(DataComponentMap.EMPTY); + } + } + @Test public void addAppendsSourcesInOrder() { List sources = new ArrayList<>(List.of("placement-native")); @@ -171,6 +202,152 @@ public class ModdedLootApplierTest { assertTrue(NativeContainerLoot.hasNativeLootTable(nativeLoot)); } + @Test + public void failedFillRestoresContentsAndDoesNotDuplicateLootOnRetry() { + RejectingContainer container = new RejectingContainer(); + container.setItem(0, diamondStack(2)); + assertEquals(2, container.getItem(0).getCount()); + container.reject = true; + ItemStack reward = diamondStack(7); + + assertThrows(IllegalStateException.class, + () -> NativeContainerLoot.fillContainer(container, List.of(reward), new RNG(17), message -> {})); + + assertEquals(2, container.getItem(0).getCount()); + assertTrue(container.getItem(1).isEmpty()); + assertEquals(7, reward.getCount()); + NativeContainerLoot.fillContainer(container, List.of(reward), new RNG(17), message -> {}); + assertEquals(9, container.getItem(0).getCount() + container.getItem(1).getCount()); + assertEquals(7, reward.getCount()); + } + + @Test + public void failedNativeUnpackRestoresTheLootTableSeedAndPriorContents() { + DeferredLootChest container = new DeferredLootChest(BlockPos.ZERO, Blocks.CHEST.defaultBlockState()); + container.setItem(0, diamondStack(2)); + container.reject = true; + ResourceKey table = ResourceKey.create(Registries.LOOT_TABLE, Identifier.parse("minecraft:chests/simple_dungeon")); + container.setLootTable(table, 41L); + + assertThrows(IllegalStateException.class, () -> NativeContainerLoot.fillContainer(container, + List.of(new ItemStack(Items.DIAMOND)), new RNG(17), message -> {})); + + assertSame(table, container.getLootTable()); + assertEquals(41L, container.getLootTableSeed()); + container.setLootTable(null); + assertEquals(2, container.getItem(0).getCount()); + for (int slot = 1; slot < container.getContainerSize(); slot++) { + assertTrue(container.getItem(slot).isEmpty()); + } + } + + @Test + public void failedDoubleChestFillRestoresBothDeferredHalvesBeforeRetry() { + BlockPos leftPos = new BlockPos(0, 64, 0); + BlockPos rightPos = new BlockPos(1, 64, 0); + BlockState leftState = Blocks.CHEST.defaultBlockState().setValue(ChestBlock.TYPE, ChestType.LEFT); + BlockState rightState = Blocks.CHEST.defaultBlockState().setValue(ChestBlock.TYPE, ChestType.RIGHT); + DeferredLootChest left = new DeferredLootChest(leftPos, leftState); + DeferredLootChest right = new DeferredLootChest(rightPos, rightState); + left.setItem(0, diamondStack(2)); + right.setItem(0, diamondStack(3)); + right.reject = true; + ResourceKey table = ResourceKey.create(Registries.LOOT_TABLE, Identifier.parse("minecraft:chests/simple_dungeon")); + left.setLootTable(table, 41L); + right.setLootTable(table, 42L); + ServerLevel level = mock(ServerLevel.class); + NativeWorld world = mock(NativeWorld.class); + NativeBlockState state = mock(NativeBlockState.class); + when(world.nativeHandle()).thenReturn(level); + when(state.nativeHandle()).thenReturn(leftState); + when(level.getBlockEntity(leftPos)).thenReturn(left); + when(level.getBlockEntity(rightPos)).thenReturn(right); + CompoundContainer inventory = new CompoundContainer(left, right); + ItemStack reward = diamondStack(7); + try (MockedStatic chests = mockStatic(ChestBlock.class)) { + chests.when(() -> ChestBlock.getContainer((ChestBlock) Blocks.CHEST, leftState, level, leftPos, true)) + .thenReturn(inventory); + chests.when(() -> ChestBlock.getConnectedBlockPos(leftPos, leftState)).thenReturn(rightPos); + NativeContainerLoot access = new NativeContainerLoot(new NativeContainerLoot.Target( + world, new NativeBlockPoint(0, 64, 0), state)); + NativeContainerLoot.Session session = access.open(); + assertNotNull(session); + session.add(List.of(NativeItemStack.of(reward))); + + assertThrows(IllegalStateException.class, () -> session.fill(new RNG(17), message -> {})); + + assertSame(table, left.getLootTable()); + assertSame(table, right.getLootTable()); + assertEquals(41L, left.getLootTableSeed()); + assertEquals(42L, right.getLootTableSeed()); + left.setLootTable(null); + right.setLootTable(null); + assertEquals(2, countItems(left)); + assertEquals(3, countItems(right)); + assertEquals(7, reward.getCount()); + left.setLootTable(table, 41L); + right.setLootTable(table, 42L); + right.reject = false; + + session.fill(new RNG(17), message -> {}); + + assertNull(left.getLootTable()); + assertNull(right.getLootTable()); + assertEquals(15, countItems(left) + countItems(right)); + assertEquals(7, reward.getCount()); + } + } + + private static int countItems(ChestBlockEntity container) { + int total = 0; + for (int slot = 0; slot < container.getContainerSize(); slot++) { + total += container.getItem(slot).getCount(); + } + return total; + } + + private static ItemStack diamondStack(int count) { + ItemStack stack = new ItemStack(Items.DIAMOND, count); + stack.set(DataComponents.MAX_STACK_SIZE, 64); + return stack; + } + + private static final class RejectingContainer extends SimpleContainer { + private boolean reject; + + private RejectingContainer() { + super(2); + } + + @Override + public void setItem(int slot, ItemStack stack) { + super.setItem(slot, stack); + if (reject) { + reject = false; + throw new IllegalStateException("Inventory write failed"); + } + } + } + + private static final class DeferredLootChest extends ChestBlockEntity { + private boolean reject; + + private DeferredLootChest(BlockPos position, BlockState state) { + super(position, state); + } + + @Override + public void unpackLootTable(Player player) { + if (getLootTable() != null) { + setLootTable(null); + setItem(0, diamondStack(4)); + if (reject) { + throw new IllegalStateException("Native loot failed"); + } + } + } + } + private RandomizableContainer containerWithLootTable(ResourceKey key) { return (RandomizableContainer) Proxy.newProxyInstance( RandomizableContainer.class.getClassLoader(), diff --git a/adapters/modded-common/src/test/java/art/arcane/iris/modded/service/ModdedChunkUpdateServiceTest.java b/adapters/modded-common/src/test/java/art/arcane/iris/modded/service/ModdedChunkUpdateServiceTest.java index dfc35ba19..3993e2a6a 100644 --- a/adapters/modded-common/src/test/java/art/arcane/iris/modded/service/ModdedChunkUpdateServiceTest.java +++ b/adapters/modded-common/src/test/java/art/arcane/iris/modded/service/ModdedChunkUpdateServiceTest.java @@ -57,7 +57,6 @@ import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.contains; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.inOrder; @@ -171,7 +170,7 @@ public class ModdedChunkUpdateServiceTest { } @Test - public void permanentSavedBiomeUnavailabilityCompletesThePassWithAWarning() { + public void permanentSavedBiomeUnavailabilityRetainsPendingLoot() { ChestUpdateMantleChunk chunk = new ChestUpdateMantleChunk(); Engine engine = updateEngine(chunk); NativeWorld level = chestLevel(); @@ -187,13 +186,46 @@ public class ModdedChunkUpdateServiceTest { return null; }); - new ModdedChunkUpdateService().updateRegeneratedChunk(engine, level, 0, 0); + assertThrows(SavedBiomeUnavailableException.class, + () -> new ModdedChunkUpdateService().updateRegeneratedChunk(engine, level, 0, 0)); + + assertFalse(chunk.isFlagged(MantleFlag.ETCHED)); + assertFalse(chunk.isFlagged(MantleFlag.UPDATE)); + assertFalse(chunk.deleted()); + verify(chunk.slice, never()).set(2, 5, 2, null); + + } + } + + @Test + public void ordinaryLootFailureRetainsPendingWorkAndRetries() { + ChestUpdateMantleChunk chunk = new ChestUpdateMantleChunk(); + Engine engine = updateEngine(chunk); + NativeWorld level = chestLevel(); + AtomicInteger attempts = new AtomicInteger(); + ModdedChunkUpdateService service = new ModdedChunkUpdateService(); + IllegalStateException failure = new IllegalStateException("Loot provider unavailable"); + try (MockedStatic loot = mockStatic(ModdedLootApplier.class)) { + loot.when(() -> ModdedLootApplier.apply(any(), any(), any(), any(), any())).thenAnswer(invocation -> { + NativeBlockPoint position = invocation.getArgument(2); + if (position.x() == 2 && attempts.getAndIncrement() == 0) { + throw failure; + } + return null; + }); + + assertSame(failure, assertThrows(IllegalStateException.class, + () -> service.updateRegeneratedChunk(engine, level, 0, 0))); + assertFalse(chunk.isFlagged(MantleFlag.UPDATE)); + assertFalse(chunk.isFlagged(MantleFlag.ETCHED)); + assertFalse(chunk.deleted()); + verify(chunk.slice, never()).set(2, 5, 2, null); + + service.updateRegeneratedChunk(engine, level, 0, 0); assertTrue(chunk.isFlagged(MantleFlag.ETCHED)); assertTrue(chunk.deleted()); - logging.verify(() -> IrisLogging.warnOnce(anyString(), contains(reason))); - logging.verify(() -> IrisLogging.reportError(any(Throwable.class)), never()); - logging.verify(() -> IrisLogging.reportError(anyString(), any(Throwable.class)), never()); + assertEquals(2, attempts.get()); } } diff --git a/core/src/main/java/art/arcane/iris/generation/runtime/WorldChunkMaintenance.java b/core/src/main/java/art/arcane/iris/generation/runtime/WorldChunkMaintenance.java index 4f6b6d082..688e45074 100644 --- a/core/src/main/java/art/arcane/iris/generation/runtime/WorldChunkMaintenance.java +++ b/core/src/main/java/art/arcane/iris/generation/runtime/WorldChunkMaintenance.java @@ -289,6 +289,8 @@ final class WorldChunkMaintenance { boolean scheduled = J.runRegion(world, chunkX, chunkZ, manager.managedTask("bukkit_world_manager_chunk_update", () -> { try { updateChunkRegion(world, chunkX, chunkZ); + } catch (RuntimeException | Error failure) { + reportMaterializationFailure(new MaterializationTarget(world, chunkX, chunkZ), failure); } finally { chunkUpdateQueue.remove(key); } @@ -508,6 +510,13 @@ final class WorldChunkMaintenance { return loadedChunkPositions; } + private void reportMaterializationFailure(MaterializationTarget target, Throwable failure) { + String context = "Failed to materialize chunk " + target.x() + "," + target.z() + " in " + target.world().getName() + "."; + if (IrisLogging.errorOnce("chunk-materialization:" + target.world().getUID() + ":" + target.x() + "," + target.z() + ":" + failure, context)) { + IrisLogging.reportError(context, failure); + } + } + private record MaterializationTarget(World world, int x, int z) { private long key() { return Cache.key(x, z); @@ -614,7 +623,7 @@ final class WorldChunkMaintenance { } spawnInitially(chunk); } catch (Throwable failure) { - IrisLogging.reportError("Failed to materialize saved chunk at " + target.x() + "," + target.z() + ".", failure); + reportMaterializationFailure(target, failure); } finally { synchronized (this) { applying = false; diff --git a/core/src/main/java/art/arcane/iris/platform/generation/EngineBukkitOps.java b/core/src/main/java/art/arcane/iris/platform/generation/EngineBukkitOps.java index fddf19988..6a1bb99ce 100644 --- a/core/src/main/java/art/arcane/iris/platform/generation/EngineBukkitOps.java +++ b/core/src/main/java/art/arcane/iris/platform/generation/EngineBukkitOps.java @@ -20,7 +20,6 @@ package art.arcane.iris.platform.generation; import art.arcane.iris.world.history.GenerationFindCatalog; -import art.arcane.iris.platform.bukkit.BukkitWorldBinding; import art.arcane.iris.world.event.IrisLootEvent; import art.arcane.iris.integration.Identifier; import art.arcane.iris.integration.ExternalDataSVC; @@ -73,6 +72,7 @@ import org.bukkit.World; import org.bukkit.block.Block; import org.bukkit.block.BlockFace; import org.bukkit.block.BlockState; +import org.bukkit.block.DoubleChest; import org.bukkit.block.data.BlockData; import org.bukkit.block.data.type.Chest; import org.bukkit.entity.Player; @@ -82,6 +82,8 @@ import org.bukkit.inventory.ItemStack; import org.bukkit.loot.Lootable; import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.List; import java.util.UUID; import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; @@ -89,9 +91,7 @@ import java.util.concurrent.CompletionException; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicLong; -import java.util.function.BooleanSupplier; import java.util.function.Consumer; -import java.util.function.Predicate; import art.arcane.iris.localization.BukkitRuntimeMessages; import art.arcane.iris.localization.IrisLanguage; @@ -124,14 +124,7 @@ public final class EngineBukkitOps { } if (!J.isFolia() && !J.isPrimaryThread()) { - CompletableFuture scheduled = J.sfut(() -> updateChunk(engine, c, mantle)); - if (scheduled != null) { - try { - scheduled.join(); - } catch (Throwable e) { - IrisLogging.reportError(e); - } - } + awaitMaterialization(J.sfut(() -> updateChunk(engine, c, mantle))); return; } @@ -303,21 +296,25 @@ public final class EngineBukkitOps { }); if (!scheduled) { - if (J.isPrimaryThread()) { - runnable.run(); - } - return; + throw new IllegalStateException("Failed to schedule chunk materialization for " + + contextChunk.getWorld().getName() + " at chunk " + contextChunk.getX() + "," + contextChunk.getZ() + "."); } + awaitMaterialization(completion); + } + + private static void awaitMaterialization(CompletableFuture completion) { try { completion.join(); } catch (CompletionException error) { Throwable cause = error.getCause(); - IrisLogging.reportError( - "Failed deferred chunk materialization for " + contextChunk.getWorld().getName() - + " at chunk " + contextChunk.getX() + "," + contextChunk.getZ() + ".", - cause == null ? error : cause - ); + if (cause instanceof RuntimeException runtimeFailure) { + throw runtimeFailure; + } + if (cause instanceof Error fatalFailure) { + throw fatalFailure; + } + throw error; } } @@ -351,8 +348,12 @@ public final class EngineBukkitOps { InventoryHolder m = (InventoryHolder) block.getState(); addItems(engine, false, m.getInventory(), tables, slot, c.getWorld(), worldX, y, worldZ); - } catch (Throwable e) { - IrisLogging.reportError(e); + } catch (RuntimeException | Error failure) { + if (IrisLogging.errorOnce("container-loot:" + world.getUID() + ":" + worldX + "," + y + "," + worldZ + ":" + failure, + "Failed to populate loot at " + world.getName() + " [" + worldX + "," + y + "," + worldZ + "].")) { + IrisLogging.reportError(failure); + } + throw failure; } } } else { @@ -515,6 +516,10 @@ public final class EngineBukkitOps { } public static void addItems(Engine engine, boolean debug, Inventory inv, KList tables, InventorySlotType slot, World world, int x, int y, int z) { + if (!J.isOwnedByCurrentRegion(world, x >> 4, z >> 4)) { + throw new IllegalStateException("Loot inventory mutation requires the owning region at " + + world.getName() + " [" + x + "," + y + "," + z + "]."); + } KList items = new KList<>(); for (IrisLootTable i : tables) { @@ -525,57 +530,69 @@ public final class EngineBukkitOps { if (IrisLootEvent.callLootEvent(items, inv, world, x, y, z)) return; - if (PaperLib.isPaper() && engine.getWorld().hasPlatformWorld()) { - PaperLib.getChunkAtAsync(BukkitWorldBinding.world(engine.getWorld()), x >> 4, z >> 4).thenAccept((c) -> { - Runnable mutation = () -> { - for (ItemStack i : items) { - inv.addItem(i); - } + populateInventory(engine, inv, items, LootResolver.containerRng(engine.getSeedManager().getLoot(), x, y, z)); + } - scramble(engine, inv, LootResolver.containerRng(engine.getSeedManager().getLoot(), x, y, z)); - }; - - LootMutationTask task = new LootMutationTask(world, x >> 4, z >> 4, mutation); - boolean scheduled = dispatchLootMutation( - task, - J.isFolia(), - Bukkit::isPrimaryThread, - regionTask -> J.runRegion( - regionTask.world(), - regionTask.chunkX(), - regionTask.chunkZ(), - regionTask.mutation()), - J::s - ); - if (!scheduled) { - IrisLogging.error("Failed to schedule loot inventory mutation for " + world.getName() + "@" + (x >> 4) + "," + (z >> 4) + "."); - } - }); + static void populateInventory(Engine engine, Inventory inv, List items, RNG rng) { + List states = new ArrayList<>(2); + InventoryHolder holder = inv.getHolder(); + if (holder instanceof DoubleChest chest) { + snapshotContainer(states, chest.getLeftSide()); + snapshotContainer(states, chest.getRightSide()); } else { - for (ItemStack i : items) { - inv.addItem(i); + snapshotContainer(states, holder); + } + ItemStack[] contents = null; + try { + if (states.isEmpty()) { + contents = copyContents(inv.getContents()); } - - scramble(engine, inv, LootResolver.containerRng(engine.getSeedManager().getLoot(), x, y, z)); + for (ItemStack i : items) { + if (i != null) { + inv.addItem(i.clone()); + } + } + scramble(engine, inv, rng); + } catch (RuntimeException | Error failure) { + if (states.isEmpty()) { + if (contents != null) { + try { + inv.setContents(contents); + } catch (RuntimeException | Error rollbackFailure) { + if (rollbackFailure != failure) { + failure.addSuppressed(rollbackFailure); + } + } + } + } else { + for (BlockState state : states) { + try { + if (!state.update(true, false)) { + throw new IllegalStateException("Failed to restore loot container at " + state.getLocation()); + } + } catch (RuntimeException | Error rollbackFailure) { + if (rollbackFailure != failure) { + failure.addSuppressed(rollbackFailure); + } + } + } + } + throw failure; } } - static boolean dispatchLootMutation( - LootMutationTask task, - boolean folia, - BooleanSupplier primaryThread, - Predicate regionScheduler, - Consumer syncScheduler) { - if (folia) { - return regionScheduler.test(task); + private static void snapshotContainer(List snapshots, InventoryHolder holder) { + if (holder instanceof BlockState state) { + snapshots.add(state.getBlock().getState()); } + } - if (primaryThread.getAsBoolean()) { - task.mutation().run(); - } else { - syncScheduler.accept(task.mutation()); + private static ItemStack[] copyContents(ItemStack[] contents) { + ItemStack[] copy = new ItemStack[contents.length]; + for (int index = 0; index < contents.length; index++) { + copy[index] = contents[index] == null ? null : contents[index].clone(); } - return true; + return copy; } public static IrisBiome getBiome(Engine engine, Location l) { @@ -838,6 +855,4 @@ public final class EngineBukkitOps { } } - record LootMutationTask(World world, int chunkX, int chunkZ, Runnable mutation) { - } } diff --git a/core/src/main/java/art/arcane/iris/structure/object/WorldObjectPlacer.java b/core/src/main/java/art/arcane/iris/structure/object/WorldObjectPlacer.java index e48f158f1..11a6eb55f 100644 --- a/core/src/main/java/art/arcane/iris/structure/object/WorldObjectPlacer.java +++ b/core/src/main/java/art/arcane/iris/structure/object/WorldObjectPlacer.java @@ -93,8 +93,10 @@ public class WorldObjectPlacer implements IObjectPlacer { } InventoryHolder holder = (InventoryHolder) block.getState(); EngineBukkitOps.addItems(engine, false, holder.getInventory(), tables, InventorySlotType.STORAGE, world, x, y, z); - } catch (Throwable e) { - IrisLogging.reportError(e); + } catch (RuntimeException | Error failure) { + IrisLogging.reportError("Failed to populate placed container at " + world.getName() + + " [" + x + "," + y + "," + z + "].", failure); + throw failure; } } diff --git a/core/src/main/java/art/arcane/iris/world/event/IrisLootEvent.java b/core/src/main/java/art/arcane/iris/world/event/IrisLootEvent.java index e3acddfdf..4c89ddf8b 100644 --- a/core/src/main/java/art/arcane/iris/world/event/IrisLootEvent.java +++ b/core/src/main/java/art/arcane/iris/world/event/IrisLootEvent.java @@ -4,7 +4,6 @@ import art.arcane.iris.generation.runtime.Engine; import art.arcane.iris.world.loot.InventorySlotType; import art.arcane.iris.world.loot.IrisLootTable; import art.arcane.iris.platform.bukkit.BukkitPlatform; -import art.arcane.iris.spi.IrisLogging; import art.arcane.volmlib.util.collection.KList; import art.arcane.iris.world.task.J; import lombok.Getter; @@ -92,6 +91,10 @@ public class IrisLootEvent extends Event { * @return true when the event was canceled */ public static boolean callLootEvent(KList loot, Inventory inv, World world, int x, int y, int z) { + if (!J.isOwnedByCurrentRegion(world, x >> 4, z >> 4)) { + throw new IllegalStateException("LootGenerateEvent requires the owning region at " + + world.getName() + " [" + x + "," + y + "," + z + "]."); + } InventoryHolder holder = inv.getHolder(); Location loc = new Location(world, x, y, z); if (holder == null) { @@ -106,39 +109,8 @@ public class IrisLootEvent extends Event { LootContext context = new LootContext.Builder(loc).build(); LootGenerateEvent event = new LootGenerateEvent(world, null, holder, EMPTY, context, loot, true); - if (!Bukkit.isPrimaryThread()) { - IrisLogging.warn("LootGenerateEvent was not called on the main thread, please report this issue."); - Thread.dumpStack(); - J.sfut(() -> { - try { - Bukkit.getPluginManager().callEvent(event); - } catch (Throwable e) { - IrisLogging.reportError("LootGenerateEvent dispatch failed at " - + world.getName() + " [" + x + "," + y + "," + z + "].", e); - if (e instanceof RuntimeException runtimeException) { - throw runtimeException; - } - if (e instanceof Error error) { - throw error; - } - throw new IllegalStateException(e); - } - }).join(); - } else { - try { - Bukkit.getPluginManager().callEvent(event); - } catch (Throwable e) { - IrisLogging.reportError("LootGenerateEvent dispatch failed at " - + world.getName() + " [" + x + "," + y + "," + z + "].", e); - if (e instanceof RuntimeException runtimeException) { - throw runtimeException; - } - if (e instanceof Error error) { - throw error; - } - throw new IllegalStateException(e); - } - } + + Bukkit.getPluginManager().callEvent(event); return event.isCancelled(); } diff --git a/core/src/main/java/art/arcane/iris/world/loot/IrisVanillaLootTable.java b/core/src/main/java/art/arcane/iris/world/loot/IrisVanillaLootTable.java index 7f8284b08..a96a9c20b 100644 --- a/core/src/main/java/art/arcane/iris/world/loot/IrisVanillaLootTable.java +++ b/core/src/main/java/art/arcane/iris/world/loot/IrisVanillaLootTable.java @@ -33,13 +33,12 @@ public class IrisVanillaLootTable extends IrisLootTable { private final String lootTableKey; /** - * The resolved table, looked up once. Every chest pull resolved it again - a NamespacedKey parse plus a registry - * lookup per container, and a dungeon room is a lot of containers. + * Cache the parsed key; resolve its table from the current registry for every container. *

* Transient for the same reason the field above is a String: it keeps the Bukkit type argument out of the Gson * field walk and out of the pack purity gate, both of which resolve generic field types for the fields they visit. */ - private final transient AtomicCache resolvedTable = new AtomicCache<>(); + private final transient AtomicCache resolvedKey = new AtomicCache<>(); @Override public String getName() { @@ -83,10 +82,8 @@ public class IrisVanillaLootTable extends IrisLootTable { } private LootTable resolveTable() { - return resolvedTable.aquire(() -> { - NamespacedKey key = NamespacedKey.fromString(lootTableKey); - return key == null ? null : Bukkit.getLootTable(key); - }); + NamespacedKey key = resolvedKey.aquire(() -> NamespacedKey.fromString(lootTableKey)); + return key == null ? null : Bukkit.getLootTable(key); } @Override diff --git a/core/src/test/java/art/arcane/iris/generation/runtime/WorldChunkMaintenanceRoutingTest.java b/core/src/test/java/art/arcane/iris/generation/runtime/WorldChunkMaintenanceRoutingTest.java index 8b4f00aaf..016a19477 100644 --- a/core/src/test/java/art/arcane/iris/generation/runtime/WorldChunkMaintenanceRoutingTest.java +++ b/core/src/test/java/art/arcane/iris/generation/runtime/WorldChunkMaintenanceRoutingTest.java @@ -175,6 +175,7 @@ public class WorldChunkMaintenanceRoutingTest { public void failedMaterializationRetainsRetryAndReportsTheFailure() throws Exception { try (Fixture fixture = new Fixture()) { IllegalStateException failure = new IllegalStateException("Provider placement failed"); + fixture.logging.when(() -> IrisLogging.errorOnce(anyString(), anyString())).thenReturn(true, false); fixture.updates.when(() -> EngineBukkitOps.updateChunk(fixture.engine, fixture.chunk, fixture.savedMantle)).thenThrow(failure); fixture.schedule(); fixture.prepare(); @@ -182,6 +183,10 @@ public class WorldChunkMaintenanceRoutingTest { fixture.apply(); fixture.awaitRelease(); + fixture.schedule(); + fixture.prepare(); + fixture.apply(); + fixture.awaitRelease(); fixture.logging.verify(() -> IrisLogging.reportError(anyString(), eq(failure))); fixture.schedule(); assertEquals(1, fixture.async.size()); diff --git a/core/src/test/java/art/arcane/iris/platform/bukkit/BukkitStorageClassificationTest.java b/core/src/test/java/art/arcane/iris/platform/bukkit/BukkitStorageClassificationTest.java new file mode 100644 index 000000000..59d22b974 --- /dev/null +++ b/core/src/test/java/art/arcane/iris/platform/bukkit/BukkitStorageClassificationTest.java @@ -0,0 +1,54 @@ +package art.arcane.iris.platform.bukkit; + +import art.arcane.volmlib.util.data.BSupport; +import org.bukkit.Bukkit; +import org.bukkit.Material; +import org.bukkit.block.data.BlockData; +import org.junit.Test; +import org.mockito.MockedStatic; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; + +public class BukkitStorageClassificationTest { + @Test + public void copperChestsAndExistingLootContainersReceiveStorageUpdates() { + BlockData air = blockData(Material.AIR); + try (MockedStatic bukkit = mockStatic(Bukkit.class)) { + bukkit.when(() -> Bukkit.createBlockData(Material.AIR)).thenReturn(air); + BSupport support = new BSupport() { + }; + for (Material material : new Material[]{Material.COPPER_CHEST, Material.EXPOSED_COPPER_CHEST, + Material.WEATHERED_COPPER_CHEST, Material.OXIDIZED_COPPER_CHEST, + Material.WAXED_COPPER_CHEST, Material.WAXED_EXPOSED_COPPER_CHEST, + Material.WAXED_WEATHERED_COPPER_CHEST, Material.WAXED_OXIDIZED_COPPER_CHEST, + Material.CHEST, Material.TRAPPED_CHEST, Material.BARREL, Material.SHULKER_BOX, + Material.BLUE_SHULKER_BOX, Material.DISPENSER, Material.DROPPER, Material.HOPPER}) { + BlockData state = blockData(material); + assertTrue(material.name(), support.isStorage(state)); + assertTrue(material.name(), support.isStorageChest(state)); + assertTrue(material.name(), support.isUpdatable(state)); + } + for (Material material : new Material[]{Material.FURNACE, Material.BLAST_FURNACE, Material.SMOKER}) { + BlockData state = blockData(material); + assertTrue(material.name(), support.isStorage(state)); + assertFalse(material.name(), support.isStorageChest(state)); + } + for (Material material : new Material[]{Material.STONE, Material.AIR, Material.ENDER_CHEST, + Material.COPPER_BLOCK, Material.COPPER_CHESTPLATE}) { + BlockData state = blockData(material); + assertFalse(material.name(), support.isStorage(state)); + assertFalse(material.name(), support.isStorageChest(state)); + } + } + } + + private static BlockData blockData(Material material) { + BlockData state = mock(BlockData.class); + when(state.getMaterial()).thenReturn(material); + return state; + } +} diff --git a/core/src/test/java/art/arcane/iris/platform/generation/EngineBukkitOpsLootTest.java b/core/src/test/java/art/arcane/iris/platform/generation/EngineBukkitOpsLootTest.java index b3c331a63..9ad22498f 100644 --- a/core/src/test/java/art/arcane/iris/platform/generation/EngineBukkitOpsLootTest.java +++ b/core/src/test/java/art/arcane/iris/platform/generation/EngineBukkitOpsLootTest.java @@ -1,20 +1,66 @@ package art.arcane.iris.platform.generation; -import org.bukkit.World; -import org.bukkit.block.Block; -import org.bukkit.block.BlockFace; -import org.bukkit.block.data.type.Chest; -import org.bukkit.loot.LootTable; -import org.junit.Test; - +import art.arcane.iris.generation.runtime.Engine; +import art.arcane.iris.generation.runtime.EngineMetrics; +import art.arcane.iris.platform.bukkit.BukkitBlockResolution; +import art.arcane.iris.spi.IrisLogging; +import art.arcane.iris.testsupport.BukkitTestServer; +import art.arcane.iris.world.event.IrisLootEvent; +import art.arcane.iris.world.loot.InventorySlotType; +import art.arcane.iris.world.loot.IrisLootTable; +import art.arcane.iris.world.task.J; +import art.arcane.volmlib.util.collection.KList; +import art.arcane.volmlib.util.function.Consumer4; +import art.arcane.volmlib.util.mantle.flag.MantleFlag; +import art.arcane.volmlib.util.mantle.runtime.MantleChunk; +import art.arcane.volmlib.util.mantle.runtime.MantleDataAdapter; +import art.arcane.volmlib.util.mantle.runtime.MantleHooks; +import art.arcane.volmlib.util.math.RNG; +import art.arcane.volmlib.util.matter.Matter; +import art.arcane.volmlib.util.matter.MatterUpdate; +import art.arcane.volmlib.util.matter.slices.UpdateMatter; +import io.papermc.paper.registry.RegistryAccess; +import java.util.HashMap; +import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import org.bukkit.Bukkit; +import org.bukkit.Chunk; +import org.bukkit.Material; +import org.bukkit.World; +import org.bukkit.block.Barrel; +import org.bukkit.block.Block; +import org.bukkit.block.BlockFace; +import org.bukkit.block.BlockState; +import org.bukkit.block.DoubleChest; +import org.bukkit.block.data.BlockData; +import org.bukkit.block.data.type.Chest; +import org.bukkit.event.world.LootGenerateEvent; +import org.bukkit.inventory.Inventory; +import org.bukkit.inventory.ItemStack; +import org.bukkit.loot.LootTable; +import org.bukkit.plugin.PluginManager; +import org.junit.Test; +import org.mockito.MockedStatic; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.RETURNS_DEEP_STUBS; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; public class EngineBukkitOpsLootTest { @@ -46,117 +92,314 @@ public class EngineBukkitOpsLootTest { } @Test - public void foliaLootMutationUsesOwningRegionOnly() { - World world = mock(World.class); - AtomicInteger primaryChecks = new AtomicInteger(); - AtomicInteger regionSchedules = new AtomicInteger(); - AtomicInteger syncSchedules = new AtomicInteger(); - AtomicBoolean mutated = new AtomicBoolean(); - Runnable mutation = () -> mutated.set(true); - EngineBukkitOps.LootMutationTask task = new EngineBukkitOps.LootMutationTask(world, 12, -3, mutation); + public void failedInsertionRestoresExistingLootBeforeRetry() { + Inventory inventory = mock(Inventory.class); + AtomicReference contents = new AtomicReference<>(new ItemStack[]{item(2), null}); + AtomicBoolean reject = new AtomicBoolean(true); + when(inventory.getSize()).thenReturn(2); + when(inventory.getContents()).thenAnswer(invocation -> contents.get()); + doAnswer(invocation -> { + contents.set(invocation.getArgument(0)); + return null; + }).when(inventory).setContents(any(ItemStack[].class)); + doAnswer(invocation -> { + ItemStack incoming = invocation.getArgument(0); + contents.get()[0].setAmount(contents.get()[0].getAmount() + incoming.getAmount()); + if (reject.getAndSet(false)) { + throw new IllegalStateException("Insertion failed"); + } + return new HashMap(); + }).when(inventory).addItem(any(ItemStack.class)); + ItemStack reward = item(7); - boolean scheduled = EngineBukkitOps.dispatchLootMutation( - task, - true, - () -> { - primaryChecks.incrementAndGet(); - return true; - }, - regionTask -> { - regionSchedules.incrementAndGet(); - assertSame(world, regionTask.world()); - assertEquals(12, regionTask.chunkX()); - assertEquals(-3, regionTask.chunkZ()); - assertSame(mutation, regionTask.mutation()); - regionTask.mutation().run(); - return true; - }, - runnable -> syncSchedules.incrementAndGet() - ); + assertThrows(IllegalStateException.class, + () -> EngineBukkitOps.populateInventory(null, inventory, List.of(reward), new RNG(17))); - assertTrue(scheduled); - assertTrue(mutated.get()); - assertEquals(1, regionSchedules.get()); - assertEquals(0, primaryChecks.get()); - assertEquals(0, syncSchedules.get()); + assertEquals(2, contents.get()[0].getAmount()); + EngineBukkitOps.populateInventory(null, inventory, List.of(reward), new RNG(17)); + int total = 0; + for (ItemStack item : contents.get()) { + total += item == null ? 0 : item.getAmount(); + } + assertEquals(9, total); + assertEquals(7, reward.getAmount()); } @Test - public void failedFoliaRegionScheduleDoesNotMutateInventoryElsewhere() { - World world = mock(World.class); - AtomicInteger primaryChecks = new AtomicInteger(); - AtomicInteger syncSchedules = new AtomicInteger(); - AtomicBoolean mutated = new AtomicBoolean(); - EngineBukkitOps.LootMutationTask task = new EngineBukkitOps.LootMutationTask(world, 1, 2, () -> mutated.set(true)); + public void failedNativeInventoryAccessRestoresBothChestSnapshots() { + Inventory inventory = mock(Inventory.class); + DoubleChest holder = mock(DoubleChest.class); + org.bukkit.block.Chest left = mock(org.bukkit.block.Chest.class); + org.bukkit.block.Chest right = mock(org.bukkit.block.Chest.class); + Block leftBlock = mock(Block.class); + Block rightBlock = mock(Block.class); + BlockState leftSnapshot = mock(BlockState.class); + BlockState rightSnapshot = mock(BlockState.class); + when(inventory.getHolder()).thenReturn(holder); + when(holder.getLeftSide()).thenReturn(left); + when(holder.getRightSide()).thenReturn(right); + when(left.getBlock()).thenReturn(leftBlock); + when(right.getBlock()).thenReturn(rightBlock); + when(leftBlock.getState()).thenReturn(leftSnapshot); + when(rightBlock.getState()).thenReturn(rightSnapshot); + when(leftSnapshot.update(true, false)).thenReturn(true); + when(rightSnapshot.update(true, false)).thenReturn(true); + IllegalStateException failure = new IllegalStateException("Native loot failed"); + doThrow(failure).when(inventory).addItem(any(ItemStack.class)); - boolean scheduled = EngineBukkitOps.dispatchLootMutation( - task, - true, - () -> { - primaryChecks.incrementAndGet(); - return true; - }, - regionTask -> false, - runnable -> syncSchedules.incrementAndGet() - ); + assertSame(failure, assertThrows(IllegalStateException.class, + () -> EngineBukkitOps.populateInventory(null, inventory, + List.of(item(1)), new RNG(17)))); - assertFalse(scheduled); - assertFalse(mutated.get()); - assertEquals(0, primaryChecks.get()); - assertEquals(0, syncSchedules.get()); + verify(leftSnapshot).update(true, false); + verify(rightSnapshot).update(true, false); } @Test - public void nonFoliaPrimaryThreadMutatesInline() { - World world = mock(World.class); - AtomicInteger regionSchedules = new AtomicInteger(); - AtomicInteger syncSchedules = new AtomicInteger(); - AtomicBoolean mutated = new AtomicBoolean(); - EngineBukkitOps.LootMutationTask task = new EngineBukkitOps.LootMutationTask(world, 4, 5, () -> mutated.set(true)); + public void failedBarrelFillRestoresItsBlockSnapshot() { + Inventory inventory = mock(Inventory.class); + Barrel holder = mock(Barrel.class); + Block block = mock(Block.class); + Barrel snapshot = mock(Barrel.class); + when(inventory.getHolder()).thenReturn(holder); + when(holder.getBlock()).thenReturn(block); + when(block.getState()).thenReturn(snapshot); + when(snapshot.update(true, false)).thenReturn(true); + IllegalStateException failure = new IllegalStateException("Native barrel loot failed"); + when(inventory.getContents()).thenThrow(failure); - boolean scheduled = EngineBukkitOps.dispatchLootMutation( - task, - false, - () -> true, - regionTask -> { - regionSchedules.incrementAndGet(); - return true; - }, - runnable -> syncSchedules.incrementAndGet() - ); + assertSame(failure, assertThrows(IllegalStateException.class, + () -> EngineBukkitOps.populateInventory(null, inventory, List.of(), new RNG(17)))); - assertTrue(scheduled); - assertTrue(mutated.get()); - assertEquals(0, regionSchedules.get()); - assertEquals(0, syncSchedules.get()); + verify(snapshot).update(true, false); } @Test - public void nonFoliaAsyncCompletionUsesSyncScheduler() { + public void failedRollbackPreservesTheCauseAndRestoresTheOtherHalf() { + Inventory inventory = mock(Inventory.class); + DoubleChest holder = mock(DoubleChest.class); + org.bukkit.block.Chest left = mock(org.bukkit.block.Chest.class, RETURNS_DEEP_STUBS); + org.bukkit.block.Chest right = mock(org.bukkit.block.Chest.class, RETURNS_DEEP_STUBS); + BlockState leftSnapshot = mock(BlockState.class); + BlockState rightSnapshot = mock(BlockState.class); + when(inventory.getHolder()).thenReturn(holder); + when(holder.getLeftSide()).thenReturn(left); + when(holder.getRightSide()).thenReturn(right); + when(left.getBlock().getState()).thenReturn(leftSnapshot); + when(right.getBlock().getState()).thenReturn(rightSnapshot); + when(rightSnapshot.update(true, false)).thenReturn(true); + IllegalStateException failure = new IllegalStateException("Native loot failed"); + IllegalStateException rollbackFailure = new IllegalStateException("Snapshot restore failed"); + when(leftSnapshot.update(true, false)).thenThrow(rollbackFailure); + doThrow(failure).when(inventory).addItem(any(ItemStack.class)); + + assertSame(failure, assertThrows(IllegalStateException.class, + () -> EngineBukkitOps.populateInventory(null, inventory, List.of(item(1)), new RNG(17)))); + + assertEquals(1, failure.getSuppressed().length); + assertSame(rollbackFailure, failure.getSuppressed()[0]); + verify(rightSnapshot).update(true, false); + } + + @Test + public void cancelledLootLeavesTheInventoryUntouched() { + Engine engine = mock(Engine.class, RETURNS_DEEP_STUBS); + Inventory inventory = mock(Inventory.class); World world = mock(World.class); - AtomicInteger regionSchedules = new AtomicInteger(); - AtomicInteger syncSchedules = new AtomicInteger(); - AtomicBoolean mutated = new AtomicBoolean(); - EngineBukkitOps.LootMutationTask task = new EngineBukkitOps.LootMutationTask(world, 6, 7, () -> mutated.set(true)); + try (MockedStatic scheduling = mockStatic(J.class); + MockedStatic events = mockStatic(IrisLootEvent.class)) { + scheduling.when(() -> J.isOwnedByCurrentRegion(world, 2, -2)).thenReturn(true); + events.when(() -> IrisLootEvent.callLootEvent(any(), eq(inventory), eq(world), eq(33), eq(64), eq(-17))) + .thenReturn(true); - boolean scheduled = EngineBukkitOps.dispatchLootMutation( - task, - false, - () -> false, - regionTask -> { - regionSchedules.incrementAndGet(); - return true; - }, - runnable -> { - syncSchedules.incrementAndGet(); - runnable.run(); - } - ); + EngineBukkitOps.addItems(engine, false, inventory, new KList<>(), InventorySlotType.STORAGE, world, 33, 64, -17); - assertTrue(scheduled); - assertTrue(mutated.get()); - assertEquals(0, regionSchedules.get()); - assertEquals(1, syncSchedules.get()); + verifyNoInteractions(inventory); + } + } + + @Test + public void offRegionLootDoesNotRollOrTouchInventory() { + Engine engine = mock(Engine.class); + Inventory inventory = mock(Inventory.class); + World world = mock(World.class); + IrisLootTable table = mock(IrisLootTable.class); + try (MockedStatic scheduling = mockStatic(J.class)) { + assertThrows(IllegalStateException.class, () -> EngineBukkitOps.addItems(engine, false, inventory, + new KList<>(table), InventorySlotType.STORAGE, world, 33, 64, -17)); + + scheduling.verify(() -> J.isOwnedByCurrentRegion(world, 2, -2)); + verifyNoInteractions(table, inventory, engine); + } + } + + @Test + public void owningRegionLootCompletesInlineWithoutGlobalDispatch() { + Engine engine = mock(Engine.class, RETURNS_DEEP_STUBS); + Inventory inventory = mock(Inventory.class); + World world = mock(World.class); + when(inventory.getContents()).thenReturn(new ItemStack[0]); + try (MockedStatic scheduling = mockStatic(J.class); + MockedStatic events = mockStatic(IrisLootEvent.class)) { + scheduling.when(() -> J.isOwnedByCurrentRegion(world, 2, -2)).thenReturn(true); + + EngineBukkitOps.addItems(engine, false, inventory, new KList<>(), InventorySlotType.STORAGE, world, 33, 64, -17); + + verify(inventory).setContents(any(ItemStack[].class)); + scheduling.verify(() -> J.isOwnedByCurrentRegion(world, 2, -2)); + scheduling.verifyNoMoreInteractions(); + } + } + + @Test + public void foliaLootEventRunsOnTheOwningRegionWithoutGlobalDispatch() { + World world = mock(World.class); + Inventory inventory = mock(Inventory.class); + PluginManager manager = mock(PluginManager.class); + Thread owner = Thread.currentThread(); + AtomicInteger eventCalls = new AtomicInteger(); + try (MockedStatic scheduling = mockStatic(J.class); + MockedStatic bukkit = mockStatic(Bukkit.class)) { + scheduling.when(() -> J.isOwnedByCurrentRegion(world, 2, -2)).thenReturn(true); + bukkit.when(Bukkit::getPluginManager).thenReturn(manager); + doAnswer(invocation -> { + assertSame(owner, Thread.currentThread()); + eventCalls.incrementAndGet(); + return null; + }).when(manager).callEvent(any(LootGenerateEvent.class)); + + assertFalse(IrisLootEvent.callLootEvent(new KList<>(), inventory, world, 33, 64, -17)); + + assertEquals(1, eventCalls.get()); + scheduling.verify(() -> J.isOwnedByCurrentRegion(world, 2, -2)); + scheduling.verifyNoMoreInteractions(); + } + } + + @Test + public void lootEventRetainsListenerChangesAndCancellation() { + World world = mock(World.class); + Inventory inventory = mock(Inventory.class); + PluginManager manager = mock(PluginManager.class); + ItemStack original = item(2); + ItemStack replacement = item(3); + KList items = new KList<>(original); + try (MockedStatic scheduling = mockStatic(J.class); + MockedStatic bukkit = mockStatic(Bukkit.class)) { + scheduling.when(() -> J.isOwnedByCurrentRegion(world, 2, -2)).thenReturn(true); + bukkit.when(Bukkit::getPluginManager).thenReturn(manager); + doAnswer(invocation -> { + LootGenerateEvent event = invocation.getArgument(0); + event.setLoot(List.of(replacement)); + event.setCancelled(true); + return null; + }).when(manager).callEvent(any(LootGenerateEvent.class)); + + assertTrue(IrisLootEvent.callLootEvent(items, inventory, world, 33, 64, -17)); + + assertEquals(List.of(replacement), items); + } + } + + @Test + @SuppressWarnings("unchecked") + public void failedLootRollKeepsPendingMarkerUntilSuccessfulRetry() { + BukkitTestServer.install(); + AtomicBoolean pending = new AtomicBoolean(true); + AtomicInteger attempts = new AtomicInteger(); + Engine engine = mock(Engine.class, RETURNS_DEEP_STUBS); + when(engine.getMetrics()).thenReturn(new EngineMetrics(8)); + World world = mock(World.class); + when(world.getMaxHeight()).thenReturn(256); + Chunk chunk = mock(Chunk.class); + when(chunk.getWorld()).thenReturn(world); + Block block = mock(Block.class); + BlockData state = mock(BlockData.class); + org.bukkit.block.Chest chest = mock(org.bukkit.block.Chest.class); + Inventory inventory = mock(Inventory.class); + when(chunk.getBlock(1, 64, 1)).thenReturn(block); + when(block.getBlockData()).thenReturn(state); + when(block.getX()).thenReturn(1); + when(block.getZ()).thenReturn(1); + when(block.getState()).thenReturn(chest); + when(chest.getInventory()).thenReturn(inventory); + when(inventory.getContents()).thenReturn(new ItemStack[0]); + MantleChunk mantle = spy(new MantleChunk<>(16, 0, 0, mock(MantleDataAdapter.class), MantleHooks.NONE)); + doAnswer(invocation -> { + if (pending.get()) { + Consumer4 visitor = invocation.getArgument(1); + visitor.accept(1, 64, 1, UpdateMatter.ON); + } + return null; + }).when(mantle).iterate(eq(MatterUpdate.class), any()); + doAnswer(invocation -> { + pending.set(false); + return null; + }).when(mantle).deleteSlices(MatterUpdate.class); + IrisLootTable table = mock(IrisLootTable.class); + IllegalStateException failure = new IllegalStateException("Loot provider unavailable"); + when(table.getLoot(false, 0L, InventorySlotType.STORAGE, world, 1, 64, 1)).thenAnswer(invocation -> { + if (attempts.getAndIncrement() < 2) { + throw failure; + } + return new KList(); + }); + KList tables = new KList<>(table); + PluginManager manager = mock(PluginManager.class); + try (MockedStatic registries = mockStatic(RegistryAccess.class)) { + registries.when(RegistryAccess::registryAccess).thenReturn(BukkitTestServer.registryAccess()); + try (MockedStatic operations = mockStatic(EngineBukkitOps.class); + MockedStatic resolution = mockStatic(BukkitBlockResolution.class); + MockedStatic bukkit = mockStatic(Bukkit.class); + MockedStatic events = mockStatic(IrisLootEvent.class); + MockedStatic scheduling = mockStatic(J.class); + MockedStatic logging = mockStatic(IrisLogging.class)) { + bukkit.when(Bukkit::getPluginManager).thenReturn(manager); + logging.when(() -> IrisLogging.errorOnce(anyString(), anyString())).thenReturn(true, false); + scheduling.when(() -> J.isOwnedByCurrentRegion(world, 0, 0)).thenReturn(true); + resolution.when(() -> BukkitBlockResolution.isStorage(state)).thenReturn(true); + resolution.when(() -> BukkitBlockResolution.isStorageChest(state)).thenReturn(true); + operations.when(() -> EngineBukkitOps.isCanonicalContainer(block)).thenReturn(true); + operations.when(() -> EngineBukkitOps.getLootTables(eq(engine), any(RNG.class), eq(block), eq(mantle), eq(false))).thenReturn(tables); + operations.when(() -> EngineBukkitOps.addItems(engine, false, inventory, tables, InventorySlotType.STORAGE, world, 1, 64, 1)).thenCallRealMethod(); + operations.when(() -> EngineBukkitOps.populateInventory(eq(engine), eq(inventory), any(), any())).thenCallRealMethod(); + operations.when(() -> EngineBukkitOps.update(engine, 1, 64, 1, chunk, mantle)).thenCallRealMethod(); + operations.when(() -> EngineBukkitOps.materializeUpdates(engine, chunk, mantle)).thenCallRealMethod(); + operations.when(() -> EngineBukkitOps.materializeUpdates(eq(engine), eq(chunk), eq(mantle), any(EngineBukkitOps.UpdateDispatcher.class))).thenCallRealMethod(); + operations.when(() -> EngineBukkitOps.runMaterializationPasses(eq(mantle), any(Runnable.class), any(Runnable.class), any(Runnable.class))).thenCallRealMethod(); + Runnable pass = () -> EngineBukkitOps.runMaterializationPasses(mantle, () -> {}, () -> {}, + () -> EngineBukkitOps.materializeUpdates(engine, chunk, mantle)); + + assertSame(failure, assertThrows(IllegalStateException.class, pass::run)); + assertTrue(pending.get()); + assertFalse(mantle.isFlagged(MantleFlag.UPDATE)); + assertFalse(mantle.isFlagged(MantleFlag.ETCHED)); + assertSame(failure, assertThrows(IllegalStateException.class, pass::run)); + assertTrue(pending.get()); + logging.verify(() -> IrisLogging.reportError(failure)); + + pass.run(); + pass.run(); + + assertFalse(pending.get()); + assertTrue(mantle.isFlagged(MantleFlag.UPDATE)); + assertTrue(mantle.isFlagged(MantleFlag.ETCHED)); + assertEquals(3, attempts.get()); + } + } + } + + private ItemStack item(int amount) { + ItemStack item = mock(ItemStack.class); + AtomicInteger count = new AtomicInteger(amount); + when(item.getType()).thenReturn(Material.DIAMOND); + when(item.getAmount()).thenAnswer(invocation -> count.get()); + when(item.clone()).thenAnswer(invocation -> item(count.get())); + doAnswer(invocation -> { + count.set(invocation.getArgument(0)); + return null; + }).when(item).setAmount(anyInt()); + return item; } private Block chestBlock(int x, int z, Chest.Type type, BlockFace facing) { diff --git a/core/src/test/java/art/arcane/iris/world/loot/IrisVanillaLootTableTest.java b/core/src/test/java/art/arcane/iris/world/loot/IrisVanillaLootTableTest.java new file mode 100644 index 000000000..c2e04eb15 --- /dev/null +++ b/core/src/test/java/art/arcane/iris/world/loot/IrisVanillaLootTableTest.java @@ -0,0 +1,76 @@ +package art.arcane.iris.world.loot; + +import art.arcane.iris.spi.IrisLogging; +import org.bukkit.Bukkit; +import org.bukkit.NamespacedKey; +import org.bukkit.World; +import org.bukkit.inventory.ItemStack; +import org.bukkit.loot.LootTable; +import org.junit.Test; +import org.mockito.MockedStatic; + +import java.util.List; + +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class IrisVanillaLootTableTest { + @Test + public void registryReplacementRemovalAndRestorationApplyToTheNextContainer() { + NamespacedKey key = new NamespacedKey("test", "reloadable_loot"); + World world = mock(World.class); + LootTable before = mock(LootTable.class); + LootTable after = mock(LootTable.class); + ItemStack previousItem = mock(ItemStack.class); + ItemStack currentItem = mock(ItemStack.class); + when(before.populateLoot(any(), any())).thenReturn(List.of(previousItem)); + when(after.populateLoot(any(), any())).thenReturn(List.of(currentItem)); + + try (MockedStatic bukkit = mockStatic(Bukkit.class); + MockedStatic logging = mockStatic(IrisLogging.class)) { + IrisVanillaLootTable table = new IrisVanillaLootTable(key.toString()); + bukkit.when(() -> Bukkit.getLootTable(key)).thenReturn(before); + assertSame(previousItem, table.getLoot(false, 1L, InventorySlotType.STORAGE, world, 0, 64, 0).getFirst()); + + bukkit.when(() -> Bukkit.getLootTable(key)).thenReturn(after); + assertSame(currentItem, table.getLoot(false, 1L, InventorySlotType.STORAGE, world, 1, 64, 0).getFirst()); + + bukkit.when(() -> Bukkit.getLootTable(key)).thenReturn(null); + assertTrue(table.getLoot(false, 1L, InventorySlotType.STORAGE, world, 2, 64, 0).isEmpty()); + + bukkit.when(() -> Bukkit.getLootTable(key)).thenReturn(after); + assertSame(currentItem, table.getLoot(false, 1L, InventorySlotType.STORAGE, world, 3, 64, 0).getFirst()); + verify(before).populateLoot(any(), any()); + verify(after, times(2)).populateLoot(any(), any()); + } + } + + @Test + public void failedTableGenerationDoesNotRetainTheReplacedTable() { + NamespacedKey key = new NamespacedKey("test", "repaired_loot"); + World world = mock(World.class); + LootTable failed = mock(LootTable.class); + LootTable repaired = mock(LootTable.class); + ItemStack item = mock(ItemStack.class); + IllegalStateException failure = new IllegalStateException("Loot generation failed"); + when(failed.populateLoot(any(), any())).thenThrow(failure); + when(repaired.populateLoot(any(), any())).thenReturn(List.of(item)); + + try (MockedStatic bukkit = mockStatic(Bukkit.class)) { + IrisVanillaLootTable table = new IrisVanillaLootTable(key.toString()); + bukkit.when(() -> Bukkit.getLootTable(key)).thenReturn(failed); + assertSame(failure, assertThrows(IllegalStateException.class, + () -> table.getLoot(false, 1L, InventorySlotType.STORAGE, world, 0, 64, 0))); + + bukkit.when(() -> Bukkit.getLootTable(key)).thenReturn(repaired); + assertSame(item, table.getLoot(false, 1L, InventorySlotType.STORAGE, world, 0, 64, 0).getFirst()); + } + } +}