EditTagDialog: Fix saving play statistics

Fixes #1124

src/collection/collection.cpp

@@ -219,9 +219,9 @@ void SCollection::SyncPlaycountAndRatingToFiles() {
 
 }
 
-void SCollection::SongsPlaycountChanged(const SongList &songs) {
+void SCollection::SongsPlaycountChanged(const SongList &songs, const bool save_tags) {
 
-  if (save_playcounts_to_files_) {
+  if (save_tags || save_playcounts_to_files_) {
     app_->tag_reader_client()->UpdateSongsPlaycount(songs);
   }
 

src/collection/collection.h

@@ -78,7 +78,7 @@ class SCollection : public QObject {
 
  private slots:
   void ExitReceived();
-  void SongsPlaycountChanged(const SongList &songs);
+  void SongsPlaycountChanged(const SongList &songs, const bool save_tags = false);
   void SongsRatingChanged(const SongList &songs, const bool save_tags = false);
 
  signals:

src/collection/collectionbackend.cpp

@@ -140,8 +140,8 @@ void CollectionBackend::IncrementSkipCountAsync(const int id, const float progre
   QMetaObject::invokeMethod(this, "IncrementSkipCount", Qt::QueuedConnection, Q_ARG(int, id), Q_ARG(float, progress));
 }
 
-void CollectionBackend::ResetStatisticsAsync(const int id) {
+void CollectionBackend::ResetStatisticsAsync(const int id, const bool save_tags) {
-  QMetaObject::invokeMethod(this, "ResetStatistics", Qt::QueuedConnection, Q_ARG(int, id));
+  QMetaObject::invokeMethod(this, "ResetStatistics", Qt::QueuedConnection, Q_ARG(int, id), Q_ARG(bool, save_tags));
 }
 
 void CollectionBackend::LoadDirectories() {
@@ -714,7 +714,7 @@ void CollectionBackend::UpdateSongsBySongID(const SongMap &new_songs) {
 
       Song old_song = old_songs[new_song.song_id()];
 
-      if (!new_song.IsMetadataEqual(old_song)) {  // Update existing song.
+      if (!new_song.IsAllMetadataEqual(old_song)) {  // Update existing song.
 
         {
           SqlQuery q(db);
@@ -1776,7 +1776,7 @@ void CollectionBackend::IncrementSkipCount(const int id, const float progress) {
 
 }
 
-void CollectionBackend::ResetStatistics(const int id) {
+void CollectionBackend::ResetStatistics(const int id, const bool save_tags) {
 
   if (id == -1) return;
 
@@ -1792,7 +1792,7 @@ void CollectionBackend::ResetStatistics(const int id) {
   }
 
   Song new_song = GetSongById(id, db);
-  emit SongsStatisticsChanged(SongList() << new_song);
+  emit SongsStatisticsChanged(SongList() << new_song, save_tags);
 
 }
 
@@ -1927,7 +1927,7 @@ void CollectionBackend::UpdateLastPlayed(const QString &artist, const QString &a
 
 }
 
-void CollectionBackend::UpdatePlayCount(const QString &artist, const QString &title, const int playcount) {
+void CollectionBackend::UpdatePlayCount(const QString &artist, const QString &title, const int playcount, const bool save_tags) {
 
   SongList songs = GetSongsBy(artist, QString(), title);
   if (songs.isEmpty()) {
@@ -1949,7 +1949,7 @@ void CollectionBackend::UpdatePlayCount(const QString &artist, const QString &ti
     }
   }
 
-  emit SongsStatisticsChanged(SongList() << songs);
+  emit SongsStatisticsChanged(SongList() << songs, save_tags);
 
 }
 

src/collection/collectionbackend.h

@@ -200,7 +200,7 @@ class CollectionBackend : public CollectionBackendInterface {
 
   void IncrementPlayCountAsync(const int id);
   void IncrementSkipCountAsync(const int id, const float progress);
-  void ResetStatisticsAsync(const int id);
+  void ResetStatisticsAsync(const int id, const bool save_tags = false);
 
   void DeleteAllAsync();
 
@@ -236,13 +236,13 @@ class CollectionBackend : public CollectionBackendInterface {
   void ForceCompilation(const QString &album, const QList<QString> &artists, const bool on);
   void IncrementPlayCount(const int id);
   void IncrementSkipCount(const int id, const float progress);
-  void ResetStatistics(const int id);
+  void ResetStatistics(const int id, const bool save_tags);
   void DeleteAll();
   void SongPathChanged(const Song &song, const QFileInfo &new_file, const std::optional<int> new_collection_directory_id);
 
   SongList GetSongsBy(const QString &artist, const QString &album, const QString &title);
   void UpdateLastPlayed(const QString &artist, const QString &album, const QString &title, const qint64 lastplayed);
-  void UpdatePlayCount(const QString &artist, const QString &title, const int playcount);
+  void UpdatePlayCount(const QString &artist, const QString &title, const int playcount, const bool save_tags = false);
 
   void UpdateSongRating(const int id, const float rating, const bool save_tags = false);
   void UpdateSongsRating(const QList<int> &id_list, const float rating, const bool save_tags = false);
@@ -256,7 +256,7 @@ class CollectionBackend : public CollectionBackendInterface {
 
   void SongsDiscovered(SongList);
   void SongsDeleted(SongList);
-  void SongsStatisticsChanged(SongList);
+  void SongsStatisticsChanged(SongList, bool = false);
 
   void DatabaseReset();
 

src/collection/collectionwatcher.cpp

@@ -763,7 +763,7 @@ void CollectionWatcher::UpdateNonCueAssociatedSong(const QString &file,
   const Song &matching_song = matching_songs.first();
   if (cue_deleted) {
     for (const Song &song : matching_songs) {
-      if (!song.IsMetadataAndMoreEqual(matching_song)) {
+      if (!song.IsAllMetadataEqual(matching_song)) {
         t->deleted_songs << song;
       }
     }
@@ -855,6 +855,14 @@ void CollectionWatcher::AddChangedSong(const QString &file, const Song &matching
       changes << "metadata";
       notify_new = true;
     }
+    if (!matching_song.IsStatisticsEqual(new_song)) {
+      changes << "statistics";
+      notify_new = true;
+    }
+    if (!matching_song.IsRatingEqual(new_song)) {
+      changes << "rating";
+      notify_new = true;
+    }
     if (matching_song.art_automatic() != new_song.art_automatic() || matching_song.art_manual() != new_song.art_manual()) {
       changes << "album art";
       notify_new = true;

src/core/song.cpp

@@ -1523,17 +1523,43 @@ bool Song::IsMetadataEqual(const Song &other) const {
          d->bitrate_ == other.d->bitrate_ &&
          d->samplerate_ == other.d->samplerate_ &&
          d->bitdepth_ == other.d->bitdepth_ &&
-         d->cue_path_ == other.d->cue_path_ &&
+         d->cue_path_ == other.d->cue_path_;
-         d->playcount_ == other.d->playcount_ &&
-         d->rating_ == other.d->rating_;
 }
 
-bool Song::IsMetadataAndMoreEqual(const Song &other) const {
+bool Song::IsStatisticsEqual(const Song &other) const {
+
+  return d->playcount_ == other.d->playcount_ &&
+         d->skipcount_ == other.d->skipcount_ &&
+         d->lastplayed_ == other.d->lastplayed_;
+
+}
+
+bool Song::IsRatingEqual(const Song &other) const {
+
+  return d->rating_ == other.d->rating_;
+
+}
+
+bool Song::IsFingerprintEqual(const Song &other) const {
+
+  return d->fingerprint_ == other.d->fingerprint_;
+
+}
+
+bool Song::IsArtEqual(const Song &other) const {
+
+  return d->art_automatic_ == other.d->art_automatic_ &&
+         d->art_manual_ == other.d->art_manual_;
+
+}
+
+bool Song::IsAllMetadataEqual(const Song &other) const {
 
   return IsMetadataEqual(other) &&
-         d->fingerprint_ == other.d->fingerprint_ &&
+         IsStatisticsEqual(other) &&
-         d->art_automatic_ == other.d->art_automatic_ &&
+         IsRatingEqual(other) &&
-         d->art_manual_ == other.d->art_manual_;
+         IsFingerprintEqual(other) &&
+         IsArtEqual(other);
 
 }
 

src/core/song.h

@@ -106,7 +106,7 @@ class Song {
     FileType_XM = 20,
     FileType_IT = 21,
     FileType_SPC = 22,
-    FileType_VGM = 23, 
+    FileType_VGM = 23,
     FileType_CDDA = 90,
     FileType_Stream = 91,
   };
@@ -390,7 +390,12 @@ class Song {
 
   // Comparison functions
   bool IsMetadataEqual(const Song &other) const;
-  bool IsMetadataAndMoreEqual(const Song &other) const;
+  bool IsStatisticsEqual(const Song &other) const;
+  bool IsRatingEqual(const Song &other) const;
+  bool IsFingerprintEqual(const Song &other) const;
+  bool IsArtEqual(const Song &other) const;
+  bool IsAllMetadataEqual(const Song &other) const;
+
   bool IsOnSameAlbum(const Song &other) const;
   bool IsSimilar(const Song &other) const;
 

src/dialogs/edittagdialog.cpp

@@ -187,7 +187,7 @@ EditTagDialog::EditTagDialog(Application *app, QWidget *parent)
 
   QObject::connect(ui_->song_list->selectionModel(), &QItemSelectionModel::selectionChanged, this, &EditTagDialog::SelectionChanged);
   QObject::connect(ui_->button_box, &QDialogButtonBox::clicked, this, &EditTagDialog::ButtonClicked);
-  QObject::connect(ui_->playcount_reset, &QPushButton::clicked, this, &EditTagDialog::ResetPlayCounts);
+  QObject::connect(ui_->playcount_reset, &QPushButton::clicked, this, &EditTagDialog::ResetStatistics);
   QObject::connect(ui_->rating, &RatingWidget::RatingChanged, this, &EditTagDialog::SongRated);
 #ifdef HAVE_MUSICBRAINZ
   QObject::connect(ui_->fetch_tag, &QPushButton::clicked, this, &EditTagDialog::FetchTag);
@@ -1145,8 +1145,25 @@ void EditTagDialog::SaveData() {
       QObject::connect(reply, &TagReaderReply::Finished, this, [this, reply, ref]() { SongSaveTagsComplete(reply, ref.current_.url().toLocalFile(), ref.current_); }, Qt::QueuedConnection);
     }
 
-    if (ref.current_.rating() != ref.original_.rating() && ref.current_.is_collection_song()) {
+    if (ref.current_.playcount() == 0 &&
-      app_->collection_backend()->UpdateSongRatingAsync(ref.current_.id(), ref.current_.rating(), true);
+        ref.current_.skipcount() == 0 &&
+        ref.current_.lastplayed() == -1 &&
+        !ref.current_.IsStatisticsEqual(ref.original_)) {
+      if (ref.current_.is_collection_song()) {
+        app_->collection_backend()->ResetStatisticsAsync(ref.current_.id());
+      }
+      else {
+        app_->tag_reader_client()->UpdateSongsPlaycount(SongList() << ref.current_);
+      }
+    }
+
+    if (!ref.current_.IsRatingEqual(ref.original_)) {
+      if (ref.current_.is_collection_song()) {
+        app_->collection_backend()->UpdateSongRatingAsync(ref.current_.id(), ref.current_.rating());
+      }
+      else {
+        app_->tag_reader_client()->UpdateSongsRating(SongList() << ref.current_);
+      }
     }
 
     QString embedded_cover_from_file;
@@ -1287,15 +1304,15 @@ void EditTagDialog::AcceptFinished() {
 
 }
 
-void EditTagDialog::ResetPlayCounts() {
+void EditTagDialog::ResetStatistics() {
 
-  const QModelIndexList sel = ui_->song_list->selectionModel()->selectedIndexes();
+  const QModelIndexList idx_list = ui_->song_list->selectionModel()->selectedIndexes();
-  if (sel.isEmpty()) return;
+  if (idx_list.isEmpty()) return;
 
-  Song *song = &data_[sel.first().row()].original_;
+  Song *song = &data_[idx_list.first().row()].current_;
-  if (!song->is_valid() || song->id() == -1) return;
+  if (!song->is_valid()) return;
 
-  if (QMessageBox::question(this, tr("Reset play counts"), tr("Are you sure you want to reset this song's statistics?"), QMessageBox::Reset, QMessageBox::Cancel) != QMessageBox::Reset) {
+  if (QMessageBox::question(this, tr("Reset song statistics"), tr("Are you sure you want to reset this song's statistics?"), QMessageBox::Reset, QMessageBox::Cancel) != QMessageBox::Reset) {
     return;
   }
 
@@ -1303,10 +1320,6 @@ void EditTagDialog::ResetPlayCounts() {
   song->set_skipcount(0);
   song->set_lastplayed(-1);
 
-  if (song->is_collection_song()) {
-    app_->collection_backend()->ResetStatisticsAsync(song->id());
-  }
-
   UpdateStatisticsTab(*song);
 
 }
@@ -1317,7 +1330,7 @@ void EditTagDialog::SongRated(const float rating) {
   if (indexes.isEmpty()) return;
 
   for (const QModelIndex &idx : indexes) {
-    if (!data_[idx.row()].current_.is_valid() || data_[idx.row()].current_.id() == -1) continue;
+    if (!data_[idx.row()].current_.is_valid()) continue;
     data_[idx.row()].current_.set_rating(rating);
   }
 

src/dialogs/edittagdialog.h

@@ -115,7 +115,7 @@ class EditTagDialog : public QDialog {
   void FieldValueEdited();
   void ResetField();
   void ButtonClicked(QAbstractButton *button);
-  void ResetPlayCounts();
+  void ResetStatistics();
   void SongRated(const float rating);
   void FetchTag();
   void FetchTagSongChosen(const Song &original_song, const Song &new_metadata);

src/scrobbler/lastfmimport.cpp

@@ -542,7 +542,7 @@ void LastFMImport::GetTopTracksRequestFinished(QNetworkReply *reply, const int p
 
       if (playcount <= 0) continue;
 
-      emit UpdatePlayCount(artist, title, playcount);
+      emit UpdatePlayCount(artist, title, playcount, false);
       UpdateProgressCheck();
 
     }

src/scrobbler/lastfmimport.h

@@ -79,7 +79,7 @@ class LastFMImport : public QObject {
   void FinishCheck();
 
  signals:
-  void UpdatePlayCount(QString, QString, int);
+  void UpdatePlayCount(QString, QString, int, bool = false);
   void UpdateLastPlayed(QString, QString, QString, qint64);
   void UpdateTotal(int, int);
   void UpdateProgress(int, int);