feat(keep-alive): emit keep_alive in CONFIGURATION and make interval configurable#79
Open
rayanbzd wants to merge 7 commits into
Open
feat(keep-alive): emit keep_alive in CONFIGURATION and make interval configurable#79rayanbzd wants to merge 7 commits into
rayanbzd wants to merge 7 commits into
Conversation
…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.
Quozul
reviewed
May 12, 2026
…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)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_aliveinpico_limbo/src/server/network.rsonly fired inState::PlayVanilla / mojmap-side,
minecraft:keep_aliveis a legit clientbound packet in CONFIGURATION state too, same wire shape, just a different protocol ID per version (resolved automatically by thePacketReportmacro from the data reports).Fix
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_alivenow dispatches onclient.state():Configuration-> emits the new variant,Play-> emits the existing one, anything else -> no-op.LoginAcknowledgedPacketcallsclient_state.set_keep_alive_should_enable()right afterset_state(Configuration). The ticker arms on the nextenable_keep_alive_if_needed()(end ofprocess_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(default15, matching vanilla). It flows fromConfig->ServerStateBuilder::keep_alive_interval_secs->ServerState::keep_alive_interval()->ClientData::newso 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_protocoltightened to assertclient_state.should_enable_keep_alive()after the handler runs, so a regression of "ticker armed in CONFIG" is caught at the unit level.login_acknowledgedtests 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 outafter 30s. After: connection stays up, PE plugins see the keep-alive flow, no Velocity log noise.Scope / risk
supports_configuration_state()check. Thekeep_aliveclientbound report is defined inconfigurationfor every version of the repo that supports CONFIG state (1.20.2 → 26.1).keep_alive_interval_seconds = 15, this matches what a vanilla server does in CONFIG state.