Stop emptied loot chests refilling after reload

Co-Authored-By: Codex <[email protected]>

crates/horizon-client/src/game/commands.rs

@@ -2692,7 +2692,7 @@ impl Game<'_> {
             .take()
             .map(std::sync::Arc::unwrap_or_clone)
             .unwrap_or_default();
-        extras.rolled_loot.retain(|r| r.pos != key);
+        horizon_client::save::consume_container_loot(&mut extras, key);
         chunk.saved = Some(std::sync::Arc::new(extras));
         self.world.modified.insert((key.0 >> 4, key.2 >> 4));
     }

crates/horizon-client/src/save.rs

@@ -52,6 +52,8 @@ use std::{
 pub const VERSION_NAME: &str = horizon_core::VANILLA_VERSION;
 
 mod gateway;
+#[cfg(test)]
+mod loot_tests;
 
 /// Snapshot live block entities and custom names for a save batch without
 /// mutating the world. Each map is visited once, retaining its iteration order
@@ -526,6 +528,25 @@ impl SaveTables {
 
 pub use crate::chunk::{ChunkExtras, RolledLoot, SavedProto};
 
+/// A container was opened: its eager roll is now the container's own inventory.
+pub fn consume_container_loot(extras: &mut ChunkExtras, pos: (i32, i32, i32)) {
+    let before = extras.rolled_loot.len();
+    extras.rolled_loot.retain(|r| r.pos != pos);
+    if extras.rolled_loot.len() == before {
+        return;
+    }
+    // The source compound participates in the save overlay. Once opened,
+    // even an emptied chest must not inherit its old loot table from there.
+    for (source_pos, source) in &mut extras.block_entity_sources {
+        if *source_pos == pos
+            && let Tag::Compound(map) = source
+        {
+            map.remove(&"LootTable".into());
+            map.remove(&"LootTableSeed".into());
+        }
+    }
+}
+
 fn tick_list(ticks: &[SavedTick], name: impl Fn(u32) -> Option<String>) -> Tag {
     Tag::List(
         ticks

crates/horizon-client/src/save/loot_tests.rs

@@ -0,0 +1,196 @@
+use super::*;
+
+fn fixture() -> (BlockRegistry, Biomes) {
+    static NEXT: AtomicU64 = AtomicU64::new(0);
+    let root = std::env::temp_dir().join(format!(
+        "horizon-opened-loot-{}-{}",
+        std::process::id(),
+        NEXT.fetch_add(1, Ordering::Relaxed)
+    ));
+    fs::create_dir(&root).unwrap();
+    let maps = root.join("assets/minecraft/textures/colormap");
+    fs::create_dir_all(&maps).unwrap();
+    // Original one-pixel fixtures: chunk persistence only needs biome names.
+    for name in ["grass", "foliage", "dry_foliage"] {
+        let file = fs::File::create(maps.join(format!("{name}.png"))).unwrap();
+        let mut encoder = png::Encoder::new(file, 1, 1);
+        encoder.set_color(png::ColorType::Rgb);
+        encoder
+            .write_header()
+            .unwrap()
+            .write_image_data(&[0, 0, 0])
+            .unwrap();
+    }
+    let folder = root.join("data/minecraft/worldgen/biome");
+    fs::create_dir_all(&folder).unwrap();
+    fs::write(folder.join("plains.json"), "{}").unwrap();
+    let biomes = Biomes::load(&root).unwrap();
+    fs::remove_dir_all(&root).unwrap();
+    let registry = BlockRegistry::from_nbt(&Tag::compound([
+        ("DataVersion", Tag::Int(DATA_VERSION)),
+        (
+            "states",
+            Tag::List(vec![
+                Tag::compound([("id", string("minecraft:air"))]),
+                Tag::compound([("id", string("minecraft:chest"))]),
+            ]),
+        ),
+        (
+            "defaults",
+            Tag::compound([
+                ("minecraft:air", Tag::Int(0)),
+                ("minecraft:chest", Tag::Int(1)),
+            ]),
+        ),
+    ]))
+    .unwrap();
+    (registry, biomes)
+}
+
+fn reload_loot_case(
+    registry: &BlockRegistry,
+    biomes: &Biomes,
+    table: &str,
+    seed: i64,
+    mode: u8,
+) -> Chunk {
+    let pos = (1, 64, 2);
+    let mut slots = vec![None; 27];
+    slots[0] = Some(crate::items::ItemStack::new("minecraft:gold_ingot", 2));
+    let source = Tag::compound([
+        ("id", string("minecraft:chest")),
+        ("x", Tag::Int(pos.0)),
+        ("y", Tag::Int(pos.1)),
+        ("z", Tag::Int(pos.2)),
+        ("LootTable", string(table)),
+        ("LootTableSeed", Tag::Long(seed)),
+        ("CustomName", string("{\"text\":\"Keep me\"}")),
+    ]);
+    let mut extras = ChunkExtras {
+        block_entity_sources: vec![(pos, source)],
+        rolled_loot: vec![RolledLoot {
+            pos,
+            table: table.into(),
+            seed: Some(seed),
+            items: encode_items(&slots),
+        }],
+        ..Default::default()
+    };
+    // Exactly the production transition used when the player opens the chest.
+    if mode != 0 {
+        consume_container_loot(&mut extras, pos);
+        slots.fill(None);
+        if mode == 2 {
+            slots[0] = Some(crate::items::ItemStack::new("minecraft:gold_ingot", 1));
+        }
+    }
+    let mut chunk = Chunk::empty(0, 0);
+    chunk.set(pos.0, pos.1, pos.2, 1);
+    chunk
+        .block_entities
+        .push((pos, crate::containers::BlockEntity::Chest(slots)));
+    chunk.saved = Some(Arc::new(extras));
+    let encoded = encode(&chunk, registry, biomes);
+    let bytes = nbt::write_named(&encoded, Limits::default()).unwrap();
+    let (encoded, used) = nbt::read_named(&bytes, Limits::default()).unwrap();
+    assert_eq!(used, bytes.len());
+    decode(&encoded, registry, biomes).unwrap()
+}
+
+#[test]
+fn emptied_opened_loot_stays_empty_after_chunk_save_and_reload() {
+    let (registry, biomes) = fixture();
+    let loaded = reload_loot_case(&registry, &biomes, "minecraft:chests/ruined_portal", 42, 1);
+    let source = &loaded.saved.as_ref().unwrap().block_entity_sources[0].1;
+    assert!(
+        source.get("LootTable").is_none(),
+        "opened chest keeps a rerollable loot table: {source:?}"
+    );
+    assert!(source.get("LootTableSeed").is_none());
+    assert_eq!(
+        text(source.get("CustomName")).as_deref(),
+        Some("{\"text\":\"Keep me\"}")
+    );
+    let crate::containers::BlockEntity::Chest(slots) = &loaded.block_entities[0].1 else {
+        panic!("chest");
+    };
+    assert!(slots.iter().all(Option::is_none));
+}
+
+#[test]
+fn consuming_one_table_preserves_unopened_neighbors_and_other_fields() {
+    let pos = (1, 64, 2);
+    let other = (2, 64, 2);
+    let tag = Tag::compound([
+        ("LootTable", string("minecraft:chests/ruined_portal")),
+        ("LootTableSeed", Tag::Long(42)),
+        ("CustomName", string("keep")),
+    ]);
+    let rolled = |pos| RolledLoot {
+        pos,
+        table: "minecraft:chests/ruined_portal".into(),
+        seed: Some(42),
+        items: Tag::List(vec![]),
+    };
+    let mut extras = ChunkExtras {
+        block_entity_sources: vec![(pos, tag.clone()), (other, tag.clone())],
+        rolled_loot: vec![rolled(pos), rolled(other)],
+        ..Default::default()
+    };
+    consume_container_loot(&mut extras, pos);
+    assert_eq!(extras.rolled_loot, vec![rolled(other)]);
+    assert_eq!(extras.block_entity_sources[1].1, tag);
+    assert_eq!(
+        extras.block_entity_sources[0].1.get("CustomName"),
+        Some(&string("keep"))
+    );
+    let saved = extras.block_entity_sources.clone();
+    consume_container_loot(&mut extras, pos);
+    assert_eq!(extras.block_entity_sources, saved);
+    // No rolled marker means an unopened source is not consumed by accident.
+    extras.rolled_loot.clear();
+    consume_container_loot(&mut extras, other);
+    assert_eq!(extras.block_entity_sources[1].1, tag);
+}
+
+#[test]
+#[ignore = "run by tools/test_opened_loot_parity.py"]
+fn opened_loot_parity_fixture() {
+    use std::io::Write;
+    let (registry, biomes) = fixture();
+    let input = fs::read_to_string(std::env::var("HORIZON_OPENED_LOOT_INPUT").unwrap()).unwrap();
+    let mut output =
+        fs::File::create(std::env::var("HORIZON_OPENED_LOOT_OUTPUT").unwrap()).unwrap();
+    for line in input.lines() {
+        let fields: Vec<_> = line.split_whitespace().collect();
+        let mode = fields[2].parse().unwrap();
+        let chunk = reload_loot_case(
+            &registry,
+            &biomes,
+            fields[0],
+            fields[1].parse().unwrap(),
+            mode,
+        );
+        let source = &chunk.saved.as_ref().unwrap().block_entity_sources[0].1;
+        let crate::containers::BlockEntity::Chest(slots) = &chunk.block_entities[0].1 else {
+            panic!("chest");
+        };
+        let count = if mode == 0 {
+            "unopened".into()
+        } else {
+            slots
+                .iter()
+                .flatten()
+                .map(|s| s.count)
+                .sum::<u32>()
+                .to_string()
+        };
+        writeln!(
+            output,
+            "{} {} {count}",
+            u8::from(source.get("LootTable").is_some()),
+            u8::from(source.get("LootTableSeed").is_some())
+        )
+        .unwrap();
+    }
+}

mods/horizon/src/test/java/dev/horizonlab/parity/VanillaOpenedLootOracle.java

@@ -0,0 +1,73 @@
+package dev.horizonlab.parity;
+
+import java.io.PrintWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import net.minecraft.core.BlockPos;
+import net.minecraft.core.registries.Registries;
+import net.minecraft.resources.Identifier;
+import net.minecraft.resources.ResourceKey;
+import net.minecraft.util.ProblemReporter;
+import net.minecraft.util.RandomSource;
+import net.minecraft.world.item.ItemStack;
+import net.minecraft.world.item.Items;
+import net.minecraft.world.level.Level;
+import net.minecraft.world.level.block.Blocks;
+import net.minecraft.world.level.block.entity.ChestBlockEntity;
+import net.minecraft.world.level.storage.TagValueInput;
+
+/** Original oracle: unpack real loot, empty/replace contents, save and reload a chest. */
+public final class VanillaOpenedLootOracle {
+    static final class Chest extends ChestBlockEntity {
+        Chest() { super(BlockPos.ZERO, Blocks.CHEST.defaultBlockState()); }
+        // Persistence is under test; no real chunk receives dirty notifications.
+        @Override public void setChanged() { }
+    }
+    public static void main(String[] args) throws Exception {
+        ParityData.init();
+        var allocator = Class.forName("sun.misc.Unsafe");
+        var field = allocator.getDeclaredField("theUnsafe");
+        field.setAccessible(true);
+        VanillaBlockUpdateOracle.unsafe = field.get(null);
+        VanillaBlockUpdateOracle.allocate = allocator.getMethod("allocateInstance", Class.class);
+        try (var out = new PrintWriter(Files.newBufferedWriter(Path.of(args[1]), StandardCharsets.UTF_8))) {
+            for (var line : Files.readAllLines(Path.of(args[0]))) {
+                var parts = line.split(" ");
+                long seed = Long.parseLong(parts[1]);
+                int mode = Integer.parseInt(parts[2]);
+                var world = VanillaBlockUpdateOracle.make(VanillaBlockUpdateOracle.World.class);
+                world.registries = ParityData.access;
+                world.cells = new java.util.HashMap<>();
+                VanillaBlockUpdateOracle.AIR = Blocks.AIR.defaultBlockState();
+                world.rng = RandomSource.create(seed);
+                VanillaBlockUpdateOracle.set(Level.class, world, "random", world.rng);
+                world.log = new StringBuilder();
+                world.server = VanillaBlockUpdateOracle.make(VanillaBlockUpdateOracle.Server.class);
+                world.server.loot = VanillaBlockUpdateOracle.make(VanillaBlockUpdateOracle.Loot.class);
+                world.server.loot.world = world;
+                world.server.loot.delegate = ParityData.loot;
+                var chest = new Chest();
+                chest.setLevel(world);
+                chest.setLootTable(ResourceKey.create(Registries.LOOT_TABLE, Identifier.parse(parts[0])));
+                chest.setLootTableSeed(seed);
+                if (mode != 0) {
+                    chest.unpackLootTable(null);
+                    if (chest.getLootTable() != null) throw new AssertionError("unpack did not consume table");
+                    chest.clearContent();
+                    if (mode == 2) chest.setItem(0, new ItemStack(Items.GOLD_INGOT, 1));
+                }
+                var saved = chest.saveWithFullMetadata(ParityData.access);
+                var reloaded = new Chest();
+                reloaded.loadWithComponents(TagValueInput.create(ProblemReporter.DISCARDING, ParityData.lookup, saved));
+                var items = ChestBlockEntity.class.getDeclaredField("items");
+                items.setAccessible(true);
+                @SuppressWarnings("unchecked")
+                var slots = (net.minecraft.core.NonNullList<ItemStack>) items.get(reloaded);
+                int count = slots.stream().mapToInt(ItemStack::getCount).sum();
+                out.println((saved.contains("LootTable") ? 1 : 0) + " "
+                    + (saved.contains("LootTableSeed") ? 1 : 0) + " " + (mode == 0 ? "unopened" : count));
+            }
+        }
+    }
+}

tools/test_opened_loot_parity.py

@@ -0,0 +1,75 @@
+#!/usr/bin/env python3
+"""Compare native chest opening/save/reload with Horizon's chunk persistence."""
+import os
+from pathlib import Path
+import random
+import tempfile
+try:
+    from . import lab, oracle_harness as harness, test_rust_parity as base, integration_env
+except ImportError:
+    import lab, oracle_harness as harness, test_rust_parity as base, integration_env
+
+
+def cases():
+    rng = random.Random(0x4C4F4F54)
+    seeds = [0, 1, -1, 42, -(1 << 63), (1 << 63) - 1]
+    seeds.extend(rng.randrange(-(1 << 63), 1 << 63) for _ in range(58))
+    for table in ['ruined_portal', 'simple_dungeon', 'abandoned_mineshaft']:
+        for seed in seeds:
+            for mode in range(3):
+                yield f'minecraft:chests/{table} {seed} {mode}'
+
+
+def main():
+    lock, directory = lab.config()
+    manifest, client = lab.import_client(lock, directory)
+    natives = {v['downloads']['artifact']['path'] for v in manifest['libraries']
+               if v['name'].split(':')[-1].startswith('natives-')}
+    libraries = [lab.import_library(a, directory / 'libraries/client')
+                 for a in lab.client_library_artifacts(manifest) if a['path'] not in natives]
+    build = lab.LOCAL / 'rust-opened-loot-parity'
+    build.mkdir(parents=True, exist_ok=True)
+    report = build / 'report.json'
+    report.unlink(missing_ok=True)
+    sources = [Path(__file__), lab.ROOT / 'tools/lab.py', lab.ROOT / 'tools/oracle_harness.py',
+               lab.ROOT / 'tools/test_rust_parity.py', lab.ROOT / 'tools/integration_env.py',
+               lab.ROOT / 'Cargo.toml', lab.ROOT / 'Cargo.lock',
+               lab.ROOT / 'crates/horizon-client/src/game/commands.rs',
+               lab.ROOT / 'crates/horizon-client/src/save.rs',
+               lab.ROOT / 'crates/horizon-client/src/save/loot_tests.rs',
+               lab.ROOT / 'crates/horizon-client-render/src/chunk.rs',
+               lab.ROOT / 'mods/horizon/src/test/java/dev/horizonlab/parity/VanillaOpenedLootOracle.java',
+               lab.ROOT / 'mods/horizon/src/test/java/dev/horizonlab/parity/VanillaBlockUpdateOracle.java',
+               lab.ROOT / 'mods/horizon/src/test/java/dev/horizonlab/parity/ParityData.java']
+    hashes = base.source_hashes(sources)
+    rows = list(cases())
+    inputs = build / 'inputs.txt'
+    inputs.write_text('\n'.join(rows) + '\n', encoding='utf-8')
+    input_hash = lab.digest(inputs, 'sha256')
+    classes, dependencies, _ = harness.compile_oracle(build, 'VanillaOpenedLootOracle',
+                                                     {'client': client}, libraries, lock['minecraft']['java_major'])
+    harness.java_run(build, classes, dependencies, 'VanillaOpenedLootOracle',
+                     [inputs, build / 'client.txt'], 'client')
+    with tempfile.TemporaryDirectory(prefix='horizon-opened-loot-parity-') as scratch:
+        env = integration_env.game_environment(os.environ, scratch)
+        env.update(HORIZON_OPENED_LOOT_INPUT=str(inputs), HORIZON_OPENED_LOOT_OUTPUT=str(build / 'rust.txt'))
+        harness.run_logged(['cargo', 'test', '--locked', '-p', 'horizon-client-lib', '--lib',
+                            'save::loot_tests::opened_loot_parity_fixture',
+                            '--', '--exact', '--ignored'], build, 'rust', env=env)
+    matched = harness.compare(build / 'client.txt', build / 'rust.txt', len(rows))
+    base.ensure_sources_unchanged(sources, hashes)
+    if lab.digest(inputs, 'sha256') != input_hash:
+        raise AssertionError('Input fixtures changed')
+    lab.write_json(report, dict(schema=1, minecraft=lock['minecraft']['version'], sources_sha256=hashes,
+        client_jar_sha256=lab.digest(client, 'sha256'), input_sha256=input_hash,
+        matched_output_lines=matched, output_sha256=lab.digest(build / 'rust.txt', 'sha256'),
+        scope='Native chest unpackLootTable, save and reload on a recording level versus Horizon opening bookkeeping, '
+              'chunk encoding, binary NBT roundtrip and chunk decoding. Compares loot table/seed retention and opened inventory counts. '
+              'Unopened inventory contents are intentionally not compared: Horizon eagerly rolls while vanilla defers. '
+              'Three real native chest tables,64 seeds,unopened/emptied/replaced contents. Native dirty notifications suppressed; '
+              'Rust begins with an original synthetic eager-roll inventory. No loot-output/RNG comparison,full world/UI run or region-file I/O.'))
+    print(f'{matched} opened-loot save/reload cases match the pinned client: {report}', flush=True)
+
+
+if __name__ == '__main__':
+    main()