MainWindow: Save tab mode immediately when changed

The sidebar mode (tabs on top, large sidebar, etc.) was only being saved
when the application exits normally. This caused the setting to be lost
if the application crashed or was force-closed, reverting to the default
"Large Sidebar" mode on next startup.

This fix connects the FancyTabWidget::ModeChanged signal to immediately
save the tab_mode setting when the user changes it via the context menu.

Fixes issue where sidebar keeps periodically reverting to Large mode.

Co-authored-by: jonaski <10343810+jonaski@users.noreply.github.com>

src/constants/filefilterconstants.h

@@ -33,7 +33,7 @@ constexpr char kFileFilter[] =
     "*.mod *.s3m *.xm *.it "
     "*.spc *.vgm";
 
-constexpr char kLoadImageFileFilter[] = QT_TRANSLATE_NOOP("FileFilter", "Images (*.png *.jpg *.jpeg *.bmp *.gif *.xpm *.pbm *.pgm *.ppm *.xbm *.webp)");
-constexpr char kSaveImageFileFilter[] = QT_TRANSLATE_NOOP("FileFilter", "Images (*.png *.jpg *.jpeg *.bmp *.xpm *.pbm *.ppm *.xbm *.webp)");
+constexpr char kLoadImageFileFilter[] = QT_TRANSLATE_NOOP("FileFilter", "Images (*.png *.jpg *.jpeg *.bmp *.gif *.xpm *.pbm *.pgm *.ppm *.xbm)");
+constexpr char kSaveImageFileFilter[] = QT_TRANSLATE_NOOP("FileFilter", "Images (*.png *.jpg *.jpeg *.bmp *.xpm *.pbm *.ppm *.xbm)");
 
 #endif  // FILEFILTERCONSTANTS_H

src/constants/scrobblersettings.h

@@ -34,7 +34,6 @@ constexpr char kShowErrorDialog[] = "show_error_dialog";
 constexpr char kStripRemastered[] = "strip_remastered";
 constexpr char kSources[] = "sources";
 constexpr char kUserToken[] = "user_token";
-constexpr char kApiKey[] = "api_key";
 
 }  // namespace ScrobblerSettings
 

src/core/mainwindow.cpp

@@ -447,6 +447,14 @@ MainWindow::MainWindow(Application *app,
   ui_->tabs->SetBackgroundPixmap(QPixmap(u":/pictures/sidebar-background.png"_s));
   ui_->tabs->LoadSettings(QLatin1String(MainWindowSettings::kSettingsGroup));
 
+  // Save tab mode immediately when changed to avoid losing the setting
+  QObject::connect(ui_->tabs, &FancyTabWidget::ModeChanged, this, [this](FancyTabWidget::Mode mode) {
+    Settings s;
+    s.beginGroup(MainWindowSettings::kSettingsGroup);
+    s.setValue("tab_mode", static_cast<int>(mode));
+    s.endGroup();
+  });
+
   track_position_timer_->setInterval(kTrackPositionUpdateTimeMs);
   QObject::connect(track_position_timer_, &QTimer::timeout, this, &MainWindow::UpdateTrackPosition);
   track_slider_timer_->setInterval(kTrackSliderUpdateTimeMs);

src/scrobbler/lastfmimport.cpp

@@ -42,22 +42,15 @@
 #include "core/logging.h"
 #include "core/networkaccessmanager.h"
 #include "core/settings.h"
-#include "constants/scrobblersettings.h"
 
 #include "lastfmimport.h"
 
 #include "lastfmscrobbler.h"
 
 using namespace Qt::Literals::StringLiterals;
-using namespace ScrobblerSettings;
 
 namespace {
 constexpr int kRequestsDelay = 2000;
-constexpr int kMaxRetries = 5;
-constexpr int kInitialBackoffMs = 5000;
-constexpr int kMaxBackoffShift = 10;  // Maximum shift value to prevent overflow
-constexpr int kRetryHttpStatusCode1 = 500;  // Internal Server Error
-constexpr int kRetryHttpStatusCode2 = 503;  // Service Unavailable
 }
 
 LastFMImport::LastFMImport(const SharedPtr<NetworkAccessManager> network, QObject *parent)
@@ -108,17 +101,14 @@ void LastFMImport::ReloadSettings() {
   Settings s;
   s.beginGroup(LastFMScrobbler::kSettingsGroup);
   username_ = s.value("username").toString();
-  api_key_ = s.value(ScrobblerSettings::kApiKey).toString();
   s.endGroup();
 
 }
 
 QNetworkReply *LastFMImport::CreateRequest(const ParamList &request_params) {
 
-  const QString api_key = !api_key_.isEmpty() ? api_key_ : QLatin1String(LastFMScrobbler::kApiKey);
-
   ParamList params = ParamList()
-    << Param(u"api_key"_s, api_key)
+    << Param(u"api_key"_s, QLatin1String(LastFMScrobbler::kApiKey))
     << Param(u"user"_s, username_)
     << Param(u"lang"_s, QLocale().name().left(2).toLower())
     << Param(u"format"_s, u"json"_s)
@@ -244,11 +234,11 @@ void LastFMImport::SendGetRecentTracksRequest(GetRecentTracksRequest request) {
   }
 
   QNetworkReply *reply = CreateRequest(params);
-  QObject::connect(reply, &QNetworkReply::finished, this, [this, reply, request]() { GetRecentTracksRequestFinished(reply, request); });
+  QObject::connect(reply, &QNetworkReply::finished, this, [this, reply, request]() { GetRecentTracksRequestFinished(reply, request.page); });
 
 }
 
-void LastFMImport::GetRecentTracksRequestFinished(QNetworkReply *reply, GetRecentTracksRequest request) {
+void LastFMImport::GetRecentTracksRequestFinished(QNetworkReply *reply, const int page) {
 
   if (!replies_.contains(reply)) return;
   replies_.removeAll(reply);
@@ -257,21 +247,10 @@ void LastFMImport::GetRecentTracksRequestFinished(QNetworkReply *reply, GetRecen
 
   const JsonObjectResult json_object_result = ParseJsonObject(reply);
   if (!json_object_result.success()) {
-    if (ShouldRetryRequest(json_object_result) && request.retry_count < kMaxRetries) {
-      const int delay_ms = CalculateBackoffDelay(request.retry_count);
-      LogRetryAttempt(json_object_result.http_status_code, request.retry_count, delay_ms);
-      QTimer::singleShot(delay_ms, this, [this, request]() {
-        GetRecentTracksRequest retry_request(request.page, request.retry_count + 1);
-        SendGetRecentTracksRequest(retry_request);
-      });
-      return;
-    }
     Error(json_object_result.error_message);
     return;
   }
 
-  const int page = request.page;
-
   QJsonObject json_object = json_object_result.json_object;
   if (json_object.isEmpty()) {
     return;
@@ -411,11 +390,11 @@ void LastFMImport::SendGetTopTracksRequest(GetTopTracksRequest request) {
   }
 
   QNetworkReply *reply = CreateRequest(params);
-  QObject::connect(reply, &QNetworkReply::finished, this, [this, reply, request]() { GetTopTracksRequestFinished(reply, request); });
+  QObject::connect(reply, &QNetworkReply::finished, this, [this, reply, request]() { GetTopTracksRequestFinished(reply, request.page); });
 
 }
 
-void LastFMImport::GetTopTracksRequestFinished(QNetworkReply *reply, GetTopTracksRequest request) {
+void LastFMImport::GetTopTracksRequestFinished(QNetworkReply *reply, const int page) {
 
   if (!replies_.contains(reply)) return;
   replies_.removeAll(reply);
@@ -424,21 +403,10 @@ void LastFMImport::GetTopTracksRequestFinished(QNetworkReply *reply, GetTopTrack
 
   const JsonObjectResult json_object_result = ParseJsonObject(reply);
   if (!json_object_result.success()) {
-    if (ShouldRetryRequest(json_object_result) && request.retry_count < kMaxRetries) {
-      const int delay_ms = CalculateBackoffDelay(request.retry_count);
-      LogRetryAttempt(json_object_result.http_status_code, request.retry_count, delay_ms);
-      QTimer::singleShot(delay_ms, this, [this, request]() {
-        GetTopTracksRequest retry_request(request.page, request.retry_count + 1);
-        SendGetTopTracksRequest(retry_request);
-      });
-      return;
-    }
     Error(json_object_result.error_message);
     return;
   }
 
-  const int page = request.page;
-
   QJsonObject json_object = json_object_result.json_object;
   if (json_object.isEmpty()) {
     return;
@@ -548,23 +516,6 @@ void LastFMImport::GetTopTracksRequestFinished(QNetworkReply *reply, GetTopTrack
 
 }
 
-bool LastFMImport::ShouldRetryRequest(const JsonObjectResult &result) const {
-  return result.http_status_code == kRetryHttpStatusCode1 ||
-         result.http_status_code == kRetryHttpStatusCode2 ||
-         result.network_error == QNetworkReply::TemporaryNetworkFailureError;
-}
-
-void LastFMImport::LogRetryAttempt(const int http_status_code, const int retry_count, const int delay_ms) const {
-  qLog(Warning) << "Last.fm request failed with status" << http_status_code
-                << ", retrying in" << delay_ms << "ms (attempt"
-                << (retry_count + 1) << "of" << kMaxRetries << ")";
-}
-
-int LastFMImport::CalculateBackoffDelay(const int retry_count) const {
-  const int safe_shift = std::min(retry_count, kMaxBackoffShift);
-  return kInitialBackoffMs * (1 << safe_shift);
-}
-
 void LastFMImport::UpdateTotalCheck() {
 
   Q_EMIT UpdateTotal(lastplayed_total_, playcount_total_);

src/scrobbler/lastfmimport.h

@@ -60,14 +60,12 @@ class LastFMImport : public JsonBaseRequest {
   using ParamList = QList<Param>;
 
   struct GetRecentTracksRequest {
-    explicit GetRecentTracksRequest(const int _page, const int _retry_count = 0) : page(_page), retry_count(_retry_count) {}
+    explicit GetRecentTracksRequest(const int _page) : page(_page) {}
     int page;
-    int retry_count;
   };
   struct GetTopTracksRequest {
-    explicit GetTopTracksRequest(const int _page, const int _retry_count = 0) : page(_page), retry_count(_retry_count) {}
+    explicit GetTopTracksRequest(const int _page) : page(_page) {}
     int page;
-    int retry_count;
   };
 
  private:
@@ -80,10 +78,6 @@ class LastFMImport : public JsonBaseRequest {
   void SendGetRecentTracksRequest(GetRecentTracksRequest request);
   void SendGetTopTracksRequest(GetTopTracksRequest request);
 
-  bool ShouldRetryRequest(const JsonObjectResult &result) const;
-  int CalculateBackoffDelay(const int retry_count) const;
-  void LogRetryAttempt(const int http_status_code, const int retry_count, const int delay_ms) const;
-
   void Error(const QString &error, const QVariant &debug = QVariant()) override;
 
   void UpdateTotalCheck();
@@ -101,15 +95,14 @@ class LastFMImport : public JsonBaseRequest {
 
  private Q_SLOTS:
   void FlushRequests();
-  void GetRecentTracksRequestFinished(QNetworkReply *reply, GetRecentTracksRequest request);
-  void GetTopTracksRequestFinished(QNetworkReply *reply, GetTopTracksRequest request);
+  void GetRecentTracksRequestFinished(QNetworkReply *reply, const int page);
+  void GetTopTracksRequestFinished(QNetworkReply *reply, const int page);
 
  private:
   SharedPtr<NetworkAccessManager> network_;
   QTimer *timer_flush_requests_;
 
   QString username_;
-  QString api_key_;
   bool lastplayed_;
   bool playcount_;
   int playcount_total_;

src/scrobbler/lastfmscrobbler.cpp

@@ -113,7 +113,6 @@ void LastFMScrobbler::ReloadSettings() {
 
   s.beginGroup(kSettingsGroup);
   enabled_ = s.value(ScrobblerSettings::kEnabled, false).toBool();
-  api_key_ = s.value(ScrobblerSettings::kApiKey).toString();
   s.endGroup();
 
   s.beginGroup(ScrobblerSettings::kSettingsGroup);

src/scrobbler/lastfmscrobbler.h

@@ -64,7 +64,6 @@ class LastFMScrobbler : public ScrobblerService {
   bool subscriber() const { return subscriber_; }
   bool submitted() const override { return submitted_; }
   QString username() const { return username_; }
-  QString api_key() const { return api_key_; }
 
   void Authenticate();
   void UpdateNowPlaying(const Song &song) override;
@@ -140,7 +139,6 @@ class LastFMScrobbler : public ScrobblerService {
   bool subscriber_;
   QString username_;
   QString session_key_;
-  QString api_key_;
 
   bool submitted_;
   Song song_playing_;

src/settings/scrobblersettingspage.cpp

@@ -106,7 +106,6 @@ void ScrobblerSettingsPage::Load() {
   ui_->checkbox_source_unknown->setChecked(scrobbler_->sources().contains(Song::Source::Unknown));
 
   ui_->checkbox_lastfm_enable->setChecked(lastfmscrobbler_->enabled());
-  ui_->lineedit_lastfm_api_key->setText(lastfmscrobbler_->api_key());
   LastFM_RefreshControls(lastfmscrobbler_->authenticated());
 
   ui_->checkbox_listenbrainz_enable->setChecked(listenbrainzscrobbler_->enabled());
@@ -153,7 +152,6 @@ void ScrobblerSettingsPage::Save() {
 
   s.beginGroup(LastFMScrobbler::kSettingsGroup);
   s.setValue(kEnabled, ui_->checkbox_lastfm_enable->isChecked());
-  s.setValue(kApiKey, ui_->lineedit_lastfm_api_key->text());
   s.endGroup();
 
   s.beginGroup(ListenBrainzScrobbler::kSettingsGroup);

src/settings/scrobblersettingspage.ui

@@ -234,43 +234,6 @@
         </property>
        </widget>
       </item>
-      <item>
-       <layout class="QHBoxLayout" name="layout_lastfm_api_key">
-        <item>
-         <widget class="QLabel" name="label_lastfm_api_key">
-          <property name="minimumSize">
-           <size>
-            <width>80</width>
-            <height>0</height>
-           </size>
-          </property>
-          <property name="text">
-           <string>API key:</string>
-          </property>
-         </widget>
-        </item>
-        <item>
-         <widget class="QLineEdit" name="lineedit_lastfm_api_key">
-          <property name="placeholderText">
-           <string>Optional - your own Last.fm API key</string>
-          </property>
-         </widget>
-        </item>
-       </layout>
-      </item>
-      <item>
-       <widget class="QLabel" name="label_lastfm_api_key_info">
-        <property name="text">
-         <string>&lt;html&gt;&lt;head/&gt;&lt;body&gt;&lt;p&gt;&lt;span style=&quot; font-size:8pt;&quot;&gt;Using your own API key can help avoid rate limiting for large libraries. Get one at &lt;/span&gt;&lt;a href=&quot;https://www.last.fm/api/account/create&quot;&gt;&lt;span style=&quot; font-size:8pt; text-decoration: underline; color:#0000ff;&quot;&gt;https://www.last.fm/api/account/create&lt;/span&gt;&lt;/a&gt;&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</string>
-        </property>
-        <property name="wordWrap">
-         <bool>true</bool>
-        </property>
-        <property name="openExternalLinks">
-         <bool>true</bool>
-        </property>
-       </widget>
-      </item>
       <item>
        <widget class="LoginStateWidget" name="widget_lastfm_login_state" native="true"/>
       </item>
@@ -431,7 +394,6 @@
   <tabstop>checkbox_source_somafm</tabstop>
   <tabstop>checkbox_source_radioparadise</tabstop>
   <tabstop>checkbox_lastfm_enable</tabstop>
-  <tabstop>lineedit_lastfm_api_key</tabstop>
   <tabstop>button_lastfm_login</tabstop>
   <tabstop>checkbox_listenbrainz_enable</tabstop>
   <tabstop>lineedit_listenbrainz_user_token</tabstop>