WStarRating: don't update track directly, use signals/slots

src/library/dlgtrackinfo.cpp

@@ -252,6 +252,14 @@ void DlgTrackInfo::init() {
             this,
             &DlgTrackInfo::slotReloadCoverArt);
 
+    connect(m_pWStarRating,
+            &WStarRating::ratingChanged,
+            this,
+            [this](int rating) {
+                m_pWStarRating->slotSetRating(rating);
+                m_trackRecord.setRating(rating);
+            });
+
     btnColorPicker->setStyle(QStyleFactory::create(QStringLiteral("fusion")));
     QMenu* pColorPickerMenu = new QMenu(this);
     pColorPickerMenu->addAction(m_pColorPicker);
@@ -338,7 +346,7 @@ void DlgTrackInfo::updateFromTrack(const Track& track) {
 
     reloadTrackBeats(track);
 
-    m_pWStarRating->slotTrackLoaded(m_pLoadedTrack);
+    m_pWStarRating->slotSetRating(m_pLoadedTrack->getRating());
 }
 
 void DlgTrackInfo::replaceTrackRecord(
@@ -446,7 +454,7 @@ void DlgTrackInfo::loadTrackInternal(const TrackPointer& pTrack) {
     updateFromTrack(*m_pLoadedTrack);
     m_pWCoverArtLabel->loadTrack(m_pLoadedTrack);
 
-    // We already listen to changed() so we don't need to listen to individual
+    // Listen to changed() so we don't need to listen to individual
     // signals such as cuesUpdates, coverArtUpdated(), etc.
     connect(pTrack.get(),
             &Track::changed,
@@ -603,6 +611,8 @@ void DlgTrackInfo::clear() {
     updateSpinBpmFromBeats();
 
     txtLocation->setText("");
+
+    m_pWStarRating->slotSetRating(0);
 }
 
 void DlgTrackInfo::slotBpmScale(mixxx::Beats::BpmScale bpmScale) {

src/library/starrating.h

@@ -13,7 +13,7 @@ QT_FORWARD_DECLARE_CLASS(QRect);
  * The StarRating class represents a rating as a number of stars.
  * In addition to holding the data, it is also capable of painting the stars on a QPaintDevice,
  * which in this example is either a view or an editor.
- * The myStarCount member variable stores the current rating, and myMaxStarCount stores
+ * The m_starCount member variable stores the current rating, and m_maxStarCount stores
  * the highest possible rating (typically 5).
  */
 class StarRating {

src/mixer/basetrackplayer.cpp

@@ -428,6 +428,14 @@ void BaseTrackPlayerImpl::connectLoadedTrack() {
             &Track::colorUpdated,
             this,
             &BaseTrackPlayerImpl::slotSetTrackColor);
+
+    // Forward the update signal, i.e. use BaseTrackPlayer as relay.
+    // Currently only used by WStarRating which is connected in
+    // LegacySkinParser::parseStarRating
+    connect(m_pLoadedTrack.get(),
+            &Track::ratingUpdated,
+            this,
+            &BaseTrackPlayerImpl::trackRatingChanged);
 }
 
 void BaseTrackPlayerImpl::disconnectLoadedTrack() {
@@ -516,6 +524,7 @@ void BaseTrackPlayerImpl::slotTrackLoaded(TrackPointer pNewTrack,
         m_pLoopOutPoint->set(kNoTrigger);
         m_pLoadedTrack.reset();
         emit playerEmpty();
+        emit trackRatingChanged(0);
     } else if (pNewTrack && pNewTrack == m_pLoadedTrack) {
         // NOTE(uklotzde): In a previous version track metadata was reloaded
         // from the source file at this point again. This is no longer necessary
@@ -596,6 +605,7 @@ void BaseTrackPlayerImpl::slotTrackLoaded(TrackPointer pNewTrack,
         }
 
         emit newTrackLoaded(m_pLoadedTrack);
+        emit trackRatingChanged(m_pLoadedTrack->getRating());
     } else {
         // this is the result from an outdated load or unload signal
         // A new load is already pending
@@ -737,6 +747,13 @@ void BaseTrackPlayerImpl::slotTrackColorChangeRequest(double v) {
     m_pLoadedTrack->setColor(color);
 }
 
+void BaseTrackPlayerImpl::slotSetTrackRating(int rating) {
+    if (!m_pLoadedTrack) {
+        return;
+    }
+    m_pLoadedTrack->setRating(rating);
+}
+
 void BaseTrackPlayerImpl::slotPlayToggled(double value) {
     if (value == 0 && m_replaygainPending) {
         setReplayGain(m_pLoadedTrack->getReplayGain().getRatio());

src/mixer/basetrackplayer.h

@@ -44,13 +44,15 @@ class BaseTrackPlayer : public BasePlayer {
     virtual void slotCloneFromGroup(const QString& group) = 0;
     virtual void slotCloneDeck() = 0;
     virtual void slotEjectTrack(double) = 0;
+    virtual void slotSetTrackRating(int rating) = 0;
 
   signals:
     void newTrackLoaded(TrackPointer pLoadedTrack);
     void trackUnloaded(TrackPointer pUnloadedTrack);
     void loadingTrack(TrackPointer pNewTrack, TrackPointer pOldTrack);
     void playerEmpty();
     void noVinylControlInputConfigured();
+    void trackRatingChanged(int rating);
 };
 
 class BaseTrackPlayerImpl : public BaseTrackPlayer {
@@ -90,6 +92,7 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer {
     // to compensate so there is no audible change in volume.
     void slotAdjustReplayGain(mixxx::ReplayGain replayGain);
     void slotSetTrackColor(const mixxx::RgbColor::optional_t& color);
+    void slotSetTrackRating(int rating) final;
     void slotPlayToggled(double);
 
   private slots:

src/skin/legacy/legacyskinparser.cpp

@@ -24,6 +24,7 @@
 #include "skin/legacy/colorschemeparser.h"
 #include "skin/legacy/launchimage.h"
 #include "skin/legacy/skincontext.h"
+#include "track/track.h"
 #include "util/cmdlineargs.h"
 #include "util/timer.h"
 #include "util/valuetransformer.h"
@@ -442,8 +443,6 @@ LaunchImage* LegacySkinParser::parseLaunchImage(const QString& skinPath, QWidget
     return pLaunchImage;
 }
 
-
-
 QList<QWidget*> wrapWidget(QWidget* pWidget) {
     QList<QWidget*> result;
     if (pWidget != nullptr) {
@@ -1163,14 +1162,18 @@ QWidget* LegacySkinParser::parseStarRating(const QDomElement& node) {
     commonWidgetSetup(node, pStarRating, false);
     pStarRating->setup(node, *m_pContext);
 
-    connect(pPlayer, &BaseTrackPlayer::newTrackLoaded, pStarRating, &WStarRating::slotTrackLoaded);
-    connect(pPlayer, &BaseTrackPlayer::playerEmpty, pStarRating, [pStarRating]() {
-        pStarRating->slotTrackLoaded(TrackPointer());
-    });
+    connect(pPlayer,
+            &BaseTrackPlayer::trackRatingChanged,
+            pStarRating,
+            &WStarRating::slotSetRating);
+    connect(pStarRating,
+            &WStarRating::ratingChanged,
+            pPlayer,
+            &BaseTrackPlayer::slotSetTrackRating);
 
     TrackPointer pTrack = pPlayer->getLoadedTrack();
     if (pTrack) {
-        pStarRating->slotTrackLoaded(pTrack);
+        pStarRating->slotSetRating(pTrack->getRating());
     }
 
     return pStarRating;

src/test/autodjprocessor_test.cpp

@@ -105,6 +105,8 @@ class FakeDeck : public BaseTrackPlayer {
     MOCK_METHOD1(slotCloneFromGroup, void(const QString& group));
     MOCK_METHOD0(slotCloneDeck, void());
 
+    void slotSetTrackRating(int /*unused*/) override{};
+
     TrackPointer loadedTrack;
     ControlObject trackSamples;
     ControlObject samplerate;

src/track/track.cpp

@@ -289,6 +289,7 @@ bool Track::replaceRecord(
     const auto newKey = newRecord.getGlobalKey();
     const auto newReplayGain = newRecord.getMetadata().getTrackInfo().getReplayGain();
     const auto newColor = newRecord.getColor();
+    const auto newRating = newRecord.getRating();
 
     auto locked = lockMutex(&m_qMutex);
     const bool recordUnchanged = m_record == newRecord;
@@ -299,6 +300,7 @@ bool Track::replaceRecord(
     const auto oldKey = m_record.getGlobalKey();
     const auto oldReplayGain = m_record.getMetadata().getTrackInfo().getReplayGain();
     const auto oldColor = m_record.getColor();
+    const auto oldRating = m_record.getRating();
 
     bool bpmUpdatedFlag;
     if (pOptionalBeats) {
@@ -334,6 +336,9 @@ bool Track::replaceRecord(
     if (oldColor != newColor) {
         emit colorUpdated(newColor);
     }
+    if (oldRating != newRating) {
+        emit ratingUpdated(newRating);
+    }
 
     emitChangedSignalsForAllMetadata();
     return true;
@@ -1360,6 +1365,7 @@ void Track::setRating (int rating) {
     auto locked = lockMutex(&m_qMutex);
     if (compareAndSet(m_record.ptrRating(), rating)) {
         markDirtyAndUnlock(&locked);
+        emit ratingUpdated(rating);
     }
 }
 

src/track/track.h

@@ -436,6 +436,7 @@ class Track : public QObject {
     // adjusted in the opposite direction to compensate (no audible change).
     void replayGainAdjusted(const mixxx::ReplayGain&);
     void colorUpdated(const mixxx::RgbColor::optional_t& color);
+    void ratingUpdated(int rating);
     void cuesUpdated();
     void loopRemove();
     void analyzed();