Merge branch 'develop' into feat/dont-hide-settings

Signed-off-by: TheKodeToad <TheKodeToad@proton.me>

5 files changed, 52 insertions, 57 deletions

diff --git a/launcher/ui/pages/instance/InstanceSettingsPage.cpp b/launcher/ui/pages/instance/InstanceSettingsPage.cpp
index 46830720..08977841 100644
--- a/launcher/ui/pages/instance/InstanceSettingsPage.cpp
+++ b/launcher/ui/pages/instance/InstanceSettingsPage.cpp
@@ -63,16 +63,14 @@ InstanceSettingsPage::InstanceSettingsPage(BaseInstance *inst, QWidget *parent)
     // As the signal will (probably) not be triggered once we click edit, let's update it manually instead.
     updateRunningStatus(m_instance->isRunning());
 
-    accountMenu = new QMenu(this);
-    // Use undocumented property... https://stackoverflow.com/questions/7121718/create-a-scrollbar-in-a-submenu-qt
-    accountMenu->setStyleSheet("QMenu { menu-scrollable: 1; }");
-    ui->instanceAccountSelector->setMenu(accountMenu);
-
     connect(m_instance, &BaseInstance::runningStatusChanged, this, &InstanceSettingsPage::updateRunningStatus);
     connect(ui->openGlobalJavaSettingsButton, &QCommandLinkButton::clicked, this, &InstanceSettingsPage::globalSettingsButtonClicked);
     connect(APPLICATION, &Application::globalSettingsAboutToOpen, this, &InstanceSettingsPage::applySettings);
     connect(APPLICATION, &Application::globalSettingsClosed, this, &InstanceSettingsPage::loadSettings);
+    connect(ui->instanceAccountSelector, QOverload<int>::of(&QComboBox::currentIndexChanged), this, &InstanceSettingsPage::changeInstanceAccount);
     loadSettings();
+
+
     updateThresholds();
 }
 
@@ -453,36 +451,17 @@ void InstanceSettingsPage::on_javaTestBtn_clicked()
 
 void InstanceSettingsPage::updateAccountsMenu()
 {
-    accountMenu->clear();
-
+    ui->instanceAccountSelector->clear();
     auto accounts = APPLICATION->accounts();
     int accountIndex = accounts->findAccountByProfileId(m_settings->get("InstanceAccountId").toString());
-    MinecraftAccountPtr defaultAccount = accounts->defaultAccount();
-
-    if (accountIndex != -1 && accounts->at(accountIndex)) {
-        defaultAccount = accounts->at(accountIndex);
-    }
-
-    if (defaultAccount) {
-        ui->instanceAccountSelector->setText(defaultAccount->profileName());
-        ui->instanceAccountSelector->setIcon(getFaceForAccount(defaultAccount));
-    } else {
-        ui->instanceAccountSelector->setText(tr("No default account"));
-        ui->instanceAccountSelector->setIcon(APPLICATION->getThemedIcon("noaccount"));
-    }
 
     for (int i = 0; i < accounts->count(); i++) {
         MinecraftAccountPtr account = accounts->at(i);
-        QAction* action = new QAction(account->profileName(), this);
-        action->setData(i);
-        action->setCheckable(true);
-        if (accountIndex == i) {
-            action->setChecked(true);
-        }
-        action->setIcon(getFaceForAccount(account));
-        accountMenu->addAction(action);
-        connect(action, SIGNAL(triggered(bool)), this, SLOT(changeInstanceAccount()));
+        ui->instanceAccountSelector->addItem(getFaceForAccount(account), account->profileName(), i);
+        if (i == accountIndex)
+            ui->instanceAccountSelector->setCurrentIndex(i);
     }
+
 }
 
 QIcon InstanceSettingsPage::getFaceForAccount(MinecraftAccountPtr account)
@@ -494,20 +473,13 @@ QIcon InstanceSettingsPage::getFaceForAccount(MinecraftAccountPtr account)
     return APPLICATION->getThemedIcon("noaccount");
 }
 
-void InstanceSettingsPage::changeInstanceAccount()
+void InstanceSettingsPage::changeInstanceAccount(int index)
 {
-    QAction* sAction = (QAction*)sender();
-
-    Q_ASSERT(sAction->data().type() == QVariant::Type::Int);
-
-    QVariant data = sAction->data();
-    int index = data.toInt();
     auto accounts = APPLICATION->accounts();
-    auto account = accounts->at(index);
-    m_settings->set("InstanceAccountId", account->profileId());
-
-    ui->instanceAccountSelector->setText(account->profileName());
-    ui->instanceAccountSelector->setIcon(getFaceForAccount(account));
+    if (index != -1 && accounts->at(index) && ui->instanceAccountGroupBox->isChecked()) {
+        auto account = accounts->at(index);
+        m_settings->set("InstanceAccountId", account->profileId());
+    }
 }
 
 void InstanceSettingsPage::on_maxMemSpinBox_valueChanged(int i)
diff --git a/launcher/ui/pages/instance/InstanceSettingsPage.h b/launcher/ui/pages/instance/InstanceSettingsPage.h
index 8bd854ad..0438fe3b 100644
--- a/launcher/ui/pages/instance/InstanceSettingsPage.h
+++ b/launcher/ui/pages/instance/InstanceSettingsPage.h
@@ -95,12 +95,11 @@ private slots:
 
     void updateAccountsMenu();
     QIcon getFaceForAccount(MinecraftAccountPtr account);
-    void changeInstanceAccount();
+    void changeInstanceAccount(int index);
 
 private:
     Ui::InstanceSettingsPage *ui;
     BaseInstance *m_instance;
     SettingsObjectPtr m_settings;
     unique_qobject_ptr<JavaCommon::TestCheck> checker;
-    QMenu *accountMenu = nullptr;
 };
diff --git a/launcher/ui/pages/instance/InstanceSettingsPage.ui b/launcher/ui/pages/instance/InstanceSettingsPage.ui
index 1b986184..19d6dc02 100644
--- a/launcher/ui/pages/instance/InstanceSettingsPage.ui
+++ b/launcher/ui/pages/instance/InstanceSettingsPage.ui
@@ -636,14 +636,7 @@
              </widget>
             </item>
             <item row="0" column="1">
-             <widget class="QToolButton" name="instanceAccountSelector">
-              <property name="popupMode">
-               <enum>QToolButton::InstantPopup</enum>
-              </property>
-              <property name="toolButtonStyle">
-               <enum>Qt::ToolButtonTextBesideIcon</enum>
-              </property>
-             </widget>
+             <widget class="QComboBox" name="instanceAccountSelector"/>
             </item>
            </layout>
           </item>
diff --git a/launcher/ui/pages/instance/ManagedPackPage.cpp b/launcher/ui/pages/instance/ManagedPackPage.cpp
index dc983d9a..d0701a7a 100644
--- a/launcher/ui/pages/instance/ManagedPackPage.cpp
+++ b/launcher/ui/pages/instance/ManagedPackPage.cpp
@@ -30,8 +30,6 @@ class NoBigComboBoxStyle : public QProxyStyle {
     Q_OBJECT
 
    public:
-    NoBigComboBoxStyle(QStyle* style) : QProxyStyle(style) {}
-
     // clang-format off
     int styleHint(QStyle::StyleHint hint, const QStyleOption* option = nullptr, const QWidget* widget = nullptr, QStyleHintReturn* returnData = nullptr) const override
     {
@@ -41,6 +39,37 @@ class NoBigComboBoxStyle : public QProxyStyle {
         return QProxyStyle::styleHint(hint, option, widget, returnData);
     }
     // clang-format on
+
+    /**
+     * Something about QProxyStyle and QStyle objects means they can't be free'd just
+     * because all the widgets using them are gone.
+     * They seems to be tied to the QApplicaiton lifecycle.
+     * So make singletons tied to the lifetime of the application to clean them up and ensure they aren't
+     * being remade over and over again, thus leaking memory.
+     */
+   public:
+    static NoBigComboBoxStyle* getInstance(QStyle* style)
+    {
+        static QHash<QStyle*, NoBigComboBoxStyle*> s_singleton_instances_ = {};
+        static std::mutex s_singleton_instances_mutex_;
+
+        std::lock_guard<std::mutex> lock(s_singleton_instances_mutex_);
+        auto inst_iter = s_singleton_instances_.constFind(style);
+        NoBigComboBoxStyle* inst = nullptr;
+        if (inst_iter == s_singleton_instances_.constEnd() || *inst_iter == nullptr) {
+            inst = new NoBigComboBoxStyle(style);
+            inst->setParent(APPLICATION);
+            s_singleton_instances_.insert(style, inst);
+            qDebug() << "QProxyStyle NoBigComboBox created for" << style->objectName() << style;
+        } else {
+            inst = *inst_iter;
+        }
+        return inst;
+    }
+
+   private:
+    NoBigComboBoxStyle(QStyle* style) : QProxyStyle(style) {}
+
 };
 
 ManagedPackPage* ManagedPackPage::createPage(BaseInstance* inst, QString type, QWidget* parent)
@@ -62,8 +91,10 @@ ManagedPackPage::ManagedPackPage(BaseInstance* inst, InstanceWindow* instance_wi
 
     // NOTE: GTK2 themes crash with the proxy style.
     // This seems like an upstream bug, so there's not much else that can be done.
-    if (!QStyleFactory::keys().contains("gtk2"))
-        ui->versionsComboBox->setStyle(new NoBigComboBoxStyle(ui->versionsComboBox->style()));
+    if (!QStyleFactory::keys().contains("gtk2")){
+        auto comboStyle = NoBigComboBoxStyle::getInstance(ui->versionsComboBox->style());
+        ui->versionsComboBox->setStyle(comboStyle);
+    }
 
     ui->reloadButton->setVisible(false);
     connect(ui->reloadButton, &QPushButton::clicked, this, [this](bool){
diff --git a/launcher/ui/pages/instance/VersionPage.cpp b/launcher/ui/pages/instance/VersionPage.cpp
index 7fff3b93..74b7ec7c 100644
--- a/launcher/ui/pages/instance/VersionPage.cpp
+++ b/launcher/ui/pages/instance/VersionPage.cpp
@@ -165,7 +165,7 @@ VersionPage::VersionPage(MinecraftInstance *inst, QWidget *parent)
     auto proxy = new IconProxy(ui->packageView);
     proxy->setSourceModel(m_profile.get());
 
-    m_filterModel = new QSortFilterProxyModel();
+    m_filterModel = new QSortFilterProxyModel(this);
     m_filterModel->setDynamicSortFilter(true);
     m_filterModel->setFilterCaseSensitivity(Qt::CaseInsensitive);
     m_filterModel->setSortCaseSensitivity(Qt::CaseInsensitive);
@@ -501,7 +501,7 @@ void VersionPage::on_actionDownload_All_triggered()
         return;
     }
     ProgressDialog tDialog(this);
-    connect(updateTask.get(), SIGNAL(failed(QString)), SLOT(onGameUpdateError(QString)));
+    connect(updateTask.get(), &Task::failed, this, &VersionPage::onGameUpdateError);
     // FIXME: unused return value
     tDialog.execWithTask(updateTask.get());
     updateButtons();