Skip to content

feat(keep-alive): emit keep_alive in CONFIGURATION and make interval configurable#79

Open
rayanbzd wants to merge 7 commits into
Quozul:masterfrom
rayanbzd:feat/keep-alive-configuration-state
Open

feat(keep-alive): emit keep_alive in CONFIGURATION and make interval configurable#79
rayanbzd wants to merge 7 commits into
Quozul:masterfrom
rayanbzd:feat/keep-alive-configuration-state

Conversation

@rayanbzd
Copy link
Copy Markdown
Contributor

If a client sits in CONFIGURATION state for more than 30s (e.g. behind a custom auth / registration dialog held server-side) Velocity and most proxies drops it w/ [connected player] read timed out. Velocity's default read-timeout is 30s and PicoLimbo wasn't sending any keep-alive until PLAY, so nothing kept the connection warm during a long hold.

Root cause

send_keep_alive in pico_limbo/src/server/network.rs only fired in State::Play

Vanilla / mojmap-side, minecraft:keep_alive is a legit clientbound packet in CONFIGURATION state too, same wire shape, just a different protocol ID per version (resolved automatically by the PacketReport macro from the data reports).

Fix

  • New variant PacketRegistry::ConfigurationClientBoundKeepAlive(ClientBoundKeepAlivePacket) w/ state = "configuration" / bound = "clientbound" / name = "minecraft:keep_alive". Reuses the existing play packet struct since the wire format is identical.
  • send_keep_alive now dispatches on client.state(): Configuration -> emits the new variant, Play -> emits the existing one, anything else -> no-op.
  • LoginAcknowledgedPacket calls client_state.set_keep_alive_should_enable() right after set_state(Configuration). The ticker arms on the next enable_keep_alive_if_needed() (end of process_packet), same machinery PLAY already uses.

The interval is also made user-configurable while we're here: new top-level TOML option keep_alive_interval_seconds (default 15, matching vanilla). It flows from Config -> ServerStateBuilder::keep_alive_interval_secs -> ServerState::keep_alive_interval() -> ClientData::new so each connection picks up the configured value at construction time. The 2-second period for 1.7.6- clients stays hard-coded since it's a protocol requirement, not a tunable.

Test

  • test_login_ack_supported_protocol tightened to assert client_state.should_enable_keep_alive() after the handler runs, so a regression of "ticker armed in CONFIG" is caught at the unit level.
  • All 4 existing login_acknowledged tests still pass.

E2E I ran locally: Velocity 3.5.x + PacketEvents 2.12.1 + a 26.1.2 client + a custom CONFIG-state hold of around 60s. Before: read timed out after 30s. After: connection stays up, PE plugins see the keep-alive flow, no Velocity log noise.

Scope / risk

  • No protocol-version gating beyond the existing supports_configuration_state() check. The keep_alive clientbound report is defined in configuration for every version of the repo that supports CONFIG state (1.20.2 → 26.1).
  • Vanilla-equivalent default behaviour: with the default keep_alive_interval_seconds = 15, this matches what a vanilla server does in CONFIG state.

…configurable

Send `minecraft:keep_alive` while the client sits in CONFIGURATION, so a long custom hold there (auth / registration / dialog screen) does not trip Velocity's 30s read-timeout. The ticker is armed when the client enters CONFIGURATION from LoginAcknowledged. `send_keep_alive` now dispatches on state, emitting the configuration variant in CONFIG and the play variant in PLAY.

The interval is now exposed as a top-level config option `keep_alive_interval_seconds` (default 15, matching vanilla). It flows from `Config` through `ServerState` into `ClientData` at connection time. Legacy 1.7.6- clients still use the protocol-mandated 2s ping regardless of this value.
Copy link
Copy Markdown
Owner

@Quozul Quozul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also update the CHANGELOG.md file? I've updated it in my PR #75 if you want to check the format.

Comment thread pico_limbo/src/configuration/config.rs
Comment thread pico_limbo/src/handlers/login/login_acknowledged.rs
rayanbzd added 4 commits May 13, 2026 00:42
…configurable

Send `minecraft:keep_alive` while the client sits in CONFIGURATION, so a long custom hold there (auth / registration / dialog screen) does not trip Velocity's 30s read-timeout. The ticker is armed when the client enters CONFIGURATION from LoginAcknowledged. `send_keep_alive` now dispatches on state, emitting the configuration variant in CONFIG and the play variant in PLAY.

The interval is now exposed as a top-level config option `keep_alive_interval_seconds` (default 15, matching vanilla). It flows from `Config` through `ServerState` into `ClientData` at connection time. Legacy 1.7.6- clients still use the protocol-mandated 2s ping regardless of this value.
…ate' into feat/keep-alive-configuration-state
Move the call to where the state transition actually happens:
- LoginStart (for < 1.20.2, which skip CONFIGURATION entirely)
- LoginAcknowledged (for >= 1.20.2, already in place)
@rayanbzd rayanbzd requested a review from Quozul May 12, 2026 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants