Merge pull request #13853 from Swiftb0y/refactor/valuetransformer

Refactor `ValueTransformer` and `WBaseWidget`

src/skin/legacy/legacyskinparser.cpp

@@ -8,8 +8,10 @@
 #include <QVBoxLayout>
 #include <QtDebug>
 #include <QtGlobal>
+#include <memory>
 
 #include "control/controlobject.h"
+#include "control/controlpushbutton.h"
 #include "controllers/controllerlearningeventfilter.h"
 #include "controllers/controllermanager.h"
 #include "controllers/keyboard/keyboardeventfilter.h"
@@ -23,6 +25,7 @@
 #include "skin/legacy/launchimage.h"
 #include "skin/legacy/skincontext.h"
 #include "track/track.h"
+#include "util/assert.h"
 #include "util/cmdlineargs.h"
 #include "util/timer.h"
 #include "util/valuetransformer.h"
@@ -2369,8 +2372,6 @@ void LegacySkinParser::setupWidget(const QDomNode& node,
 }
 
 void LegacySkinParser::setupConnections(const QDomNode& node, WBaseWidget* pWidget) {
-    ControlParameterWidgetConnection* pLastLeftOrNoButtonConnection = nullptr;
-
     for (QDomNode con = m_pContext->selectNode(node, "Connection");
             !con.isNull();
             con = con.nextSibling()) {
@@ -2381,20 +2382,22 @@ void LegacySkinParser::setupConnections(const QDomNode& node, WBaseWidget* pWidg
             continue;
         }
 
-        ValueTransformer* pTransformer = nullptr;
+        std::unique_ptr<ValueTransformer> pTransformer = nullptr;
         QDomElement transform = m_pContext->selectElement(con, "Transform");
         if (!transform.isNull()) {
-            pTransformer = ValueTransformer::parseFromXml(transform, *m_pContext);
+            pTransformer =
+                    ValueTransformer::parseFromXml(transform, *m_pContext);
         }
 
         QString property;
         if (m_pContext->hasNodeSelectString(con, "BindProperty", &property)) {
             //qDebug() << "Making property connection for" << property;
 
-            ControlWidgetPropertyConnection* pConnection =
-                    new ControlWidgetPropertyConnection(pWidget, control->getKey(),
-                                                        pTransformer, property);
-            pWidget->addPropertyConnection(pConnection);
+            pWidget->addPropertyConnection(
+                    std::make_unique<ControlWidgetPropertyConnection>(pWidget,
+                            control->getKey(),
+                            std::move(pTransformer),
+                            property));
         } else {
             bool nodeValue;
             Qt::MouseButton state = parseButtonState(con, *m_pContext);
@@ -2455,26 +2458,36 @@ void LegacySkinParser::setupConnections(const QDomNode& node, WBaseWidget* pWidg
                 emitOption |= ControlParameterWidgetConnection::EMIT_DEFAULT;
             }
 
-            ControlParameterWidgetConnection* pConnection = new ControlParameterWidgetConnection(
-                    pWidget, control->getKey(), pTransformer,
-                    static_cast<ControlParameterWidgetConnection::DirectionOption>(directionOption),
-                    static_cast<ControlParameterWidgetConnection::EmitOption>(emitOption));
+            auto pConnection =
+                    std::make_unique<ControlParameterWidgetConnection>(pWidget,
+                            control->getKey(),
+                            std::move(pTransformer),
+                            static_cast<ControlParameterWidgetConnection::
+                                            DirectionOption>(directionOption),
+                            static_cast<ControlParameterWidgetConnection::
+                                            EmitOption>(emitOption));
 
             switch (state) {
             case Qt::NoButton:
-                pWidget->addConnection(pConnection);
                 if (directionOption & ControlParameterWidgetConnection::DIR_TO_WIDGET) {
-                    pLastLeftOrNoButtonConnection = pConnection;
+                    pWidget->addAndSetDisplayConnection(std::move(pConnection),
+                            WBaseWidget::ConnectionSide::Right);
+                } else {
+                    pWidget->addConnection(std::move(pConnection),
+                            WBaseWidget::ConnectionSide::Right);
                 }
                 break;
             case Qt::LeftButton:
-                pWidget->addLeftConnection(pConnection);
                 if (directionOption & ControlParameterWidgetConnection::DIR_TO_WIDGET) {
-                    pLastLeftOrNoButtonConnection = pConnection;
+                    pWidget->addAndSetDisplayConnection(std::move(pConnection),
+                            WBaseWidget::ConnectionSide::Left);
+                } else {
+                    pWidget->addConnection(std::move(pConnection),
+                            WBaseWidget::ConnectionSide::Left);
                 }
                 break;
             case Qt::RightButton:
-                pWidget->addRightConnection(pConnection);
+                pWidget->addConnection(std::move(pConnection), WBaseWidget::ConnectionSide::Right);
                 break;
             default:
                 // can't happen. Nothing else is returned by parseButtonState();
@@ -2564,14 +2577,6 @@ void LegacySkinParser::setupConnections(const QDomNode& node, WBaseWidget* pWidg
             }
         }
     }
-
-    // Legacy behavior: The last left-button or no-button connection with
-    // connectValueToWidget is the display connection. If no left-button or
-    // no-button connection exists, use the last right-button connection as the
-    // display connection.
-    if (pLastLeftOrNoButtonConnection != nullptr) {
-        pWidget->setDisplayConnection(pLastLeftOrNoButtonConnection);
-    }
 }
 
 void LegacySkinParser::addShortcutToToolTip(WBaseWidget* pWidget,

src/test/wpushbutton_test.cpp

@@ -1,82 +1,59 @@
+#include "widget/wpushbutton.h"
+
 #include <gtest/gtest.h>
 
 #include <QTestEventList>
-#include <QScopedPointer>
+#include <memory>
 
-#include "mixxxtest.h"
-#include "control/controlobject.h"
-#include "control/controlproxy.h"
 #include "control/controlpushbutton.h"
-#include "widget/wpushbutton.h"
+#include "util/valuetransformer.h"
 #include "widget/controlwidgetconnection.h"
+#include "widget/wbasewidget.h"
 
-class WPushButtonTest : public MixxxTest {
+class WPushButtonTest : public ::testing::Test {
   public:
-    WPushButtonTest()
-          : m_pGroup("[Channel1]") {
-    }
-
-  protected:
-    void SetUp() override {
-        m_pTouchShift.reset(new ControlPushButton(ConfigKey("[Controls]", "touch_shift")));
-        m_pButton.reset(new WPushButton());
-        m_pButton->setStates(2);
+    // touchShift is needed internally to avoid a DEBUG_ASSERT
+    ControlPushButton touchShift = ConfigKey(
+            QStringLiteral("[Controls]"), QStringLiteral("touch_shift"));
+    ControlPushButton pushControl = ConfigKey(QStringLiteral("[Test]"), QStringLiteral("push"));
+    WPushButton pushButton = WPushButton(nullptr,
+            mixxx::control::ButtonMode::LongPressLatching,
+            mixxx::control::ButtonMode::Push);
+
+    WPushButtonTest() {
+        pushControl.setButtonMode(mixxx::control::ButtonMode::LongPressLatching);
+        pushButton.setStates(2);
+        pushButton.addConnection(
+                std::make_unique<ControlParameterWidgetConnection>(
+                        &pushButton,
+                        pushControl.getKey(),
+                        nullptr,
+                        ControlParameterWidgetConnection::DIR_FROM_AND_TO_WIDGET,
+                        ControlParameterWidgetConnection::EMIT_ON_PRESS_AND_RELEASE),
+                WBaseWidget::ConnectionSide::Left);
     }
-
-    QScopedPointer<WPushButton> m_pButton;
-    QScopedPointer<ControlPushButton> m_pTouchShift;
-    QTestEventList m_Events;
-    const char* m_pGroup;
 };
 
 TEST_F(WPushButtonTest, QuickPressNoLatchTest) {
-    QScopedPointer<ControlPushButton> pPushControl(
-        new ControlPushButton(ConfigKey("[Test]", "push")));
-    pPushControl->setButtonMode(mixxx::control::ButtonMode::LongPressLatching);
-
-    m_pButton.reset(new WPushButton(NULL,
-            mixxx::control::ButtonMode::LongPressLatching,
-            mixxx::control::ButtonMode::Push));
-    m_pButton->setStates(2);
-    m_pButton->addLeftConnection(
-        new ControlParameterWidgetConnection(
-            m_pButton.data(),
-            pPushControl->getKey(), NULL,
-            ControlParameterWidgetConnection::DIR_FROM_AND_TO_WIDGET,
-            ControlParameterWidgetConnection::EMIT_ON_PRESS_AND_RELEASE));
-
     // This test can be flaky if the event simulator takes too long to deliver
     // the event.
-    m_Events.addMousePress(Qt::LeftButton);
-    m_Events.addDelay(100);
-    m_Events.addMouseRelease(Qt::LeftButton);
+    QTestEventList events;
+    events.addMousePress(Qt::LeftButton);
+    events.addDelay(100);
+    events.addMouseRelease(Qt::LeftButton);
 
-    m_Events.simulate(m_pButton.data());
+    events.simulate(&pushButton);
 
-    ASSERT_EQ(0.0, m_pButton->getControlParameterLeft());
+    ASSERT_EQ(0.0, pushButton.getControlParameterLeft());
 }
 
 TEST_F(WPushButtonTest, LongPressLatchTest) {
-    QScopedPointer<ControlPushButton> pPushControl(
-        new ControlPushButton(ConfigKey("[Test]", "push")));
-    pPushControl->setButtonMode(mixxx::control::ButtonMode::LongPressLatching);
-
-    m_pButton.reset(new WPushButton(NULL,
-            mixxx::control::ButtonMode::LongPressLatching,
-            mixxx::control::ButtonMode::Push));
-    m_pButton->setStates(2);
-    m_pButton->addLeftConnection(
-        new ControlParameterWidgetConnection(
-            m_pButton.data(),
-            pPushControl->getKey(), NULL,
-            ControlParameterWidgetConnection::DIR_FROM_AND_TO_WIDGET,
-            ControlParameterWidgetConnection::EMIT_ON_PRESS_AND_RELEASE));
-
-    m_Events.addMousePress(Qt::LeftButton);
-    m_Events.addDelay(1000);
-    m_Events.addMouseRelease(Qt::LeftButton);
+    QTestEventList events;
+    events.addMousePress(Qt::LeftButton);
+    events.addDelay(1000);
+    events.addMouseRelease(Qt::LeftButton);
 
-    m_Events.simulate(m_pButton.data());
+    events.simulate(&pushButton);
 
-    ASSERT_EQ(1.0, m_pButton->getControlParameterLeft());
+    ASSERT_EQ(1.0, pushButton.getControlParameterLeft());
 }