From 7c0479b098b29df219084db3bf427c073eefe15c Mon Sep 17 00:00:00 2001 From: isanae <14251494+isanae@users.noreply.github.com> Date: Mon, 4 Jan 2021 09:40:52 -0500 Subject: fixes for the profiles dialog: - don't refresh before showing the dialog, this happened because setCurrentIndex() was called to reselect the old item after was selected - can't use previousIndex after the dialog has been shown, the ordering might have changed if profiles were created/removed - don't call activateSelectedProfile() manually when 'select' is clicked from the dialog, just call setCurrentText() and let it fire the event again, then return immediately so it's not done twice --- src/mainwindow.cpp | 121 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 73 insertions(+), 48 deletions(-) (limited to 'src/mainwindow.cpp') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index cfce9bef..3d7526f6 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1686,66 +1686,91 @@ void MainWindow::activateSelectedProfile() void MainWindow::on_profileBox_currentIndexChanged(int index) { - if (ui->profileBox->isEnabled()) { - int previousIndex = m_OldProfileIndex; - m_OldProfileIndex = index; - - if ((previousIndex != -1) && - (m_OrganizerCore.currentProfile() != nullptr) && - m_OrganizerCore.currentProfile()->exists()) { - m_OrganizerCore.saveCurrentLists(); - } - - // Avoid doing any refresh if currentProfile is already set but previous index was -1 - // as it means that this is happening during initialization so everything has already been set. - if (previousIndex == -1 - && m_OrganizerCore.currentProfile() != nullptr - && m_OrganizerCore.currentProfile()->exists() - && ui->profileBox->currentText() == m_OrganizerCore.currentProfile()->name()){ - return; - } + if (!ui->profileBox->isEnabled()) { + return; + } - // ensure the new index is valid - if (index < 0 || index >= ui->profileBox->count()) { - log::debug("invalid profile index, using last profile"); - ui->profileBox->setCurrentIndex(ui->profileBox->count() - 1); - } + int previousIndex = m_OldProfileIndex; + m_OldProfileIndex = index; + + // select has changed, save stuff + if ((previousIndex != -1) && + (m_OrganizerCore.currentProfile() != nullptr) && + m_OrganizerCore.currentProfile()->exists()) { + m_OrganizerCore.saveCurrentLists(); + } + + // Avoid doing any refresh if currentProfile is already set but previous + // index was -1 as it means that this is happening during initialization so + // everything has already been set. + if (previousIndex == -1 + && m_OrganizerCore.currentProfile() != nullptr + && m_OrganizerCore.currentProfile()->exists() + && ui->profileBox->currentText() == m_OrganizerCore.currentProfile()->name()){ + return; + } + + // ensure the new index is valid + if (index < 0 || index >= ui->profileBox->count()) { + log::debug("invalid profile index, using last profile"); + ui->profileBox->setCurrentIndex(ui->profileBox->count() - 1); + } + + + // handle item + if (ui->profileBox->currentIndex() == 0) { + // remember the profile name that was selected before, previousIndex can't + // be used again because adding/deleting profiles will change the order + // in the list + const QString previousName = ui->profileBox->itemText(previousIndex); - if (ui->profileBox->currentIndex() == 0) { - ui->profileBox->setCurrentIndex(previousIndex); + // show the dialog + ProfilesDialog dlg(previousName, m_OrganizerCore, this); + dlg.exec(); - std::optional newSelection; + // check if the user clicked 'select' to select another profile + std::optional newSelection = dlg.selectedProfile(); - ProfilesDialog dlg(ui->profileBox->currentText(), m_OrganizerCore, this); + // refresh the profile box; this loops until there is at least one profile + // available, which shouldn't really happen because the dialog won't allow + // it + // + // the `false` to refreshProfiles() is so it doesn't try to select the + // profile in the list because 1) it's done just below, and 2) it might be + // wrong profile if there's something in newSelection + while (!refreshProfiles(false)) { + ProfilesDialog dlg(previousName, m_OrganizerCore, this); dlg.exec(); newSelection = dlg.selectedProfile(); + } - while (!refreshProfiles()) { - ProfilesDialog dlg(ui->profileBox->currentText(), m_OrganizerCore, this); - dlg.exec(); - newSelection = dlg.selectedProfile(); - } - - if (newSelection) { - ui->profileBox->setCurrentText(*newSelection); - activateSelectedProfile(); - } + // note that setCurrentText() is recursive, it will re-execute this function + if (newSelection) { + ui->profileBox->setCurrentText(*newSelection); } else { - activateSelectedProfile(); + ui->profileBox->setCurrentText(previousName); } - LocalSavegames *saveGames = m_OrganizerCore.managedGame()->feature(); - if (saveGames != nullptr) { - if (saveGames->prepareProfile(m_OrganizerCore.currentProfile())) { - m_SavesTab->refreshSaveList(); - } + // nothing else to do because setCurrentText() is recursive and will + // have re-executed on_profileBox_currentIndexChanged() again, doing all + // the stuff below for the new selection + return; + } + + + activateSelectedProfile(); + + LocalSavegames *saveGames = m_OrganizerCore.managedGame()->feature(); + if (saveGames != nullptr) { + if (saveGames->prepareProfile(m_OrganizerCore.currentProfile())) { + m_SavesTab->refreshSaveList(); } + } - BSAInvalidation *invalidation = m_OrganizerCore.managedGame()->feature(); - if (invalidation != nullptr) { - if (invalidation->prepareProfile(m_OrganizerCore.currentProfile())) { - QTimer::singleShot(5, &m_OrganizerCore, SLOT(profileRefresh())); - } + BSAInvalidation *invalidation = m_OrganizerCore.managedGame()->feature(); + if (invalidation != nullptr) { + if (invalidation->prepareProfile(m_OrganizerCore.currentProfile())) { + QTimer::singleShot(5, &m_OrganizerCore, SLOT(profileRefresh())); } } } -- cgit v1.3.1