fix: lockstep mode requires one-way latency instead of round-trip

[gschup]

The lockstep gate previously required last_confirmed_frame == current_frame, forcing both sides to be phase-locked. With input_delay=D and one-way latency L, this caused a stall/burst cycle of length L on every frame regardless of D, effectively requiring D=2L (round-trip) to run smoothly.

The fix relaxes the gate to frames_ahead <= input_delay, allowing each side to pipeline up to D committed frames ahead of confirmation. Since those frames use delayed (not predicted) inputs, no rollback is ever needed. With D=L, remote inputs arrive just in time every frame with no stalls. D=0 collapses back to the old behaviour (frames_ahead must be 0).

Implementation:

• InputQueue: add frame_delay() getter
• SyncLayer: add min_local_input_delay(local_handles) to read the minimum delay across all local player queues
• P2PSession: apply new gate; extract rollback+save block into handle_rollback_and_save(); deduplicate frames_ahead calculation

Tests:

• test_lockstep_stalls_without_remote_input: gate stalls when remote input has not arrived (no cross-poll)
• test_lockstep_advances_with_zero_latency_zero_delay: both sessions advance every frame when cross-polled before each advance

Fixes #116

[gschup]

@loganmc10 please take a look!

[loganmc10]

This seems to run a little better for a few seconds, but then I get this crash:

thread 'main' (1238769) panicked at /var/home/loganmc10/.cargo/git/checkouts/ggrs-5de1f5c5d7148c68/9d8104d/src/sync_layer.rs:340:9:
assertion failed: first_incorrect == NULL_FRAME || first_incorrect >= frame

[gschup]

hmm. the more I think about it, the more I think that the pre-existing logic was correct in the first place. I wonder if there is something else I am forgetting about...

[loganmc10]

I believe your original comment is on the right track (this caused a stall/burst cycle of length L on every frame). When I printed out the confirmed frame and current frame, it seemed like the confirmed frame would jump every so often, probably related to the input delay. I test this locally by using tc/netem to simulate latency, for example:

sudo tc qdisc add dev lo root netem delay 80ms 10ms

[gschup]

@loganmc10 another try with a wild new approach. Let me know if you have tried it out on your setup!

[loganmc10]

Now I get this:

thread 'main' (1283913) panicked at /var/home/loganmc10/.cargo/git/checkouts/ggrs-5de1f5c5d7148c68/67fe0f6/src/input_queue.rs:221:9:
assertion failed: self.length <= INPUT_QUEUE_LENGTH
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The workaround seems to work:

 if p2p_session.current_frame() > p2p_session.confirmed_frame()
        && p2p_session.confirmed_frame() != ggrs::NULL_FRAME
{
    poll clients
}
advance_frame()

Could you not just have advance_frame() return an empty requests list if p2p_session.current_frame() > p2p_session.confirmed_frame()? Since advance_frame() calls poll_remote_clients, I could just have it continuously call advance_frame() until the Requests list is not empty.

[gschup]

I tried to fix the fix. its a bit messy, but cleanup will follow after it starts working for you :D

[loganmc10]

It is better, but it still seems to add 1-2 extra frames of latency? With my current workaround, on a simulated 80ms link, I can get roughly 60FPS with 5 frames of input delay (which makes sense, since 80/16.66 is almost 5). But with this PR, I have to set the input delay to 7 to get 50 a full 60 FPS.

[gschup]

Hmm, this is quite a tricky one to figure out. I will keep going, but I might need some time to track down the proper fix. I will ping you once I have a new try.

I hope you achieve loading and saving before I get there, though :) rollback to the future!

[loganmc10]

yes, saving/loading does work, but the problem is the graphics. The N64 used a shared memory architecture (the CPU and GPU can write to the same memory), and the graphics are emulated using Vulkan.

I don't know if I can do a good job explaining the problem, but basically, when you save (or load) the state, you need to tell the Vulkan side "finish what you are doing" to make sure that the memory is correct for the state. But this kills the performance, especially when you are possibly doing one load and many saves per frame. The emulator already didn't have a ton of wiggle room to be emulating/simulating multiple frames per step.

Some games rely on the data that the GPU writes to the memory for the game itself (if you are familiar with Pokémon Snap for example, the game actually "looks" at the rendered picture to decide if you got a good picture), so you can't just skip the graphics when making the state.

Anyway, one day I'm sure I will figure something out, but for now, just having it spin before calling advance_frame is enough to avoid triggering a rollback.

[gschup]

I think this is an improvement to the codebase in any case. Will merge, but leave the issue open with a few comments.