Show online players on servers screen #3112

iTrooz · 2024-11-16T22:27:13Z

No description provided.

…callback See https://discord.com/channels/1031648380885147709/1031823065937629267/1307471566166167696

https://doc.qt.io/qt-6/qtcpsocket.html#dtor.QTcpSocket

…/catch

launcher/ui/pages/instance/ServersPage.cpp

…jet is deleted, just to be sure

iTrooz · 2024-11-28T23:59:21Z

Ok, all conversations have been resolved, and the feature feels complete to me. I think this PR can be considered for merging

I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: fac521a I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 43a54ca I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: ee35ac5 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 99ac11b I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 8fa1dff I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 2f70115 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: ea2a234 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 87c9066 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: fe28a05 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 0a379a0 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 6a7678a I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: cba7e2d I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 8cf0c20 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 0d830e5 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: b35cffb I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 1f094b9 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 2d06e0a I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: c3543b1 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 8b7040d I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: b8035ca I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 9d5727e I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 7cf2458 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 7d04f0e I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 60fb922 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: a79a66c I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 1fb0fe0 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 5cfb5a6 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 9ce5eaa I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 0c6f78d I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 7c8d2c9 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 24b9815 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 7d2da19 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: ca6d669 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 520d6b0 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 4fad298 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 087ab70 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 7c61fec I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: fef8ee2 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 3a9c030 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: f05548f I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 66f3619 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: ca52d00 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 26f50f9 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 873232e I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 8b90a9f I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: dbb88ca I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 6f9be25 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: d124e2e I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: cfb0c97 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 5eb417f I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 3fb6764 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: ae7d337 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 0978274 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: b0778e7 I, iTrooz <hey@itrooz.fr>, hereby add my Signed-off-by to this commit: 01db826 Signed-off-by: iTrooz <hey@itrooz.fr>

launcher/tasks/Task.h

launcher/ui/pages/instance/ServersPage.cpp

launcher/ui/pages/instance/McClient.h

launcher/ui/pages/instance/McResolver.h

Signed-off-by: iTrooz <hey@itrooz.fr>

launcher/ui/pages/instance/ServersPage.cpp

Signed-off-by: iTrooz <hey@itrooz.fr>

TheKodeToad · 2024-12-12T16:59:37Z

launcher/ui/pages/instance/McClient.cpp

+    return QtConcurrent::run([this]() {
+        try {
+            auto status = getStatusDataBlocking();
+            auto players = Json::requireObject(status, "players");
+            return Json::requireInteger(players, "online");
+        } catch (const Exception &e) {
+            qDebug() << "Error: " << e.what();
+            return -1;
+        }
+    });


It feels a bit pointless to use a thread when it is just waiting doing nothing while another thread does the actual work...

Also, this method is not recommended on Windows

hey ! Sorry for the long delay

It feels a bit pointless to use a thread when it is just waiting doing nothing while another thread does the actual work...

In this case the main thread is not really waiting, since QtConcurrent::run() returns a future, that is later "awaited" here:

PrismLauncher/launcher/ui/pages/instance/ServerPingTask.cpp

Line 21 in 5eb417f

QObject::connect(watcher, &QFutureWatcher<int>::finished, this, [this, client, onlineFuture, watcher]() {

If you want, I can try to handle the socket IO operations from the main GUI thread, but it didn't seem like a good option to me (shouldn't the main thread only do GUI stuff ?)

Also, this method is not recommended on Windows

I do not have much experience with Windows, could you clarify what is the bad pattern here ?

I'm with iTrooz here:

it is bad to do this in the UI thread

we already use a similar pattern when extracting files

Pretty sure all the waitFor methods do is make the thread sleep until the work is done. The rest of the work the thread does is just light parsing

I do not have much experience with Windows, could you clarify what is the bad pattern here ?

https://doc.qt.io/qt-6/qabstractsocket.html#waitForConnected

Note: This function may fail randomly on Windows. Consider using the event loop and the connected() signal if your software will run on Windows.

iTrooz added 17 commits November 17, 2024 19:10

Add dummy UI button

fac521a

add my classes

43a54ca

add method queryStatus() to servers and use it on click

ee35ac5

add my classes to CMakeLists

99ac11b

remove space

8fa1dff

add debug print

2f70115

make splitAddress() const

ea2a234

run the code in tasks

87c9066

make MCResolver a dynamic object so it doesnt get deleted before the …

fe28a05

…callback See https://discord.com/channels/1031648380885147709/1031823065937629267/1307471566166167696

replace my printf calls with qDebug

0a379a0

Actually show online players when clicking on the button

6a7678a

use std::optional<> to signify when there is no value

cba7e2d

No need to close the socket, it is done automatically

8cf0c20

https://doc.qt.io/qt-6/qtcpsocket.html#dtor.QTcpSocket

replace std::runtime_exception with PrismLauncher Exception + add try…

0d830e5

…/catch

fix warning about unused jsonLength

b35cffb

avoid translating back to stf string without reason

1f094b9

Fix after rebase

2d06e0a

iTrooz force-pushed the online_count branch from e39aa09 to 2d06e0a Compare November 17, 2024 18:21

iTrooz added 6 commits November 17, 2024 19:26

fix qDebug() call with string << append

c3543b1

use Qt5-compatible writeString() impl

8b7040d

refactor header and code into 2 files

b8035ca

MCResolver -> McResolver

9d5727e

.hpp -> .h

7cf2458

remove useless code when reading response

7d04f0e

Trial97 reviewed Nov 18, 2024

View reviewed changes

launcher/ui/pages/instance/ServersPage.cpp Outdated Show resolved Hide resolved

Trial97 reviewed Nov 18, 2024

View reviewed changes

launcher/ui/pages/instance/ServersPage.cpp Outdated Show resolved Hide resolved

Trial97 reviewed Nov 18, 2024

View reviewed changes

launcher/ui/pages/instance/ServersPage.cpp Outdated Show resolved Hide resolved

Trial97 reviewed Nov 18, 2024

View reviewed changes

launcher/ui/pages/instance/ServersPage.cpp Outdated Show resolved Hide resolved

Trial97 reviewed Nov 18, 2024

View reviewed changes

launcher/ui/pages/instance/ServersPage.cpp Outdated Show resolved Hide resolved

Trial97 reviewed Nov 18, 2024

View reviewed changes

launcher/ui/pages/instance/ServersPage.cpp Show resolved Hide resolved

iTrooz added 6 commits November 28, 2024 18:09

use Json::requireInteger()

cfb0c97

Always autodelete signal connections with this captured when the ob…

5eb417f

…jet is deleted, just to be sure

fix typo

3fb6764

abort current query task if present when refreshing

ae7d337

make writePacketToSocket() clear data automatically

0978274

chore

b0778e7

iTrooz changed the title ~~[DRAFT] Show online players on servers screen~~ Show online players on servers screen Nov 28, 2024

iTrooz marked this pull request as ready for review November 28, 2024 23:49

Reset online players UI component when refreshing

01db826

Trial97 reviewed Nov 29, 2024

View reviewed changes

launcher/tasks/Task.h Outdated Show resolved Hide resolved

Trial97 reviewed Nov 29, 2024

View reviewed changes

launcher/ui/pages/instance/ServersPage.cpp Outdated Show resolved Hide resolved

Trial97 reviewed Nov 29, 2024

View reviewed changes

launcher/ui/pages/instance/ServersPage.cpp Outdated Show resolved Hide resolved

Trial97 reviewed Nov 29, 2024

View reviewed changes

launcher/ui/pages/instance/ServersPage.cpp Outdated Show resolved Hide resolved

Trial97 reviewed Nov 29, 2024

View reviewed changes

launcher/ui/pages/instance/ServersPage.cpp Outdated Show resolved Hide resolved

Trial97 reviewed Nov 29, 2024

View reviewed changes

launcher/ui/pages/instance/McClient.h Outdated Show resolved Hide resolved

Trial97 reviewed Nov 29, 2024

View reviewed changes

launcher/ui/pages/instance/McResolver.h Outdated Show resolved Hide resolved

chore

4aaf7b9

Signed-off-by: iTrooz <hey@itrooz.fr>

iTrooz force-pushed the online_count branch from c2f5c7a to 4aaf7b9 Compare November 29, 2024 13:07

iTrooz added 2 commits November 29, 2024 19:42

add m_ prefix to class members

1477d64

Signed-off-by: iTrooz <hey@itrooz.fr>

Use shared pointer to store server query task

ba0bd5f

Signed-off-by: iTrooz <hey@itrooz.fr>

iTrooz force-pushed the online_count branch from 584c7af to ba0bd5f Compare November 29, 2024 18:43

Trial97 reviewed Dec 2, 2024

View reviewed changes

launcher/ui/pages/instance/ServersPage.cpp Outdated Show resolved Hide resolved

Trial97 reviewed Dec 2, 2024

View reviewed changes

launcher/ui/pages/instance/ServersPage.cpp Outdated Show resolved Hide resolved

iTrooz added 2 commits December 5, 2024 20:43

currentQueryTask -> m_currentQueryTask

4e48015

Signed-off-by: iTrooz <hey@itrooz.fr>

do not destroy the ping ConcurrentTask directly at the end of the task

f9e450a

Signed-off-by: iTrooz <hey@itrooz.fr>

iTrooz force-pushed the online_count branch from 1225971 to f9e450a Compare December 5, 2024 19:43

Trial97 approved these changes Dec 6, 2024

View reviewed changes

TheKodeToad reviewed Dec 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show online players on servers screen #3112

Show online players on servers screen #3112

iTrooz commented Nov 16, 2024

iTrooz commented Nov 28, 2024 •

edited

Loading

TheKodeToad Dec 12, 2024

iTrooz Jan 12, 2025

Trial97 Jan 16, 2025

TheKodeToad Jan 16, 2025

Show online players on servers screen #3112

Are you sure you want to change the base?

Show online players on servers screen #3112

Conversation

iTrooz commented Nov 16, 2024

iTrooz commented Nov 28, 2024 • edited Loading

TheKodeToad Dec 12, 2024

Choose a reason for hiding this comment

iTrooz Jan 12, 2025

Choose a reason for hiding this comment

Trial97 Jan 16, 2025

Choose a reason for hiding this comment

TheKodeToad Jan 16, 2025

Choose a reason for hiding this comment

iTrooz commented Nov 28, 2024 •

edited

Loading