-
-
Notifications
You must be signed in to change notification settings - Fork 455
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
Ensure live status requests are always batched #4713
Conversation
bb9a845
to
204d18e
Compare
This matches the previous implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
@@ -86,6 +86,12 @@ class Helix : public IHelix | |||
std::function<void()> finallyCallback), | |||
(override)); | |||
|
|||
MOCK_METHOD(void, fetchChannels, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: invalid case style for function 'MOCK_METHOD' [readability-identifier-naming]
MOCK_METHOD(void, fetchChannels, | |
mockMethod(void, fetchChannels, |
QStringList channelIDs; | ||
|
||
{ | ||
std::unique_lock immediateRequestsLock( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: variable 'immediateRequestsLock' of type 'std::unique_lock' (aka 'std::unique_lockstd::mutex') can be declared 'const' [misc-const-correctness]
std::unique_lock immediateRequestsLock( | |
std::unique_lock const immediateRequestsLock( |
assert(!channelID.isEmpty()); | ||
|
||
{ | ||
std::unique_lock lock(this->channelsMutex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: variable 'lock' of type 'std::unique_lock<shared_mutex>' (aka 'std::unique_lockstd::shared_mutex') can be declared 'const' [misc-const-correctness]
std::unique_lock lock(this->channelsMutex); | |
std::unique_lock const lock(this->channelsMutex); |
} | ||
|
||
{ | ||
std::unique_lock immediateRequestsLock(this->immediateRequestsMutex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: variable 'immediateRequestsLock' of type 'std::unique_lock' (aka 'std::unique_lockstd::mutex') can be declared 'const' [misc-const-correctness]
std::unique_lock immediateRequestsLock(this->immediateRequestsMutex); | |
std::unique_lock const immediateRequestsLock(this->immediateRequestsMutex); |
} | ||
else | ||
{ | ||
std::shared_lock lock(this->channelsMutex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: variable 'lock' of type 'std::shared_lock<shared_mutex>' (aka 'std::shared_lockstd::shared_mutex') can be declared 'const' [misc-const-correctness]
std::shared_lock lock(this->channelsMutex); | |
std::shared_lock const lock(this->channelsMutex); |
this->addMessage(builder.release()); | ||
|
||
// "delete" old 'CHANNEL is live' message | ||
LimitedQueueSnapshot<MessagePtr> snapshot = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: variable 'snapshot' of type 'LimitedQueueSnapshotchatterino::MessagePtr' (aka 'LimitedQueueSnapshot<shared_ptr>') can be declared 'const' [misc-const-correctness]
LimitedQueueSnapshot<MessagePtr> snapshot = | |
LimitedQueueSnapshot<MessagePtr> const snapshot = |
// "delete" old 'CHANNEL is live' message | ||
LimitedQueueSnapshot<MessagePtr> snapshot = | ||
getApp()->twitch->liveChannel->getMessageSnapshot(); | ||
int snapshotLength = snapshot.size(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: variable 'snapshotLength' of type 'int' can be declared 'const' [misc-const-correctness]
int snapshotLength = snapshot.size(); | |
int const snapshotLength = snapshot.size(); |
// "delete" old 'CHANNEL is live' message | ||
LimitedQueueSnapshot<MessagePtr> snapshot = | ||
getApp()->twitch->liveChannel->getMessageSnapshot(); | ||
int snapshotLength = snapshot.size(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: narrowing conversion from 'size_t' (aka 'unsigned long') to signed type 'int' is implementation-defined [bugprone-narrowing-conversions]
int snapshotLength = snapshot.size();
^
int snapshotLength = snapshot.size(); | ||
|
||
// MSVC hates this code if the parens are not there | ||
int end = (std::max)(0, snapshotLength - 200); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: variable 'end' of type 'int' can be declared 'const' [misc-const-correctness]
int end = (std::max)(0, snapshotLength - 200); | |
int const end = (std::max)(0, snapshotLength - 200); |
status->gameId = stream.gameId; | ||
status->game = stream.gameName; | ||
status->title = stream.title; | ||
QDateTime since = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: variable 'since' of type 'QDateTime' can be declared 'const' [misc-const-correctness]
QDateTime since = | |
QDateTime const since = |
Description
This makes sure live status requests are always batched, including the first one
Titles are also now always batched, and updated frequently rather than just randomly when we hover over the split header.
The display name of a channel is now also updated, if necessary, alongside the livestatus changes. Technically unrelated to the live status-ness but since we have that data in the same request, we might as well update it.