Merged container loot fixes

- Container loot rolls retain retry state and restore contents after failed writes
2026-10-25 13:30:08 +00:00 · 2026-10-13 16:40:04 -04:00
commit 57882a3f1c
@@ -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<Integer> 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));
@@ -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);
        }
@@ -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));
@@ -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<String> 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<LootTable> 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<LootTable> 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<ChestBlock> 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<LootTable> key) {
        return (RandomizableContainer) Proxy.newProxyInstance(
                RandomizableContainer.class.getClassLoader(),
@@ -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<ModdedLootApplier> 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());
        }
    }