Merge pull request #15815 from rdbtCVS/t3/fix-nightly-parity-regressions

Fix seven parity regressions blocking nightly builds

crates/horizon-client-render/src/hud_overlays.rs

@@ -39,14 +39,13 @@ pub fn text_with_backdrop(
     background: u32,
 ) {
     if background != 0 {
-        // Font.PreparedTextBuilder.markBackground: one pixel above and left of
-        // the glyphs, the glyph advance to the right and nine high: not a
-        // fixed two-pixel frame.
+        // GuiGraphicsExtractor.textWithBackdrop adds two pixels around the
+        // text's advance and the font's nine-pixel line height.
         out.push(fill(
-            x - 1,
-            y - 1,
-            x + w,
-            y + 9,
+            x - 2,
+            y - 2,
+            x + w + 2,
+            y + 11,
             multiply(background, color),
         ));
     }
@@ -1491,10 +1490,10 @@ mod boss_bar_room_tests {
 mod backdrop_tests {
     use super::*;
 
-    /// `markBackground`: the backdrop reaches one pixel above and left of the
-    /// glyphs, their advance to the right and nine down, not 2 on every side.
+    /// `GuiGraphicsExtractor.textWithBackdrop`: two pixels around the advance
+    /// and the nine-pixel font line, independently of glyph-background drawing.
     #[test]
-    fn the_backdrop_frames_the_glyphs_by_one_pixel() {
+    fn the_backdrop_frames_the_text_by_two_pixels() {
         let mut out = Vec::new();
         text_with_backdrop(
             &mut out,
@@ -1508,6 +1507,6 @@ mod backdrop_tests {
         let Some(GuiDraw::Fill { x0, y0, x1, y1, .. }) = out.first() else {
             panic!("a backdrop is a fill first");
         };
-        assert_eq!((*x0, *y0, *x1, *y1), (9, 19, 16, 29));
+        assert_eq!((*x0, *y0, *x1, *y1), (8, 18, 18, 31));
     }
 }

crates/horizon-client-render/src/skins.rs

@@ -93,18 +93,17 @@ pub fn unpack(value: &str, signature: Signature, valid_uris: &[impl AsRef<str>])
         signed: signature == Signature::Signed,
         ..empty.clone()
     };
-    // Each texture is loaded independently: an invalid or disallowed URL for one
-    // texture (cape, elytra) must not drop another (the skin). Vanilla's
-    // SkinManager works the same way.
+    // Authlib's MinecraftServicesSessionService validates every URL in
+    // unpackTextures before returning any textures. A bad cape rejects the skin too.
     for (kind, texture) in textures {
         let Some(url) = texture.get("url").and_then(|u| u.as_str()) else {
-            continue;
+            return empty;
         };
         let allowed = valid_uris
             .iter()
             .any(|u| url.starts_with(&u.as_ref().replace("{textureId}", "")));
         if !allowed {
-            continue;
+            return empty;
         }
         if kind == "SKIN" {
             out.slim = texture
@@ -1388,10 +1387,9 @@ mod tests {
                     &super::TEXTURE_URIS,
                 );
                 assert_eq!(textures.slim, slim && !url.contains("evil"));
-                // Textures are judged per URL (vanilla's SkinManager): the
-                // cape's URL is always allowed here, so it is kept even when
-                // the skin's URL is not.
-                assert!(textures.cape.is_some());
+                // Authlib rejects the whole payload if any URL is invalid,
+                // including the allowed cape when the skin is disallowed.
+                assert_eq!(textures.cape.is_some(), !url.contains("evil"));
                 assert_eq!(parsed.skin.is_some(), !url.contains("evil"));
                 assert!(std::sync::Arc::ptr_eq(&parsed, &cache.get(&profile)));
             }
@@ -1544,18 +1542,14 @@ mod tests {
         assert!(t.slim);
         assert!(!unpack(wide, Signature::Signed, &TEXTURE_URIS).slim);
     }
-    /// `unpack` keeps each texture independently: a cape from another origin
-    /// must not drop the skin.
+    /// Authlib's `MinecraftServicesSessionService` rejects the whole profile in
+    /// `unpackTextures` when any texture URL is outside the allowed origins.
     #[test]
-    fn unpack_keeps_the_skin_when_another_texture_is_rejected() {
+    fn unpack_rejects_the_skin_when_another_texture_is_rejected() {
         use super::{Signature, TEXTURE_URIS, unpack};
         let mixed = "eyJ0ZXh0dXJlcyI6IHsiU0tJTiI6IHsidXJsIjogImh0dHBzOi8vdGV4dHVyZXMubWluZWNyYWZ0Lm5ldC90ZXh0dXJlL3NraW4xIn0sICJDQVBFIjogeyJ1cmwiOiAiaHR0cHM6Ly9ldmlsLmV4YW1wbGUvdGV4dHVyZS9jYXBlMSJ9fX0=";
         let t = unpack(mixed, Signature::Unsigned, &TEXTURE_URIS);
-        assert_eq!(
-            t.skin.as_deref(),
-            Some("https://textures.minecraft.net/texture/skin1")
-        );
-        assert_eq!(t.cape, None);
+        assert_eq!(t, super::Textures::default());
     }
     #[test]
     fn profiles_parse_like_resolvable_profiles() {

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

@@ -237,7 +237,7 @@ fn goal_name(m: &Mob, name: &str) -> String {
         "RestrictSun" => "RestrictSunGoal",
         "FleeSun" => "FleeSunGoal",
         // `Cat.CatAvoidEntityGoal`, `Fox.FoxAvoidEntityGoal`... name their subclass.
-        "AvoidEntity" if m.species() == Species::Cat => "CatAvoidEntityGoal",
+        "AvoidEntity" | "CatAvoid" if m.species() == Species::Cat => "CatAvoidEntityGoal",
         "AvoidEntity" => "AvoidEntityGoal",
         "BowAttack" => "RangedBowAttackGoal",
         "Swell" => "SwellGoal",

crates/horizon-client/src/game/boats/below_world.rs

@@ -18,7 +18,7 @@ impl Game<'_> {
     /// `Entity.checkBelowWorld` for the boat at `i`: true when it fell out of
     /// the world and is gone.
     pub(in crate::game) fn boat_below_world(&mut self, i: usize) -> bool {
-        if !below_world(self.boats[i].position.y, self.world.bottom_y()) {
+        if !below_world(self.boats[i].position.y, self.world.shape.min_y) {
             return false;
         }
         self.eject_boat_mobs(i, true);
@@ -41,7 +41,7 @@ impl Game<'_> {
         kind: &str,
     ) -> Option<usize> {
         let i = cart_at(&self.carts, id, slot)?;
-        if !below_world(self.carts[i].position.y, self.world.bottom_y()) {
+        if !below_world(self.carts[i].position.y, self.world.shape.min_y) {
             return Some(i);
         }
         self.eject_cart_mobs(i);
@@ -68,44 +68,51 @@ mod tests {
         assert!(below_world(-64.5, 0));
     }
 
-    /// A tick discards the boat and the chest minecart under -128 in the
-    /// overworld, before they move, and spills the cart's slots; the two on
-    /// the line or above it stay, whatever their fall this tick.
+    /// A tick discards vehicles below each dimension's own floor, even when
+    /// its chunk storage extends lower, and spills the container's contents.
+    /// Vehicles on the line or above it survive this tick.
     #[test]
     #[ignore = "needs prepared resources in HORIZON_TEST_RESOURCES"]
     fn a_tick_discards_the_vehicles_under_the_world() {
         use horizon_client::chunk::Chunk;
         use horizon_core::position::ChunkPos;
-        let mut g = crate::game::native_machines::tests::server_game();
-        g.build_registries();
-        g.set_allow_commands(true);
-        g.world
-            .chunks
-            .insert((0, 0), std::sync::Arc::new(Chunk::empty(0, 0)));
-        g.block_ticks.add_empty_chunk(ChunkPos::new(0, 0)).unwrap();
-        assert!(g.run_command("forceload add 0 0"));
-        for summon in [
-            "summon oak_boat 0.5 -128 0.5",
-            "summon oak_boat 4.5 -128.5 4.5",
-            "summon minecart 6.5 -127 6.5",
-            "summon chest_minecart 2.5 -129 2.5 {Items:[{Slot:0b,id:\"minecraft:diamond\",count:3}]}",
-        ] {
-            assert!(g.run_command(summon), "{summon}");
+        for min_y in [-64, 0] {
+            let mut g = crate::game::native_machines::tests::server_game();
+            g.world.shape.min_y = min_y;
+            g.build_registries();
+            g.set_allow_commands(true);
+            g.world
+                .chunks
+                .insert((0, 0), std::sync::Arc::new(Chunk::empty(0, 0)));
+            g.block_ticks.add_empty_chunk(ChunkPos::new(0, 0)).unwrap();
+            assert!(g.run_command("forceload add 0 0"));
+            let line = min_y - 64;
+            for summon in [
+                format!("summon oak_boat 0.5 {line} 0.5"),
+                format!("summon oak_boat 4.5 {} 4.5", f64::from(line) - 0.5),
+                format!("summon minecart 6.5 {} 6.5", line + 1),
+                format!(
+                    "summon chest_minecart 2.5 {} 2.5 {{Items:[{{Slot:0b,id:\"minecraft:diamond\",count:3}}]}}",
+                    line - 1
+                ),
+            ] {
+                assert!(g.run_command(&summon), "{summon}");
+            }
+            g.sync_tick_chunks();
+            g.tick_boats();
+            g.tick_carts();
+            let boats: Vec<f64> = g.boats.iter().map(|b| b.position.x).collect();
+            assert_eq!(boats, [0.5]);
+            let carts: Vec<f64> = g.carts.iter().map(|c| c.position.x).collect();
+            assert_eq!(carts, [6.5]);
+            let spilled: u32 = g
+                .dropped
+                .iter()
+                .filter(|i| &*i.stack.item == "minecraft:diamond")
+                .map(|i| i.stack.count)
+                .sum();
+            assert_eq!(spilled, 3);
         }
-        g.sync_tick_chunks();
-        g.tick_boats();
-        g.tick_carts();
-        let boats: Vec<f64> = g.boats.iter().map(|b| b.position.x).collect();
-        assert_eq!(boats, [0.5]);
-        let carts: Vec<f64> = g.carts.iter().map(|c| c.position.x).collect();
-        assert_eq!(carts, [6.5]);
-        let spilled: u32 = g
-            .dropped
-            .iter()
-            .filter(|i| &*i.stack.item == "minecraft:diamond")
-            .map(|i| i.stack.count)
-            .sum();
-        assert_eq!(spilled, 3);
     }
 
     #[test]

docs/parity-workflow.md

@@ -72,6 +72,7 @@ A harness can declare, at the top level of its script:
 
 - `PARITY_SHARDS = N`: it runs as `NAME#1` to `NAME#N`, in parallel, each told its share in `PARITY_SHARD=i/N` (`test_datafix_parity.py` upgrades every sixth world). `NAME` on the command line or in `--known-failures` means all of them.
 - `PARITY_RUST_SCOPE = [...]`: the Rust sources its probes can reach, as path prefixes. Cargo's dep-info names every file of every crate a probe links, so without it a probe of `horizon-world` depends on all 350 of its files. The runner keeps only the scope's files if the scope is closed: nothing in it names a module, crate-root re-export or macro outside it (`crate::`, `super::`, other workspace crates; comments and strings ignored), declares a module it doesn't cover, or includes source, and nothing outside it implements anything for a type it defines. Crate roots then count without their declarations of other modules, so adding a module elsewhere doesn't rerun the harness. If the scope isn't closed the runner says why and keeps every file. `datafix` depends on 41 files this way, not 354.
+- `PARITY_MANUAL = "<how to run it>"`: the script is a proof someone runs by hand with arguments (a Java side, then a native side against its reference), not a harness. `parity.py` leaves it out of every run, `--changed` and parity-main included, and names that text when it is asked for it by name.
 
 ### Which harnesses a change affects
 

tests/test_parity_runner.py

@@ -413,6 +413,15 @@ class ShardTests(unittest.TestCase):
             self.assertEqual(parity.SHARDS["x#2"], "2/3")
             self.assertEqual(parity.base_name("x#2"), "x")
 
+    def test_a_manual_proof_script_is_left_out(self):
+        with tempfile.TemporaryDirectory() as temp, mock.patch.object(parity, "ROOT", Path(temp)), \
+                mock.patch.dict(parity.MANUAL, clear=True):
+            (Path(temp) / "tools").mkdir()
+            (Path(temp) / "tools/test_x_parity.py").write_text('import sys\nPARITY_MANUAL = "--side java, then --side native"\n')
+            (Path(temp) / "tools/test_y_parity.py").write_text("")
+            self.assertEqual(sorted(parity.harnesses()), ["y"])
+            self.assertEqual(parity.MANUAL, {"x": "--side java, then --side native"})
+
 
 class OracleArgumentTests(unittest.TestCase):
     def test_an_agent_jar_is_an_input(self):

tests/test_save_roundtrip_parity.py

@@ -72,5 +72,16 @@ class SaveRoundtripPathsTests(unittest.TestCase):
                 self.assertEqual(roundtrip.summarize(report), 2)
 
 
+class SaveRoundtripTimersTests(unittest.TestCase):
+    def test_elapsed_despawn_timers_do_not_hide_other_saved_state_changes(self):
+        report = [
+            ('entity.DespawnDelay @minecraft:trader_llama#57', 47384, 47375),
+            ('entity.Passengers[].DespawnDelay @minecraft:trader_llama#58', 100, 91),
+            ('entity.PersistenceRequired @minecraft:trader_llama#57', 0, 1),
+        ]
+        with contextlib.redirect_stdout(io.StringIO()):
+            self.assertEqual(roundtrip.summarize(report), 1)
+
+
 if __name__ == '__main__':
     unittest.main()

tools/parity.py

@@ -90,27 +90,33 @@ FAILURE_TAILS, FAILURE_TAIL_LINES = 10, 30  # the summary prints the end of the
 
 
 def harness_settings(path):
-    """PARITY_SHARDS and PARITY_RUST_SCOPE, read from a harness's source without running it."""
+    """PARITY_SHARDS, PARITY_RUST_SCOPE and PARITY_MANUAL, read from a harness's source without running it."""
     import ast
     text = Path(path).read_text(encoding="utf-8")
     settings = {}
     if "PARITY_" in text:
         for node in ast.parse(text).body:
             if isinstance(node, ast.Assign) and len(node.targets) == 1 and isinstance(node.targets[0], ast.Name) \
-                    and node.targets[0].id in ("PARITY_SHARDS", "PARITY_RUST_SCOPE"):
+                    and node.targets[0].id in ("PARITY_SHARDS", "PARITY_RUST_SCOPE", "PARITY_MANUAL"):
                 settings[node.targets[0].id] = ast.literal_eval(node.value)
     return settings
 
 
 SHARDS = {}  # "datafix#2" -> "2/8": the share of a sharded harness each entry runs (PARITY_SHARD)
+MANUAL = {}  # "guest_close_event" -> how to run it: a proof script that needs arguments, which no run starts
 
 
 def harnesses():
-    """Harness names and scripts. A harness declaring PARITY_SHARDS = N runs as N entries, NAME#1 to NAME#N."""
+    """Harness names and scripts. A harness declaring PARITY_SHARDS = N runs as N entries, NAME#1 to NAME#N.
+    A script declaring PARITY_MANUAL = "<how to run it>" is a proof run by hand with arguments: left out (MANUAL)."""
     found = {}
     for path in sorted((ROOT / "tools").glob("test_*_parity.py")):
         name = path.name[len("test_"):-len("_parity.py")]
-        count = harness_settings(path).get("PARITY_SHARDS", 1)
+        settings = harness_settings(path)
+        if settings.get("PARITY_MANUAL"):
+            MANUAL[name] = settings["PARITY_MANUAL"]
+            continue
+        count = settings.get("PARITY_SHARDS", 1)
         if count > 1:
             for index in range(1, count + 1):
                 found[f"{name}#{index}"] = path
@@ -1652,6 +1658,9 @@ def main():
     # A sharded harness's name stands for all its shards.
     args.names = [entry for name in args.names for entry in ([name] if name in found else
                   [shard for shard in found if base_name(shard) == name] or [name])]
+    by_hand = [name for name in args.names if name in MANUAL]
+    if by_hand:
+        parser.error("; ".join(f"{name} is run by hand, not by parity.py: {MANUAL[name]}" for name in by_hand))
     unknown = [name for name in args.names if name not in found]
     if unknown:
         parser.error("unknown harnesses: " + ", ".join(unknown))

tools/test_block_event_readiness_parity.py

@@ -16,7 +16,8 @@ except ImportError:
     import lab
     import test_rust_parity as base
 
-TEST = 'level::tests::fresh_chunk_'
+# The sculk regression lives in level::tests::trigger_tests after the test split.
+TEST = 'fresh_chunk_'
 
 
 def main():
@@ -41,7 +42,8 @@ def main():
                lab.ROOT / 'crates/horizon-world/src/level/mod.rs',
                lab.ROOT / 'crates/horizon-world/src/level/pistons.rs',
                lab.ROOT / 'crates/horizon-world/src/level/sculk.rs',
-               lab.ROOT / 'crates/horizon-world/src/level/tests.rs']
+               lab.ROOT / 'crates/horizon-world/src/level/tests.rs',
+               lab.ROOT / 'crates/horizon-world/src/level/tests/trigger_tests.rs']
     hashes = base.source_hashes(sources)
 
     def run(command, label, **kwargs):

tools/test_builtin_component_delivery_parity.py

@@ -17,6 +17,7 @@ import struct
 import subprocess
 import time
 import uuid
+PARITY_MANUAL = "a proof run by hand, once per side: --side java, then --side native, each with --binary BIN"  # not a harness: tools/parity.py leaves it out
 try:
     from . import anvil_dump as nbt, net_capture, test_locator_bar_parity as net
 except ImportError:

tools/test_entity_movement_predicate_parity.py

@@ -1,6 +1,7 @@
 """Actual named movement predicates over a frozen, UUID-bound unmounted pig."""
 import argparse,hashlib,json,sys
 from pathlib import Path
+PARITY_MANUAL = "a proof run by hand: --work-dir DIR --reference-only for the Java side, then --work-dir DIR2 --reference-dir DIR for the native one"  # not a harness: tools/parity.py leaves it out
 try:
  from . import test_command_exec_parity as driver
 except ImportError:

tools/test_guest_close_event_parity.py

@@ -11,6 +11,7 @@ import json
 from pathlib import Path
 import subprocess
 import sys
+PARITY_MANUAL = "a proof run by hand: --side java --work-dir DIR, then --side native --work-dir DIR2 --binary BIN --reference DIR/vanilla.jsonl"  # not a harness: tools/parity.py leaves it out
 
 ROOT = Path(__file__).resolve().parents[1]
 

tools/test_machine_recipe_placement_parity.py

@@ -14,6 +14,7 @@ import hashlib
 import struct
 import uuid
 import re
+PARITY_MANUAL = "a proof run by hand, once per side: --side java, then --side native, each with --binary BIN"  # not a harness: tools/parity.py leaves it out
 
 try:
     from . import anvil_dump as nbt, net_capture, test_locator_bar_parity as net

tools/test_mob_display_name_parity.py

@@ -24,18 +24,20 @@ except ImportError:
 
 NAME = 'mob-display-name-parity'
 SCRIPT = [
-    ('setup', ['chat on']),
-    ('summon', ['cmd Alice summon minecraft:wolf 2.5 -60 4.5 {NoAI:1b}']),
-    ('named', ['cmd Alice summon minecraft:pig 4.5 -60 4.5 {NoAI:1b,CustomName:"Bob"}']),
+    # Only the script's own mobs may match its selectors. Both servers load a
+    # real generated world and may otherwise spawn different numbers of pigs.
+    ('setup', ['chat on', 'cmd Alice gamerule spawn_mobs false']),
+    ('summon', ['cmd Alice summon minecraft:wolf 2.5 -60 4.5 {NoAI:1b,Tags:["display_name_parity"]}']),
+    ('named', ['cmd Alice summon minecraft:pig 4.5 -60 4.5 {NoAI:1b,CustomName:"Bob",Tags:["display_name_parity"]}']),
     # /team names the team with PlayerTeam.getFormattedDisplayName and its member with
     # ScoreHolder.getFeedbackDisplayName (the display name, its scoreboard name shown on hover).
     ('team', ['cmd Alice team add reds', 'cmd Alice team modify reds color red',
-              'cmd Alice team join reds @e[type=minecraft:pig]']),
-    ('data', ['cmd Alice data get entity @e[type=minecraft:pig,limit=1] Health']),
-    ('kill', ['cmd Alice kill @e[type=minecraft:wolf]']),
+              'cmd Alice team join reds @e[type=minecraft:pig,tag=display_name_parity]']),
+    ('data', ['cmd Alice data get entity @e[type=minecraft:pig,tag=display_name_parity,limit=1] Health']),
+    ('kill', ['cmd Alice kill @e[type=minecraft:wolf,tag=display_name_parity]']),
     # /scoreboard names score holders with getFeedbackDisplayName too: an entity's UUID, a player's name on hover.
     ('scores', ['cmd Alice scoreboard objectives add points dummy',
-                'cmd Alice scoreboard players set @e[type=minecraft:pig] points 3',
+                'cmd Alice scoreboard players set @e[type=minecraft:pig,tag=display_name_parity] points 3',
                 'cmd Alice scoreboard players set Alice points 1']),
 ]
 INTS = re.compile(r'"uuid":\[(-?\d+),(-?\d+),(-?\d+),(-?\d+)\]')

tools/test_predicate_reference_startup_parity.py

@@ -31,6 +31,7 @@ from pathlib import Path
 ROOT = Path(__file__).resolve().parents[1]
 sys.path.insert(0, str(ROOT / 'tools'))
 import lab  # noqa: E402
+PARITY_MANUAL = "a proof run by hand: --kind java --output DIR, then --kind native --output DIR2 --binary BIN"  # not a harness: tools/parity.py leaves it out
 
 META = '{"pack":{"description":"predicate reference graph","min_format":[121,0],"max_format":[121,0]}}\n'
 # 26.3 dispatches a predicate on `type`; a named reference is a holder: an id where a predicate goes.

tools/test_save_roundtrip_parity.py

@@ -86,7 +86,7 @@ IGNORED = {
     r'server_data\.state_updating_resumes_at)$':
         'block entity timers advance',
     r'^entity\.(Passengers\[\]\.)*(Age|Motion\[\]|Rotation\[\]|Pos\[\]|OnGround|HurtTime|Health|TimeInOverworld|fall_distance|'
-    r'wasOnGround|Fire|Air|DeathTime|Fuse|life|inGround|ticks_in_ground|Time|EggLayTime)$': 'entity simulation advances',
+    r'wasOnGround|Fire|Air|DeathTime|Fuse|life|inGround|ticks_in_ground|Time|EggLayTime|DespawnDelay)$': 'entity simulation advances',
     r'^level\.dat\.Data\.(Time|LastPlayed)$': 'clock and wall time of the save',
     r'^level\.dat\.Data\.(ServerBrands.*|singleplayer_uuid)$': 'a client world names its brand and its player',
     r'^entity\.listener\.selector\.(event|tick)$': 'the warden listener holds the last vibration it heard; timing',

tools/test_self_pose_metadata_parity.py

@@ -4,6 +4,7 @@ Initial default delivery is observational; flags never stand in for a typed Pose
 """
 import argparse,hashlib,json,os,re,time
 from pathlib import Path
+PARITY_MANUAL = "a proof run by hand: --work-dir DIR --reference-only for the Java side, then --work-dir DIR2 --reference-dir DIR --binary BIN"  # not a harness: tools/parity.py leaves it out
 try:
     from . import net_capture,test_locator_bar_parity as net
     from .test_builtin_component_delivery_parity import Commands,generate,offline,uuid_words