Repository navigation
fix(resourcepack): wait for the client to drop the pack before kicking - #289
Merged
Merged
Conversation
Contributor
Test results 516 files 516 suites 3m 22s ⏱️ Results for commit 82cffa3. |
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.
Proposed changes
Players (reported for the slender) kept the Cygnus resource pack in the lobby after the
RestartPhasekick orSpectatorService.leave. Behind Velocity a kick moves the player to the lobby and the client connection survives. The pop was sent immediately before the disconnect, and nothing guaranteed the client had processed it before being rerouted.Fix.
CygnusPlayer.kicksends the pop and defers the disconnect until either:DISCARDED) for our pack id viaonResourcePackStatus, orAn exactly-once guard covers repeated kicks and the ack/timeout race. Kicks without a pack id, or outside the PLAY state, stay immediate.
ResourcePackService.handleStatusignores a leaving player'sDISCARDED, so the "must accept" message no longer replaces the leave message. 10 ticks is well below the 1 sRestartPhasewaits before shutdown.Minestom notes.
PlayerResourcePackStatusEvent#getPackUuididentifies the pack.ResourcePackListenerfires the event, then callsPlayer#onResourcePackStatus.Types of changes
Checklist
Further comments
Limitation. A player-initiated proxy switch (
/hub,/server) closes the connection without warning. That cannot be solved here. A related lobby-side safeguard is tracked in OneLiteFeatherNET/Titan#361.Test plan.
CygnusPlayerResourcePackKickTestandResourcePackServiceTestcover: the pop is sent before the kick, the disconnect is deferred untilDISCARDEDor the 10 tick timeout, repeated kicks and ack+timeout disconnect only once, kicks without a pack id or outside PLAY stay immediate, and a leaving player'sDISCARDEDdoes not trigger the "must accept" message../gradlew :game:testis green (658 tests).