Give chest loot tables precedence over saved items

Leave randomizable-container inventories empty when their NBT supplies a valid loot table, allowing the existing command and chunk loading paths to retain the table. Add a red/green regression and a real vanilla inventory-load comparison.

Fixes #8920

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

crates/horizon-client/examples/probe/chest_load_fixture.rs

@@ -0,0 +1,22 @@
+//! Chest inventory loading without unpacking loot, for the vanilla differential harness.
+use horizon_client::save;
+use std::{fs, io::Write};
+
+pub fn main() -> Result<(), Box<dyn std::error::Error>> {
+    let args: Vec<_> = super::args().collect();
+    let input = fs::read_to_string(&args[1])?;
+    let mut out = fs::File::create(&args[2])?;
+    for line in input.lines() {
+        let tag = horizon_world::nbt::parse_snbt(line)?.0;
+        let (_, chest) = save::decode_block_entity(&tag).ok_or("not a chest")?;
+        let mut result = String::new();
+        for (i, stack) in chest.slots().iter().enumerate() {
+            if let Some(stack) = stack {
+                use std::fmt::Write;
+                write!(result, "{i}:{}:{};", stack.item, stack.count)?;
+            }
+        }
+        writeln!(out, "{}", if result.is_empty() { "empty" } else { &result })?;
+    }
+    Ok(())
+}

crates/horizon-client/examples/probe/main.rs

@@ -26,6 +26,7 @@ mod breaking_fixture;
 mod camera_probe;
 mod cauldron_fixture;
 mod chat_screen_fixture;
+mod chest_load_fixture;
 mod chest_pair_fixture;
 mod cloud_fixture;
 mod command_parse_fixture;
@@ -128,6 +129,7 @@ probes! {
     cauldron_fixture,
     chat_screen_fixture,
     chest_pair_fixture,
+    chest_load_fixture,
     cloud_fixture,
     command_parse_fixture,
     container_screen_probe,

crates/horizon-client/src/save.rs

@@ -969,7 +969,16 @@ pub fn decode_block_entity(tag: &Tag) -> Option<((i32, i32, i32), crate::contain
     };
     let mut entity = BlockEntity::for_block(block)?;
     match &mut entity {
-        BlockEntity::Chest(items) => *items = decode_items(tag.get("Items"), items.len()),
+        BlockEntity::Chest(items) => {
+            // Randomizable containers reset their inventory, then load Items only
+            // when tryLoadLootTable did not read a valid loot-table key.
+            let has_table = text(tag.get("LootTable"))
+                .and_then(|id| horizon_client_core::identifier::normalize_id(&id))
+                .is_some();
+            if !has_table {
+                *items = decode_items(tag.get("Items"), items.len());
+            }
+        }
         BlockEntity::Furnace(f) => {
             let items = decode_items(tag.get("Items"), 3);
             f.items = [items[0].clone(), items[1].clone(), items[2].clone()];
@@ -6117,6 +6126,46 @@ mod tests {
                 .expect("update tag");
         assert!(update.get("Items").is_some() && update.get("CookingTimes").is_none());
     }
+    #[test]
+    fn a_chest_loot_table_takes_precedence_over_saved_items() {
+        let mut saved = horizon_world::nbt::parse_snbt(
+            r#"{id:"minecraft:chest",x:1,y:2,z:3,Items:[{Slot:3b,id:"minecraft:diamond",count:7}],LootTable:"minecraft:chests/simple_dungeon",LootTableSeed:42L}"#,
+        )
+        .unwrap()
+        .0;
+        let (_, mut chest) = decode_block_entity(&saved).expect("chest");
+        assert!(
+            chest.slots().iter().all(Option::is_none),
+            "ChestBlockEntity.loadAdditional ignores Items when a loot table loads"
+        );
+        // The command path rolls an empty inventory and keeps the table/seed
+        // for data get and saving. No loot resources are needed to keep that state.
+        let rolled = crate::structure_blocks::roll_container(
+            chest.slots_mut(),
+            None,
+            "minecraft:chests/simple_dungeon".into(),
+            Some(42),
+            (1, 2, 3),
+            "minecraft:plains",
+        );
+        let mut stored = block_entity_data((1, 2, 3), &chest, "minecraft:chest", None);
+        let Tag::Compound(fields) = &mut stored else {
+            panic!("compound");
+        };
+        assert!(rolled.restore(fields));
+        assert_eq!(stored.get("LootTable"), saved.get("LootTable"));
+        assert_eq!(stored.get("LootTableSeed"), saved.get("LootTableSeed"));
+        assert!(stored.get("Items").is_none());
+        let Tag::Compound(fields) = &mut saved else {
+            panic!("compound");
+        };
+        fields.remove(&"LootTable".into());
+        let (_, chest) = decode_block_entity(&saved).expect("chest without table");
+        let item = chest.slots()[3].as_ref().expect("saved item");
+        assert_eq!(item.item.as_ref(), "minecraft:diamond");
+        assert_eq!(item.count, 7);
+    }
+
     /// `LockCode`: a chest's `lock` is not in Horizon's model, so `data get`
     /// takes it from the tag the block entity was loaded (or set) with.
     #[test]

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

@@ -0,0 +1,45 @@
+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.NonNullList;
+import net.minecraft.core.registries.BuiltInRegistries;
+import net.minecraft.nbt.TagParser;
+import net.minecraft.util.ProblemReporter;
+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.entity.ChestBlockEntity;
+import net.minecraft.world.level.storage.TagValueInput;
+
+/** Original oracle: load real chest NBT without opening or unpacking its loot. */
+public final class VanillaChestLoadOracle {
+    public static void main(String[] args) throws Exception {
+        ParityData.init();
+        var itemsField = ChestBlockEntity.class.getDeclaredField("items");
+        itemsField.setAccessible(true);
+        try (var out = new PrintWriter(Files.newBufferedWriter(Path.of(args[1]), StandardCharsets.UTF_8));
+             var lines = Files.lines(Path.of(args[0]))) {
+            for (String line : (Iterable<String>) lines::iterator) {
+                var chest = new ChestBlockEntity(BlockPos.ZERO, Blocks.CHEST.defaultBlockState());
+                chest.setItem(0, new ItemStack(Items.GOLD_INGOT, 5));
+                chest.loadWithComponents(TagValueInput.create(ProblemReporter.DISCARDING,
+                    ParityData.lookup, TagParser.parseCompoundFully(line)));
+                @SuppressWarnings("unchecked")
+                var slots = (NonNullList<ItemStack>) itemsField.get(chest);
+                var result = new StringBuilder();
+                for (int i = 0; i < slots.size(); i++) {
+                    var stack = slots.get(i);
+                    if (!stack.isEmpty()) {
+                        result.append(i).append(':').append(BuiltInRegistries.ITEM.getKey(stack.getItem()))
+                            .append(':').append(stack.getCount()).append(';');
+                    }
+                }
+                out.println(result.isEmpty() ? "empty" : result);
+            }
+        }
+    }
+}

tools/test_chest_load_parity.py

@@ -0,0 +1,74 @@
+#!/usr/bin/env python3
+"""Compare chest inventory loading with the pinned client, without unpacking loot."""
+import json
+import os
+from pathlib import Path
+try:
+    from . import lab, oracle_harness as harness, test_rust_parity as base
+except ImportError:
+    import lab, oracle_harness as harness, test_rust_parity as base
+
+
+def cases():
+    tables = [None, '"minecraft:chests/simple_dungeon"', '"chests/simple_dungeon"',
+              '"example:missing"', '"INVALID TABLE"', '42', '""', '":chests/simple_dungeon"']
+    for table in tables:
+        for items in [None, '[]', '[{Slot:3b,id:"minecraft:diamond",count:7}]',
+                      '[{Slot:0b,id:"minecraft:stone",count:1},{Slot:26b,id:"minecraft:diamond",count:64}]']:
+            for seed in [None, '42L', '-9223372036854775808L']:
+                fields = ['id:"minecraft:chest"', 'x:1', 'y:2', 'z:3']
+                if items is not None:
+                    fields.append('Items:' + items)
+                if table is not None:
+                    fields.append('LootTable:' + table)
+                if seed is not None:
+                    fields.append('LootTableSeed:' + seed)
+                yield '{' + ','.join(fields) + '}'
+
+
+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-chest-load-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 / 'Cargo.toml', lab.ROOT / 'Cargo.lock',
+               lab.ROOT / 'crates/horizon-client/src/save.rs',
+               lab.ROOT / 'crates/horizon-client-core/src/identifier.rs',
+               lab.ROOT / 'crates/horizon-client/examples/probe/chest_load_fixture.rs',
+               lab.ROOT / 'mods/horizon/src/test/java/dev/horizonlab/parity/VanillaChestLoadOracle.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')
+    inputs_hash = lab.digest(inputs, 'sha256')
+    classes, dependencies, _ = harness.compile_oracle(build, 'VanillaChestLoadOracle',
+                                                     {'client': client}, libraries, lock['minecraft']['java_major'])
+    harness.java_run(build, classes, dependencies, 'VanillaChestLoadOracle',
+                     [inputs, build / 'client.txt'], 'client')
+    command = harness.build_probe(build, 'chest_load_fixture')
+    if os.name == 'nt':
+        command[0] = command[0].with_suffix('.exe')
+    harness.run_logged([*command, inputs, build / 'rust.txt'], build, 'rust')
+    matched = harness.compare(build / 'client.txt', build / 'rust.txt', len(rows))
+    base.ensure_sources_unchanged(sources, hashes)
+    if lab.digest(inputs, 'sha256') != inputs_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=inputs_hash,
+        matched_output_lines=matched, output_sha256=lab.digest(build / 'rust.txt', 'sha256'),
+        scope='Real ChestBlockEntity.loadWithComponents on a previously filled chest. Compare decoded inventory '
+              'for present/absent/invalid loot-table keys, item lists and seeds without unpacking loot. '
+              'Loot generation, command integration and network synchronization are not exercised.'))
+    print(f'{matched} chest load cases match the pinned client: {report}', flush=True)
+
+
+if __name__ == '__main__':
+    main()