Merge pull request #12591 from ronso0/stars-co-crash-workaround

avoid issue with `stars_up/_down` ControlObjects

src/library/dlgtrackinfo.cpp

@@ -45,7 +45,7 @@ DlgTrackInfo::DlgTrackInfo(
           m_tapFilter(this, kFilterLength, kMaxInterval),
           m_pWCoverArtMenu(make_parented<WCoverArtMenu>(this)),
           m_pWCoverArtLabel(make_parented<WCoverArtLabel>(this, m_pWCoverArtMenu)),
-          m_pWStarRating(make_parented<WStarRating>(nullptr, this)),
+          m_pWStarRating(make_parented<WStarRating>(this)),
           m_pColorPicker(make_parented<WColorPickerAction>(
                   WColorPicker::Option::AllowNoColor |
                           // TODO(xxx) remove this once the preferences are themed via QSS

src/mixer/basetrackplayer.cpp

@@ -702,6 +702,31 @@ void BaseTrackPlayerImpl::loadTrackFromGroup(const QString& group) {
     slotLoadTrack(pTrack, false);
 }
 
+void BaseTrackPlayerImpl::ensureStarControlsArePrepared() {
+    if (m_pStarsUp == nullptr) {
+        m_pStarsUp = std::make_unique<ControlPushButton>(ConfigKey(getGroup(), "stars_up"));
+        connect(m_pStarsUp.get(),
+                &ControlObject::valueChanged,
+                this,
+                [this](double value) {
+                    if (value > 0) {
+                        emit trackRatingChangeRequest(1);
+                    }
+                });
+    }
+    if (m_pStarsDown == nullptr) {
+        m_pStarsDown = std::make_unique<ControlPushButton>(ConfigKey(getGroup(), "stars_down"));
+        connect(m_pStarsDown.get(),
+                &ControlObject::valueChanged,
+                this,
+                [this](double value) {
+                    if (value > 0) {
+                        emit trackRatingChangeRequest(-1);
+                    }
+                });
+    }
+}
+
 void BaseTrackPlayerImpl::slotSetReplayGain(mixxx::ReplayGain replayGain) {
     // Do not change replay gain when track is playing because
     // this may lead to an unexpected volume change.

src/mixer/basetrackplayer.h

@@ -37,13 +37,14 @@ class BaseTrackPlayer : public BasePlayer {
 
     virtual TrackPointer getLoadedTrack() const = 0;
     virtual void setupEqControls() = 0;
+    virtual void ensureStarControlsArePrepared(){};
 
   public slots:
     virtual void slotLoadTrack(TrackPointer pTrack, bool bPlay = false) = 0;
     virtual void slotCloneFromGroup(const QString& group) = 0;
     virtual void slotCloneDeck() = 0;
     virtual void slotEjectTrack(double) = 0;
-    virtual void slotSetTrackRating(int rating) = 0;
+    virtual void slotSetTrackRating(int){};
 
   signals:
     void newTrackLoaded(TrackPointer pLoadedTrack);
@@ -52,6 +53,7 @@ class BaseTrackPlayer : public BasePlayer {
     void playerEmpty();
     void noVinylControlInputConfigured();
     void trackRatingChanged(int rating);
+    void trackRatingChangeRequest(int change);
 };
 
 class BaseTrackPlayerImpl : public BaseTrackPlayer {
@@ -70,13 +72,18 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer {
 
     TrackPointer getLoadedTrack() const final;
 
-    // TODO(XXX): Only exposed to let the passthrough AudioInput get
-    // connected. Delete me when EngineMixer supports AudioInput assigning.
+    /// TODO(XXX): Only exposed to let the passthrough AudioInput get
+    /// connected. Delete me when EngineMixer supports AudioInput assigning.
     EngineDeck* getEngineDeck() const;
 
     void setupEqControls() final;
 
-    // For testing, loads a fake track.
+    /// Controls to change the star rating.
+    /// Created on request only, because we need them only when there is
+    /// a star rating widget for this player.
+    void ensureStarControlsArePrepared() final;
+
+    /// For testing, loads a fake track.
     TrackPointer loadFakeTrack(bool bPlay, double filebpm);
 
   public slots:
@@ -87,8 +94,8 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer {
     void slotTrackLoaded(TrackPointer pNewTrack, TrackPointer pOldTrack);
     void slotLoadFailed(TrackPointer pTrack, const QString& reason);
     void slotSetReplayGain(mixxx::ReplayGain replayGain);
-    // When the replaygain is adjusted, we modify the track pregain
-    // to compensate so there is no audible change in volume.
+    /// When the replaygain is adjusted, we modify the track pregain
+    /// 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;
@@ -164,6 +171,9 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer {
     std::unique_ptr<ControlPushButton> m_pShiftCuesLaterSmall;
     std::unique_ptr<ControlObject> m_pShiftCues;
 
+    std::unique_ptr<ControlPushButton> m_pStarsUp;
+    std::unique_ptr<ControlPushButton> m_pStarsDown;
+
     std::unique_ptr<ControlObject> m_pUpdateReplayGainFromPregain;
 
     parented_ptr<ControlProxy> m_pReplayGain;

src/skin/legacy/legacyskinparser.cpp

@@ -1171,14 +1171,23 @@ QWidget* LegacySkinParser::parseStarRating(const QDomElement& node) {
         return nullptr;
     }
 
-    WStarRating* pStarRating = new WStarRating(group, m_pParent);
+    // Ensure stars_up/down controls are created and slots available.
+    // If they have already been created, existing connections were
+    // removed when the previous star widget was destroyed.
+    pPlayer->ensureStarControlsArePrepared();
+
+    WStarRating* pStarRating = new WStarRating(m_pParent);
     commonWidgetSetup(node, pStarRating, false);
     pStarRating->setup(node, *m_pContext);
 
     connect(pPlayer,
             &BaseTrackPlayer::trackRatingChanged,
             pStarRating,
             &WStarRating::slotSetRating);
+    connect(pPlayer,
+            &BaseTrackPlayer::trackRatingChangeRequest,
+            pStarRating,
+            &WStarRating::slotRatingUpDownRequest);
     connect(pStarRating,
             &WStarRating::ratingChanged,
             pPlayer,

src/widget/wstarrating.cpp

@@ -9,22 +9,10 @@
 class QEvent;
 class QWidgets;
 
-WStarRating::WStarRating(const QString& group, QWidget* pParent)
+WStarRating::WStarRating(QWidget* pParent)
         : WWidget(pParent),
           m_starCount(0),
           m_visualStarRating(m_starCount, 5) {
-    // Controls to change the star rating with controllers.
-    // Note that 'group' maybe NULLPTR, e.g. when called from DlgTrackInfo,
-    // so only create rate change COs if there's a group passed when creating deck widgets.
-    if (!group.isEmpty()) {
-        m_pStarsUp = std::make_unique<ControlPushButton>(ConfigKey(group, "stars_up"));
-        m_pStarsDown = std::make_unique<ControlPushButton>(ConfigKey(group, "stars_down"));
-        connect(m_pStarsUp.get(), &ControlObject::valueChanged, this, &WStarRating::slotStarsUp);
-        connect(m_pStarsDown.get(),
-                &ControlObject::valueChanged,
-                this,
-                &WStarRating::slotStarsDown);
-    }
 }
 
 void WStarRating::setup(const QDomNode& node, const SkinContext& context) {
@@ -54,6 +42,10 @@ void WStarRating::slotSetRating(int starCount) {
     emit ratingChanged(m_starCount);
 }
 
+void WStarRating::slotRatingUpDownRequest(int change) {
+    slotSetRating(m_starCount + change);
+}
+
 void WStarRating::paintEvent(QPaintEvent * /*unused*/) {
     QStyleOption option;
     option.initFrom(this);
@@ -80,18 +72,6 @@ void WStarRating::mouseMoveEvent(QMouseEvent *event) {
     }
 }
 
-void WStarRating::slotStarsUp(double v) {
-    if (v > 0 && m_starCount < m_visualStarRating.maxStarCount()) {
-        slotSetRating(m_starCount + 1);
-    }
-}
-
-void WStarRating::slotStarsDown(double v) {
-    if (v > 0 && m_starCount > StarRating::kMinStarCount) {
-        slotSetRating(m_starCount - 1);
-    }
-}
-
 void WStarRating::leaveEvent(QEvent* /*unused*/) {
     resetVisualRating();
 }

src/widget/wstarrating.h

@@ -1,32 +1,26 @@
 #pragma once
 
-#include "control/controlpushbutton.h"
 #include "library/starrating.h"
 #include "widget/wwidget.h"
 
-class ControlObject;
-class ControlPushButton;
 class QDomNode;
 class SkinContext;
 
 class WStarRating : public WWidget {
     Q_OBJECT
   public:
-    WStarRating(const QString& group, QWidget* pParent);
+    WStarRating(QWidget* pParent);
 
     virtual void setup(const QDomNode& node, const SkinContext& context);
     QSize sizeHint() const override;
 
   public slots:
     void slotSetRating(int starCount);
+    void slotRatingUpDownRequest(int change);
 
   signals:
     void ratingChanged(int starCount);
 
-  private slots:
-    void slotStarsUp(double v);
-    void slotStarsDown(double v);
-
   protected:
     void paintEvent(QPaintEvent* e) override;
     void mouseMoveEvent(QMouseEvent *event) override;
@@ -44,7 +38,4 @@ class WStarRating : public WWidget {
     void resetVisualRating() {
         updateVisualRating(m_starCount);
     }
-
-    std::unique_ptr<ControlPushButton> m_pStarsUp;
-    std::unique_ptr<ControlPushButton> m_pStarsDown;
 };