style: modernize net sinks and skin dialog code

Signed-off-by: Trial97 <alexandru.tripon97@gmail.com>

launcher/net/ByteArraySink.h

@@ -45,14 +45,15 @@ namespace Net {
  */
 class ByteArraySink : public Sink {
    public:
-    virtual ~ByteArraySink() = default;
+    ~ByteArraySink() override = default;
 
    public:
     auto init(QNetworkRequest& request) -> Task::State override
     {
         m_output.clear();
-        if (initAllValidators(request))
+        if (initAllValidators(request)) {
             return Task::State::Running;
+        }
         m_fail_reason = "Failed to initialize validators";
         return Task::State::Failed;
     };
@@ -60,8 +61,9 @@ class ByteArraySink : public Sink {
     auto write(QByteArray& data) -> Task::State override
     {
         m_output.append(data);
-        if (writeAllValidators(data))
+        if (writeAllValidators(data)) {
             return Task::State::Running;
+        }
         m_fail_reason = "Failed to write validators";
         return Task::State::Failed;
     }
@@ -75,8 +77,9 @@ class ByteArraySink : public Sink {
 
     auto finalize(QNetworkReply& reply) -> Task::State override
     {
-        if (finalizeAllValidators(reply))
+        if (finalizeAllValidators(reply)) {
             return Task::State::Succeeded;
+        }
         m_fail_reason = "Failed to finalize validators";
         return Task::State::Failed;
     }

launcher/net/DummySink.h

@@ -23,12 +23,12 @@ namespace Net {
 
 class DummySink : public Sink {
    public:
-    explicit DummySink() {}
-    ~DummySink() override {}
-    auto init(QNetworkRequest& request) -> Task::State override { return Task::State::Running; }
-    auto write(QByteArray& data) -> Task::State override { return Task::State::Succeeded; }
+    explicit DummySink() = default;
+    ~DummySink() override = default;
+    auto init(QNetworkRequest& /*request*/) -> Task::State override { return Task::State::Running; }
+    auto write(QByteArray& /*data*/) -> Task::State override { return Task::State::Succeeded; }
     auto abort() -> Task::State override { return Task::State::AbortedByUser; }
-    auto finalize(QNetworkReply& reply) -> Task::State override { return Task::State::Succeeded; }
+    auto finalize(QNetworkReply& /*reply*/) -> Task::State override { return Task::State::Succeeded; }
     auto hasLocalData() -> bool override { return false; }
 };
 

launcher/net/Request.cpp

@@ -52,7 +52,7 @@
 #include <utility>
 #include <variant>
 
-#if defined(LAUNCHER_APPLICATION)
+#ifdef LAUNCHER_APPLICATION
 #include "Application.h"
 #include "net/ApiHeaderProxy.h"
 #include "net/ChecksumValidator.h"
@@ -63,6 +63,7 @@
 #endif
 #include "net/ByteArraySink.h"
 #include "net/FileSink.h"
+#include "net/Logging.h"
 
 #include "MMCTime.h"
 #include "StringUtils.h"
@@ -104,7 +105,7 @@ Request::Request(const Spec& spec) : m_options(spec.options), m_url(spec.url), m
         setObjectName(spec.name);
     }
     m_logCat = logCatForMethod(m_httpMethod);
-#if defined(LAUNCHER_APPLICATION)
+#ifdef LAUNCHER_APPLICATION
     if (spec.options.testFlag(Option::AddAPIHeaders)) {
         addHeaderProxy(std::make_unique<ApiHeaderProxy>());
     }
@@ -121,7 +122,7 @@ void Request::executeTask()
     setStatus(tr("Requesting %1").arg(StringUtils::truncateUrlHumanFriendly(m_url, 80)));
 
     if (m_network == nullptr) {
-#if defined(LAUNCHER_APPLICATION)
+#ifdef LAUNCHER_APPLICATION
         m_network = APPLICATION->network();
 #else
         qCCritical(m_logCat) << getUid().toString() << "No network manager set for request:" << m_url.toString();
@@ -160,7 +161,7 @@ void Request::executeTask()
             return;
     }
 
-#if defined(LAUNCHER_APPLICATION)
+#ifdef LAUNCHER_APPLICATION
     auto userAgent = APPLICATION->getUserAgent();
 #else
     auto userAgent = BuildConfig.USER_AGENT;
@@ -170,7 +171,7 @@ void Request::executeTask()
         headerProxy->writeHeaders(request);
     }
 
-#if defined(LAUNCHER_APPLICATION)
+#ifdef LAUNCHER_APPLICATION
     request.setTransferTimeout(APPLICATION->settings()->get("RequestTimeout").toInt() * 1000);
 #else
     request.setTransferTimeout();
@@ -545,7 +546,7 @@ QNetworkReply* Request::getReply(QNetworkRequest& request)
         m_postData);
 }
 
-#if defined(LAUNCHER_APPLICATION)
+#ifdef LAUNCHER_APPLICATION
 auto Request::makeCached(const QUrl& url, MetaEntryPtr entry, Options options) -> Ptr
 {
     auto dl = Ptr(new Request(url, options, (QString("CACHE:") + url.toString())));

launcher/net/Request.h

@@ -57,14 +57,13 @@
 #include "Validator.h"
 
 #include "QObjectPtr.h"
-#include "net/Logging.h"
+
 #include "tasks/Task.h"
 
 class QIODevice;
 class QHttpMultiPart;
 
 namespace Net {
-class ByteArraySink;
 
 enum class HttpMethodValue : std::uint8_t {
     Get,

launcher/ui/dialogs/skins/SkinManageDialog.cpp

@@ -34,18 +34,17 @@
 #include <QUrl>
 
 #include "Application.h"
-#include "settings/SettingsObject.h"
 #include "DesktopServices.h"
-#include "Json.h"
 #include "QObjectPtr.h"
+#include "settings/SettingsObject.h"
 
 #include "minecraft/auth/Parsers.h"
 #include "minecraft/skins/SkinList.h"
 #include "minecraft/skins/SkinModel.h"
 #include "minecraft/skins/SkinRequests.h"
 
-#include "net/Request.h"
 #include "net/NetJob.h"
+#include "net/Request.h"
 #include "tasks/Task.h"
 
 #include "ui/dialogs/CustomMessageBox.h"
@@ -66,7 +65,7 @@ SkinManageDialog::SkinManageDialog(QWidget* parent, MinecraftAccountPtr acct)
 
     setWindowModality(Qt::WindowModal);
 
-    auto contentsWidget = m_ui->listView;
+    auto* contentsWidget = m_ui->listView;
     contentsWidget->setViewMode(QListView::IconMode);
     contentsWidget->setFlow(QListView::LeftToRight);
     contentsWidget->setIconSize(QSize(48, 48));
@@ -120,9 +119,8 @@ SkinManageDialog::SkinManageDialog(QWidget* parent, MinecraftAccountPtr acct)
 SkinManageDialog::~SkinManageDialog()
 {
     delete m_ui;
-    if (m_skinPreview) {
-        delete m_skinPreview;
-    }
+
+    delete m_skinPreview;
 }
 
 void SkinManageDialog::activated(QModelIndex index)
@@ -131,18 +129,21 @@ void SkinManageDialog::activated(QModelIndex index)
     accept();
 }
 
-void SkinManageDialog::selectionChanged(QItemSelection selected, [[maybe_unused]] QItemSelection deselected)
+void SkinManageDialog::selectionChanged(const QItemSelection& selected, [[maybe_unused]] const QItemSelection& deselected)
 {
-    if (selected.empty())
+    if (selected.empty()) {
         return;
+    }
 
     QString key = selected.first().indexes().first().data(Qt::UserRole).toString();
-    if (key.isEmpty())
+    if (key.isEmpty()) {
         return;
+    }
     m_selectedSkinKey = key;
-    auto skin = getSelectedSkin();
-    if (!skin)
+    auto* skin = getSelectedSkin();
+    if (!skin) {
         return;
+    }
 
     if (m_skinPreview) {
         m_skinPreview->updateScene(skin);
@@ -155,10 +156,10 @@ void SkinManageDialog::selectionChanged(QItemSelection selected, [[maybe_unused]
     m_ui->alexBtn->setChecked(skin->getModel() == SkinModel::SLIM);
 }
 
-void SkinManageDialog::delayed_scroll(QModelIndex model_index)
+void SkinManageDialog::delayed_scroll(QModelIndex modelIndex)
 {
-    auto contentsWidget = m_ui->listView;
-    contentsWidget->scrollTo(model_index);
+    auto* contentsWidget = m_ui->listView;
+    contentsWidget->scrollTo(modelIndex);
 }
 
 void SkinManageDialog::on_openDirBtn_clicked()
@@ -169,24 +170,24 @@ void SkinManageDialog::on_openDirBtn_clicked()
 void SkinManageDialog::on_fileBtn_clicked()
 {
     auto filter = QMimeDatabase().mimeTypeForName("image/png").filterString();
-    QString raw_path = QFileDialog::getOpenFileName(this, tr("Select Skin Texture"), QString(), filter);
-    if (raw_path.isNull()) {
+    QString rawPath = QFileDialog::getOpenFileName(this, tr("Select Skin Texture"), QString(), filter);
+    if (rawPath.isNull()) {
         return;
     }
-    auto message = m_list.installSkin(raw_path, {});
+    auto message = m_list.installSkin(rawPath, {});
     if (!message.isEmpty()) {
         CustomMessageBox::selectable(this, tr("Selected file is not a valid skin"), message, QMessageBox::Critical)->show();
         return;
     }
 }
-
-QPixmap previewCape(QImage capeImage, bool elytra = false)
+namespace {
+QPixmap previewCape(const QImage& capeImage, bool elytra = false)
 {
     if (elytra) {
         auto wing = capeImage.copy(34, 2, 12, 20);
         QImage mirrored = wing.mirrored(true, false);
 
-        QImage combined(wing.width() * 2 + 1, wing.height() + 14, capeImage.format());
+        QImage combined((wing.width() * 2) + 1, wing.height() + 14, capeImage.format());
         combined.fill(Qt::transparent);
 
         QPainter painter(&combined);
@@ -197,6 +198,7 @@ QPixmap previewCape(QImage capeImage, bool elytra = false)
     }
     return QPixmap::fromImage(capeImage.copy(1, 1, 10, 16).scaled(80, 128, Qt::IgnoreAspectRatio, Qt::FastTransformation));
 }
+}  // namespace
 
 void SkinManageDialog::setupCapes()
 {
@@ -214,7 +216,7 @@ void SkinManageDialog::setupCapes()
     bool needsToDownload = false;
     for (auto& cape : accountData.minecraftProfile.capes) {
         auto path = FS::PathCombine(capesDir, cape.id + ".png");
-        if (cape.data.size()) {
+        if (!cape.data.isEmpty()) {
             QImage capeImage;
             if (capeImage.loadFromData(cape.data, "PNG") && capeImage.save(path)) {
                 m_capes[cape.id] = capeImage;
@@ -252,7 +254,7 @@ void SkinManageDialog::setupCapes()
     }
 }
 
-void SkinManageDialog::on_capeCombo_currentIndexChanged(int index)
+void SkinManageDialog::on_capeCombo_currentIndexChanged(int /*index*/)
 {
     auto id = m_ui->capeCombo->currentData();
     auto cape = m_capes.value(id.toString(), {});
@@ -265,7 +267,7 @@ void SkinManageDialog::on_capeCombo_currentIndexChanged(int index)
     if (m_skinPreview) {
         m_skinPreview->updateCape(cape);
     }
-    if (auto skin = getSelectedSkin(); skin) {
+    if (auto* skin = getSelectedSkin(); skin) {
         skin->setCapeId(id.toString());
         if (m_skinPreview) {
             m_skinPreview->updateScene(skin);
@@ -278,7 +280,7 @@ void SkinManageDialog::on_capeCombo_currentIndexChanged(int index)
 
 void SkinManageDialog::on_steveBtn_toggled(bool checked)
 {
-    if (auto skin = getSelectedSkin(); skin) {
+    if (auto* skin = getSelectedSkin(); skin) {
         skin->setModel(checked ? SkinModel::CLASSIC : SkinModel::SLIM);
         if (m_skinPreview) {
             m_skinPreview->updateScene(skin);
@@ -291,7 +293,7 @@ void SkinManageDialog::on_steveBtn_toggled(bool checked)
 
 void SkinManageDialog::accept()
 {
-    auto skin = m_list.skin(m_selectedSkinKey);
+    auto* skin = m_list.skin(m_selectedSkinKey);
     if (!skin) {
         reject();
         return;
@@ -351,7 +353,7 @@ bool SkinManageDialog::eventFilter(QObject* obj, QEvent* ev)
 {
     if (obj == m_ui->listView) {
         if (ev->type() == QEvent::KeyPress) {
-            QKeyEvent* keyEvent = static_cast<QKeyEvent*>(ev);
+            auto* keyEvent = static_cast<QKeyEvent*>(ev);
             switch (keyEvent->key()) {
                 case Qt::Key_Delete:
                     on_action_Delete_Skin_triggered(false);
@@ -367,26 +369,28 @@ bool SkinManageDialog::eventFilter(QObject* obj, QEvent* ev)
     return QDialog::eventFilter(obj, ev);
 }
 
-void SkinManageDialog::on_action_Rename_Skin_triggered(bool)
+void SkinManageDialog::on_action_Rename_Skin_triggered(bool /*unused*/)
 {
     if (!m_selectedSkinKey.isEmpty()) {
         m_ui->listView->edit(m_ui->listView->currentIndex());
     }
 }
 
-void SkinManageDialog::on_action_Delete_Skin_triggered(bool)
+void SkinManageDialog::on_action_Delete_Skin_triggered(bool /*unused*/)
 {
-    if (m_selectedSkinKey.isEmpty())
+    if (m_selectedSkinKey.isEmpty()) {
         return;
+    }
 
     if (m_list.getSkinIndex(m_selectedSkinKey) == m_list.getSelectedAccountSkin()) {
         CustomMessageBox::selectable(this, tr("Delete error"), tr("Can not delete skin that is in use."), QMessageBox::Warning)->exec();
         return;
     }
 
-    auto skin = m_list.skin(m_selectedSkinKey);
-    if (!skin)
+    auto* skin = m_list.skin(m_selectedSkinKey);
+    if (!skin) {
         return;
+    }
 
     auto response = CustomMessageBox::selectable(this, tr("Confirm Deletion"),
                                                  tr("You are about to delete \"%1\".\n"
@@ -433,10 +437,12 @@ void SkinManageDialog::on_urlBtn_clicked()
     }
 }
 
+namespace {
+
 class WaitTask : public Task {
    public:
-    WaitTask() : m_loop(), m_done(false) {};
-    virtual ~WaitTask() = default;
+    WaitTask() = default;
+    ~WaitTask() override = default;
 
    public slots:
     void quit()
@@ -446,17 +452,19 @@ class WaitTask : public Task {
     }
 
    protected:
-    virtual void executeTask()
+    void executeTask() override
     {
-        if (!m_done)
+        if (!m_done) {
             m_loop.exec();
+        }
         emitSucceeded();
     };
 
    private:
     QEventLoop m_loop;
-    bool m_done;
+    bool m_done{};
 };
+}  // namespace
 
 void SkinManageDialog::on_userBtn_clicked()
 {
@@ -480,29 +488,29 @@ void SkinManageDialog::on_userBtn_clicked()
     QString failReason;
 
     connect(getUUID.get(), &Task::aborted, uuidLoop.get(), &WaitTask::quit);
-    connect(getUUID.get(), &Task::failed, this, [&failReason](QString reason) {
+    connect(getUUID.get(), &Task::failed, this, [&failReason](const QString& reason) {
         qCritical() << "Couldn't get user UUID:" << reason;
         failReason = tr("failed to get user UUID");
     });
     connect(getUUID.get(), &Task::failed, uuidLoop.get(), &WaitTask::quit);
     connect(getProfile.get(), &Task::aborted, profileLoop.get(), &WaitTask::quit);
     connect(getProfile.get(), &Task::failed, profileLoop.get(), &WaitTask::quit);
-    connect(getProfile.get(), &Task::failed, this, [&failReason](QString reason) {
+    connect(getProfile.get(), &Task::failed, this, [&failReason](const QString& reason) {
         qCritical() << "Couldn't get user profile:" << reason;
         failReason = tr("failed to get user profile");
     });
-    connect(downloadSkin.get(), &Task::failed, this, [&failReason](QString reason) {
+    connect(downloadSkin.get(), &Task::failed, this, [&failReason](const QString& reason) {
         qCritical() << "Couldn't download skin:" << reason;
         failReason = tr("failed to download skin");
     });
 
     connect(getUUID.get(), &Task::succeeded, this, [uuidLoop, uuidOut, job, getProfile, &failReason] {
         try {
-            QJsonParseError parse_error{};
-            QJsonDocument doc = QJsonDocument::fromJson(*uuidOut, &parse_error);
-            if (parse_error.error != QJsonParseError::NoError) {
-                qWarning() << "Error while parsing JSON response from Minecraft skin service at" << parse_error.offset
-                           << "reason:" << parse_error.errorString();
+            QJsonParseError parseError{};
+            QJsonDocument doc = QJsonDocument::fromJson(*uuidOut, &parseError);
+            if (parseError.error != QJsonParseError::NoError) {
+                qWarning() << "Error while parsing JSON response from Minecraft skin service at" << parseError.offset
+                           << "reason:" << parseError.errorString();
                 failReason = tr("failed to parse get user UUID response");
                 uuidLoop->quit();
                 return;
@@ -562,7 +570,7 @@ void SkinManageDialog::on_userBtn_clicked()
 
 void SkinManageDialog::resizeEvent(QResizeEvent* event)
 {
-    QWidget::resizeEvent(event);
+    QDialog::resizeEvent(event);
     QSize s = size() * (1. / 3);
 
     auto id = m_ui->capeCombo->currentData();
@@ -572,7 +580,7 @@ void SkinManageDialog::resizeEvent(QResizeEvent* event)
     } else {
         m_ui->capeImage->clear();
     }
-    if (auto skin = getSelectedSkin(); skin && !m_skinPreview) {
+    if (auto* skin = getSelectedSkin(); skin && !m_skinPreview) {
         m_skinPreviewLabel->setPixmap(
             QPixmap::fromImage(skin->getPreview()).scaled(m_skinPreviewLabel->size(), Qt::KeepAspectRatio, Qt::FastTransformation));
     }
@@ -580,7 +588,7 @@ void SkinManageDialog::resizeEvent(QResizeEvent* event)
 
 SkinModel* SkinManageDialog::getSelectedSkin()
 {
-    if (auto skin = m_list.skin(m_selectedSkinKey); skin && skin->isValid()) {
+    if (auto* skin = m_list.skin(m_selectedSkinKey); skin && skin->isValid()) {
         return skin;
     }
     return nullptr;

launcher/ui/dialogs/skins/SkinManageDialog.h

@@ -35,14 +35,14 @@ class SkinManageDialog : public QDialog, public SkinProvider {
     Q_OBJECT
    public:
     explicit SkinManageDialog(QWidget* parent, MinecraftAccountPtr acct);
-    virtual ~SkinManageDialog();
+    ~SkinManageDialog() override;
     void resizeEvent(QResizeEvent* event) override;
 
-    virtual SkinModel* getSelectedSkin() override;
-    virtual QHash<QString, QImage> capes() override;
+    SkinModel* getSelectedSkin() override;
+    QHash<QString, QImage> capes() override;
 
    public slots:
-    void selectionChanged(QItemSelection, QItemSelection);
+    void selectionChanged(const QItemSelection&, const QItemSelection&);
     void activated(QModelIndex);
     void delayed_scroll(QModelIndex);
     void on_openDirBtn_clicked();