From 0efe1c2c13459829f03763fc6adc76c38ce7ae82 Mon Sep 17 00:00:00 2001 From: Horizon agent Date: Fri, 6 Nov 2026 13:11:18 +0100 Subject: [PATCH] Player effects also leave out ambient false and show_particles true MobEffectInstance.Details.MAP_CODEC uses optionalFieldOf("ambient", false) and ("show_particles", true), so like amplifier 0 those defaults are not written (the mob writer already does this). The harness gets a default work dir, so tools/parity.py can run it with no arguments; the run-log doc is dropped. Co-Authored-By: Claude Opus 5.5 --- crates/horizon-client/src/save.rs | 26 +++--- .../parity/player-effect-amplifier-default.md | 88 ------------------- tools/test_player_effect_amplifier_parity.py | 6 +- 3 files changed, 21 insertions(+), 99 deletions(-) delete mode 100644 docs/parity/player-effect-amplifier-default.md diff --git a/crates/horizon-client/src/save.rs b/crates/horizon-client/src/save.rs index b16108bd48..9740ad2d65 100644 --- a/crates/horizon-client/src/save.rs +++ b/crates/horizon-client/src/save.rs @@ -4222,11 +4222,16 @@ impl Level { Tag::Byte((*amplifier).clamp(0, 255) as u8 as i8), )); } - effect.extend([ - ("ambient", Tag::Byte(i8::from(*ambient))), - ("show_particles", Tag::Byte(i8::from(*particles))), - ("show_icon", Tag::Byte(i8::from(*particles))), - ]); + // optionalFieldOf("ambient", false) and + // ("show_particles", true): defaults are not written; + // show_icon is always written. + if *ambient { + effect.push(("ambient", Tag::Byte(1))); + } + if !*particles { + effect.push(("show_particles", Tag::Byte(0))); + } + effect.push(("show_icon", Tag::Byte(i8::from(*particles)))); Tag::compound(effect) }) .collect(), @@ -7200,11 +7205,12 @@ mod tests { for (entry, (id, duration, _, ambient, particles)) in entries.iter().zip(&effects) { assert_eq!(entry.get("id"), Some(&string(id))); assert_eq!(entry.get("duration"), Some(&Tag::Int(*duration))); - assert_eq!(entry.get("ambient"), Some(&Tag::Byte(i8::from(*ambient)))); - assert_eq!( - entry.get("show_particles"), - Some(&Tag::Byte(i8::from(*particles))) - ); + // MobEffectInstance.Details.MAP_CODEC leaves out ambient false + // and show_particles true. + let ambient_tag = ambient.then_some(Tag::Byte(1)); + assert_eq!(entry.get("ambient"), ambient_tag.as_ref()); + let particles_tag = (!*particles).then_some(Tag::Byte(0)); + assert_eq!(entry.get("show_particles"), particles_tag.as_ref()); assert_eq!( entry.get("show_icon"), Some(&Tag::Byte(i8::from(*particles))) diff --git a/docs/parity/player-effect-amplifier-default.md b/docs/parity/player-effect-amplifier-default.md deleted file mode 100644 index a31dd2de89..0000000000 --- a/docs/parity/player-effect-amplifier-default.md +++ /dev/null @@ -1,88 +0,0 @@ -# Player saved effect amplifier default - -H100 changes only `save.rs::Level::player_tag`: omit an amplifier whose modeled -value is zero. The existing clamp to 0..255 and unsigned Byte conversion stay -unchanged for other values. The loader, live effects, other effect fields and -separate Mob writer are untouched. - -Pinned Java 26.3 `MobEffectInstance.Details` builds its map codec with -`ExtraCodecs.UNSIGNED_BYTE.optionalFieldOf("amplifier", 0)`. PR6369 changed only -the Mob writer. PR9061 supplied the Player unsigned-byte reload/clamp behavior, -which this fix preserves. - -## Actual Game proof - -Frozen base: `0d21d5520ec26c49b774bf1533e53ea8af81278d`. -Script: `tools/test_player_effect_amplifier_parity.py`, SHA-256 -`2b2023bb710b24dac97953ee87765a9c1585c3c90ce4842cb3f0eed2e023605c`. -Fresh Java reference SHA-256: -`b4a8db3e861ac399f61093270580a5e2917882a40fa2ce9c1ca615763011f805`. - -Real Player commands apply/clear an infinite Luck effect at amplifiers 0/1/255. -Saved reads use the existing `/data get entity @s` route: Game synchronizes the -live player and calls `Level::player_tag`, the same Player saved encoder used by -world/player file writes. No observer or packet implementation was added. This -proves that saved-NBT projection, without a save/reopen or disk-I/O claim. - -The predeclared 71 command records compare as complete raw JSON, including all -messages/styles and callbacks, without masks/tolerances. UUID/type/Health20, -position/mode, load/floor/air, exact effect id/duration/type, one-effect count, -particle/icon flags, effective nonambient state, real effect removal and zero -relative world-clock controls stay strict. Each amplifier read has fresh -result/success sentinels. Whole effect/body compounds and absolute startup -clocks are separate complete raw traces, not whole-record equality claims. - -Untouched Game matches 68/71; fixed Game matches 71/71 with identical script -and fresh Java reference bytes. Exactly three old records differ for amplifier 0: - -| Record | Fresh Java | Untouched Game | -| --- | --- | --- | -| Saved amplifier read | Missing field; callback`[false,0]` | `0b`; callback`[true,0]` | -| Success store | `0` | `1` | -| Field-absence predicate | Test passed;`[true,1]` | Test failed, count1;`[false,0]` | - -The positive 1 and 255 controls pass; 255 saves as typed Byte -1. Live effect -presence is independently witnessed by successful real effect removal, rather -than inferred from the saved amplifier's absence. - -The resource-free save regression starts both with no raw input and with stale -raw effects. It checks 0 omission and Byte 1/-128/-1, unchanged id/duration/flags, -unrelated raw data and unchanged live model. On the old writer it fails at -`Some(Byte(0))` versus`None`; the existing 255-reload and ended-effect controls pass. - -Source checkpoint `932d351e115e9a3e31f29eaaba0614004bbb3621` adds unrelated -region compression selection/tests in `save.rs`; the Player writer and command -read path are unchanged. The paired runtime proof remains qualified to 0d21. - -## Retained whole-effect diagnostic - -The original 68-record program/reference and complete old output remain local -and executable. It matches 62/68: the three H100 records above plus -`amp0-count`, `amp1-count`, `amp255-count`, which read whole effect lists. -Those lists also contain native `ambient:0b`, whereas Java omits the default. -The 71-record program replaces those three compound comparisons with exact -first-entry/absent-second-entry controls. It preserves their full raw reads as -traces. No ambient/default-flag repair belongs to H100; no whole-diagnostic equality claim. The fixed diagnostic matches 65/68; every -remaining exact saved-list value difference is below (all three callbacks -remain `[true,1]`). - - -| Label | Java saved list | Fixed Game saved list | -| --- | --- | --- | -| `amp0-count` | `[{duration: -1, show_icon: 0b, id: "minecraft:luck", show_particles: 0b}]` | `[{duration: -1, show_icon: 0b, ambient: 0b, id: "minecraft:luck", show_particles: 0b}]` | -| `amp1-count` | `[{duration: -1, show_icon: 0b, amplifier: 1b, id: "minecraft:luck", show_particles: 0b}]` | `[{duration: -1, show_icon: 0b, ambient: 0b, amplifier: 1b, id: "minecraft:luck", show_particles: 0b}]` | -| `amp255-count` | `[{duration: -1, show_icon: 0b, amplifier: -1b, id: "minecraft:luck", show_particles: 0b}]` | `[{duration: -1, show_icon: 0b, ambient: 0b, amplifier: -1b, id: "minecraft:luck", show_particles: 0b}]` | - -Run in a fresh destination using the existing command harness: - -```sh -python3 tools/test_player_effect_amplifier_parity.py --reference-only \ - --work-dir .local/player-effect-reference -python3 tools/test_player_effect_amplifier_parity.py --binary /path/to/horizon-client \ - --work-dir .local/player-effect-native --reference .local/player-effect-reference/vanilla.jsonl -``` - -The native invocation independently reruns Java and requires the exact same -reference bytes. All commands, original diagnostic scripts, raw records and -traces are retained locally, including the earlier 68-record compound admission -failure. The focused 71-record proof independently admits every actual subject/state. diff --git a/tools/test_player_effect_amplifier_parity.py b/tools/test_player_effect_amplifier_parity.py index 5d3cf4f546..916dcaa631 100644 --- a/tools/test_player_effect_amplifier_parity.py +++ b/tools/test_player_effect_amplifier_parity.py @@ -85,7 +85,11 @@ def main(): 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: - raise SystemExit('a fresh --work-dir is required') + # tools/parity.py runs every harness with no arguments: a fresh folder under .local. + import shutil + default = Path(__file__).resolve().parents[1] / '.local' / 'player-effect-amplifier-parity' + shutil.rmtree(default, ignore_errors=True) + sys.argv += ['--work-dir', str(default)] build = Path(sys.argv[sys.argv.index('--work-dir')+1]).resolve() assert not build.exists(), 'preserve earlier attempts; use a fresh directory' build.mkdir(parents=True)