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

[Refactoring]: Server initialization #3790

Open
thelsing opened this issue Dec 26, 2022 · 2 comments
Open

[Refactoring]: Server initialization #3790

thelsing opened this issue Dec 26, 2022 · 2 comments
Assignees
Labels
code-maintenance Adding/editing javadocs, unit tests, formatting.

Comments

@thelsing
Copy link
Collaborator

thelsing commented Dec 26, 2022

Describe the problem

Currently server initialization is a mess. We start a server and immediately try to connect to it. If the server is already running the first step fails, but we still try to connect to the server. If we failed the connection because the server is already running we get confusion messages ("Player already exists" if we used the same name the existing server uses") or even the "Please notify your GM of your PIN .." because we connected to some server via webrtc.

I often see the "Failed to start personal server" message when something went wrong with server start.

The huge catch bloc for many different exceptions in the START_SERVER action isn't nice too. I'd like to remove that.

The improvement you'd like to see

IMO we need some clean chain of events with onComplete and onError handlers. We already have something similar for establishing client connections. The whole connection establishment should probably be refactored to use CompleteableFuture.

Expected Benefits

Clean way to init server, better error handling, code that is easier to understand.

Additional Context

No response

@thelsing thelsing added the code-maintenance Adding/editing javadocs, unit tests, formatting. label Dec 26, 2022
@thelsing
Copy link
Collaborator Author

We should also get rid of the "please wait until map reloads" and block the screen until server start and connection to it is done.

@FullBleed
Copy link

We should also get rid of the "please wait until map reloads" and block the screen until server start and connection to it is done.

Definitely. Has always been a bugaboo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-maintenance Adding/editing javadocs, unit tests, formatting.
Projects
Status: In Progress
Development

No branches or pull requests

2 participants