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

Fix for disconnect causing instant loss #13390

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ssk97
Copy link
Contributor

@ssk97 ssk97 commented Feb 25, 2025

For the past several months, users with somewhat spotty connections have sometimes experienced disconnects causing instant concessions with no chance to reconnect. Upon examination of server logs, it is seemingly entirely due to CLIENT DISCONNECTED.

I'm unsure as to why this is occurring, as the jboss documentation suggests that that the ClientDisconnectedException should only happen if the client does a normal client termination, but it has been consistent enough that I think it's worth putting this fix in.

@ssk97 ssk97 requested a review from JayDi85 February 25, 2025 22:57
@JayDi85
Copy link
Member

JayDi85 commented Feb 25, 2025

It can broke normal disconnection without tables wait (player will be active and lock game for 5 minutes until lose by timeout).

@ssk97
Copy link
Contributor Author

ssk97 commented Feb 25, 2025

It can broke normal disconnection without tables wait (player will be active and lock game for 5 minutes until lose by timeout).

If the player quits normally then it should still go through the DisconnectedByUser path via removeListener, right?

@JayDi85
Copy link
Member

JayDi85 commented Feb 25, 2025

Need tests (is it works or not). It’s important to test on real server (another machine) cause jboss uses diff logic for local client and server.

Just an idea: original problem in wrong lease timeouts — client wrongly catch broken connection status and try to close it (it’s use bisocket, e.g. both client and server keeps two depended connections and try to close another one on “fail”).

@JayDi85
Copy link
Member

JayDi85 commented Feb 25, 2025

I’m stopped here.

Client side can auto-call disconnection on bad connection due failed pings:
IMG_1159

Client side can auto-call disconnection on connection change due IP change:
IMG_1160

P.S. There are also problems with hard coded pings and timeout settings (there are no tools to sync it between client and server).

@JayDi85 JayDi85 self-assigned this Feb 26, 2025
Copy link
Contributor

@xenohedron xenohedron left a comment

Choose a reason for hiding this comment

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

I tested this today on the XDHS server (applying just this change on top of the tagged release from Feb 9, and building custom).

It works successfully to allow players to reconnect after network blips without getting kicked from their matches.

We also tested what happens on deliberate disconnect, and that does quit from active tables as intended.

It may be a good idea to add a comment in here explaining the workaround (to reduce confusion for future investigation), but this successfully addresses a critical bug without introducing other problems, so let's merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants