Preserve tick-step requests and send them before piston events

Co-Authored-By: Codex GPT-6.1 <[email protected]>

crates/horizon-client/src/clock.rs

@@ -278,6 +278,16 @@ impl ServerTickRateManager {
 const SPRINT_PER_FRAME: i64 = 40;
 const SAMPLES: usize = 100;
 
+/// A step command's broadcast value and the ticks still owed to a new client.
+/// `ServerTickRateManager.updateStepTicks` sends the former immediately;
+/// `ServerTickRateManager.updateJoiningPlayer` sends the latter on a join.
+#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
+pub struct StepState {
+    pub revision: u32,
+    pub requested: i32,
+    pub remaining: i32,
+}
+
 /// The game's clock: `DeltaTracker.Timer` pacing driven by the
 /// `ServerTickRateManager` the `/tick` command controls, over a virtual
 /// millisecond clock built from the frame times it is fed. The integrated
@@ -300,6 +310,7 @@ pub struct FixedStep {
     /// How many times `/tick step` started or stopped a step
     /// (`ServerTickRateManager.updateStepTicks` broadcasts each one).
     step_changes: u32,
+    step_requested: i32,
     /// In-frame ticks dropped since the last overload warning (`drop_ticks`).
     dropped: u32,
     /// When the last overload warning was due, on the frame clock.
@@ -320,6 +331,7 @@ impl Default for FixedStep {
             tick_count: 0,
             client: false,
             step_changes: 0,
+            step_requested: 0,
             dropped: 0,
             last_overload_warning: std::time::Duration::ZERO,
         }
@@ -488,18 +500,29 @@ impl FixedStep {
         let started = self
             .manager
             .step_game_if_paused(ticks.min(i32::MAX as u32) as i32);
-        self.step_changes += u32::from(started);
+        if started {
+            self.step_changes += 1;
+            self.step_requested = self.manager.base.frozen_ticks_to_run();
+        }
         started
     }
     pub fn stop_step(&mut self) -> bool {
         let stopped = self.manager.stop_stepping();
-        self.step_changes += u32::from(stopped);
+        if stopped {
+            self.step_changes += 1;
+            self.step_requested = 0;
+        }
         stopped
     }
-    /// What `ClientboundTickingStepPacket` carries (`frozenTicksToRun`), and
-    /// a count that changes each time `updateStepTicks` would broadcast it.
-    pub fn step_state(&self) -> (u32, i32) {
-        (self.step_changes, self.manager.base.frozen_ticks_to_run())
+    /// Preserve the command's count until the network pass, even if a world
+    /// tick has already consumed it.
+    /// New arrivals instead receive the current remaining ticks.
+    pub fn step_state(&self) -> StepState {
+        StepState {
+            revision: self.step_changes,
+            requested: self.step_requested,
+            remaining: self.manager.base.frozen_ticks_to_run(),
+        }
     }
     /// Record how long a tick took (`MinecraftServer.tickServer` bookkeeping
     /// and `endTickWork`).
@@ -607,15 +630,55 @@ mod tests {
     fn only_a_started_or_stopped_step_is_broadcast() {
         let mut clock = FixedStep::default();
         assert!(!clock.start_step(5), "not frozen");
-        assert_eq!(clock.step_state(), (0, 0));
+        assert_eq!(clock.step_state(), StepState::default());
         clock.set_frozen(true);
         assert!(clock.start_step(5));
-        assert_eq!(clock.step_state(), (1, 5));
+        assert_eq!(
+            clock.step_state(),
+            StepState {
+                revision: 1,
+                requested: 5,
+                remaining: 5
+            }
+        );
         clock.world_runs();
-        assert_eq!(clock.step_state(), (1, 4), "a step tick is not broadcast");
+        assert_eq!(
+            clock.step_state(),
+            StepState {
+                revision: 1,
+                requested: 5,
+                remaining: 4
+            },
+            "consuming a tick preserves the command broadcast"
+        );
         assert!(clock.stop_step());
         assert!(!clock.stop_step());
-        assert_eq!(clock.step_state(), (2, 0));
+        assert_eq!(
+            clock.step_state(),
+            StepState {
+                revision: 2,
+                requested: 0,
+                remaining: 0
+            }
+        );
+    }
+    #[test]
+    fn a_consumed_single_step_still_advances_the_client_once() {
+        let mut server = FixedStep::default();
+        server.set_frozen(true);
+        assert!(server.start_step(1));
+        assert!(server.world_runs());
+        assert!(!server.world_runs());
+        let state = server.step_state();
+        assert_eq!(state.remaining, 0, "a late join has no pending ticks");
+        let mut client = FixedStep::client();
+        client.set_frozen(true);
+        client.set_frozen_ticks_to_run(state.requested);
+        assert!(
+            client.world_runs(),
+            "the connected client runs the command's tick"
+        );
+        assert!(!client.world_runs(), "then remains frozen");
     }
     #[test]
     fn a_servers_tick_rate_only_slows_its_client_down() {

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

@@ -2213,6 +2213,13 @@ impl Game<'_> {
             for d in &mut self.dropped {
                 d.needs_sync = false;
             }
+            // The step command reaches clients before the events its tick
+            // generated, as ServerTickRateManager.updateStepTicks does.
+            host.hub.sync_ticking(
+                self.clock.rate(),
+                self.clock.frozen(),
+                self.clock.step_state(),
+            );
             // `runBlockEvents`' broadcast precedes the tick's block changes
             // (`ChunkHolder.broadcastChanges` in `tick_level` below): the
             // guest re-runs a piston's `triggerEvent` while its level still

crates/horizon-client/src/lan/hub.rs

@@ -144,9 +144,8 @@ pub struct TickCtx<'a> {
     /// TickRateManager: the tick rate and whether the game is frozen.
     pub tick_rate: f32,
     pub tick_frozen: bool,
-    /// `/tick step`: a count that changes with each step started or
-    /// stopped, and the ticks the frozen game still runs (`FixedStep::step_state`).
-    pub tick_steps: (u32, i32),
+    /// The last step command's broadcast and the remaining ticks for joins.
+    pub tick_steps: crate::clock::StepState,
     /// Block collisions, for ServerGamePacketListenerImpl's moved-wrongly
     /// check (none: moves are not checked against blocks).
     pub collision: Option<&'a BlockCollisionRegistry<'a>>,
@@ -593,6 +592,7 @@ impl Hub {
         stack_to_network(item_id, |registry, name| sync.id(registry, name), s)
     }
 
+    /// `PlayerList.broadcastAll`: send a packet to every placed player.
     pub fn broadcast(&self, packet: &C) {
         let body = packet.encode();
         for g in self.guests.iter().filter(|g| g.placed) {
@@ -1283,7 +1283,7 @@ impl Hub {
         // TickRateManager.updateJoiningPlayer.
         g.send(&ticking_state(ctx.tick_rate, ctx.tick_frozen));
         g.send(&C::TickingStep(play::TickingStep {
-            steps: ctx.tick_steps.1,
+            steps: ctx.tick_steps.remaining,
         }));
     }
 
@@ -1475,7 +1475,7 @@ impl Hub {
             // (`updateJoiningPlayer`).
             if !self.guests.iter().any(|g| g.placed) {
                 self.ticking = (ctx.tick_rate, ctx.tick_frozen);
-                self.stepping = ctx.tick_steps.0;
+                self.stepping = ctx.tick_steps.revision;
             }
             if !self
                 .guests
@@ -1580,6 +1580,22 @@ impl Hub {
         g
     }
 
+    /// `ServerTickRateManager.updateStepTicks` / `updateStateToClients`:
+    /// send clock changes before the block events caused by their ticks.
+    pub fn sync_ticking(&mut self, rate: f32, frozen: bool, steps: crate::clock::StepState) {
+        if (rate, frozen) != self.ticking {
+            self.ticking = (rate, frozen);
+            self.broadcast(&ticking_state(rate, frozen));
+        }
+        // ServerTickRateManager.updateStepTicks: a step started or stopped.
+        if steps.revision != self.stepping {
+            self.stepping = steps.revision;
+            self.broadcast(&C::TickingStep(play::TickingStep {
+                steps: steps.requested,
+            }));
+        }
+    }
+
     /// Time and weather (ServerLevel.tickTime / advanceWeatherCycle broadcasts).
     fn sync_world(&mut self, ctx: &TickCtx) {
         let world = &*ctx.world;
@@ -1614,17 +1630,7 @@ impl Hub {
         }
         self.last_clock = day;
         self.last_rate = rate;
-        if (ctx.tick_rate, ctx.tick_frozen) != self.ticking {
-            self.ticking = (ctx.tick_rate, ctx.tick_frozen);
-            self.broadcast(&ticking_state(ctx.tick_rate, ctx.tick_frozen));
-        }
-        // ServerTickRateManager.updateStepTicks: a step started or stopped.
-        if ctx.tick_steps.0 != self.stepping {
-            self.stepping = ctx.tick_steps.0;
-            self.broadcast(&C::TickingStep(play::TickingStep {
-                steps: ctx.tick_steps.1,
-            }));
-        }
+        self.sync_ticking(ctx.tick_rate, ctx.tick_frozen, ctx.tick_steps);
         let w = &world.weather;
         let (raining, rain, thunder) = self.weather;
         if w.raining != raining {

crates/horizon-client/tests/lan_bot.rs

@@ -203,7 +203,7 @@ impl Server {
             clock_rate: 1.0,
             tick_rate: 20.0,
             tick_frozen: false,
-            tick_steps: (0, 0),
+            tick_steps: Default::default(),
             collision: None,
             border: &horizon_client::worldborder::WorldBorder::default(),
             streaming: None,
@@ -1419,7 +1419,7 @@ fn a_respawned_guest_is_immune_until_its_client_loads_the_world() {
         clock_rate: 1.0,
         tick_rate: 20.0,
         tick_frozen: false,
-        tick_steps: (0, 0),
+        tick_steps: Default::default(),
         collision: None,
         border: &border,
         streaming: None,

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

@@ -104,6 +104,7 @@ import net.minecraft.world.phys.Vec3;
  * {@code paddle NAME LEFT RIGHT} sends the two boolean paddle states.
  */
 public final class LocatorBarClient {
+    private static volatile boolean tickSteps;
     private static volatile String step = "start";
     private static volatile boolean players;
     private static volatile boolean chat;
@@ -140,6 +141,7 @@ public final class LocatorBarClient {
                         abilities.flying = w[2].equals("on");
                         players.get(w[1]).send(new net.minecraft.network.protocol.game.ServerboundPlayerAbilitiesPacket(abilities));
                     }
+                    case "ticksteps" -> tickSteps = w[1].equals("on");
                     case "still" -> players.get(w[1]).still();
                     case "respawn" -> players.get(w[1]).send(new net.minecraft.network.protocol.game.ServerboundClientCommandPacket(
                         net.minecraft.network.protocol.game.ServerboundClientCommandPacket.Action.PERFORM_RESPAWN));
@@ -435,6 +437,11 @@ public final class LocatorBarClient {
                     if (p instanceof net.minecraft.network.protocol.game.ClientboundLoginPacket l) {
                         id = l.playerId();
                     }
+                    if (tickSteps && p instanceof net.minecraft.network.protocol.game.ClientboundTickingStepPacket t)
+                        record("tick-step " + t.tickSteps());
+                    if (tickSteps && p instanceof net.minecraft.network.protocol.game.ClientboundBlockEventPacket e
+                        && e.getPos().equals(new net.minecraft.core.BlockPos(0, 80, 0)))
+                        record("piston-event " + e.getB0() + " " + e.getB1());
                     if (state) recordState(p);
                     if (boats && p instanceof net.minecraft.network.protocol.game.ClientboundAddEntityPacket a
                         && isBoat(a.getType())) {

tools/test_tick_step_network_parity.py

@@ -0,0 +1,136 @@
+#!/usr/bin/env python3
+"""Compare /tick step broadcasts and late-join state on real 26.3 servers.
+
+The native-codec client records the actual ClientboundTickingStepPacket counts.
+One-tick steps are synchronized by server game time, so the network pass must
+still send the requested count after the server has consumed it. A late join
+receives the remaining count instead of replaying a completed step command.
+"""
+import platform
+import time
+
+try:
+    from . import lab, test_rust_parity as base, test_locator_bar_parity as net
+    from .test_dedicated_server_parity import Server, game_time
+except ImportError:
+    import lab
+    import test_rust_parity as base
+    import test_locator_bar_parity as net
+    from test_dedicated_server_parity import Server, game_time
+
+
+def run_side(label, command, client_command, build, world):
+    directory = build / label
+    port = net.server_dir(directory, world)
+    out = build / f'{label}.txt'
+    server = Server(command, directory, label)
+    try:
+        server.wait_for(r'Done \(')
+        with (build / f'{label}-client.log').open('w') as log:
+            client = net.Client(client_command, port, out, log)
+            try:
+                for line in ['ticksteps on', 'step joins', 'join Alice', 'fly Alice on']:
+                    client.tell(line)
+                server.wait_for('Alice joined the game', timeout=120)
+                client.wait(server, ['Alice'], 5)
+                server.console('tick freeze')
+                game_time(server)
+                client.sync(['Alice'])
+                for name, ticks in [('single', 1), ('five', 5), ('single-again', 1)]:
+                    client.tell(f'step {name}')
+                    before = game_time(server)
+                    server.console(f'tick step {ticks}')
+                    deadline = time.monotonic() + 20
+                    while game_time(server) < before + ticks:
+                        if time.monotonic() > deadline:
+                            raise TimeoutError(f'{label}: {name} did not finish')
+                        time.sleep(0.02)
+                    client.sync(['Alice'])
+                client.tell('step long-step')
+                server.console('tick step 1000')
+                game_time(server)
+                client.sync(['Alice'])
+                client.tell('step stop')
+                server.console('tick step stop')
+                game_time(server)
+                client.sync(['Alice'])
+                client.tell('step late-join')
+                client.tell('join Bob')
+                server.wait_for('Bob joined the game', timeout=120)
+                client.sync(['Alice', 'Bob'])
+                # A rejected step while unfrozen sends no update.
+                client.tell('step unfrozen')
+                server.console('tick unfreeze')
+                server.console('tick step 1')
+                game_time(server)
+                client.sync(['Alice', 'Bob'])
+                # The step notification must precede the piston event it causes.
+                # Both clients are nearby so the server sends them the event.
+                for command in ['tp Alice 0.5 82 -4', 'tp Bob 2.5 82 -4', 'tick freeze',
+                                'setblock 0 80 0 sticky_piston[facing=east]',
+                                'setblock 1 80 0 stone', 'setblock -1 80 0 redstone_block']:
+                    server.console(command)
+                game_time(server)
+                client.sync(['Alice', 'Bob'])
+                client.tell('step piston')
+                before = game_time(server)
+                server.console('tick step 1')
+                deadline = time.monotonic() + 20
+                while game_time(server) == before:
+                    if time.monotonic() > deadline:
+                        raise TimeoutError(f'{label}: piston step did not run')
+                    time.sleep(0.02)
+                client.sync(['Alice', 'Bob'])
+                client.end()
+            finally:
+                client.kill()
+    finally:
+        server.stop()
+    observations = [line for line in out.read_text().splitlines()
+                    if ' tick-step ' in line or ' piston-event ' in line]
+    # Separate receiver threads may interleave, but preserve each one's wire order.
+    return sorted(observations, key=lambda line: line.split(' ', 1)[0])
+
+
+def main():
+    lock, build, vanilla, horizon, client = net.sides('tick-step-network-parity')
+    sources = [lab.ROOT / 'crates/horizon-client/src/clock.rs',
+               lab.ROOT / 'crates/horizon-client/src/lan/hub.rs',
+               lab.ROOT / 'mods/horizon/src/test/java/dev/horizonlab/parity/LocatorBarClient.java',
+               lab.ROOT / 'tools/test_locator_bar_parity.py',
+               lab.ROOT / 'tools/test_dedicated_server_parity.py', lab.ROOT / __file__]
+    hashes = base.source_hashes(sources)
+    world = net.generate_world(vanilla, build)
+    expected = [
+        'Alice step=joins tick-step 0',
+        'Alice step=single tick-step 1',
+        'Alice step=five tick-step 5',
+        'Alice step=single-again tick-step 1',
+        'Alice step=long-step tick-step 1000',
+        'Alice step=stop tick-step 0',
+        'Alice step=piston tick-step 1',
+        'Alice step=piston piston-event 0 5',
+        'Bob step=late-join tick-step 0',
+        'Bob step=piston tick-step 1',
+        'Bob step=piston piston-event 0 5',
+    ]
+    reference = run_side('vanilla', vanilla, client, build, world)
+    if reference != expected:
+        raise AssertionError(f'vanilla scenario did not establish the contract: {reference}')
+    actual = run_side('horizon', horizon, client, build, world)
+    if actual != reference:
+        raise AssertionError(f'Horizon differs from vanilla:\n{actual}\nexpected:\n{reference}')
+    base.ensure_sources_unchanged(sources, hashes)
+    report = {'schema': 1, 'minecraft': lock['minecraft']['version'],
+              'platform': platform.platform(), 'sources_sha256': hashes,
+              'matched_output_lines': len(actual),
+              'scope': 'Actual dedicated servers and native-codec client. Requested counts for '
+                       'single/multiple/repeated steps, cancellation, rejected unfrozen steps, '
+                       'remaining ticks on a late join, and step-before-piston-event ordering for '
+                       'both clients. Does not render a graphical client.'}
+    lab.write_json(build / 'report.json', report)
+    print(f'Tick-step network parity passed: {len(actual)} native packet observations')
+
+
+if __name__ == '__main__':
+    main()