-
Notifications
You must be signed in to change notification settings - Fork 19
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
woc #17
woc #17
Conversation
BREAKING CHANGE: SongLyric class change
@@ -1,6 +1,6 @@ | |||
#include "Tools.h" | |||
|
|||
QString Tools::milsec2Time(quint64 milisec) { | |||
QString Tools::milsec2Time(const quint64& milisec) { |
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.
it makes no sense to use const reference for number types.
@@ -5,7 +5,7 @@ | |||
|
|||
struct Tools | |||
{ | |||
static QString milsec2Time(quint64 milisec); | |||
static QString milsec2Time(const quint64& milisec); |
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.
Same as above.
break; | ||
} | ||
case PlayMode::Shuffle: { | ||
// TODO | ||
auto index = randomInt(0, model.count() - 1); |
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.
Actually, shuffle doesn't mean random in official clients.
But it's okey.
@@ -39,6 +39,8 @@ class OutputDeviceViewModel : public QAbstractListModel { | |||
signals: | |||
void currentIndexChanged(); | |||
|
|||
//TODO: listen to system audio output device change |
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.
Implement or not?
} | ||
|
||
PlayingSongViewModel::~PlayingSongViewModel() { | ||
save(); | ||
} | ||
|
||
PlayingSongViewModel* PlayingSongViewModel::create(QQmlEngine*, QJSEngine*) { | ||
return new PlayingSongViewModel(); | ||
return getInstance(); |
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.
Call QQmlEngine::setObjectOwnership
to set correct ownership, otherwise the object will be freed twice, causing crash when quitting the application.
} | ||
|
||
// remove extra metadata | ||
std::regex metaDataRegex(R"(\[by:.*?\])", std::regex_constants::ECMAScript); |
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.
There may be more field names aside from by
for lrc. How about them?
选做:歌词(丑,但能用