Fix client steering for saddled pigs and striders

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

@@ -31,7 +31,7 @@ fn stick_on_break(stack: &ItemStack, wear: u32, max: u32) -> Option<ItemStack> {
     Some(rod)
 }
 
-fn steering_item(species: Species, main: Option<&str>, offhand: Option<&str>) -> bool {
+pub(super) fn steering_item(species: Species, main: Option<&str>, offhand: Option<&str>) -> bool {
     let item = match species {
         Species::Pig => "minecraft:carrot_on_a_stick",
         Species::Strider => "minecraft:warped_fungus_on_a_stick",

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

@@ -9,7 +9,9 @@
 //! server (`LocalPlayer.tick`: `ServerboundMoveVehiclePacket`,
 //! `AbstractBoat.tick`: `ServerboundPaddleBoatPacket`). So is a saddled horse,
 //! donkey, mule or camel: its rider's client moves it and reports the moves,
-//! and tells the server of a riding jump (`START_RIDING_JUMP`). A minecart
+//! and tells the server of a riding jump (`START_RIDING_JUMP`). Pigs and striders
+//! also move on their rider's client when saddled and steered with the right stick.
+//! A minecart
 //! moves on the server with the rider's input, and the rider sits in it.
 use super::{Game, Keys, ray_box};
 use crate::Result;
@@ -431,7 +433,19 @@ impl Game<'_> {
     /// saddle (`isLocalInstanceAuthoritative`).
     fn drives(&self, m: &Mob) -> bool {
         self.riding == Some(m.id)
-            && client_moves(m, self.remote.as_ref().is_some_and(|r| r.first_passenger))
+            && client_moves(
+                m,
+                self.remote.as_ref().is_some_and(|r| r.first_passenger),
+                self.steers_mount(m),
+            )
+    }
+
+    fn steers_mount(&self, m: &Mob) -> bool {
+        super::mounts::steering_item(
+            m.species(),
+            self.held().map(|s| s.item.as_ref()),
+            self.offhand.as_ref().map(|s| s.item.as_ref()),
+        )
     }
 
     /// The mount this client moves, if the player rides one.
@@ -452,6 +466,7 @@ impl Game<'_> {
         }
         let strafe = f32::from(u8::from(keys.left)) - f32::from(u8::from(keys.right));
         let forward = f32::from(u8::from(keys.forward)) - f32::from(u8::from(keys.backward));
+        let steer = self.steers_mount(&self.mobs[i]);
         let m = &mut self.mobs[i];
         if jump > 0.0 && equine::is_equine(m.species()) {
             // LocalPlayer.aiStep also runs `onPlayerJump` here: the mount slides
@@ -470,7 +485,7 @@ impl Game<'_> {
             yaw: self.yaw,
             pitch: self.pitch,
             jump: if jump > 0.0 { jump } else { pending },
-            steer: false,
+            steer,
             sprint: sprinting,
             jumping: keys.jump,
             client: false,
@@ -498,12 +513,15 @@ impl Game<'_> {
 /// (`HappyGhast.getControllingPassenger`: wearing body armor, player on
 /// top, not on a still timeout — the rider steers it with `travelRidden`
 /// on their own client, and the mount goes to the server as `MoveVehicle`).
-fn client_moves(m: &Mob, first_passenger: bool) -> bool {
+fn client_moves(m: &Mob, first_passenger: bool, steer: bool) -> bool {
     if !first_passenger || !m.alive() {
         return false;
     }
     if equine::moved_by_rider(m.species()) && m.extra.saddled {
-        return true;
+        return !matches!(
+            m.species(),
+            horizon_client::mobs::Species::Pig | horizon_client::mobs::Species::Strider
+        ) || steer;
     }
     m.species() == horizon_client::mobs::Species::HappyGhast
         && happy_ghast::harnessed_adult(m)
@@ -536,6 +554,129 @@ mod tests {
     use super::*;
     use horizon_client::mobs::MobTypes;
 
+    #[test]
+    #[ignore = "requires prepared resources and the vanilla mount-control reference"]
+    fn steering_mount_control_matches_vanilla() {
+        use super::super::{
+            Audio, Options, Progress, Settings, execution_role::ExecutionRole, load_resources,
+        };
+        use horizon_client::{items::ItemStack, motion::PlayerMotion};
+        use horizon_protocol::connection::Pipe;
+        let root = std::path::PathBuf::from(std::env::var_os("HORIZON_TEST_RESOURCES").unwrap());
+        let reference = std::env::var_os("HORIZON_MOUNT_CONTROL_REFERENCE").unwrap();
+        let rows: Vec<serde_json::Value> =
+            serde_json::from_slice(&std::fs::read(reference).unwrap()).unwrap();
+        assert_eq!(rows.len(), 32, "missing vanilla control cases");
+        let settings = Settings::default();
+        let loaded = load_resources(
+            root.clone(),
+            None,
+            &root,
+            &settings,
+            std::time::Instant::now(),
+            false,
+            true,
+            &Progress::new(),
+            &mut |_, _| unreachable!("no menu handoff"),
+        )
+        .unwrap();
+        let mut options = Options::parse(std::iter::empty()).unwrap().unwrap();
+        options.resources = root.clone();
+        options.execution_role = ExecutionRole::Client;
+        let mut game = Game::new(
+            &loaded.resources,
+            loaded.pack,
+            loaded.gui.unwrap(),
+            options,
+            settings,
+            Audio::silent(&root),
+            None,
+        )
+        .unwrap();
+        let (client, _server) = Pipe::pair();
+        game.connect_local(client, "SteeringTest").unwrap();
+        game.remote.as_mut().unwrap().joined = true;
+        game.player = Some(PlayerMotion::new(game.motion, Vec3::new(0.5, 300.0, 0.5)).unwrap());
+        let types = MobTypes::load(&root).unwrap();
+        for row in rows {
+            let kind = row["kind"].as_str().unwrap();
+            let t = std::sync::Arc::new(types.types[&format!("minecraft:{kind}")].clone());
+            let mut m = Mob::new(7, t, Vec3::new(0.5, 300.0, 0.5), 0.0);
+            if row["saddled"].as_bool().unwrap() {
+                m.set_saddle(Some(ItemStack::new("minecraft:saddle", 1)));
+            }
+            let stick = if kind == "pig" {
+                "minecraft:carrot_on_a_stick"
+            } else {
+                "minecraft:warped_fungus_on_a_stick"
+            };
+            let wrong = if kind == "pig" {
+                "minecraft:warped_fungus_on_a_stick"
+            } else {
+                "minecraft:carrot_on_a_stick"
+            };
+            let hand = row["hand"].as_str().unwrap();
+            game.inventory[game.selected] = match hand {
+                "main" => Some(ItemStack::new(stick, 1)),
+                "wrong" => Some(ItemStack::new(wrong, 1)),
+                _ => None,
+            };
+            game.offhand = (hand == "off").then(|| ItemStack::new(stick, 1));
+            game.mobs.clear();
+            game.mobs.push(m);
+            game.riding = Some(7);
+            game.remote.as_mut().unwrap().first_passenger = row["first"].as_bool().unwrap();
+            let controlled = row["controlled"].as_bool().unwrap();
+            assert_eq!(game.driven_mount().is_some(), controlled, "{row}");
+            let packets = game.remote_vehicle_packets();
+            assert_eq!(packets.len(), usize::from(controlled), "{row}");
+            if controlled {
+                assert!(matches!(packets[0], S::MoveVehicle(_)), "{row}");
+                let before = game.mobs[0].position;
+                assert!(game.remote_ride(Keys::default(), 0).unwrap());
+                let m = &game.mobs[0];
+                assert_eq!(
+                    f64::from(m.ai.zza),
+                    row["forward"].as_f64().unwrap(),
+                    "{row}"
+                );
+                assert!(m.position.z > before.z, "{row}: mount did not move forward");
+                match &game.remote_vehicle_packets()[0] {
+                    S::MoveVehicle(p) => assert_eq!(
+                        p.position,
+                        [m.position.x, m.position.y, m.position.z],
+                        "{row}"
+                    ),
+                    packet => panic!("wrong movement packet: {packet:?}"),
+                }
+            }
+            // The server reads the same held-item requirement, then waits for
+            // this controlling client's MoveVehicle rather than traveling twice.
+            if row["first"].as_bool().unwrap() {
+                assert!(game.tick_mount_riding(Keys::default()));
+                game.mobs[0].rider.as_mut().unwrap().client = true;
+            } else {
+                game.mobs[0].rider = None;
+            }
+            assert_eq!(
+                equine::moved_by_client(&game.mobs[0]),
+                !row["server_simulates"].as_bool().unwrap(),
+                "{row}"
+            );
+            if controlled {
+                let mut server_mount = game.mobs[0].clone();
+                let before = server_mount.position;
+                let mut random = game.world.random.clone();
+                let mut events = Vec::new();
+                server_mount.tick(&game.mob_world_base(&[]), &mut random, &mut events);
+                assert_eq!(
+                    server_mount.position, before,
+                    "server traveled twice: {row}"
+                );
+            }
+        }
+    }
+
     /// A mob of the prepared resources' type `kind` (None without them).
     fn mob(kind: &str) -> Option<Mob> {
         let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR"))
@@ -547,7 +688,7 @@ mod tests {
     }
 
     /// #2920: on a vanilla server the rider's client moves a saddled horse,
-    /// donkey, mule or camel; a pig, a llama, an unsaddled horse or a second
+    /// donkey, mule or camel; a pig without its stick, a llama, an unsaddled horse or a second
     /// passenger's ride is the server's.
     #[test]
     fn the_rider_moves_a_saddled_horse_on_its_client() {
@@ -555,14 +696,17 @@ mod tests {
             let Some(mut m) = mob(&format!("minecraft:{kind}")) else {
                 return;
             };
-            assert!(!client_moves(&m, true), "{kind} without a saddle");
+            assert!(!client_moves(&m, true, false), "{kind} without a saddle");
             m.set_saddle(Some(horizon_client::items::ItemStack::new(
                 "minecraft:saddle",
                 1,
             )));
             let horse = matches!(kind, "horse" | "donkey" | "mule" | "camel");
-            assert_eq!(client_moves(&m, true), horse, "{kind}");
-            assert!(!client_moves(&m, false), "{kind} behind another rider");
+            assert_eq!(client_moves(&m, true, false), horse, "{kind}");
+            assert!(
+                !client_moves(&m, false, false),
+                "{kind} behind another rider"
+            );
         }
     }
 
@@ -574,12 +718,12 @@ mod tests {
         let Some(mut m) = mob("minecraft:happy_ghast") else {
             return;
         };
-        assert!(!client_moves(&m, true), "ghast without a harness");
+        assert!(!client_moves(&m, true, false), "ghast without a harness");
         m.extra.body = Some("minecraft:white_happy_ghast_harness".into());
-        assert!(client_moves(&m, true), "harnessed adult");
-        assert!(!client_moves(&m, false), "behind another rider");
+        assert!(client_moves(&m, true, false), "harnessed adult");
+        assert!(!client_moves(&m, false, false), "behind another rider");
         m.extra.age = -1;
-        assert!(!client_moves(&m, true), "baby ghast");
+        assert!(!client_moves(&m, true, false), "baby ghast");
     }
 
     /// #2920: the client reports where it moved the mount, and a released

crates/horizon-client/src/mobs/equine.rs

@@ -285,12 +285,12 @@ pub fn controlled(m: &Mob) -> bool {
         _ => m.extra.saddled,
     }
 }
-/// `AbstractHorse`s (camels too, not llamas, which no player steers): the
-/// player controlling one moves it on its own client
+/// Saddled equines (not llamas), pigs and striders: the controlling player
+/// moves the mount on their own client
 /// (`Entity.isLocalInstanceAuthoritative`, `LivingEntity.travelRidden`) and
 /// sends the moves (`ServerboundMoveVehiclePacket`).
 pub fn moved_by_rider(s: Species) -> bool {
-    is_equine(s) && !super::llama::is_llama(s)
+    matches!(s, Species::Pig | Species::Strider) || (is_equine(s) && !super::llama::is_llama(s))
 }
 /// A server's mount that its rider's client moves: `travelRidden` without
 /// `canSimulateMovement` turns it (`tickRidden`) and stops it there.

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

@@ -200,6 +200,71 @@ public final class VanillaCommandExecOracle {
             Files.writeString(output.resolveSibling("fishing-fallback-reference.json"), rows.toString());
         }
 
+        void writeMountControlReference() throws Exception {
+            if (!Boolean.getBoolean("horizon.mountControlReference")) return;
+            var level = this.overworld();
+            var oldMain = player.getMainHandItem();
+            var oldOff = player.getOffhandItem();
+            var oldPosition = player.position();
+            JsonArray rows = new JsonArray();
+            for (String kind : new String[] {"pig", "strider"}) {
+                for (boolean saddled : new boolean[] {false, true}) {
+                    for (boolean first : new boolean[] {false, true}) {
+                        for (String hand : new String[] {"none", "wrong", "main", "off"}) {
+                            net.minecraft.world.entity.Mob mount = kind.equals("pig")
+                                ? new net.minecraft.world.entity.animal.pig.Pig(net.minecraft.world.entity.EntityTypes.PIG, level)
+                                : new net.minecraft.world.entity.monster.Strider(net.minecraft.world.entity.EntityTypes.STRIDER, level);
+                            mount.setPos(oldPosition);
+                            mount.setItemSlot(net.minecraft.world.entity.EquipmentSlot.SADDLE,
+                                saddled ? new net.minecraft.world.item.ItemStack(net.minecraft.world.item.Items.SADDLE)
+                                    : net.minecraft.world.item.ItemStack.EMPTY);
+                            var stick = kind.equals("pig") ? net.minecraft.world.item.Items.CARROT_ON_A_STICK
+                                : net.minecraft.world.item.Items.WARPED_FUNGUS_ON_A_STICK;
+                            var wrong = kind.equals("pig") ? net.minecraft.world.item.Items.WARPED_FUNGUS_ON_A_STICK
+                                : net.minecraft.world.item.Items.CARROT_ON_A_STICK;
+                            player.setItemInHand(net.minecraft.world.InteractionHand.MAIN_HAND,
+                                hand.equals("main") ? new net.minecraft.world.item.ItemStack(stick)
+                                    : hand.equals("wrong") ? new net.minecraft.world.item.ItemStack(wrong)
+                                    : net.minecraft.world.item.ItemStack.EMPTY);
+                            player.setItemInHand(net.minecraft.world.InteractionHand.OFF_HAND,
+                                hand.equals("off") ? new net.minecraft.world.item.ItemStack(stick)
+                                    : net.minecraft.world.item.ItemStack.EMPTY);
+                            var other = new net.minecraft.world.entity.player.Player(level,
+                                new GameProfile(new UUID(0, 2), "OtherRider")) {
+                                public net.minecraft.world.level.GameType gameMode() {
+                                    return net.minecraft.world.level.GameType.SURVIVAL;
+                                }
+                            };
+                            if (!first && !other.startRiding(mount, true, true)) throw new IllegalStateException("first passenger rejected");
+                            if (!player.startRiding(mount, true, true)) throw new IllegalStateException("player passenger rejected");
+                            if ((mount.getFirstPassenger() == player) != first) throw new IllegalStateException("passenger order differs");
+                            var input = mount.getClass().getDeclaredMethod("getRiddenInput", net.minecraft.world.entity.player.Player.class,
+                                net.minecraft.world.phys.Vec3.class);
+                            input.setAccessible(true);
+                            var direction = (net.minecraft.world.phys.Vec3)input.invoke(mount, player, net.minecraft.world.phys.Vec3.ZERO);
+                            JsonObject row = new JsonObject();
+                            row.addProperty("kind", kind);
+                            row.addProperty("saddled", saddled);
+                            row.addProperty("first", first);
+                            row.addProperty("hand", hand);
+                            row.addProperty("controlled", mount.getControllingPassenger() == player);
+                            row.addProperty("server_simulates", mount.canSimulateMovement());
+                            row.addProperty("forward", direction.z);
+                            rows.add(row);
+                            player.stopRiding();
+                            other.stopRiding();
+                            other.discard();
+                            mount.discard();
+                        }
+                    }
+                }
+            }
+            player.setItemInHand(net.minecraft.world.InteractionHand.MAIN_HAND, oldMain);
+            player.setItemInHand(net.minecraft.world.InteractionHand.OFF_HAND, oldOff);
+            player.setPos(oldPosition);
+            Files.writeString(output.resolveSibling("mount-control-reference.json"), rows.toString());
+        }
+
         // Direct reference for FishingHook.retrieve on a real server player and level.
         void writeFishingReference() throws Exception {
             if (!Boolean.getBoolean("horizon.fishingReference")) return;
@@ -419,6 +484,7 @@ public final class VanillaCommandExecOracle {
                 Files.writeString(output.resolveSibling("client_config.json"), login.toString());
             }
             writeOrbWaterReference();
+            try { writeMountControlReference(); } catch (Exception e) { throw new IOException(e); }
             try { writeFishingReference(); } catch (Exception e) { throw new IOException(e); }
             try { writeFishingFallbackReference(); } catch (Exception e) { throw new IOException(e); }
             StringBuilder out = new StringBuilder();

tools/test_mount_control_parity.py

@@ -0,0 +1,43 @@
+#!/usr/bin/env python3
+"""Compare pig/strider client authority with the real 26.3 controlling-passenger methods.
+
+The ignored Rust fixture exercises actual client prediction and MoveVehicle packet
+selection, plus the server's moved-by-client guard, without a window or network server.
+"""
+import json
+import os
+from pathlib import Path
+import subprocess
+import sys
+import tempfile
+
+try:
+    from . import lab
+    from .integration_env import game_environment
+except ImportError:
+    import lab
+    from integration_env import game_environment
+
+
+def main():
+    work = Path(tempfile.mkdtemp(prefix="mount-control-", dir=lab.LOCAL))
+    env = game_environment(os.environ, work)
+    env["JAVA_TOOL_OPTIONS"] = env.get("JAVA_TOOL_OPTIONS", "") + " -Dhorizon.mountControlReference=true"
+    subprocess.run([sys.executable, str(lab.ROOT / "tools/test_command_exec_parity.py"),
+                    "--only", "scoreboard", "--reference-only", "--work-dir", str(work)],
+                   cwd=lab.ROOT, env=env, check=True)
+    reference = work / "mount-control-reference.json"
+    rows = json.loads(reference.read_text())
+    if (len(rows) != 32 or sum(r["controlled"] for r in rows) != 4
+            or sum(not r["server_simulates"] for r in rows) != 4):
+        raise SystemExit("vanilla mount-control fixture did not cover 32 cases / four controllers")
+    env["HORIZON_MOUNT_CONTROL_REFERENCE"] = str(reference)
+    env["HORIZON_TEST_RESOURCES"] = os.environ.get("HORIZON_TEST_RESOURCES", str(lab.RESOURCES))
+    subprocess.run(["cargo", "test", "-p", "horizon-client", "--lib",
+                    "game::remote_vehicles::tests::steering_mount_control_matches_vanilla",
+                    "--", "--exact", "--ignored"], cwd=lab.ROOT, env=env, check=True)
+    print("mount control: 32 vanilla cases match; four controllers predict forward movement and send MoveVehicle")
+
+
+if __name__ == "__main__":
+    main()