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: server does not terminate the process #1023

Merged
merged 1 commit into from
May 28, 2022
Merged

Fix: server does not terminate the process #1023

merged 1 commit into from
May 28, 2022

Conversation

NIKEA-SOFT
Copy link
Contributor

Server may not shutdown the process, it gets stuck in a loop, waiting for input to the console.

server/server.cpp Show resolved Hide resolved
@@ -21,6 +21,7 @@

// Qt
#include <QObject>
#include <atomic>
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think <atomic> is part of Qt... can we put an empty line between them?

Copy link
Contributor Author

@NIKEA-SOFT NIKEA-SOFT May 27, 2022

Choose a reason for hiding this comment

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

I don't think <atomic> is part of Qt... can we put an empty line between them?

Why, if std::atomic is used there, we do not need to guess whether atomic is included somewhere or not

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm simply suggesting this:

// Qt
#include <QObject>

#include <atomic>

So if in the future we include more of Qt or the STL, it doesn't become inconsistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@lmoureaux lmoureaux requested a review from jwrober May 27, 2022 11:33
@lmoureaux
Copy link
Contributor

Requesting a test from @jwrober -- start a local game, end it, the server should be gone

@lmoureaux lmoureaux merged commit c8e26a0 into longturn:master May 28, 2022
@NIKEA-SOFT NIKEA-SOFT deleted the bugfix/server_stuck branch May 30, 2022 05:46
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.

3 participants