Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spectator session doesn't advance when started before hosting P2P session #97

Open
caspark opened this issue Dec 20, 2024 · 0 comments
Open
Labels
bug Something isn't working

Comments

@caspark
Copy link
Contributor

caspark commented Dec 20, 2024

Describe the bug
When a spectator session is started well before a host session, the spectator is never requested to advance frame.

To Reproduce
Using example game, on Linux or MacOS:

cargo run --example ex_game_spectator -- --local-port 7001 --host 127.0.0.1:7000 --num-players 1 | sed 's/^/spec: /' &
sleep 1 # make the host start later
cargo run --example ex_game_p2p -- --local-port 7000 --players localhost --spectators 127.0.0.1:7001 | sed 's/^/host: /' &

Should be easy enough to adjust for a Windows repro: remove the piping to sed (which is only there to attribute log lines to processes easily) and manually wait a second or so before starting the second command.

Observed behavior
Spectator game starts but stays at frame 0 indefinitely.

Expected behavior
Spectator game starts and advances frame.

Additional context

  • If you start the host then the spectator, everything works fine.
  • I haven't bisected, but it repros at least as far back as dec7a16, and possibly further - I just stopped there because it satisfied me that at least I didn't break it with all my recent PRs :)
  • I never see a message from the spectator saying it has synchronized, so this seems like a bug in the "synchronize before we start advancing frames" logic; I debugged for 10 mins and it seems like the problem comes from this:
    • if the spectator starts first, then its initial sync message is missed by the host
    • the host then starts and syncs with the spectator, successfully
    • since the host->spectator sync completes successfully, the host starts sending inputs to the spectator
    • the spectator acks the messages that it receives from the host
    • the spectator's ack messages reset its UdpProtocol::last_send_time, so the logic to resend sync requests doesn't actually ever trigger (i.e. if self.last_send_time + SYNC_RETRY_INTERVAL < now { self.send_sync_request(); } never actually sends a sync request)
  • If that diagnosis is correct, it should be an easy fix to keep a separate bit of state to track "last sync request send time" (leaving "last send time" to only be used by the keepalive send mechanism).
    • Alternatively, it seems likely that moving the synchronization step to happen at the UDP protocol layer (as discussed in Make Synchronization step optional for non-UDP transport layers #95) would incidentally solve this too, as the timer for "when should we send the next sync requested message" being at the UDP socket level would make it easy to keep it separate from "when did we last send any message".
@caspark caspark added the bug Something isn't working label Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant