From 9c5982d8e0cb8116ae98195ffaae4cb12a4889e4 Mon Sep 17 00:00:00 2001 From: Florian ERNST <30944236+TheMrZZ@users.noreply.github.com> Date: Thu, 5 Nov 2026 23:07:30 +0100 Subject: [PATCH] Default loaded item frames down when Facing is absent Co-Authored-By: Codex --- crates/horizon-client/src/entity_nbt.rs | 51 ++++++- docs/parity/frame-missing-facing.md | 156 ++++++++++++++++++++++ tools/port_map/game2-002.tsv | 2 +- tools/test_frame_facing_default_parity.py | 114 ++++++++++++++++ 4 files changed, 321 insertions(+), 2 deletions(-) create mode 100644 docs/parity/frame-missing-facing.md create mode 100644 tools/test_frame_facing_default_parity.py diff --git a/crates/horizon-client/src/entity_nbt.rs b/crates/horizon-client/src/entity_nbt.rs index bbc81a32ad..1178465357 100644 --- a/crates/horizon-client/src/entity_nbt.rs +++ b/crates/horizon-client/src/entity_nbt.rs @@ -3217,12 +3217,13 @@ pub fn item_frame_to_nbt(f: &ItemFrame, uuid: [i32; 4], raw: Option<&Tag>) -> Ta } tag } +/// `ItemFrame.readAdditionalSaveData`: an absent Facing defaults to Direction.DOWN. pub fn item_frame_from_nbt(tag: &Tag, id: u32) -> Option { let pos = attached_block_pos(tag)?; Some(ItemFrame { id, pos, - facing: number(tag.get("Facing")).unwrap_or(2).clamp(0, 5) as u8, + facing: number(tag.get("Facing")).unwrap_or(0).clamp(0, 5) as u8, glow: kind(tag).as_deref() == Some("minecraft:glow_item_frame"), item: tag.get("Item").and_then(item_from_nbt), rotation: (number(tag.get("ItemRotation")).unwrap_or(0) & 7) as u8, @@ -6340,6 +6341,54 @@ mod fireball_nbt_tests { assert_eq!(item_frame_from_nbt(&frame, 1).unwrap().pos, [-1, 80, 6]); } + #[test] + fn frame_missing_facing_loads_down_and_valid_directions_round_trip() { + // ItemFrame.readAdditionalSaveData: LEGACY_ID_CODEC, orElse(DOWN). + for kind in ["minecraft:item_frame", "minecraft:glow_item_frame"] { + // Explicit controls run before the missing-field assertion. + for facing in [Some(0), Some(1), Some(2), Some(3), Some(4), Some(5), None] { + let mut tag = Tag::compound([ + ("id", string(kind)), + ("block_pos", Tag::IntArray(vec![4, 102, 6])), + ("Fixed", Tag::Byte(1)), + ("ItemRotation", Tag::Byte(5)), + ("ItemDropChance", Tag::Float(0.25_f32.to_bits())), + ( + "Item", + item_to_nbt(&crate::items::ItemStack::new("minecraft:diamond", 1)), + ), + ]); + if let Some(direction) = facing { + put(&mut tag, "Facing", Tag::Byte(direction)); + } + let frame = item_frame_from_nbt(&tag, 17).unwrap(); + assert_eq!(frame.kind(), kind); + assert_eq!(frame.id, 17); + assert!(frame.fixed); + assert_eq!( + frame.item.as_ref().unwrap().item.as_ref(), + "minecraft:diamond" + ); + for raw in [None, Some(&tag)] { + let saved = item_frame_to_nbt(&frame, [1, 2, 3, 4], raw); + assert_eq!( + saved.get("Facing"), + Some(&Tag::Byte(facing.unwrap_or(0))), + "{kind}" + ); + assert_eq!(saved.get("Item"), tag.get("Item")); + assert_eq!(saved.get("ItemRotation"), Some(&Tag::Byte(5))); + assert_eq!(saved.get("ItemDropChance"), tag.get("ItemDropChance")); + assert_eq!(saved.get("Fixed"), Some(&Tag::Byte(1))); + assert_eq!(saved.get("block_pos"), tag.get("block_pos")); + if facing.is_none() { + assert_eq!(saved.get("Pos"), Some(&doubles(&[4.5, 102.96875, 6.5]))); + } + } + } + } + } + #[test] fn a_named_frame_drops_with_its_custom_name() { // `ItemFrame.getFrameItemStackWithData`: the frame item, then diff --git a/docs/parity/frame-missing-facing.md b/docs/parity/frame-missing-facing.md new file mode 100644 index 0000000000..4987a3e38b --- /dev/null +++ b/docs/parity/frame-missing-facing.md @@ -0,0 +1,156 @@ +# Item frames with an absent Facing + +The only gameplay change is the fallback in `item_frame_from_nbt`: DOWN (0) rather +than NORTH (2). The pinned 26.3 `ItemFrame.readAdditionalSaveData` reads +`Direction.LEGACY_ID_CODEC` and calls `orElse(Direction.DOWN)`. + +Both ordinary and glow frames use the actual loader. Explicit directions 0..5, +including SOUTH (3), keep their existing behavior. The landed immutable-glow and +old-map-detachment code in `Game::apply_entity_nbt` is untouched. No missing +`block_pos`, invalid-direction, attachment lifecycle or map behavior claim. + +## Reproduction and declared comparison + +``` +python3 tools/test_frame_facing_default_parity.py --binary /path/to/game --work-dir /scratch/run +python3 tools/test_frame_facing_default_parity.py --full-diagnostic --binary /path/to/game --work-dir /scratch/full +``` + +`--reference /scratch/old/vanilla.jsonl` requires byte-identical fresh Java output +before comparing a second binary. The driver uses the existing command oracle and +actual authoritative Game command route with a connected owner; no synthetic +frame state or new observer. Every run gets fresh isolated data. + +Fourteen frames: each actual class crossed with omitted Facing and explicit 0..5. +Each has a unique fixed UUID, explicit `block_pos`, `Fixed:1b`, diamond count 1, +ItemRotation 5 and ItemDropChance 0.25f. Positive UUID/type, loaded chunk, stone floor, +air gap, owner UUID/Health/mode/Pos and command callbacks guard admission. Setup +settles before actors; after summons there are zero world ticks, verified by an +actual relative gametime delta of zero. No `/data` writes. + +Predeclared before native captures: the scoped **198** complete raw command records +include every Facing/Pos observation, setup acknowledgement, class/UUID/block_pos, +Fixed/Item/ItemRotation/drop-chance and relative-clock record. The additional **30** +records are diagnostic from the start: both absolute clocks and each actor's +Rotation and full tag. Those commands remain executable and traced in scoped +mode; `--full-diagnostic` records and strictly compares all **228**. No projections, +style normalization or numeric tolerances are applied to the selected records. + +Frozen base: `3c7bb44aa12407aff43fe8c9adab51a9b9ebc61c`. + +| Program | Exact script SHA256 | Java reference SHA256 | +|---|---|---| +| Scoped198 | `5eb2c1e15f38b887b6eace1eb01206f1b8ceb249c45924f7f6b1559125662cfd` | `8a01196b9da48bec92c5f3470272c3b577d09d9ca7e3e710eaf178e5323b6c8f` | +| Full228 | `bb36f1aa4bf5976ed6421a2d3141195c34b72edf61007bdd390c6bf58771c157` | `1d43e476c7a2c3368bbd3634e7b204a34f724999a1894a5d905b5ba7c399b765` | + +The unchanged Game passes **194/198**, with exactly these target failures: + +| Label | Java | Old Game | +|---|---|---| +| item_frame-absent-Facing | 0b | 2b | +| item_frame-absent-Pos | [2.5d,102.96875d,4.5d] | [2.5d,102.5d,4.96875d] | +| glow_item_frame-absent-Facing | 0b | 2b | +| glow_item_frame-absent-Pos | [2.5d,102.96875d,10.5d] | [2.5d,102.5d,10.96875d] | + +## Regression and retained evidence + +`frame_missing_facing_loads_down_and_valid_directions_round_trip` calls the real +loader and saver for both actual classes, with and without raw NBT. Explicit +valid-direction controls precede the absent-field assertion. The old code fails +at `Facing` Byte2 versus Byte0; the identical test passes fixed, including the +DOWN-derived position, Fixed, Item, ItemRotation and drop-chance controls. + +Complete programs, fresh references, old/fixed raw JSONL, full styled feedback and +callbacks, transcripts, reports, pinned bytecode, source/checkpoint hashes and +immutable old executables are retained under the owner's `.local/h134`. They +contain no inferred event timestamps. Capture/log filesystem timestamps are +preserved; unobserved stage timestamps remain unknown. Prepared resources/jars +are not published. + +The first Java-only setup draft used item count2; it is retained separately and +was revised to count1 before either paired program. A draft unit missing a type +qualification failed compilation and was corrected before the old assertion run. +Neither is counted as a parity failure. + +## Fixed result and complete diagnostic residuals + +The fixed actual Game passes **198/198**, with byte-identical scoped script and +fresh Java reference bytes on old/fixed. All explicit-direction and item controls +remain exact. Full diagnostics are **198/228 old** and **202/228 fixed**; there is +no full-tag or 228-green claim. The full program is byte-identical across builds. +Its independent Java captures have different absolute startup clocks, so their +full references are not byte-identical. Their non-clock records are identical. +The scoped reference above excludes exactly the predeclared 30 diagnostic labels. + +A first fixed full run attempted to reuse the old full reference and stopped on +its byte-identity assertion: Java clocks 325 versus 313, solely at clock-start/end. +That complete capture is retained. The independent full fixed reference below +has clocks 301; native clocks are 0. Each run's actual relative delta is 0. No clock +values were edited, replaced or subtracted inside raw captured records. + +The 26 remaining full-record differences are listed below. For full-tag rows, +the table gives the differing field; complete bodies, styles and callbacks stay +in raw evidence. Every fixed full-tag residual differs only in Rotation. This is +the existing `item_frame_to_nbt` Rotation projection, separate from Facing default; +no encoder/rotation production repair is included. Old absent full tags also had +the Facing/Pos differences tabulated above. + +| Exact label | Java expected | Native actual (old and fixed unless noted) | +|---|---|---| +| `clock-start` | `325 old / 301 fixed` | `0` | +| `item_frame-absent-Rotation` | `[0.0f, 90.0f]` | `[0.0f, 0.0f]` | +| `item_frame-absent-full-tag` | `Rotation: [0.0f, 90.0f]` | `Rotation: [0.0f, 0.0f]` | +| `item_frame-0-Rotation` | `[0.0f, 90.0f]` | `[0.0f, 0.0f]` | +| `item_frame-0-full-tag` | `Rotation: [0.0f, 90.0f]` | `Rotation: [0.0f, 0.0f]` | +| `item_frame-1-Rotation` | `[0.0f, -90.0f]` | `[0.0f, 0.0f]` | +| `item_frame-1-full-tag` | `Rotation: [0.0f, -90.0f]` | `Rotation: [0.0f, 0.0f]` | +| `item_frame-2-Rotation` | `[180.0f, 0.0f]` | `[0.0f, 0.0f]` | +| `item_frame-2-full-tag` | `Rotation: [180.0f, 0.0f]` | `Rotation: [0.0f, 0.0f]` | +| `item_frame-4-Rotation` | `[90.0f, 0.0f]` | `[0.0f, 0.0f]` | +| `item_frame-4-full-tag` | `Rotation: [90.0f, 0.0f]` | `Rotation: [0.0f, 0.0f]` | +| `item_frame-5-Rotation` | `[270.0f, 0.0f]` | `[0.0f, 0.0f]` | +| `item_frame-5-full-tag` | `Rotation: [270.0f, 0.0f]` | `Rotation: [0.0f, 0.0f]` | +| `glow_item_frame-absent-Rotation` | `[0.0f, 90.0f]` | `[0.0f, 0.0f]` | +| `glow_item_frame-absent-full-tag` | `Rotation: [0.0f, 90.0f]` | `Rotation: [0.0f, 0.0f]` | +| `glow_item_frame-0-Rotation` | `[0.0f, 90.0f]` | `[0.0f, 0.0f]` | +| `glow_item_frame-0-full-tag` | `Rotation: [0.0f, 90.0f]` | `Rotation: [0.0f, 0.0f]` | +| `glow_item_frame-1-Rotation` | `[0.0f, -90.0f]` | `[0.0f, 0.0f]` | +| `glow_item_frame-1-full-tag` | `Rotation: [0.0f, -90.0f]` | `Rotation: [0.0f, 0.0f]` | +| `glow_item_frame-2-Rotation` | `[180.0f, 0.0f]` | `[0.0f, 0.0f]` | +| `glow_item_frame-2-full-tag` | `Rotation: [180.0f, 0.0f]` | `Rotation: [0.0f, 0.0f]` | +| `glow_item_frame-4-Rotation` | `[90.0f, 0.0f]` | `[0.0f, 0.0f]` | +| `glow_item_frame-4-full-tag` | `Rotation: [90.0f, 0.0f]` | `Rotation: [0.0f, 0.0f]` | +| `glow_item_frame-5-Rotation` | `[270.0f, 0.0f]` | `[0.0f, 0.0f]` | +| `glow_item_frame-5-full-tag` | `Rotation: [270.0f, 0.0f]` | `Rotation: [0.0f, 0.0f]` | +| `clock-end` | `325 old / 301 fixed` | `0` | + +Fixed full Java reference SHA256: `a82805f86496c65dd5b872bbc3d63a82cd71971e234eaa840d0dafd106b6977c`. + +## Local readiness and source checkpoints + +Required `tools/pr_ready.py --quick --base 3c7bb44aa12407aff43fe8c9adab51a9b9ebc61c` +ran once on the final production and test patch. Formatting, workspace/headless +clippy, public tree and doc tests pass. Rust: **1885 passed, 1 failed, 62 ignored**. +The failure is unchanged `entities::drop_motion_tests::drops_match_vanillas_motion_bit_for_bit`: +Z bits 4597448471913462162 versus 4597448471913462163 at yaw 37.5/pitch -21.25. The archived +old unit executable reproduces that exact failure; `entities.rs` is unchanged. +The added frame regression passes separately and in the full suite. + +Python: **686 run, 4 failures, 1 skipped**. Existing release-packaging tests fail on +this ARM64 Mac: the install-signature mock only supplies an x86_64 archive, and +Bash 3.2 optional-archive subcases access unset musl/arm64 arrays. A narrow unchanged +install-signature rerun retains the exact missing `horizon-macos-arm64.zip` error. +No packaging repair is included. Broad parity was skipped by `--quick`; the +explicit actual Game/Java fixture above was run independently. + +The ledger check reports direct-callee heuristics after adding the loader's +method citation; no broader ItemFrame method parity is claimed. Full map-check +also reports the pre-existing `PrimedTnt.explode -> Game::tick_tnt` stale mapping +at row 45 of the same TSV, identical at the frozen base. No unrelated map repair. + +Actual main was pulled before/after batches. Checkpoints 83da211d3, 0f5dc8d6d and +4f4a931b21d37bc64ce77081bb0a674248f71ad7 add schematic/redstone test tooling only; +the frame loader, serializer, model, Game route and command oracle are unchanged +from the frozen proof base. These are source reviews, not combined latest-main +runtime claims. Only evidence text was appended after required checks; public-tree +and whitespace checks were repeated on the final staged tree. diff --git a/tools/port_map/game2-002.tsv b/tools/port_map/game2-002.tsv index ac6c3cb836..266303e525 100644 --- a/tools/port_map/game2-002.tsv +++ b/tools/port_map/game2-002.tsv @@ -8,7 +8,7 @@ ItemFrame.removeFramedMap port crates/horizon-client/src/game/maps.rs::Game::rem ItemFrame.getSlot inline crates/horizon-client/src/game/commands/items.rs::Game::slot_get slots_named maps container.0 and contents on a frame to Slot::Frame; slot_get reads the frame's item and slot_set writes it ItemFrame.onSyncedDataUpdated skip:other network data-sync hook; the item is read from the frame field on demand ItemFrame.setRotation inline crates/horizon-client/src/game/frames.rs::Game::interact_frame the rotate-by-one step; the neighbour update is in frame_output_changed -ItemFrame.readAdditionalSaveData port crates/horizon-client/src/entity_nbt.rs::item_frame_from_nbt live command adoption preserves immutable frame type and detaches changed old stacks before replacement; gap: minor: Facing defaults to 2 where vanilla defaults to DOWN +ItemFrame.readAdditionalSaveData port crates/horizon-client/src/entity_nbt.rs::item_frame_from_nbt live command adoption preserves immutable frame type and detaches changed old stacks before replacement; absent Facing defaults to Direction.DOWN ItemFrame.getFrameItemStackWithData port crates/horizon-client/src/entities.rs::ItemFrame::item_stack_with_data drop_frame_items spawns that stack, so a broken item frame or glow item frame keeps its custom name ItemFrame.getVisualRotationYInDegrees inline crates/horizon-client/src/game/frames.rs::Game::frame_geometry the 45 degree step per rotation step is drawn in frame_geometry LeashFenceKnotEntity.recalculateBoundingBox port crates/horizon-client/src/mobs/leash.rs::knot_state position and box from the block position diff --git a/tools/test_frame_facing_default_parity.py b/tools/test_frame_facing_default_parity.py new file mode 100644 index 0000000000..df3c6e1136 --- /dev/null +++ b/tools/test_frame_facing_default_parity.py @@ -0,0 +1,114 @@ +#!/usr/bin/env python3 +"""Actual Game/Java frame loaders: absent Facing, and every explicit valid direction. + +--full-diagnostic also compares whole tags, Rotation and absolute clocks. These +remain complete raw observations in either mode; the scoped gate compares Facing, +Pos and the explicit class/UUID/block_pos/item/Fixed/rotation controls. +""" +import hashlib +import json +from pathlib import Path +import sys + +import test_command_exec_parity as driver + + +def program(full=False): + ops = [] + + def cmd(text, label, result=None, admission=True, diagnostic=False): + op = {'op': 'cmd', 'cmd': text, 'label': label, 'admission': admission} + if result is not None: + op['expect_result'] = result + if diagnostic and not full: + op.update(record=False, trace=True) + ops.append(op) + + 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({'op': 'settle', 'required_chunks': [[0, 0]]}) + cmd('/fill 0 99 0 15 99 15 stone', 'floor', 256) + cmd('/fill 0 100 0 15 105 15 air', 'air') + cmd('/tp @s 8.5 100 14.5 0 0', 'owner-position', 1) + cmd('/gamemode creative @s', 'owner-mode') + cmd('/scoreboard objectives add h134 dummy', 'objective') + cmd('/execute store result score start h134 run time query gametime', + 'clock-start', diagnostic=True) + cmd('/execute if entity @s[type=player,name=Player]', 'owner-type-name', 1) + for field, result in [('UUID', 4), ('Health', 20), ('playerGameType', 1), ('Pos', 3)]: + cmd(f'/data get entity @s {field}', 'owner-' + field, result) + for glow in [False, True]: + kind = 'glow_item_frame' if glow else 'item_frame' + z = 10 if glow else 4 + for n, facing in enumerate([None, 0, 1, 2, 3, 4, 5]): + x = 2 + n * 2 + index = n + 1 + (7 if glow else 0) + uuid = f'00000086-0000-0000-0000-{index:012x}' + label = kind + '-' + ('absent' if facing is None else str(facing)) + direction = '' if facing is None else f',Facing:{facing}b' + cmd(f'/execute if loaded {x} 102 {z}', label + '-loaded', 1) + cmd(f'/execute if block {x} 99 {z} stone', label + '-floor', 1) + cmd(f'/execute if block {x} 102 {z} air', label + '-gap', 1) + cmd(f'/summon {kind} {x + .5} 102.5 {z + .5} ' + f'{{UUID:[I;134,0,0,{index}],block_pos:[I;{x},102,{z}],Fixed:1b,' + f'Item:{{id:"minecraft:diamond",count:1}},ItemRotation:5b,ItemDropChance:0.25f{direction}}}', + label + '-summon', 1) + cmd(f'/execute as {uuid} if entity @s[type=minecraft:{kind}]', label + '-actual-type', 1) + for field, result in [('UUID', 4), ('block_pos', 3), ('Fixed', 1), + ('ItemRotation', 5), ('ItemDropChance', 0), ('Item', None)]: + cmd(f'/data get entity {uuid} {field}', label + '-' + field, result) + cmd(f'/data get entity {uuid} Facing', label + '-Facing', + 0 if facing is None else facing, admission=facing is not None) + cmd(f'/data get entity {uuid} Pos', label + '-Pos', 3, admission=facing is not None) + cmd(f'/data get entity {uuid} Rotation', label + '-Rotation', diagnostic=True) + cmd(f'/data get entity {uuid}', label + '-full-tag', diagnostic=True) + cmd('/execute store result score now h134 run time query gametime', 'clock-end', diagnostic=True) + cmd('/scoreboard players operation now h134 -= start h134', 'clock-delta-operation', 0) + cmd('/scoreboard players get now h134', 'clock-delta', 0) + return ops + + +def main(): + full = '--full-diagnostic' in sys.argv + if full: + sys.argv.remove('--full-diagnostic') + 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: + sys.argv += ['--work-dir', str(driver.lab.LOCAL / 'frame-facing-default-parity')] + build = Path(sys.argv[sys.argv.index('--work-dir') + 1]).resolve() + ops = program(full) + driver.corpus_module.groups = lambda: [('frame absent Facing and valid directions', ops)] + sys.argv.append('--allow-gaps') + driver.main() + if '--reference-only' in sys.argv: + return + recorded = [op for op in ops if op['op'] == 'cmd' and op.get('record', True)] + source = reference or build / 'vanilla.jsonl' + if reference: + assert source.read_bytes() == (build / 'vanilla.jsonl').read_bytes(), 'fresh Java bytes changed' + java = [json.loads(s) for s in source.read_text().splitlines()] + native = [json.loads(s) for s in (build / 'rust.jsonl').read_text().splitlines()] + assert len(java) == len(native) == len(recorded) + differences = [{'label': op['label'], 'expected': a, 'actual': b} + for op, a, b in zip(recorded, java, native) if a != b] + report = {'records': len(native), 'passed': len(native) - len(differences), + 'reference_sha256': hashlib.sha256(source.read_bytes()).hexdigest(), + 'comparison': 'complete raw records, no masks/tolerances', 'differences': differences} + (build / 'strict-report.json').write_text(json.dumps(report, indent=2) + '\n') + if not full: + for op, a, b in zip(recorded, java, native): + if op['admission']: + assert a == b, ('actual fixture admission', op, a, b) + print(f'Strict raw records: {report["passed"]}/{report["records"]}') + if differences: + raise SystemExit(1) + + +if __name__ == '__main__': + main()