Merge pull request #11371 from TheMrZZ/fix/saved-max-stack-prototype

Omit prototype max-stack patches in saved ItemEntity stacks

crates/horizon-client/src/entity_nbt.rs

@@ -2368,6 +2368,25 @@ pub fn mob_from_nbt(
 
 /// `ItemEntity`: Health, Age, PickupDelay, Item, Thrower.
 pub fn item_entity_to_nbt(e: &ItemEntity, uuid: [i32; 4], raw: Option<&Tag>) -> Tag {
+    item_entity_to_nbt_saved(e, uuid, raw, item_to_nbt(&e.stack))
+}
+
+/// ItemEntity saved with the actual loaded item prototype, including transfers.
+pub fn item_entity_to_nbt_with_items(
+    e: &ItemEntity,
+    uuid: [i32; 4],
+    raw: Option<&Tag>,
+    items: &crate::items::Items,
+) -> Tag {
+    item_entity_to_nbt_saved(
+        e,
+        uuid,
+        raw,
+        horizon_items::item_nbt::item_to_nbt_with_items(&e.stack, items),
+    )
+}
+
+fn item_entity_to_nbt_saved(e: &ItemEntity, uuid: [i32; 4], raw: Option<&Tag>, item: Tag) -> Tag {
     let base = Base {
         id: "minecraft:item",
         uuid,
@@ -2379,7 +2398,7 @@ pub fn item_entity_to_nbt(e: &ItemEntity, uuid: [i32; 4], raw: Option<&Tag>) ->
     };
     let mut tag = base.non_mob_tag(raw);
     for (key, value) in [
-        ("Item", item_to_nbt(&e.stack)),
+        ("Item", item),
         ("Health", Tag::Short(e.health as i16)),
         ("Fire", Tag::Short(e.fire_ticks as i16)),
         (

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

@@ -118,7 +118,12 @@ impl Game<'_> {
             Target::Item(id) => {
                 let uuid = self.uuid_for(id);
                 let e = self.dropped.iter().find(|e| e.id == id)?;
-                entity_nbt::item_entity_to_nbt(e, uuid, self.persist.raw.get(&id))
+                entity_nbt::item_entity_to_nbt_with_items(
+                    e,
+                    uuid,
+                    self.persist.raw.get(&id),
+                    self.items(),
+                )
             }
             Target::Stand(id) => {
                 let uuid = self.uuid_for(id);

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

@@ -1534,10 +1534,11 @@ impl Game<'_> {
         }
         let tag = if let Some(i) = self.dropped.iter().position(|e| e.id == id) {
             let uuid = self.uuid_for(id);
-            let tag = horizon_client::entity_nbt::item_entity_to_nbt(
+            let tag = horizon_client::entity_nbt::item_entity_to_nbt_with_items(
                 &self.dropped[i],
                 uuid,
                 self.persist_raw(id),
+                self.items(),
             );
             // A gateway keeps the item in its level (teleportSameDimension
             // moves it): it doesn't leave, so no native_entity_leave_level.

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

@@ -973,10 +973,11 @@ impl Game<'_> {
                 return;
             }
             let uuid = self.uuid_for(id);
-            let tag = horizon_client::entity_nbt::item_entity_to_nbt(
+            let tag = horizon_client::entity_nbt::item_entity_to_nbt_with_items(
                 &self.dropped[i],
                 uuid,
                 self.persist_raw(id),
+                self.items(),
             );
             self.remove_dropped(i);
             self.detach_entity(id);

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

@@ -1768,8 +1768,12 @@ impl Game<'_> {
         keys.contains(&chunk_of(at)).then(|| {
             let id = self.dropped[i].id;
             let uuid = self.uuid_for(id);
-            let tag =
-                entity_nbt::item_entity_to_nbt(&self.dropped[i], uuid, self.persist.raw.get(&id));
+            let tag = entity_nbt::item_entity_to_nbt_with_items(
+                &self.dropped[i],
+                uuid,
+                self.persist.raw.get(&id),
+                self.items(),
+            );
             (chunk_of(at), tag)
         })
     }

crates/horizon-items/src/item_data.rs

@@ -778,6 +778,27 @@ pub fn encode_components(stack: &ItemStack) -> Option<Tag> {
     }
     Some(components)
 }
+/// Saved patch with the loaded item's prototype available.
+/// Context-free callers and the live `components_tag` view stay unchanged.
+pub fn encode_components_with_items(stack: &ItemStack, items: &crate::items::Items) -> Option<Tag> {
+    let mut components = encode_components(stack)?;
+    // PatchedDataComponentMap.set removes values equal to the prototype.
+    if let Some(prototype) = items.get(&stack.item)
+        && prototype
+            .components
+            .iter()
+            .any(|id| id == "minecraft:max_stack_size")
+        && matches!(components.get("minecraft:max_stack_size"), Some(Tag::Int(size))
+            if *size > 0 && *size as u32 == prototype.max_stack)
+        && let Tag::Compound(entries) = &mut components
+    {
+        entries.remove(&JavaString::from("minecraft:max_stack_size"));
+        if entries.is_empty() {
+            return None;
+        }
+    }
+    Some(components)
+}
 /// `encode_components` from the damage value and extra components.
 pub fn components_tag(damage: u32, data: Option<&StackData>) -> Option<Tag> {
     let mut entries: Vec<(&str, Tag)> = Vec::new();
@@ -1524,6 +1545,116 @@ mod tests {
         }
     }
 
+    fn check_max_stack_saved_prototype(decoded: bool) {
+        use crate::items::Items;
+        use horizon_world::loot::PackOverlay;
+        let root =
+            std::env::temp_dir().join(format!("h088-prototypes-{}-{decoded}", std::process::id()));
+        std::fs::create_dir_all(root.join("data/minecraft/loot_table")).unwrap();
+        std::fs::write(root.join("items.json"), r#"{"minecraft:iron_helmet":{"minecraft:max_stack_size":1},"minecraft:stone":{"minecraft:max_stack_size":64},"minecraft:ender_pearl":{"minecraft:max_stack_size":16},"audit:changed":{"minecraft:max_stack_size":7},"audit:missing":{}}"#).unwrap();
+        let items = Items::catalog(&root, &PackOverlay::NONE).unwrap();
+        let parse = |text| horizon_world::nbt::parse_snbt(text).unwrap().0;
+        for (id, prototype, has_default) in [
+            ("minecraft:iron_helmet", 1, true),
+            ("minecraft:stone", 64, true),
+            ("minecraft:ender_pearl", 16, true),
+            ("audit:changed", 7, true),
+            ("audit:missing", 64, false),
+            ("audit:unknown", 7, false),
+        ] {
+            assert!(encode_components_with_items(&ItemStack::new(id, 1), &items).is_none());
+            for (key, value) in [
+                ("minecraft:max_stack_size", Tag::Int(prototype)),
+                ("minecraft:max_stack_size", Tag::Int(3)),
+                ("!minecraft:max_stack_size", parse("{}")),
+            ] {
+                let input = Tag::compound([
+                    (key, value.clone()),
+                    ("minecraft:custom_data", parse("{keep:88}")),
+                ]);
+                let mut stack = ItemStack::new(id, 1);
+                if decoded {
+                    (stack.damage, stack.extra) = decode_components(Some(&input));
+                } else {
+                    assert!(stack.extra.is_none());
+                    stack.data_mut().set_other(key, Some(value.clone()));
+                    stack
+                        .data_mut()
+                        .set_other("minecraft:custom_data", Some(parse("{keep:88}")));
+                }
+                let before = stack.clone();
+                assert_eq!(
+                    components_tag(stack.damage, stack.data()),
+                    Some(input.clone())
+                );
+                assert_eq!(
+                    encode_components(&stack),
+                    Some(input.clone()),
+                    "context-free API stays unchanged"
+                );
+                let default = key == "minecraft:max_stack_size"
+                    && value == Tag::Int(prototype)
+                    && has_default;
+                let expected = if default {
+                    Some(parse(r#"{"minecraft:custom_data":{keep:88}}"#))
+                } else {
+                    Some(input.clone())
+                };
+                assert_eq!(
+                    encode_components_with_items(&stack, &items),
+                    expected,
+                    "{id}, decoded={decoded}"
+                );
+                assert_eq!(
+                    crate::item_nbt::item_to_nbt_with_items(&stack, &items).get("components"),
+                    expected.as_ref()
+                );
+                assert_eq!(stack, before, "no live stack mutation");
+                assert_eq!(components_tag(stack.damage, stack.data()), Some(input));
+            }
+            // Removing a sole default must also omit the empty components compound.
+            let mut only = ItemStack::new(id, 1);
+            only.data_mut()
+                .set_other("minecraft:max_stack_size", Some(Tag::Int(prototype)));
+            assert_eq!(
+                encode_components_with_items(&only, &items).is_none(),
+                has_default
+            );
+        }
+        // Reload the same known item's prototype through the real catalog loader.
+        std::fs::write(
+            root.join("items.json"),
+            r#"{"minecraft:stone":{"minecraft:max_stack_size":7}}"#,
+        )
+        .unwrap();
+        let changed = Items::catalog(&root, &PackOverlay::NONE).unwrap();
+        assert_eq!(items.get("minecraft:stone").unwrap().max_stack, 64);
+        assert_eq!(changed.get("minecraft:stone").unwrap().max_stack, 7);
+        for size in [7, 64] {
+            let mut stack = ItemStack::new("minecraft:stone", 1);
+            stack
+                .data_mut()
+                .set_other("minecraft:max_stack_size", Some(Tag::Int(size)));
+            assert_eq!(
+                encode_components_with_items(&stack, &items).is_none(),
+                size == 64
+            );
+            assert_eq!(
+                encode_components_with_items(&stack, &changed).is_none(),
+                size == 7
+            );
+        }
+        std::fs::remove_dir_all(root).unwrap();
+    }
+
+    #[test]
+    fn decoded_max_stack_saved_patch_uses_loaded_prototype() {
+        check_max_stack_saved_prototype(true);
+    }
+    #[test]
+    fn modeled_max_stack_saved_patch_uses_loaded_prototype() {
+        check_max_stack_saved_prototype(false);
+    }
     #[test]
     fn decoded_hive_saved_patch_uses_whole_prototype() {
         check_hive_saved_patch(true);

crates/horizon-items/src/item_nbt.rs

@@ -18,6 +18,17 @@ pub fn encode_item(stack: &crate::items::ItemStack) -> Vec<(&'static str, Tag)>
 pub fn item_to_nbt(stack: &crate::items::ItemStack) -> Tag {
     Tag::compound(encode_item(stack))
 }
+/// ItemStack.CODEC saved against the actual loaded item prototype.
+pub fn item_to_nbt_with_items(stack: &crate::items::ItemStack, items: &crate::items::Items) -> Tag {
+    let mut entries = vec![
+        ("id", string(&stack.item)),
+        ("count", Tag::Int(stack.count as i32)),
+    ];
+    if let Some(components) = crate::item_data::encode_components_with_items(stack, items) {
+        entries.push(("components", components));
+    }
+    Tag::compound(entries)
+}
 /// ItemStack.CODEC's count range is independent of the item's stack limit;
 /// an out-of-range count uses the field's default of one. The id is read as an
 /// `Identifier`: `stone` is `minecraft:stone` (a summoned display's or a

tools/test_item_entity_max_stack_parity.py

@@ -0,0 +1,84 @@
+#!/usr/bin/env python3
+"""ItemEntity saved max-stack default patches; zero observation ticks."""
+import hashlib,json,sys,tempfile,uuid
+from pathlib import Path
+import test_command_exec_parity as driver
+CASES=[('helmet-plain','iron_helmet',None),('helmet-default','iron_helmet','{"minecraft:max_stack_size":1}'),('helmet-keep','iron_helmet','{"minecraft:max_stack_size":1,"minecraft:custom_data":{keep:88}}'),('stone-plain','stone',None),('stone-default','stone','{"minecraft:max_stack_size":64}'),('stone-nondefault','stone','{"minecraft:max_stack_size":16}'),('stone-removed','stone','{"!minecraft:max_stack_size":{}}'),('pearl-plain','ender_pearl',None),('pearl-default','ender_pearl','{"minecraft:max_stack_size":16}'),('pearl-nondefault','ender_pearl','{"minecraft:max_stack_size":8}')]
+def program():
+ ops=[]
+ def cmd(text,label,result=None,admission=True,diagnostic=False):
+  op=dict(op='cmd',cmd=text,label=label,admission=admission)
+  if result is not None:op['expect_result']=result
+  if diagnostic:op.update(record=False,trace=True)
+  ops.append(op)
+ def read(text,label,admission=False):
+  for k in ['result','success']:cmd(f'/scoreboard players set {k} h088 -99',label+'-reset-'+k,-99)
+  cmd('/execute store result score result h088 store success score success h088 run '+text,label,admission=admission)
+  for k in ['result','success']:cmd(f'/scoreboard players get {k} h088',label+'-'+k,admission=admission)
+ cmd('/tick freeze','freeze');cmd('/gamerule spawn_mobs false','no-spawn');cmd('/gamerule advance_time false','no-daylight')
+ cmd('/forceload add 0 0','force',1);ops.append(dict(op='settle',required_chunks=[[0,0]]))
+ cmd('/fill 0 199 0 15 199 15 stone','floor',256)
+ cmd('/tp @s 14.5 200 14.5 0 0','owner-position',1);cmd('/gamemode creative @s','owner-mode')
+ cmd('/execute if entity @s[type=player,name=Player,nbt={Health:20f,playerGameType:1,Pos:[14.5d,200d,14.5d]}]','owner',1)
+ cmd('/data get entity @s UUID','owner-uuid',4)
+ cmd('/scoreboard objectives add h088 dummy','objective')
+ cmd('/execute store result score start h088 run time query gametime','clock-start',diagnostic=True)
+ for n,(label,item,components) in enumerate(CASES,1):
+  u=str(uuid.UUID(int=(880<<96)|n));x=2+n
+  cmd(f'/execute if loaded {x} 200 4',label+'-load',1)
+  cmd(f'/execute if block {x} 199 4 stone',label+'-floor',1)
+  stack='{id:"minecraft:'+item+'",count:1'+(',components:'+components if components else '')+'}'
+  cmd(f'/summon item {x}.5 200 4.5 '+f'{{UUID:[I;880,0,0,{n}],NoGravity:1b,Motion:[0d,0d,0d],Health:5s,Item:'+stack+'}',label+'-summon',1)
+  guard=f'{{UUID:[I;880,0,0,{n}],Pos:[{x}.5d,200d,4.5d],Motion:[0d,0d,0d],NoGravity:1b,Health:5s,Item:{{id:"minecraft:{item}",count:1}}}}'
+  cmd(f'/execute as {u} if entity @s[type=item,nbt='+guard+']',label+'-actor',1)
+  for f in ['UUID','Pos','Motion','NoGravity','Health']:cmd(f'/data get entity {u} {f}',label+'-'+f)
+  read(f'data get entity {u} Item',label+'-saved-stack')
+  read(f'data get entity {u} Item.components."minecraft:max_stack_size"',label+'-saved-size')
+  read(f'execute if data entity {u} Item.components."minecraft:max_stack_size"',label+'-size-presence')
+  read(f'data get entity {u} Item.components."!minecraft:max_stack_size"',label+'-removal')
+  read(f'data get entity {u} Item.components."minecraft:custom_data"',label+'-custom')
+  cmd(f'/data get entity {u}',label+'-full',diagnostic=True)
+ cmd('/execute store result score end h088 run time query gametime','clock-end',diagnostic=True)
+ cmd('/scoreboard players operation end h088 -= start h088','clock-delta-operation',0)
+ cmd('/scoreboard players get end h088','clock-delta',0)
+ return ops
+
+def main():
+    reference = None
+    if '--reference' in sys.argv:
+        i = sys.argv.index('--reference'); reference = Path(sys.argv[i+1]).resolve()
+        del sys.argv[i:i+2]
+    if '--work-dir' not in sys.argv:
+        parent = Path(tempfile.mkdtemp(prefix='item-max-stack-', dir=driver.lab.LOCAL))
+        sys.argv += ['--work-dir', str(parent / 'run')]
+    build = Path(sys.argv[sys.argv.index('--work-dir')+1]).resolve()
+    assert not build.exists(), 'preserve attempts; choose a fresh directory'
+    build.mkdir(parents=True)
+    ops = program(); recorded = [op for op in ops if op['op']=='cmd' and op.get('record', True)]
+    (build/'declaration.json').write_text(json.dumps(dict(
+        script_sha256=hashlib.sha256(Path(__file__).read_bytes()).hexdigest(), records=len(recorded),
+         labels=[op['label'] for op in recorded],
+        raw_diagnostics=[op['label'] for op in ops if op.get('trace')],
+        comparison='complete records, no masks/tolerances; bodies and absolute clocks are separate raw traces'), indent=2)+'\n')
+    expected_bytes = reference.read_bytes() if reference else None
+    driver.corpus_module.groups = lambda: [('ItemEntity saved max-stack prototype', ops)]
+    sys.argv.append('--allow-gaps'); driver.main()
+    java_bytes = (build/'vanilla.jsonl').read_bytes()
+    if expected_bytes is not None:
+        assert java_bytes==expected_bytes and reference.read_bytes()==expected_bytes, 'fresh Java reference changed'
+    java = [json.loads(s) for s in java_bytes.splitlines()]
+    if '--reference-only' in sys.argv: return
+    native = [json.loads(s) for s in (build/'rust.jsonl').read_bytes().splitlines()]
+    assert len(recorded)==len(java)==len(native)
+    residuals = [dict(label=op['label'], expected=a, actual=b) for op,a,b in zip(recorded,java,native) if a!=b]
+    report = dict(records=len(recorded), passed=len(recorded)-len(residuals), residuals=residuals,
+                  reference_sha256=hashlib.sha256(java_bytes).hexdigest())
+    (build/'strict-report.json').write_text(json.dumps(report, indent=2)+'\n')
+    for op,a,b in zip(recorded,java,native):
+        if op['admission']: assert a==b, ('admission',op,a,b)
+    print(f'Strict records: {report["passed"]}/{report["records"]}')
+    if residuals: raise SystemExit(1)
+
+
+if __name__ == '__main__':
+    main()