From 7380ae5431e962e0dbeec18fb59dfcfd055502aa Mon Sep 17 00:00:00 2001 From: Antoine C Date: Thu, 23 Feb 2023 22:38:44 +0000 Subject: [PATCH 01/17] feat(Controller): add support for settings This commit adds basics for support of controller settings. --- CMakeLists.txt | 1 + src/controllers/dlgprefcontroller.cpp | 18 ++ src/controllers/dlgprefcontrollerdlg.ui | 238 +++++++++--------- .../legacyhidcontrollermappingfilehandler.cpp | 1 + src/controllers/legacycontrollermapping.h | 25 ++ .../legacycontrollermappingfilehandler.cpp | 29 +++ .../legacycontrollermappingfilehandler.h | 3 + src/controllers/legacycontrollersettings.cpp | 93 +++++++ src/controllers/legacycontrollersettings.h | 117 +++++++++ .../legacycontrollersettingsfactory.h | 77 ++++++ 10 files changed, 485 insertions(+), 117 deletions(-) create mode 100644 src/controllers/legacycontrollersettings.cpp create mode 100644 src/controllers/legacycontrollersettings.h create mode 100644 src/controllers/legacycontrollersettingsfactory.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 7c67775d2566..5e7e40f500b0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -731,6 +731,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/controllers/controllerlearningeventfilter.cpp src/controllers/controllermanager.cpp src/controllers/controllermappinginfo.cpp + src/controllers/legacycontrollersettings.cpp src/controllers/controllermappinginfoenumerator.cpp src/controllers/controllermappingtablemodel.cpp src/controllers/controlleroutputmappingtablemodel.cpp diff --git a/src/controllers/dlgprefcontroller.cpp b/src/controllers/dlgprefcontroller.cpp index 2c6f09dee856..70e96200ce30 100644 --- a/src/controllers/dlgprefcontroller.cpp +++ b/src/controllers/dlgprefcontroller.cpp @@ -614,6 +614,8 @@ void DlgPrefController::slotMappingSelected(int chosenIndex) { } enableWizardAndIOTabs(false); } + + m_ui.groupBoxSettings->setVisible(false); } else { // User picked a mapping m_ui.chkEnabledDevice->setEnabled(true); @@ -823,6 +825,22 @@ void DlgPrefController::slotShowMapping(std::shared_ptr m_ui.labelLoadedMappingSupportLinks->setText(mappingSupportLinks(pMapping)); m_ui.labelLoadedMappingScriptFileLinks->setText(mappingFileLinks(pMapping)); + if (pMapping) { + const QList>& + settings = pMapping->getSettings(); + + qDeleteAll(m_ui.groupBoxSettings->findChildren("", Qt::FindDirectChildrenOnly)); + + foreach (std::shared_ptr setting, settings) { + QWidget* settingWidget = setting->buildWidget(this); + // connect(settingWidget, &AbstractLegacyControllerSetting::changed, + // this, [this] { setDirty(true); }); + m_ui.groupBoxSettings->layout()->addWidget(settingWidget); + } + + m_ui.groupBoxSettings->setVisible(!settings.isEmpty()); + } + // We mutate this mapping so keep a reference to it while we are using it. // TODO(rryan): Clone it? Technically a waste since nothing else uses this // copy but if someone did they might not expect it to change. diff --git a/src/controllers/dlgprefcontrollerdlg.ui b/src/controllers/dlgprefcontrollerdlg.ui index 1d391eb06aad..4d971609ea15 100644 --- a/src/controllers/dlgprefcontrollerdlg.ui +++ b/src/controllers/dlgprefcontrollerdlg.ui @@ -6,8 +6,8 @@ 0 0 - 507 - 437 + 837 + 945 @@ -29,110 +29,8 @@ Controller Setup - - - 0 - 0 - - - - - - - true - - - - 0 - 0 - - - - - 14 - 75 - true - - - - Controller Name - - - - - - - true - - - - 0 - 0 - - - - - - - (device category goes here) - - - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter - - - - - - - Enabled - - - - - - - Click to start the Controller Learning wizard. - - - - - - Learning Wizard (MIDI Only) - - - false - - - false - - - - - - - - 0 - 0 - - - - - 16777215 - 16777215 - - - - Load Mapping: - - - Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter - - - comboBoxMapping - - - - + + @@ -191,6 +89,53 @@ + + + + + 0 + 0 + + + + + 16777215 + 16777215 + + + + Load Mapping: + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + comboBoxMapping + + + + + + + true + + + + 0 + 0 + + + + + + + (device category goes here) + + + Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter + + + @@ -207,6 +152,68 @@ + + + + Click to start the Controller Learning wizard. + + + + + + Learning Wizard (MIDI Only) + + + false + + + false + + + + + + + true + + + + 0 + 0 + + + + + 14 + 75 + true + + + + Controller Name + + + + + + + Enabled + + + + + + + Qt::Vertical + + + + 20 + 40 + + + + @@ -421,18 +428,16 @@ - - - - Qt::Vertical + + + + Mapping settings - - - 20 - 40 - + + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter - + + @@ -525,7 +530,6 @@ - Output Mappings diff --git a/src/controllers/hid/legacyhidcontrollermappingfilehandler.cpp b/src/controllers/hid/legacyhidcontrollermappingfilehandler.cpp index 1b715ea8cd60..08b4a66aa6b2 100644 --- a/src/controllers/hid/legacyhidcontrollermappingfilehandler.cpp +++ b/src/controllers/hid/legacyhidcontrollermappingfilehandler.cpp @@ -24,6 +24,7 @@ LegacyHidControllerMappingFileHandler::load(const QDomElement& root, auto pMapping = std::make_shared(); pMapping->setFilePath(filePath); parseMappingInfo(root, pMapping); + parseMappingSettings(root, pMapping); addScriptFilesToMapping(controller, pMapping, systemMappingsPath); return pMapping; } diff --git a/src/controllers/legacycontrollermapping.h b/src/controllers/legacycontrollermapping.h index ca0e369b96c4..21080bee88b5 100644 --- a/src/controllers/legacycontrollermapping.h +++ b/src/controllers/legacycontrollermapping.h @@ -1,13 +1,18 @@ #pragma once +#include + #include #include +#include #include #include #include #include +#include #include +#include "controllers/legacycontrollersettings.h" #include "defs_urls.h" /// This class represents a controller mapping, containing the data elements that @@ -50,10 +55,29 @@ class LegacyControllerMapping { setDirty(true); } + /// Adds a setting option to the list of setting option for this mapping. + /// The option added must be a valid option. + /// @param option The option to add + void addSetting(std::shared_ptr option) { + // if (m_settings.contains(option->variableName())){ + // qWarning() << QString("Mapping setting duplication detected. + // Keeping the first version of '%1'.").arg(option->variableName()); + // return; + // } + VERIFY_OR_DEBUG_ASSERT(option->valid()) { + return; + } + m_settings.append(option); + } + const QList& getScriptFiles() const { return m_scripts; } + const QList>& getSettings() const { + return m_settings; + } + inline void setDirty(bool bDirty) { m_bDirty = bDirty; } @@ -191,5 +215,6 @@ class LegacyControllerMapping { QString m_schemaVersion; QString m_mixxxVersion; + QList> m_settings; QList m_scripts; }; diff --git a/src/controllers/legacycontrollermappingfilehandler.cpp b/src/controllers/legacycontrollermappingfilehandler.cpp index 754c972ba78f..fd821ce639d8 100644 --- a/src/controllers/legacycontrollermappingfilehandler.cpp +++ b/src/controllers/legacycontrollermappingfilehandler.cpp @@ -108,6 +108,35 @@ void LegacyControllerMappingFileHandler::parseMappingInfo( mapping->setWikiLink(wiki.isNull() ? "" : wiki.text()); } +void LegacyControllerMappingFileHandler::parseMappingSettings( + const QDomElement& root, std::shared_ptr mapping) const { + if (root.isNull() || !mapping) { + return; + } + + QDomElement settings = root.firstChildElement("settings"); + if (settings.isNull()) { + return; + } + + for (QDomElement option = settings.firstChildElement("option"); + !option.isNull(); + option = option.nextSiblingElement("option")) { + AbstractLegacyControllerSetting* setting = LegacyControllerSettingBuilder::build(option); + if (setting == nullptr) { + qDebug() << "Could not parse the unknown controller setting. Ignoring it."; + continue; + } + if (!setting->valid()) { + qDebug() << "The parsed setting appears to be invalid. Discarding it."; + delete setting; + continue; + } + + mapping->addSetting(std::shared_ptr(setting)); + } +} + QDomElement LegacyControllerMappingFileHandler::getControllerNode( const QDomElement& root) { if (root.isNull()) { diff --git a/src/controllers/legacycontrollermappingfilehandler.h b/src/controllers/legacycontrollermappingfilehandler.h index c7bc6377b6d2..ee23268ca74c 100644 --- a/src/controllers/legacycontrollermappingfilehandler.h +++ b/src/controllers/legacycontrollermappingfilehandler.h @@ -41,6 +41,9 @@ class LegacyControllerMappingFileHandler { void parseMappingInfo(const QDomElement& root, std::shared_ptr mapping) const; + void parseMappingSettings(const QDomElement& root, + std::shared_ptr mapping) const; + /// Adds script files from XML to the LegacyControllerMapping. /// /// This function parses the supplied QDomElement structure, finds the diff --git a/src/controllers/legacycontrollersettings.cpp b/src/controllers/legacycontrollersettings.cpp new file mode 100644 index 000000000000..54e059f87bba --- /dev/null +++ b/src/controllers/legacycontrollersettings.cpp @@ -0,0 +1,93 @@ +#include "controllers/legacycontrollersettings.h" + +#include "moc_legacycontrollersettings.cpp" + +LegacyControllerSettingBuilder* LegacyControllerSettingBuilder::__self = nullptr; + +LegacyControllerSettingBuilder* LegacyControllerSettingBuilder::instance() { + if (__self == nullptr) + __self = new LegacyControllerSettingBuilder(); + + return __self; +} + +QWidget* LegacyControllerIntegerSetting::buildWidget(QWidget* parent) { + QWidget* root = new QWidget(parent); + root->setLayout(new QHBoxLayout()); + root->layout()->setContentsMargins(0, 0, 0, 0); + + QLabel* labelWidget = new QLabel(root); + labelWidget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum); + labelWidget->setText(label()); + + if (!description().isEmpty()) { + auto layout = new QVBoxLayout(root); + + root->layout()->setContentsMargins(0, 0, 0, 0); + + auto descriptionWidget = new QLabel(root); + descriptionWidget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum); + descriptionWidget->setText(description()); + QFont descriptionFont; + descriptionFont.setPointSize(8); + descriptionFont.setItalic(true); + descriptionWidget->setFont(descriptionFont); + + layout->addWidget(labelWidget); + layout->addWidget(descriptionWidget); + root->layout()->addItem(layout); + } else { + root->layout()->addWidget(labelWidget); + } + + QSpinBox* spinBox = new QSpinBox(root); + spinBox->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Fixed); + root->layout()->addWidget(spinBox); + + spinBox->setRange(this->m_minValue, this->m_maxValue); + spinBox->setSingleStep(this->m_stepValue); + spinBox->setValue(this->m_currentValue); + + connect(spinBox, QOverload::of(&QSpinBox::valueChanged), this, [this] { + emit changed(); + }); + + static_cast(root->layout())->setStretch(0, 3); + static_cast(root->layout())->setStretch(1, 1); + return root; +} + +AbstractLegacyControllerSetting* LegacyControllerIntegerSetting::createFrom( + const QDomElement& element) { + int defaultValue, minValue, maxValue, stepValue; + + bool isOk = false; + minValue = element.attribute("min").toInt(&isOk); + if (!isOk) { + minValue = std::numeric_limits::min(); + } + maxValue = element.attribute("max").toInt(&isOk); + if (!isOk) { + maxValue = std::numeric_limits::max(); + } + stepValue = element.attribute("step").toInt(&isOk); + if (!isOk) { + stepValue = 1; + } + defaultValue = element.attribute("default").toInt(&isOk); + if (!isOk) { + defaultValue = 0; + } + + return new LegacyControllerIntegerSetting( + element, defaultValue, defaultValue, minValue, maxValue, stepValue); +} + +bool LegacyControllerIntegerSetting::match(const QDomElement& element) { + return element.hasAttribute("type") && + QString::compare(element.attribute("type"), + "integer", + Qt::CaseInsensitive) == 0; +}; + +REGISTER_LEGACY_CONTROLLER_SETTING(LegacyControllerIntegerSetting); diff --git a/src/controllers/legacycontrollersettings.h b/src/controllers/legacycontrollersettings.h new file mode 100644 index 000000000000..3c76939c1426 --- /dev/null +++ b/src/controllers/legacycontrollersettings.h @@ -0,0 +1,117 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "controllers/legacycontrollersettingsfactory.h" + +/// @brief The abstract controller setting. Any type of setting will have to +/// implement this base class +class AbstractLegacyControllerSetting : public QObject { + Q_OBJECT + public: + virtual ~AbstractLegacyControllerSetting() = default; + + /// @brief Build a widget that can be used to interact with this setting. It + /// shouldn't mutate the state of the setting. + /// @param parent The parent widget for which this widget is being created. + /// The parent widget will own the newly created widget + /// @return a new widget + virtual QWidget* buildWidget(QWidget* parent) = 0; + + // virtual void reset() const = 0; + + /// @brief Whether of not this setting definition is valid. Validity scope + /// includes things like default value within range' for example. + /// @return true if valid + virtual inline bool valid() const { + return !m_variableName.isEmpty(); + } + + /// @brief The variable name as perceived within the mapping definition. + /// @return a string + inline QString variableName() const { + return m_variableName; + } + + /// @brief The user-friendly label to be display in the UI + /// @return a string + inline const QString& label() const { + return m_label; + } + + /// @brief A description of what this setting does + /// @return a string + inline const QString& description() const { + return m_description; + } + + protected: + AbstractLegacyControllerSetting(QString variableName, QString label, QString description) + : m_variableName(variableName), m_label(label), m_description(description) { + } + AbstractLegacyControllerSetting(const QDomElement& element) { + m_variableName = element.attribute("variable").trimmed(); + m_label = element.attribute("label", m_variableName).trimmed(); + + QDomElement description = element.firstChildElement("description"); + if (description.isNull()) { + m_description = QString(); + return; + } + m_description = description.text().trimmed(); + } + + signals: + /// This signal will be emitted when the user has interacted with the + /// setting and changed its value + void changed(); + + private: + QString m_variableName; + QString m_label; + QString m_description; +}; + +class LegacyControllerIntegerSetting + : public LegacyControllerSettingFactory, + public AbstractLegacyControllerSetting { + public: + LegacyControllerIntegerSetting(const QDomElement& element, + int currentValue, + int defaultValue, + int minValue, + int maxValue, + int stepValue) + : AbstractLegacyControllerSetting(element), + m_currentValue(currentValue), + m_defaultValue(defaultValue), + m_minValue(minValue), + m_maxValue(maxValue), + m_stepValue(stepValue) { + } + + virtual ~LegacyControllerIntegerSetting() = default; + + QWidget* buildWidget(QWidget* parent); + + static AbstractLegacyControllerSetting* createFrom(const QDomElement& element); + static bool match(const QDomElement& element); + + private: + int m_currentValue; + int m_defaultValue; + int m_minValue; + int m_maxValue; + int m_stepValue; +}; diff --git a/src/controllers/legacycontrollersettingsfactory.h b/src/controllers/legacycontrollersettingsfactory.h new file mode 100644 index 000000000000..3b2a0b9ecbce --- /dev/null +++ b/src/controllers/legacycontrollersettingsfactory.h @@ -0,0 +1,77 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "controllers/legacycontrollersettingsfactory.h" + +class AbstractLegacyControllerSetting; + +/// @brief This class defines an interface that a controller setting type must +/// implement so it can be used properly by the builder +/// @tparam T The class implementing this interface +template +class LegacyControllerSettingFactory { + inline static LegacyControllerSettingFactory* createFrom(const QDomElement& element) { + return T::createFrom(element); + } + inline static bool match(const QDomElement& element) { + return T::match(element); + } +}; + +/// @brief This class is used to dynamically instantiate a controller setting based on its type +class LegacyControllerSettingBuilder { + public: + static LegacyControllerSettingBuilder* instance(); + + /// @brief Register a new type of setting. This method is used by the + /// REGISTER macro, it shouldn't be used directly + /// @param match the match function of the new setting + /// @param creator the creator function of the new setting + /// @return Always true + inline bool registerType(bool (*match)(const QDomElement&), + AbstractLegacyControllerSetting* (*creator)(const QDomElement&)) { + m_supportedSettings.append(std::make_tuple(match, creator)); + return true; + } + + /// @brief instantiate a new setting from a an XML definition if any valid + /// setting was found. The caller is the owner of the instance + /// @param element The XML element to parse to build the new setting + /// @return an instance if a a supported setting has been found, null + /// otherwise + static AbstractLegacyControllerSetting* build(const QDomElement& element) { + foreach (auto settingType, instance()->m_supportedSettings) { + if (std::get<0>(settingType)(element)) { + return std::get<1>(settingType)(element); + } + } + + return nullptr; + } + + private: + LegacyControllerSettingBuilder() = default; + + QList> + m_supportedSettings; + + static LegacyControllerSettingBuilder* __self; +}; + +#define REGISTER_LEGACY_CONTROLLER_SETTING(S) \ + bool kSettingRegistered_##T = \ + LegacyControllerSettingBuilder::instance()->registerType( \ + S::match, S::createFrom) From 2c6f071933f5e858cf52f192b022c548daaa72bd Mon Sep 17 00:00:00 2001 From: Antoine C Date: Fri, 24 Feb 2023 11:20:20 +0000 Subject: [PATCH 02/17] feat(Controller): basic concept of setting injection in JS engine --- src/controllers/controller.cpp | 6 ++++++ src/controllers/legacycontrollersettings.h | 7 +++++++ .../scripting/legacy/controllerscriptenginelegacy.cpp | 9 +++++++++ .../scripting/legacy/controllerscriptenginelegacy.h | 5 +++++ 4 files changed, 27 insertions(+) diff --git a/src/controllers/controller.cpp b/src/controllers/controller.cpp index c9cb45c7e5eb..63b0c67e626a 100644 --- a/src/controllers/controller.cpp +++ b/src/controllers/controller.cpp @@ -76,6 +76,12 @@ bool Controller::applyMapping() { } m_pScriptEngineLegacy->setScriptFiles(scriptFiles); + + const QList>& settings = + pMapping->getSettings(); + if (!settings.isEmpty()) { + m_pScriptEngineLegacy->setSettings(settings); + } return m_pScriptEngineLegacy->initialize(); } diff --git a/src/controllers/legacycontrollersettings.h b/src/controllers/legacycontrollersettings.h index 3c76939c1426..5c9a56ed4110 100644 --- a/src/controllers/legacycontrollersettings.h +++ b/src/controllers/legacycontrollersettings.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include #include @@ -29,6 +30,8 @@ class AbstractLegacyControllerSetting : public QObject { /// @return a new widget virtual QWidget* buildWidget(QWidget* parent) = 0; + virtual QJSValue value() const = 0; + // virtual void reset() const = 0; /// @brief Whether of not this setting definition is valid. Validity scope @@ -105,6 +108,10 @@ class LegacyControllerIntegerSetting QWidget* buildWidget(QWidget* parent); + inline QJSValue value() const { + return QJSValue(m_currentValue); + } + static AbstractLegacyControllerSetting* createFrom(const QDomElement& element); static bool match(const QDomElement& element); diff --git a/src/controllers/scripting/legacy/controllerscriptenginelegacy.cpp b/src/controllers/scripting/legacy/controllerscriptenginelegacy.cpp index a5886204ec61..f880b1694622 100644 --- a/src/controllers/scripting/legacy/controllerscriptenginelegacy.cpp +++ b/src/controllers/scripting/legacy/controllerscriptenginelegacy.cpp @@ -123,6 +123,15 @@ bool ControllerScriptEngineLegacy::initialize() { QJSValue engineGlobalObject = m_pJSEngine->globalObject(); ControllerScriptInterfaceLegacy* legacyScriptInterface = new ControllerScriptInterfaceLegacy(this, m_logger); + + // TODO(acolombier): move into a dedicated protected method + QJSValue settingsObject = m_pJSEngine->newObject(); + foreach (const std::shared_ptr& setting, m_settings) { + settingsObject.setProperty(setting->variableName(), setting->value()); + } + engineGlobalObject.setProperty( + "mixxxControllerSettings", settingsObject); + engineGlobalObject.setProperty( "engine", m_pJSEngine->newQObject(legacyScriptInterface)); diff --git a/src/controllers/scripting/legacy/controllerscriptenginelegacy.h b/src/controllers/scripting/legacy/controllerscriptenginelegacy.h index 70dea6a392e0..b71c366cbd33 100644 --- a/src/controllers/scripting/legacy/controllerscriptenginelegacy.h +++ b/src/controllers/scripting/legacy/controllerscriptenginelegacy.h @@ -28,6 +28,10 @@ class ControllerScriptEngineLegacy : public ControllerScriptEngineBase { public slots: void setScriptFiles(const QList& scripts); + inline void setSettings( + const QList>& settings) { + m_settings = settings; + } private: bool evaluateScriptFile(const QFileInfo& scriptFile); @@ -44,6 +48,7 @@ class ControllerScriptEngineLegacy : public ControllerScriptEngineBase { QList m_incomingDataFunctions; QHash m_scriptWrappedFunctionCache; QList m_scriptFiles; + QList> m_settings; QFileSystemWatcher m_fileWatcher; From 69a57d42a8012fa4ce1456a2431cdf28328d6d64 Mon Sep 17 00:00:00 2001 From: Antoine C Date: Sat, 25 Feb 2023 00:04:18 +0000 Subject: [PATCH 03/17] feat(Controller): settings persistence and HID support --- CMakeLists.txt | 2 + src/controllers/controller.cpp | 6 +- src/controllers/controllermanager.cpp | 1 + src/controllers/dlgprefcontroller.cpp | 34 ++- src/controllers/legacycontrollermapping.cpp | 47 ++++ src/controllers/legacycontrollermapping.h | 14 +- .../legacycontrollermappingfilehandler.cpp | 10 +- src/controllers/legacycontrollersettings.cpp | 258 ++++++++++++++++-- src/controllers/legacycontrollersettings.h | 250 +++++++++++++++-- .../legacycontrollersettingsfactory.h | 12 +- .../legacy/controllerscriptenginelegacy.cpp | 5 +- .../legacy/controllerscriptenginelegacy.h | 8 +- 12 files changed, 571 insertions(+), 76 deletions(-) create mode 100644 src/controllers/legacycontrollermapping.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 5e7e40f500b0..104ae222d318 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -736,6 +736,8 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/controllers/controllermappingtablemodel.cpp src/controllers/controlleroutputmappingtablemodel.cpp src/controllers/controlpickermenu.cpp + src/controllers/legacycontrollermappingfilehandler.cpp + src/controllers/legacycontrollermapping.cpp src/controllers/delegates/controldelegate.cpp src/controllers/delegates/midibytedelegate.cpp src/controllers/delegates/midichanneldelegate.cpp diff --git a/src/controllers/controller.cpp b/src/controllers/controller.cpp index 63b0c67e626a..e37933858d18 100644 --- a/src/controllers/controller.cpp +++ b/src/controllers/controller.cpp @@ -77,11 +77,7 @@ bool Controller::applyMapping() { m_pScriptEngineLegacy->setScriptFiles(scriptFiles); - const QList>& settings = - pMapping->getSettings(); - if (!settings.isEmpty()) { - m_pScriptEngineLegacy->setSettings(settings); - } + m_pScriptEngineLegacy->setSettings(pMapping->getSettings()); return m_pScriptEngineLegacy->initialize(); } diff --git a/src/controllers/controllermanager.cpp b/src/controllers/controllermanager.cpp index 52501776ffb7..a2562ed94842 100644 --- a/src/controllers/controllermanager.cpp +++ b/src/controllers/controllermanager.cpp @@ -282,6 +282,7 @@ void ControllerManager::slotSetUpDevices() { if (!pMapping) { continue; } + pMapping->restoreSettings(mappingFile, m_pConfig, pController->getName()); // This runs on the main thread but LegacyControllerMapping is not thread safe, so clone it. pController->setMapping(pMapping->clone()); diff --git a/src/controllers/dlgprefcontroller.cpp b/src/controllers/dlgprefcontroller.cpp index 70e96200ce30..28a809fa5f33 100644 --- a/src/controllers/dlgprefcontroller.cpp +++ b/src/controllers/dlgprefcontroller.cpp @@ -22,6 +22,7 @@ #include "preferences/usersettings.h" #include "util/desktophelper.h" #include "util/string.h" +#include "util/parented_ptr.h" namespace { const QString kMappingExt(".midi.xml"); @@ -533,7 +534,7 @@ void DlgPrefController::slotApply() { applyMappingChanges(); // If no changes were made, do nothing - if (!(isDirty() || (m_pMapping && m_pMapping->isDirty()))) { + if (!(isDirty() || (m_pMapping && (m_pMapping->isDirty())))) { return; } @@ -558,8 +559,18 @@ void DlgPrefController::slotApply() { } QString mappingPath = mappingPathFromIndex(m_ui.comboBoxMapping->currentIndex()); + QFileInfo mappingFileInfo(mappingPath); + + m_pMapping->saveSettings(mappingFileInfo, m_pConfig, m_pController->getName()); + m_pMapping = LegacyControllerMappingFileHandler::loadMapping( - QFileInfo(mappingPath), QDir(resourceMappingsPath(m_pConfig))); + mappingFileInfo, QDir(resourceMappingsPath(m_pConfig))); + + if (m_pMapping) { + m_pMapping->restoreSettings(mappingFileInfo, m_pConfig, m_pController->getName()); + } + + slotShowMapping(m_pMapping); // Load the resulting mapping (which has been mutated by the input/output // table models). The controller clones the mapping so we aren't touching @@ -645,12 +656,14 @@ void DlgPrefController::slotMappingSelected(int chosenIndex) { } } + QFileInfo mappingFileInfo(mappingPath); std::shared_ptr pMapping = LegacyControllerMappingFileHandler::loadMapping( - QFileInfo(mappingPath), QDir(resourceMappingsPath(m_pConfig))); + mappingFileInfo, QDir(resourceMappingsPath(m_pConfig))); if (pMapping) { DEBUG_ASSERT(!pMapping->isDirty()); + pMapping->restoreSettings(mappingFileInfo, m_pConfig, m_pController->getName()); } if (previousMappingSaved) { @@ -826,16 +839,17 @@ void DlgPrefController::slotShowMapping(std::shared_ptr m_ui.labelLoadedMappingScriptFileLinks->setText(mappingFileLinks(pMapping)); if (pMapping) { - const QList>& - settings = pMapping->getSettings(); + auto settings = pMapping->getSettings(); qDeleteAll(m_ui.groupBoxSettings->findChildren("", Qt::FindDirectChildrenOnly)); - foreach (std::shared_ptr setting, settings) { - QWidget* settingWidget = setting->buildWidget(this); - // connect(settingWidget, &AbstractLegacyControllerSetting::changed, - // this, [this] { setDirty(true); }); - m_ui.groupBoxSettings->layout()->addWidget(settingWidget); + for (auto setting : settings) { + parented_ptr pSettingWidget(setting->buildWidget(m_ui.groupBoxSettings)); + connect(setting.get(), + &AbstractLegacyControllerSetting::changed, + this, + [this] { setDirty(true); }); + m_ui.groupBoxSettings->layout()->addWidget(pSettingWidget); } m_ui.groupBoxSettings->setVisible(!settings.isEmpty()); diff --git a/src/controllers/legacycontrollermapping.cpp b/src/controllers/legacycontrollermapping.cpp new file mode 100644 index 000000000000..7bfb8ce7595c --- /dev/null +++ b/src/controllers/legacycontrollermapping.cpp @@ -0,0 +1,47 @@ +#include "controllers/legacycontrollermapping.h" + +void LegacyControllerMapping::restoreSettings(const QFileInfo& mappingFile, + UserSettingsPointer pConfig, + const QString& controllerName) { + QString controllerKey = QString(CONTROLLER_SETTINGS_PREFERENCE_GROUP_KEY) + .arg(controllerName) + .arg(mappingFile.absoluteFilePath()); + for (auto setting : getSettings()) { + bool ok; + QString value = pConfig->getValueString(ConfigKey(controllerKey, setting->variableName())); + setting->parse(value, &ok); + if (!ok) { + qWarning() << "The setting" << setting->variableName() + << "for the mapping" << mappingFile.absoluteFilePath() + << "could not be restore. Removing"; + pConfig->remove(ConfigKey(controllerKey, setting->variableName())); + } + } + // TODO (acolombier): If there are other settings that aren't defined + // anymore, they should be removed. +} + +void LegacyControllerMapping::saveSettings(const QFileInfo& mappingFile, + UserSettingsPointer pConfig, + const QString& controllerName) { + QString controllerKey = QString(CONTROLLER_SETTINGS_PREFERENCE_GROUP_KEY) + .arg(controllerName) + .arg(mappingFile.absoluteFilePath()); + for (auto setting : getSettings()) { + if (!setting->isDirty()) { + continue; + } + setting->save(); + if (!setting->valid()) { + qWarning() << "Setting" << setting->variableName() + << "for controller" << controllerName + << "is invalid. Its value will not be saved."; + continue; + } + if (setting->isDefault()) { + pConfig->remove(ConfigKey(controllerKey, setting->variableName())); + } else { + pConfig->set(ConfigKey(controllerKey, setting->variableName()), setting->stringify()); + } + } +} diff --git a/src/controllers/legacycontrollermapping.h b/src/controllers/legacycontrollermapping.h index 21080bee88b5..158d70b04dba 100644 --- a/src/controllers/legacycontrollermapping.h +++ b/src/controllers/legacycontrollermapping.h @@ -11,16 +11,21 @@ #include #include #include +#include #include "controllers/legacycontrollersettings.h" #include "defs_urls.h" +#include "preferences/usersettings.h" + +// TODO (acolombier) is it okay to keep it as it? Or shall we generate a UUID from that pair? +#define CONTROLLER_SETTINGS_PREFERENCE_GROUP_KEY "[ControllerSettings_%1_%2]" /// This class represents a controller mapping, containing the data elements that /// make it up. class LegacyControllerMapping { public: LegacyControllerMapping() - : m_bDirty(false) { + : m_bDirty(false), m_settings() { } virtual ~LegacyControllerMapping() = default; @@ -198,6 +203,13 @@ class LegacyControllerMapping { virtual bool isMappable() const = 0; + void restoreSettings(const QFileInfo& mappingFile, + UserSettingsPointer pConfig, + const QString& controllerName); + void saveSettings(const QFileInfo& mappingFile, + UserSettingsPointer pConfig, + const QString& controllerName); + // Optional list of controller device match details QList> m_productMatches; diff --git a/src/controllers/legacycontrollermappingfilehandler.cpp b/src/controllers/legacycontrollermappingfilehandler.cpp index fd821ce639d8..9a8509bdcd3d 100644 --- a/src/controllers/legacycontrollermappingfilehandler.cpp +++ b/src/controllers/legacycontrollermappingfilehandler.cpp @@ -122,18 +122,18 @@ void LegacyControllerMappingFileHandler::parseMappingSettings( for (QDomElement option = settings.firstChildElement("option"); !option.isNull(); option = option.nextSiblingElement("option")) { - AbstractLegacyControllerSetting* setting = LegacyControllerSettingBuilder::build(option); - if (setting == nullptr) { + std::shared_ptr pSetting( + LegacyControllerSettingBuilder::build(option)); + if (pSetting.get() == nullptr) { qDebug() << "Could not parse the unknown controller setting. Ignoring it."; continue; } - if (!setting->valid()) { + if (!pSetting->valid()) { qDebug() << "The parsed setting appears to be invalid. Discarding it."; - delete setting; continue; } - mapping->addSetting(std::shared_ptr(setting)); + mapping->addSetting(pSetting); } } diff --git a/src/controllers/legacycontrollersettings.cpp b/src/controllers/legacycontrollersettings.cpp index 54e059f87bba..76cf1013f42d 100644 --- a/src/controllers/legacycontrollersettings.cpp +++ b/src/controllers/legacycontrollersettings.cpp @@ -1,6 +1,10 @@ #include "controllers/legacycontrollersettings.h" #include "moc_legacycontrollersettings.cpp" +#include + +#include +#include LegacyControllerSettingBuilder* LegacyControllerSettingBuilder::__self = nullptr; @@ -11,7 +15,7 @@ LegacyControllerSettingBuilder* LegacyControllerSettingBuilder::instance() { return __self; } -QWidget* LegacyControllerIntegerSetting::buildWidget(QWidget* parent) { +QWidget* AbstractLegacyControllerSetting::buildWidget(QWidget* parent) { QWidget* root = new QWidget(parent); root->setLayout(new QHBoxLayout()); root->layout()->setContentsMargins(0, 0, 0, 0); @@ -40,54 +44,254 @@ QWidget* LegacyControllerIntegerSetting::buildWidget(QWidget* parent) { root->layout()->addWidget(labelWidget); } - QSpinBox* spinBox = new QSpinBox(root); - spinBox->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Fixed); - root->layout()->addWidget(spinBox); + root->layout()->addWidget(buildInputWidget(root)); - spinBox->setRange(this->m_minValue, this->m_maxValue); - spinBox->setSingleStep(this->m_stepValue); - spinBox->setValue(this->m_currentValue); + static_cast(root->layout())->setStretch(0, 3); + static_cast(root->layout())->setStretch(1, 1); + return root; +} + +LegacyControllerBooleanSetting::LegacyControllerBooleanSetting( + const QDomElement& element) + : AbstractLegacyControllerSetting(element) { + m_defaultValue = parseValue(element.attribute("default")); + m_currentValue = m_defaultValue; + m_dirtyValue = m_defaultValue; +} - connect(spinBox, QOverload::of(&QSpinBox::valueChanged), this, [this] { +QWidget* LegacyControllerBooleanSetting::buildWidget(QWidget* parent) { + QWidget* root = new QWidget(parent); + root->setLayout(new QHBoxLayout()); + root->layout()->setContentsMargins(0, 0, 0, 0); + + QLabel* labelWidget = new QLabel(root); + labelWidget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum); + labelWidget->setText(label()); + + root->layout()->addWidget(buildInputWidget(root)); + + if (!description().isEmpty()) { + auto layout = new QVBoxLayout(); + + layout->setContentsMargins(0, 0, 0, 0); + + auto descriptionWidget = new QLabel(root); + descriptionWidget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum); + descriptionWidget->setText(description()); + QFont descriptionFont; + descriptionFont.setPointSize(8); + descriptionFont.setItalic(true); + descriptionWidget->setFont(descriptionFont); + + layout->addWidget(labelWidget); + layout->addWidget(descriptionWidget); + + root->layout()->addItem(layout); + } else { + root->layout()->addWidget(labelWidget); + } + + return root; +} + +QWidget* LegacyControllerBooleanSetting::buildInputWidget(QWidget* parent) { + QCheckBox* checkBox = new QCheckBox("", parent); + checkBox->setSizePolicy(QSizePolicy::Maximum, QSizePolicy::Fixed); + if (m_currentValue) { + checkBox->setCheckState(Qt::Checked); + } + + connect(checkBox, &QCheckBox::stateChanged, this, [this](int state) { + m_dirtyValue = state == Qt::Checked; emit changed(); }); - static_cast(root->layout())->setStretch(0, 3); - static_cast(root->layout())->setStretch(1, 1); - return root; + return checkBox; } -AbstractLegacyControllerSetting* LegacyControllerIntegerSetting::createFrom( - const QDomElement& element) { - int defaultValue, minValue, maxValue, stepValue; +bool LegacyControllerBooleanSetting::match(const QDomElement& element) { + // TODO(acolombier) improve the function so it can detect the type from the + // spec if there is no type attribute + return element.hasAttribute("type") && + QString::compare(element.attribute("type"), + "boolean", + Qt::CaseInsensitive) == 0; +} + +REGISTER_LEGACY_CONTROLLER_SETTING(LegacyControllerBooleanSetting); +template ValueSerializer, + Deserializer ValueDeserializer, + class InputWidget> +LegacyControllerNumberSetting::LegacyControllerNumberSetting(const QDomElement& element) + : AbstractLegacyControllerSetting(element) { bool isOk = false; - minValue = element.attribute("min").toInt(&isOk); + m_minValue = ValueDeserializer(element.attribute("min"), &isOk); if (!isOk) { - minValue = std::numeric_limits::min(); + m_minValue = std::numeric_limits::min(); } - maxValue = element.attribute("max").toInt(&isOk); + m_maxValue = ValueDeserializer(element.attribute("max"), &isOk); if (!isOk) { - maxValue = std::numeric_limits::max(); + m_maxValue = std::numeric_limits::max(); } - stepValue = element.attribute("step").toInt(&isOk); + m_stepValue = ValueDeserializer(element.attribute("step"), &isOk); if (!isOk) { - stepValue = 1; + m_stepValue = 1; } - defaultValue = element.attribute("default").toInt(&isOk); + m_defaultValue = ValueDeserializer(element.attribute("default"), &isOk); if (!isOk) { - defaultValue = 0; + m_defaultValue = 0; } + reset(); +} + +template ValueSerializer, + Deserializer ValueDeserializer, + class InputWidget> +QWidget* LegacyControllerNumberSetting::buildInputWidget(QWidget* parent) { + InputWidget* spinBox = new InputWidget(parent); + spinBox->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Fixed); + + spinBox->setRange(this->m_minValue, this->m_maxValue); + spinBox->setSingleStep(this->m_stepValue); + spinBox->setValue(this->m_currentValue); + + connect(spinBox, + QOverload::of(&InputWidget::valueChanged), + this, + [this](SettingType value) { + m_dirtyValue = value; + emit changed(); + }); + + return spinBox; +} - return new LegacyControllerIntegerSetting( - element, defaultValue, defaultValue, minValue, maxValue, stepValue); +template ValueSerializer, + Deserializer ValueDeserializer, + class InputWidget> +bool LegacyControllerNumberSetting::match(const QDomElement& element) { + return matchSetting(element); } -bool LegacyControllerIntegerSetting::match(const QDomElement& element) { +template<> +bool matchSetting(const QDomElement& element) { + // TODO(acolombier) improve the function so it can detect the type from the + // spec if there is no type attribute return element.hasAttribute("type") && QString::compare(element.attribute("type"), "integer", Qt::CaseInsensitive) == 0; -}; +} + +REGISTER_LEGACY_CONTROLLER_SETTING(LegacyControllerNumberSetting); + +LegacyControllerRealSetting::LegacyControllerRealSetting(const QDomElement& element) + : LegacyControllerNumberSetting(element) { + bool isOk = false; + m_precisionValue = element.attribute("precision").toInt(&isOk); + if (!isOk) { + m_precisionValue = 2; + } +} + +QWidget* LegacyControllerRealSetting::buildInputWidget(QWidget* parent) { + QDoubleSpinBox* spinBox = dynamic_cast( + LegacyControllerNumberSetting::buildInputWidget(parent)); + VERIFY_OR_DEBUG_ASSERT(spinBox != nullptr) { + qWarning() << "Unable to set precision on the controller setting " + "input: tt does not appear to be a valid QDoubleSpinBox"; + return spinBox; + } + spinBox->setDecimals(m_precisionValue); + + return spinBox; +} + +template<> +bool matchSetting(const QDomElement& element) { + // TODO(acolombier) improve the function so it can detect the type from the + // spec if there is no type attribute + return element.hasAttribute("type") && + QString::compare(element.attribute("type"), + "real", + Qt::CaseInsensitive) == 0; +} + +REGISTER_LEGACY_CONTROLLER_SETTING(LegacyControllerRealSetting); + +LegacyControllerEnumSetting::LegacyControllerEnumSetting( + const QDomElement& element) + : AbstractLegacyControllerSetting(element), m_options() { + for (QDomElement value = element.firstChildElement("value"); + !value.isNull(); + value = value.nextSiblingElement("value")) { + QString val = value.text(); + m_options.append(std::tuple(val, value.attribute("label", val))); + } + m_defaultValue = extractSettingIntegerValue(element.attribute("default")); + reset(); +} + +void LegacyControllerEnumSetting::parse(const QString& in, bool* ok) { + if (ok != nullptr) + *ok = false; + reset(); + + size_t pos = 0; + for (const auto& value : m_options) { + if (std::get<0>(value) == in) { + if (ok != nullptr) + *ok = true; + m_currentValue = pos; + m_dirtyValue = m_currentValue; + return; + } + pos++; + } +} + +QWidget* LegacyControllerEnumSetting::buildInputWidget(QWidget* parent) { + QComboBox* comboBox = new QComboBox(parent); + + for (const auto& value : m_options) { + comboBox->addItem(std::get<1>(value)); + } + comboBox->setCurrentIndex(m_currentValue); + + connect(comboBox, + QOverload::of(&QComboBox::currentIndexChanged), + this, + [this](int selected) { + m_dirtyValue = selected; + emit changed(); + }); + + return comboBox; +} + +bool LegacyControllerEnumSetting::match(const QDomElement& element) { + // TODO(acolombier) improve the function so it can detect the type from the + // spec if there is no type attribute + return element.hasAttribute("type") && + QString::compare(element.attribute("type"), + "enum", + Qt::CaseInsensitive) == 0; +} -REGISTER_LEGACY_CONTROLLER_SETTING(LegacyControllerIntegerSetting); +REGISTER_LEGACY_CONTROLLER_SETTING(LegacyControllerEnumSetting); diff --git a/src/controllers/legacycontrollersettings.h b/src/controllers/legacycontrollersettings.h index 5c9a56ed4110..f1a08a0c6e24 100644 --- a/src/controllers/legacycontrollersettings.h +++ b/src/controllers/legacycontrollersettings.h @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -28,10 +29,18 @@ class AbstractLegacyControllerSetting : public QObject { /// @param parent The parent widget for which this widget is being created. /// The parent widget will own the newly created widget /// @return a new widget - virtual QWidget* buildWidget(QWidget* parent) = 0; + virtual QWidget* buildWidget(QWidget* parent); virtual QJSValue value() const = 0; + virtual QString stringify() const = 0; + virtual void parse(const QString&, bool*) = 0; + virtual bool isDefault() const = 0; + virtual bool isDirty() const = 0; + + virtual void save() = 0; + virtual void reset() = 0; + // virtual void reset() const = 0; /// @brief Whether of not this setting definition is valid. Validity scope @@ -75,6 +84,8 @@ class AbstractLegacyControllerSetting : public QObject { m_description = description.text().trimmed(); } + virtual QWidget* buildInputWidget(QWidget* parent) = 0; + signals: /// This signal will be emitted when the user has interacted with the /// setting and changed its value @@ -86,16 +97,133 @@ class AbstractLegacyControllerSetting : public QObject { QString m_description; }; -class LegacyControllerIntegerSetting - : public LegacyControllerSettingFactory, +class LegacyControllerBooleanSetting + : public LegacyControllerSettingFactory, + public AbstractLegacyControllerSetting { + public: + LegacyControllerBooleanSetting(const QDomElement& element); + + virtual ~LegacyControllerBooleanSetting() = default; + + QWidget* buildWidget(QWidget* parent) override; + + inline QJSValue value() const { + return QJSValue(m_currentValue); + } + + inline QString stringify() const { + return m_currentValue ? "true" : "false"; + } + inline void parse(const QString& in, bool* ok = nullptr) { + if (ok != nullptr) + *ok = true; + m_currentValue = parseValue(in); + m_dirtyValue = m_currentValue; + } + inline bool isDefault() const { + return m_currentValue == m_defaultValue; + } + inline bool isDirty() const { + return m_currentValue != m_dirtyValue; + } + + virtual void save() { + m_currentValue = m_dirtyValue; + } + + virtual void reset() { + m_currentValue = m_defaultValue; + m_dirtyValue = m_defaultValue; + } + + static inline AbstractLegacyControllerSetting* createFrom(const QDomElement& element) { + return new LegacyControllerBooleanSetting(element); + } + static bool match(const QDomElement& element); + + protected: + LegacyControllerBooleanSetting(const QDomElement& element, + bool currentValue, + bool defaultValue) + : AbstractLegacyControllerSetting(element), + m_currentValue(currentValue), + m_defaultValue(defaultValue) { + } + + inline bool parseValue(const QString& in) { + return QString::compare(in, "true", Qt::CaseInsensitive) == 0 || in == "1"; + } + + virtual QWidget* buildInputWidget(QWidget* parent); + + private: + bool m_currentValue; + bool m_defaultValue; + bool m_dirtyValue; +}; + +template +using Serializer = QString (*)(const SettingType&); + +template +using Deserializer = SettingType (*)(const QString&, bool*); + +template ValueSerializer, + Deserializer ValueDeserializer, + class InputWidget> +class LegacyControllerNumberSetting + : public LegacyControllerSettingFactory< + LegacyControllerNumberSetting>, public AbstractLegacyControllerSetting { public: - LegacyControllerIntegerSetting(const QDomElement& element, - int currentValue, - int defaultValue, - int minValue, - int maxValue, - int stepValue) + LegacyControllerNumberSetting(const QDomElement& element); + + virtual ~LegacyControllerNumberSetting() = default; + + inline QJSValue value() const { + return QJSValue(m_currentValue); + } + + inline QString stringify() const { + return ValueSerializer(m_currentValue); + } + inline void parse(const QString& in, bool* ok) { + m_currentValue = ValueDeserializer(in, ok); + m_dirtyValue = m_currentValue; + } + + inline bool isDefault() const { + return m_currentValue == m_defaultValue; + } + inline bool isDirty() const { + return m_currentValue != m_dirtyValue; + } + + virtual void save() { + m_currentValue = m_dirtyValue; + } + + virtual void reset() { + m_currentValue = m_defaultValue; + m_dirtyValue = m_defaultValue; + } + + static inline AbstractLegacyControllerSetting* createFrom(const QDomElement& element) { + return new LegacyControllerNumberSetting(element); + } + static bool match(const QDomElement& element); + + protected: + LegacyControllerNumberSetting(const QDomElement& element, + SettingType currentValue, + SettingType defaultValue, + SettingType minValue, + SettingType maxValue, + SettingType stepValue) : AbstractLegacyControllerSetting(element), m_currentValue(currentValue), m_defaultValue(defaultValue), @@ -104,21 +232,107 @@ class LegacyControllerIntegerSetting m_stepValue(stepValue) { } - virtual ~LegacyControllerIntegerSetting() = default; + virtual QWidget* buildInputWidget(QWidget* parent); + + private: + SettingType m_currentValue; + SettingType m_defaultValue; + SettingType m_minValue; + SettingType m_maxValue; + SettingType m_stepValue; + + SettingType m_dirtyValue; +}; + +template +bool matchSetting(const QDomElement& element); + +inline int extractSettingIntegerValue(const QString& str, bool* ok = nullptr) { + return str.toInt(ok); +} +inline double extractSettingDoubleValue(const QString& str, bool* ok = nullptr) { + return str.toDouble(ok); +} + +inline QString packSettingIntegerValue(const int& in) { + return QString::number(in); +} +inline QString packSettingDoubleValue(const double& in) { + return QString::number(in); +} + +class LegacyControllerRealSetting : public LegacyControllerNumberSetting { + public: + LegacyControllerRealSetting(const QDomElement& element); + + static inline AbstractLegacyControllerSetting* createFrom(const QDomElement& element) { + return new LegacyControllerRealSetting(element); + } + + QWidget* buildInputWidget(QWidget* parent) override; - QWidget* buildWidget(QWidget* parent); + private: + int m_precisionValue; +}; + +class LegacyControllerEnumSetting + : public LegacyControllerSettingFactory, + public AbstractLegacyControllerSetting { + public: + LegacyControllerEnumSetting(const QDomElement& element); + + virtual ~LegacyControllerEnumSetting() = default; inline QJSValue value() const { - return QJSValue(m_currentValue); + return QJSValue(stringify()); + } + + QString stringify() const { + return std::get<0>(m_options.value(m_currentValue)); + } + void parse(const QString& in, bool* ok); + bool isDefault() const { + return m_currentValue == m_defaultValue; + } + inline bool isDirty() const { + return m_currentValue != m_dirtyValue; } - static AbstractLegacyControllerSetting* createFrom(const QDomElement& element); + virtual void save() { + m_currentValue = m_dirtyValue; + } + + virtual void reset() { + m_currentValue = m_defaultValue; + m_dirtyValue = m_defaultValue; + } + + static inline AbstractLegacyControllerSetting* createFrom(const QDomElement& element) { + return new LegacyControllerEnumSetting(element); + } static bool match(const QDomElement& element); + protected: + LegacyControllerEnumSetting(const QDomElement& element, + QList> options, + size_t currentValue, + size_t defaultValue) + : AbstractLegacyControllerSetting(element), + m_options(options), + m_currentValue(currentValue), + m_defaultValue(defaultValue) { + } + + virtual QWidget* buildInputWidget(QWidget* parent); + private: - int m_currentValue; - int m_defaultValue; - int m_minValue; - int m_maxValue; - int m_stepValue; + // We use a QList instead of QHash here because we want to keep the natural order + QList> m_options; + size_t m_currentValue; + size_t m_defaultValue; + + size_t m_dirtyValue; }; diff --git a/src/controllers/legacycontrollersettingsfactory.h b/src/controllers/legacycontrollersettingsfactory.h index 3b2a0b9ecbce..151de97ac515 100644 --- a/src/controllers/legacycontrollersettingsfactory.h +++ b/src/controllers/legacycontrollersettingsfactory.h @@ -23,7 +23,7 @@ class AbstractLegacyControllerSetting; template class LegacyControllerSettingFactory { inline static LegacyControllerSettingFactory* createFrom(const QDomElement& element) { - return T::createFrom(element); + return new T(element); } inline static bool match(const QDomElement& element) { return T::match(element); @@ -52,7 +52,7 @@ class LegacyControllerSettingBuilder { /// @return an instance if a a supported setting has been found, null /// otherwise static AbstractLegacyControllerSetting* build(const QDomElement& element) { - foreach (auto settingType, instance()->m_supportedSettings) { + for (auto settingType : instance()->m_supportedSettings) { if (std::get<0>(settingType)(element)) { return std::get<1>(settingType)(element); } @@ -71,7 +71,9 @@ class LegacyControllerSettingBuilder { static LegacyControllerSettingBuilder* __self; }; -#define REGISTER_LEGACY_CONTROLLER_SETTING(S) \ - bool kSettingRegistered_##T = \ +#define CONCAT_(x, y) x##y +#define CONCAT(x, y) CONCAT_(x, y) +#define REGISTER_LEGACY_CONTROLLER_SETTING(...) \ + bool CONCAT(kSettingRegistered_, __COUNTER__) = \ LegacyControllerSettingBuilder::instance()->registerType( \ - S::match, S::createFrom) + __VA_ARGS__::match, __VA_ARGS__::createFrom) diff --git a/src/controllers/scripting/legacy/controllerscriptenginelegacy.cpp b/src/controllers/scripting/legacy/controllerscriptenginelegacy.cpp index f880b1694622..f760bbeabe3e 100644 --- a/src/controllers/scripting/legacy/controllerscriptenginelegacy.cpp +++ b/src/controllers/scripting/legacy/controllerscriptenginelegacy.cpp @@ -124,10 +124,9 @@ bool ControllerScriptEngineLegacy::initialize() { ControllerScriptInterfaceLegacy* legacyScriptInterface = new ControllerScriptInterfaceLegacy(this, m_logger); - // TODO(acolombier): move into a dedicated protected method QJSValue settingsObject = m_pJSEngine->newObject(); - foreach (const std::shared_ptr& setting, m_settings) { - settingsObject.setProperty(setting->variableName(), setting->value()); + for (auto setting : m_settings) { + settingsObject.setProperty(std::get<0>(setting), std::get<1>(setting)); } engineGlobalObject.setProperty( "mixxxControllerSettings", settingsObject); diff --git a/src/controllers/scripting/legacy/controllerscriptenginelegacy.h b/src/controllers/scripting/legacy/controllerscriptenginelegacy.h index b71c366cbd33..e0392a819bfd 100644 --- a/src/controllers/scripting/legacy/controllerscriptenginelegacy.h +++ b/src/controllers/scripting/legacy/controllerscriptenginelegacy.h @@ -30,7 +30,11 @@ class ControllerScriptEngineLegacy : public ControllerScriptEngineBase { void setScriptFiles(const QList& scripts); inline void setSettings( const QList>& settings) { - m_settings = settings; + m_settings.clear(); + for (auto pSetting : settings) { + m_settings.append(std::tuple( + pSetting->variableName(), pSetting->value())); + } } private: @@ -48,7 +52,7 @@ class ControllerScriptEngineLegacy : public ControllerScriptEngineBase { QList m_incomingDataFunctions; QHash m_scriptWrappedFunctionCache; QList m_scriptFiles; - QList> m_settings; + QList> m_settings; QFileSystemWatcher m_fileWatcher; From 0f01d3e7874d14e248bf902cf6ef1e490d623dd6 Mon Sep 17 00:00:00 2001 From: Antoine C Date: Sun, 26 Feb 2023 12:51:50 +0000 Subject: [PATCH 04/17] feat(Controller): UI improvement with custom layout support --- CMakeLists.txt | 2 + src/controllers/dlgprefcontroller.cpp | 22 +- src/controllers/legacycontrollermapping.cpp | 47 ++-- src/controllers/legacycontrollermapping.h | 39 +++- .../legacycontrollermappingfilehandler.cpp | 58 +++-- .../legacycontrollermappingfilehandler.h | 14 ++ src/controllers/legacycontrollersettings.cpp | 126 +++++------ src/controllers/legacycontrollersettings.h | 143 ++++++++---- .../legacycontrollersettingsfactory.h | 2 +- .../legacycontrollersettingslayout.cpp | 78 +++++++ .../legacycontrollersettingslayout.h | 108 +++++++++ ...legacymidicontrollermappingfilehandler.cpp | 1 + .../legacy/controllerscriptenginelegacy.cpp | 11 +- .../legacy/controllerscriptenginelegacy.h | 17 +- src/test/controller_mapping_settings_test.cpp | 214 ++++++++++++++++++ 15 files changed, 714 insertions(+), 168 deletions(-) create mode 100644 src/controllers/legacycontrollersettingslayout.cpp create mode 100644 src/controllers/legacycontrollersettingslayout.h create mode 100644 src/test/controller_mapping_settings_test.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 104ae222d318..70d4954448f1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -732,6 +732,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/controllers/controllermanager.cpp src/controllers/controllermappinginfo.cpp src/controllers/legacycontrollersettings.cpp + src/controllers/legacycontrollersettingslayout.cpp src/controllers/controllermappinginfoenumerator.cpp src/controllers/controllermappingtablemodel.cpp src/controllers/controlleroutputmappingtablemodel.cpp @@ -2073,6 +2074,7 @@ add_executable(mixxx-test src/test/colorpalette_test.cpp src/test/configobject_test.cpp src/test/controller_mapping_validation_test.cpp + src/test/controller_mapping_settings_test.cpp src/test/controllerscriptenginelegacy_test.cpp src/test/controlobjecttest.cpp src/test/controlobjectaliastest.cpp diff --git a/src/controllers/dlgprefcontroller.cpp b/src/controllers/dlgprefcontroller.cpp index 28a809fa5f33..8f385f97fcc6 100644 --- a/src/controllers/dlgprefcontroller.cpp +++ b/src/controllers/dlgprefcontroller.cpp @@ -559,9 +559,12 @@ void DlgPrefController::slotApply() { } QString mappingPath = mappingPathFromIndex(m_ui.comboBoxMapping->currentIndex()); + QFileInfo mappingFileInfo(mappingPath); - m_pMapping->saveSettings(mappingFileInfo, m_pConfig, m_pController->getName()); + if (m_pMapping) { + m_pMapping->saveSettings(mappingFileInfo, m_pConfig, m_pController->getName()); + } m_pMapping = LegacyControllerMappingFileHandler::loadMapping( mappingFileInfo, QDir(resourceMappingsPath(m_pConfig))); @@ -840,16 +843,19 @@ void DlgPrefController::slotShowMapping(std::shared_ptr if (pMapping) { auto settings = pMapping->getSettings(); + const auto& layout = pMapping->getSettingsLayout(); qDeleteAll(m_ui.groupBoxSettings->findChildren("", Qt::FindDirectChildrenOnly)); - for (auto setting : settings) { - parented_ptr pSettingWidget(setting->buildWidget(m_ui.groupBoxSettings)); - connect(setting.get(), - &AbstractLegacyControllerSetting::changed, - this, - [this] { setDirty(true); }); - m_ui.groupBoxSettings->layout()->addWidget(pSettingWidget); + if (layout.get() != nullptr && !settings.isEmpty()) { + m_ui.groupBoxSettings->layout()->addWidget(layout->build(m_ui.groupBoxSettings)); + + for (const auto& setting : qAsConst(settings)) { + connect(setting.get(), + &AbstractLegacyControllerSetting::changed, + this, + [this] { setDirty(true); }); + } } m_ui.groupBoxSettings->setVisible(!settings.isEmpty()); diff --git a/src/controllers/legacycontrollermapping.cpp b/src/controllers/legacycontrollermapping.cpp index 7bfb8ce7595c..1c6863e040e0 100644 --- a/src/controllers/legacycontrollermapping.cpp +++ b/src/controllers/legacycontrollermapping.cpp @@ -4,29 +4,48 @@ void LegacyControllerMapping::restoreSettings(const QFileInfo& mappingFile, UserSettingsPointer pConfig, const QString& controllerName) { QString controllerKey = QString(CONTROLLER_SETTINGS_PREFERENCE_GROUP_KEY) - .arg(controllerName) - .arg(mappingFile.absoluteFilePath()); - for (auto setting : getSettings()) { - bool ok; - QString value = pConfig->getValueString(ConfigKey(controllerKey, setting->variableName())); - setting->parse(value, &ok); - if (!ok) { - qWarning() << "The setting" << setting->variableName() + .arg(controllerName, mappingFile.absoluteFilePath()); + + auto availableSettings = getSettings(); + QList definedSettings = pConfig->getKeysWithGroup(controllerKey); + + QList availableSettingKeys; + for (const auto& pSetting : qAsConst(availableSettings)) { + availableSettingKeys.append(pSetting->variableName()); + } + + bool ok; + for (const auto& key : definedSettings) { + if (!availableSettingKeys.contains(key.item)) { + qDebug() << "The setting" << key.item + << "does not seem to exist in the mapping" << mappingFile.absoluteFilePath() + << ". It may be invalid or may have been removed."; + pConfig->remove(key); + continue; + } + auto& pSetting = availableSettings.at(availableSettingKeys.indexOf(key.item)); + QString value = pConfig->getValueString(key); + if (!pSetting->valid()) { + qWarning() << "The setting" << pSetting->variableName() << "for the mapping" << mappingFile.absoluteFilePath() - << "could not be restore. Removing"; - pConfig->remove(ConfigKey(controllerKey, setting->variableName())); + << "appears to be invalid. Its saved value won't be restored."; + } + pSetting->parse(value, &ok); + if (!ok || !pSetting->valid()) { + qWarning() << "The setting" << pSetting->variableName() + << "for the mapping" << mappingFile.absoluteFilePath() + << "could not be restore. Removing and resetting the setting default value."; + pConfig->remove(key); + pSetting->reset(); } } - // TODO (acolombier): If there are other settings that aren't defined - // anymore, they should be removed. } void LegacyControllerMapping::saveSettings(const QFileInfo& mappingFile, UserSettingsPointer pConfig, const QString& controllerName) { QString controllerKey = QString(CONTROLLER_SETTINGS_PREFERENCE_GROUP_KEY) - .arg(controllerName) - .arg(mappingFile.absoluteFilePath()); + .arg(controllerName, mappingFile.absoluteFilePath()); for (auto setting : getSettings()) { if (!setting->isDirty()) { continue; diff --git a/src/controllers/legacycontrollermapping.h b/src/controllers/legacycontrollermapping.h index 158d70b04dba..a0934281e7bd 100644 --- a/src/controllers/legacycontrollermapping.h +++ b/src/controllers/legacycontrollermapping.h @@ -14,6 +14,7 @@ #include #include "controllers/legacycontrollersettings.h" +#include "controllers/legacycontrollersettingslayout.h" #include "defs_urls.h" #include "preferences/usersettings.h" @@ -25,7 +26,26 @@ class LegacyControllerMapping { public: LegacyControllerMapping() - : m_bDirty(false), m_settings() { + : m_bDirty(false) { + } + LegacyControllerMapping(const LegacyControllerMapping& other) + : m_productMatches(other.m_productMatches), + m_bDirty(other.m_bDirty), + m_deviceId(other.m_deviceId), + m_filePath(other.m_filePath), + m_name(other.m_name), + m_author(other.m_author), + m_description(other.m_description), + m_forumlink(other.m_forumlink), + m_manualPage(other.m_manualPage), + m_wikilink(other.m_wikilink), + m_schemaVersion(other.m_schemaVersion), + m_mixxxVersion(other.m_mixxxVersion), + m_settings(other.m_settings), + m_settingsLayout(other.m_settingsLayout.get() != nullptr + ? other.m_settingsLayout->clone() + : nullptr), + m_scripts(other.m_scripts) { } virtual ~LegacyControllerMapping() = default; @@ -75,14 +95,28 @@ class LegacyControllerMapping { m_settings.append(option); } + /// @brief Set a setting layout as they should be perceived when edited in + /// the preference dialog. + /// @param layout The layout root element + void setSettingLayout(std::unique_ptr&& layout) { + VERIFY_OR_DEBUG_ASSERT(layout.get()) { + return; + } + m_settingsLayout = std::move(layout); + } + const QList& getScriptFiles() const { return m_scripts; } - const QList>& getSettings() const { + inline const QList>& getSettings() const { return m_settings; } + inline const std::unique_ptr& getSettingsLayout() const { + return m_settingsLayout; + } + inline void setDirty(bool bDirty) { m_bDirty = bDirty; } @@ -228,5 +262,6 @@ class LegacyControllerMapping { QString m_mixxxVersion; QList> m_settings; + std::unique_ptr m_settingsLayout; QList m_scripts; }; diff --git a/src/controllers/legacycontrollermappingfilehandler.cpp b/src/controllers/legacycontrollermappingfilehandler.cpp index 9a8509bdcd3d..a491149ae468 100644 --- a/src/controllers/legacycontrollermappingfilehandler.cpp +++ b/src/controllers/legacycontrollermappingfilehandler.cpp @@ -119,21 +119,53 @@ void LegacyControllerMappingFileHandler::parseMappingSettings( return; } - for (QDomElement option = settings.firstChildElement("option"); - !option.isNull(); - option = option.nextSiblingElement("option")) { - std::shared_ptr pSetting( - LegacyControllerSettingBuilder::build(option)); - if (pSetting.get() == nullptr) { - qDebug() << "Could not parse the unknown controller setting. Ignoring it."; - continue; - } - if (!pSetting->valid()) { - qDebug() << "The parsed setting appears to be invalid. Discarding it."; + std::unique_ptr settingLayout = + std::make_unique( + LegacyControllerSettingsLayoutContainer::Disposition:: + VERTICAL); + parseMappingSettingsElement(settings, mapping, settingLayout); + mapping->setSettingLayout(std::move(settingLayout)); +} + +void LegacyControllerMappingFileHandler::parseMappingSettingsElement( + const QDomElement& current, + std::shared_ptr mapping, + const std::unique_ptr& layout) + const { + // TODO (acolombier) Add test for the parser + for (QDomElement element = current.firstChildElement(); + !element.isNull(); + element = element.nextSiblingElement()) { + const QString& tagName = element.tagName(); + if (tagName == "option") { + std::shared_ptr pSetting( + LegacyControllerSettingBuilder::build(element)); + if (pSetting.get() == nullptr) { + qDebug() << "Could not parse the unknown controller setting. Ignoring it."; + continue; + } + if (!pSetting->valid()) { + qDebug() << "The parsed setting appears to be invalid. Discarding it."; + continue; + } + layout->addItem(pSetting); + mapping->addSetting(pSetting); + } else if (tagName == "row") { + std::unique_ptr row = + std::make_unique(); + parseMappingSettingsElement(element, mapping, row); + layout->addItem(std::move(row)); + } else if (tagName == "group") { + std::unique_ptr group = + std::make_unique( + element.attribute("label")); + parseMappingSettingsElement(element, mapping, group); + layout->addItem(std::move(group)); + } else { + qDebug() << "Unsupported tag" << tagName + << "for controller layout settings. Discarding it."; continue; } - - mapping->addSetting(pSetting); } } diff --git a/src/controllers/legacycontrollermappingfilehandler.h b/src/controllers/legacycontrollermappingfilehandler.h index ee23268ca74c..8daf2f21602a 100644 --- a/src/controllers/legacycontrollermappingfilehandler.h +++ b/src/controllers/legacycontrollermappingfilehandler.h @@ -41,9 +41,23 @@ class LegacyControllerMappingFileHandler { void parseMappingInfo(const QDomElement& root, std::shared_ptr mapping) const; + /// @brief Parse the setting definition block from the root node if any. + /// @param root The root node (MixxxControllerPreset) + /// @param mapping The mapping object to populate with the gathered data void parseMappingSettings(const QDomElement& root, std::shared_ptr mapping) const; + /// @brief Recursively parse setting definition and layout information + /// within a setting node + /// @param current The setting node (MixxxControllerPreset.settings) or any + /// children nodes + /// @param mapping The mapping object to populate with the gathered data + /// @param layout The currently active layout, on which new setting item + /// (leaf) should be attached + void parseMappingSettingsElement(const QDomElement& current, + std::shared_ptr mapping, + const std::unique_ptr& layout) const; + /// Adds script files from XML to the LegacyControllerMapping. /// /// This function parses the supplied QDomElement structure, finds the diff --git a/src/controllers/legacycontrollersettings.cpp b/src/controllers/legacycontrollersettings.cpp index 76cf1013f42d..1772e2538a86 100644 --- a/src/controllers/legacycontrollersettings.cpp +++ b/src/controllers/legacycontrollersettings.cpp @@ -6,49 +6,50 @@ #include #include +// TODO (acolombier): remove "new" where possible and use parented_ptr + LegacyControllerSettingBuilder* LegacyControllerSettingBuilder::__self = nullptr; LegacyControllerSettingBuilder* LegacyControllerSettingBuilder::instance() { - if (__self == nullptr) + if (__self == nullptr) { __self = new LegacyControllerSettingBuilder(); + } return __self; } +AbstractLegacyControllerSetting::AbstractLegacyControllerSetting(const QDomElement& element) { + m_variableName = element.attribute("variable").trimmed(); + m_label = element.attribute("label", m_variableName).trimmed(); + + QDomElement description = element.firstChildElement("description"); + if (!description.isNull()) { + m_description = description.text().trimmed(); + } +} + QWidget* AbstractLegacyControllerSetting::buildWidget(QWidget* parent) { - QWidget* root = new QWidget(parent); - root->setLayout(new QHBoxLayout()); - root->layout()->setContentsMargins(0, 0, 0, 0); + QWidget* pRoot = new QWidget(parent); + QBoxLayout* pLayout = new QBoxLayout(QBoxLayout::LeftToRight); + pLayout->setContentsMargins(0, 0, 0, 0); - QLabel* labelWidget = new QLabel(root); - labelWidget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum); - labelWidget->setText(label()); + QLabel* pLabelWidget = new QLabel(pRoot); + pLabelWidget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum); + pLabelWidget->setText(label()); if (!description().isEmpty()) { - auto layout = new QVBoxLayout(root); - - root->layout()->setContentsMargins(0, 0, 0, 0); - - auto descriptionWidget = new QLabel(root); - descriptionWidget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum); - descriptionWidget->setText(description()); - QFont descriptionFont; - descriptionFont.setPointSize(8); - descriptionFont.setItalic(true); - descriptionWidget->setFont(descriptionFont); - - layout->addWidget(labelWidget); - layout->addWidget(descriptionWidget); - root->layout()->addItem(layout); - } else { - root->layout()->addWidget(labelWidget); + pRoot->setToolTip(QString("

%1

").arg(description())); } - root->layout()->addWidget(buildInputWidget(root)); + pLayout->addWidget(pLabelWidget); + pLayout->addWidget(buildInputWidget(pRoot)); + + pLayout->setStretch(0, 3); + pLayout->setStretch(1, 1); - static_cast(root->layout())->setStretch(0, 3); - static_cast(root->layout())->setStretch(1, 1); - return root; + pRoot->setLayout(pLayout); + + return pRoot; } LegacyControllerBooleanSetting::LegacyControllerBooleanSetting( @@ -60,38 +61,24 @@ LegacyControllerBooleanSetting::LegacyControllerBooleanSetting( } QWidget* LegacyControllerBooleanSetting::buildWidget(QWidget* parent) { - QWidget* root = new QWidget(parent); - root->setLayout(new QHBoxLayout()); - root->layout()->setContentsMargins(0, 0, 0, 0); - - QLabel* labelWidget = new QLabel(root); - labelWidget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum); - labelWidget->setText(label()); + QWidget* pRoot = new QWidget(parent); + QBoxLayout* pLayout = new QBoxLayout(QBoxLayout::LeftToRight); + pLayout->setContentsMargins(0, 0, 0, 0); - root->layout()->addWidget(buildInputWidget(root)); + QLabel* pLabelWidget = new QLabel(pRoot); + pLabelWidget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum); + pLabelWidget->setText(label()); if (!description().isEmpty()) { - auto layout = new QVBoxLayout(); - - layout->setContentsMargins(0, 0, 0, 0); - - auto descriptionWidget = new QLabel(root); - descriptionWidget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum); - descriptionWidget->setText(description()); - QFont descriptionFont; - descriptionFont.setPointSize(8); - descriptionFont.setItalic(true); - descriptionWidget->setFont(descriptionFont); + pRoot->setToolTip(QString("

%1

").arg(description())); + } - layout->addWidget(labelWidget); - layout->addWidget(descriptionWidget); + pLayout->addWidget(buildInputWidget(pRoot)); + pLayout->addWidget(pLabelWidget); - root->layout()->addItem(layout); - } else { - root->layout()->addWidget(labelWidget); - } + pRoot->setLayout(pLayout); - return root; + return pRoot; } QWidget* LegacyControllerBooleanSetting::buildInputWidget(QWidget* parent) { @@ -110,8 +97,6 @@ QWidget* LegacyControllerBooleanSetting::buildInputWidget(QWidget* parent) { } bool LegacyControllerBooleanSetting::match(const QDomElement& element) { - // TODO(acolombier) improve the function so it can detect the type from the - // spec if there is no type attribute return element.hasAttribute("type") && QString::compare(element.attribute("type"), "boolean", @@ -188,18 +173,13 @@ bool LegacyControllerNumberSetting bool matchSetting(const QDomElement& element) { - // TODO(acolombier) improve the function so it can detect the type from the - // spec if there is no type attribute return element.hasAttribute("type") && QString::compare(element.attribute("type"), "integer", Qt::CaseInsensitive) == 0; } -REGISTER_LEGACY_CONTROLLER_SETTING(LegacyControllerNumberSetting); +REGISTER_LEGACY_CONTROLLER_SETTING(LegacyControllerIntegerSetting); LegacyControllerRealSetting::LegacyControllerRealSetting(const QDomElement& element) : LegacyControllerNumberSetting(element) { @@ -225,8 +205,6 @@ QWidget* LegacyControllerRealSetting::buildInputWidget(QWidget* parent) { template<> bool matchSetting(const QDomElement& element) { - // TODO(acolombier) improve the function so it can detect the type from the - // spec if there is no type attribute return element.hasAttribute("type") && QString::compare(element.attribute("type"), "real", @@ -237,27 +215,33 @@ REGISTER_LEGACY_CONTROLLER_SETTING(LegacyControllerRealSetting); LegacyControllerEnumSetting::LegacyControllerEnumSetting( const QDomElement& element) - : AbstractLegacyControllerSetting(element), m_options() { + : AbstractLegacyControllerSetting(element), m_options(), m_defaultValue(0) { + size_t pos = 0; for (QDomElement value = element.firstChildElement("value"); !value.isNull(); value = value.nextSiblingElement("value")) { QString val = value.text(); m_options.append(std::tuple(val, value.attribute("label", val))); + if (value.hasAttribute("default")) { + m_defaultValue = pos; + } + pos++; } - m_defaultValue = extractSettingIntegerValue(element.attribute("default")); reset(); } void LegacyControllerEnumSetting::parse(const QString& in, bool* ok) { - if (ok != nullptr) + if (ok != nullptr) { *ok = false; + } reset(); size_t pos = 0; - for (const auto& value : m_options) { + for (const auto& value : qAsConst(m_options)) { if (std::get<0>(value) == in) { - if (ok != nullptr) + if (ok != nullptr) { *ok = true; + } m_currentValue = pos; m_dirtyValue = m_currentValue; return; @@ -269,7 +253,7 @@ void LegacyControllerEnumSetting::parse(const QString& in, bool* ok) { QWidget* LegacyControllerEnumSetting::buildInputWidget(QWidget* parent) { QComboBox* comboBox = new QComboBox(parent); - for (const auto& value : m_options) { + for (const auto& value : qAsConst(m_options)) { comboBox->addItem(std::get<1>(value)); } comboBox->setCurrentIndex(m_currentValue); @@ -286,8 +270,6 @@ QWidget* LegacyControllerEnumSetting::buildInputWidget(QWidget* parent) { } bool LegacyControllerEnumSetting::match(const QDomElement& element) { - // TODO(acolombier) improve the function so it can detect the type from the - // spec if there is no type attribute return element.hasAttribute("type") && QString::compare(element.attribute("type"), "enum", diff --git a/src/controllers/legacycontrollersettings.h b/src/controllers/legacycontrollersettings.h index f1a08a0c6e24..01c7bebe2960 100644 --- a/src/controllers/legacycontrollersettings.h +++ b/src/controllers/legacycontrollersettings.h @@ -31,58 +31,74 @@ class AbstractLegacyControllerSetting : public QObject { /// @return a new widget virtual QWidget* buildWidget(QWidget* parent); + /// @brief Build a JSValue with the current setting value. The JSValue + /// variant will use the appropriate type + /// @return A QJSValue with the current value virtual QJSValue value() const = 0; + /// @brief Serialize the current value in a string format + /// @return A String with current setting value virtual QString stringify() const = 0; + + /// @brief Parse a string that contains the value in a compatible format. + /// @param in The string containing the data + /// @param ok A pointer to a boolean in which the result of the + /// deserialisation will be stored (true means the operation was successful) virtual void parse(const QString&, bool*) = 0; + + /// @brief Indicate if the setting is currently not using a user-specified value + /// @return Whether or not the setting is currently set to its default value virtual bool isDefault() const = 0; + + /// @brief Indicate if the setting is currently being mutated and if the + /// edited value is different than its its currently known value. This would + /// indicate that the user may need to save the changes or acknowledge + /// otherwise. + /// @return Whether or not the setting value is dirty virtual bool isDirty() const = 0; + /// @brief Commit the the edited value to become the currently known value. + /// Note that if `isDirty() == false`, this have no effect virtual void save() = 0; - virtual void reset() = 0; - // virtual void reset() const = 0; + /// @brief Reset the current value, as well as the editing value to use the + /// default, as specified in the spec + virtual void reset() = 0; /// @brief Whether of not this setting definition is valid. Validity scope /// includes things like default value within range' for example. /// @return true if valid - virtual inline bool valid() const { + virtual bool valid() const { return !m_variableName.isEmpty(); } /// @brief The variable name as perceived within the mapping definition. /// @return a string - inline QString variableName() const { + QString variableName() const { return m_variableName; } /// @brief The user-friendly label to be display in the UI /// @return a string - inline const QString& label() const { + const QString& label() const { return m_label; } /// @brief A description of what this setting does /// @return a string - inline const QString& description() const { + const QString& description() const { return m_description; } protected: - AbstractLegacyControllerSetting(QString variableName, QString label, QString description) - : m_variableName(variableName), m_label(label), m_description(description) { - } - AbstractLegacyControllerSetting(const QDomElement& element) { - m_variableName = element.attribute("variable").trimmed(); - m_label = element.attribute("label", m_variableName).trimmed(); - - QDomElement description = element.firstChildElement("description"); - if (description.isNull()) { - m_description = QString(); - return; - } - m_description = description.text().trimmed(); + AbstractLegacyControllerSetting(const QString& variableName, + const QString& label, + const QString& description) + : m_variableName(variableName), + m_label(label), + m_description(description) { } + AbstractLegacyControllerSetting(const QDomElement& element); virtual QWidget* buildInputWidget(QWidget* parent) = 0; @@ -107,36 +123,36 @@ class LegacyControllerBooleanSetting QWidget* buildWidget(QWidget* parent) override; - inline QJSValue value() const { + QJSValue value() const override { return QJSValue(m_currentValue); } - inline QString stringify() const { + QString stringify() const override { return m_currentValue ? "true" : "false"; } - inline void parse(const QString& in, bool* ok = nullptr) { + void parse(const QString& in, bool* ok = nullptr) override { if (ok != nullptr) *ok = true; m_currentValue = parseValue(in); m_dirtyValue = m_currentValue; } - inline bool isDefault() const { + bool isDefault() const override { return m_currentValue == m_defaultValue; } - inline bool isDirty() const { + bool isDirty() const override { return m_currentValue != m_dirtyValue; } - virtual void save() { + virtual void save() override { m_currentValue = m_dirtyValue; } - virtual void reset() { + virtual void reset() override { m_currentValue = m_defaultValue; m_dirtyValue = m_defaultValue; } - static inline AbstractLegacyControllerSetting* createFrom(const QDomElement& element) { + static AbstractLegacyControllerSetting* createFrom(const QDomElement& element) { return new LegacyControllerBooleanSetting(element); } static bool match(const QDomElement& element); @@ -150,16 +166,18 @@ class LegacyControllerBooleanSetting m_defaultValue(defaultValue) { } - inline bool parseValue(const QString& in) { + bool parseValue(const QString& in) { return QString::compare(in, "true", Qt::CaseInsensitive) == 0 || in == "1"; } - virtual QWidget* buildInputWidget(QWidget* parent); + virtual QWidget* buildInputWidget(QWidget* parent) override; private: bool m_currentValue; bool m_defaultValue; bool m_dirtyValue; + + friend class LegacyControllerMappingSettingsTest_booleanSettingEditing_Test; }; template @@ -184,35 +202,47 @@ class LegacyControllerNumberSetting virtual ~LegacyControllerNumberSetting() = default; - inline QJSValue value() const { + QJSValue value() const override { return QJSValue(m_currentValue); } - inline QString stringify() const { + QString stringify() const override { return ValueSerializer(m_currentValue); } - inline void parse(const QString& in, bool* ok) { + void parse(const QString& in, bool* ok) override { m_currentValue = ValueDeserializer(in, ok); m_dirtyValue = m_currentValue; } - inline bool isDefault() const { + bool isDefault() const override { return m_currentValue == m_defaultValue; } - inline bool isDirty() const { + bool isDirty() const override { return m_currentValue != m_dirtyValue; } - virtual void save() { + virtual void save() override { m_currentValue = m_dirtyValue; } - virtual void reset() { + virtual void reset() override { m_currentValue = m_defaultValue; m_dirtyValue = m_defaultValue; } - static inline AbstractLegacyControllerSetting* createFrom(const QDomElement& element) { + /// @brief Whether of not this setting definition and its current state are + /// valid. Validity scope includes default/current/dirty value within range + /// and a strictly positive step, strictly less than max.. + /// @return true if valid + bool valid() const override { + return AbstractLegacyControllerSetting::valid() && + m_defaultValue >= m_minValue && m_currentValue >= m_minValue && + m_dirtyValue >= m_minValue && m_defaultValue <= m_maxValue && + m_currentValue <= m_maxValue && m_dirtyValue <= m_maxValue && + m_stepValue > 0 && m_stepValue < m_maxValue; + } + + static AbstractLegacyControllerSetting* createFrom(const QDomElement& element) { return new LegacyControllerNumberSetting(element); } static bool match(const QDomElement& element); @@ -232,7 +262,7 @@ class LegacyControllerNumberSetting m_stepValue(stepValue) { } - virtual QWidget* buildInputWidget(QWidget* parent); + virtual QWidget* buildInputWidget(QWidget* parent) override; private: SettingType m_currentValue; @@ -261,6 +291,11 @@ inline QString packSettingDoubleValue(const double& in) { return QString::number(in); } +using LegacyControllerIntegerSetting = LegacyControllerNumberSetting; + class LegacyControllerRealSetting : public LegacyControllerNumberSetting(m_options.value(m_currentValue)); } - void parse(const QString& in, bool* ok); - bool isDefault() const { + void parse(const QString& in, bool* ok) override; + bool isDefault() const override { return m_currentValue == m_defaultValue; } - inline bool isDirty() const { + bool isDirty() const override { return m_currentValue != m_dirtyValue; } - virtual void save() { + virtual void save() override { m_currentValue = m_dirtyValue; } - virtual void reset() { + virtual void reset() override { m_currentValue = m_defaultValue; m_dirtyValue = m_defaultValue; } - static inline AbstractLegacyControllerSetting* createFrom(const QDomElement& element) { + /// @brief Whether of not this setting definition and its current state are + /// valid. Validity scope includes a known default/current/dirty option. + /// @return true if valid + bool valid() const override { + return AbstractLegacyControllerSetting::valid() && + (int)m_defaultValue < m_options.size() && + (int)m_currentValue < m_options.size() && + (int)m_dirtyValue < m_options.size(); + } + + static AbstractLegacyControllerSetting* createFrom(const QDomElement& element) { return new LegacyControllerEnumSetting(element); } static bool match(const QDomElement& element); protected: LegacyControllerEnumSetting(const QDomElement& element, - QList> options, + const QList>& options, size_t currentValue, size_t defaultValue) : AbstractLegacyControllerSetting(element), @@ -326,7 +371,7 @@ class LegacyControllerEnumSetting m_defaultValue(defaultValue) { } - virtual QWidget* buildInputWidget(QWidget* parent); + virtual QWidget* buildInputWidget(QWidget* parent) override; private: // We use a QList instead of QHash here because we want to keep the natural order diff --git a/src/controllers/legacycontrollersettingsfactory.h b/src/controllers/legacycontrollersettingsfactory.h index 151de97ac515..92b8de773979 100644 --- a/src/controllers/legacycontrollersettingsfactory.h +++ b/src/controllers/legacycontrollersettingsfactory.h @@ -52,7 +52,7 @@ class LegacyControllerSettingBuilder { /// @return an instance if a a supported setting has been found, null /// otherwise static AbstractLegacyControllerSetting* build(const QDomElement& element) { - for (auto settingType : instance()->m_supportedSettings) { + for (const auto& settingType : qAsConst(instance()->m_supportedSettings)) { if (std::get<0>(settingType)(element)) { return std::get<1>(settingType)(element); } diff --git a/src/controllers/legacycontrollersettingslayout.cpp b/src/controllers/legacycontrollersettingslayout.cpp new file mode 100644 index 000000000000..01cfb6f26170 --- /dev/null +++ b/src/controllers/legacycontrollersettingslayout.cpp @@ -0,0 +1,78 @@ + +#include "controllers/legacycontrollersettingslayout.h" + +#include +#include +#include +#include +#include +#include +#include + +void LegacyControllerSettingsLayoutContainer::addItem( + std::shared_ptr setting) { + m_elements.push_back(std::make_unique(setting)); +} + +QBoxLayout* LegacyControllerSettingsLayoutContainer::buildLayout(QWidget* pParent) const { + QBoxLayout* pLayout = nullptr; + + /// Find the active screen. If its size is too small, we force vertical orientation + QScreen* pActive = nullptr; + QWidget* pWidget = pParent; + + while (pWidget) { + auto* pW = pWidget->windowHandle(); + if (pW != nullptr) { + pActive = pW->screen(); + break; + } else { + pWidget = pWidget->parentWidget(); + } + } + + if (m_disposition == VERTICAL || + (pActive != nullptr && + pActive->availableSize().width() < + MIN_SCREEN_SIZE_FOR_CONTROLLER_SETTING_ROW)) { + pLayout = new QVBoxLayout(); + } else { + pLayout = new QHBoxLayout(); + pLayout->setSpacing(16); + } + + pParent->setLayout(pLayout); + + return pLayout; +} + +QWidget* LegacyControllerSettingsLayoutContainer::build(QWidget* pParent) { + QWidget* pContainer = new QWidget(pParent); + QBoxLayout* pLayout = buildLayout(pContainer); + + pLayout->setContentsMargins(0, 0, 0, 0); + + for (auto& element : m_elements) { + auto* pWidget = element->build(pContainer); + if (pLayout->direction() == QBoxLayout::LeftToRight) { + auto* pLayout = dynamic_cast(pWidget->layout()); + if (pLayout != nullptr) { + pLayout->setDirection(QBoxLayout::TopToBottom); + } + } + pLayout->addWidget(pWidget); + } + + return pContainer; +} + +QWidget* LegacyControllerSettingsGroup::build(QWidget* pParent) { + QWidget* pContainer = new QGroupBox(m_label, pParent); + QBoxLayout* pLayout = buildLayout(pContainer); + + for (auto& element : m_elements) { + pLayout->addWidget(element->build(pContainer)); + } + + return pContainer; +} diff --git a/src/controllers/legacycontrollersettingslayout.h b/src/controllers/legacycontrollersettingslayout.h new file mode 100644 index 000000000000..3ce83cb596cb --- /dev/null +++ b/src/controllers/legacycontrollersettingslayout.h @@ -0,0 +1,108 @@ +#pragma once + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "controllers/legacycontrollersettings.h" +#include "defs_urls.h" +#include "preferences/usersettings.h" + +#define MIN_SCREEN_SIZE_FOR_CONTROLLER_SETTING_ROW 1280 + +class LegacyControllerSettingsLayoutElement { + public: + LegacyControllerSettingsLayoutElement() { + } + virtual ~LegacyControllerSettingsLayoutElement() = default; + + virtual std::unique_ptr clone() const = 0; + + virtual QWidget* build(QWidget* parent) = 0; +}; + +class LegacyControllerSettingsLayoutContainer : public LegacyControllerSettingsLayoutElement { + public: + enum Disposition { + HORIZONTAL = 0, + VERTICAL, + }; + + LegacyControllerSettingsLayoutContainer(Disposition disposition = HORIZONTAL) + : LegacyControllerSettingsLayoutElement(), m_disposition(disposition) { + } + LegacyControllerSettingsLayoutContainer(const LegacyControllerSettingsLayoutContainer& other) { + m_elements.reserve(other.m_elements.size()); + for (const auto& e : other.m_elements) + m_elements.push_back(e->clone()); + } + virtual ~LegacyControllerSettingsLayoutContainer() = default; + + virtual std::unique_ptr clone() const override { + return std::make_unique(*this); + } + + void addItem(std::shared_ptr setting); + void addItem(std::unique_ptr&& container) { + m_elements.push_back(std::move(container)); + } + + virtual QWidget* build(QWidget* parent) override; + + protected: + QBoxLayout* buildLayout(QWidget* parent) const; + + Disposition m_disposition; + std::vector> m_elements; +}; + +class LegacyControllerSettingsGroup : public LegacyControllerSettingsLayoutContainer { + public: + LegacyControllerSettingsGroup(const QString& label, + LegacyControllerSettingsLayoutContainer::Disposition disposition = + VERTICAL) + : LegacyControllerSettingsLayoutContainer(disposition), + m_label(label) { + } + virtual ~LegacyControllerSettingsGroup() = default; + + std::unique_ptr clone() const override { + return std::make_unique(*this); + } + + QWidget* build(QWidget* parent) override; + + private: + QString m_label; +}; + +class LegacyControllerSettingsLayoutItem : public LegacyControllerSettingsLayoutElement { + public: + LegacyControllerSettingsLayoutItem(std::shared_ptr setting) + : LegacyControllerSettingsLayoutElement(), m_setting(setting) { + } + virtual ~LegacyControllerSettingsLayoutItem() = default; + + std::unique_ptr clone() const override { + return std::make_unique(*this); + } + + QWidget* build(QWidget* parent) override { + VERIFY_OR_DEBUG_ASSERT(m_setting.get() != nullptr) { + return nullptr; + } + return m_setting->buildWidget(parent); + } + + private: + std::shared_ptr m_setting; +}; diff --git a/src/controllers/midi/legacymidicontrollermappingfilehandler.cpp b/src/controllers/midi/legacymidicontrollermappingfilehandler.cpp index e0fcea473cf3..48d97d2ea830 100644 --- a/src/controllers/midi/legacymidicontrollermappingfilehandler.cpp +++ b/src/controllers/midi/legacymidicontrollermappingfilehandler.cpp @@ -11,6 +11,7 @@ std::shared_ptr LegacyMidiControllerMappingFileHandler::load(const QDomElement& root, const QString& filePath, const QDir& systemMappingsPath) { + // TODO (acolombier): support for controller settings if (root.isNull()) { return nullptr; } diff --git a/src/controllers/scripting/legacy/controllerscriptenginelegacy.cpp b/src/controllers/scripting/legacy/controllerscriptenginelegacy.cpp index f760bbeabe3e..6f089cb767f7 100644 --- a/src/controllers/scripting/legacy/controllerscriptenginelegacy.cpp +++ b/src/controllers/scripting/legacy/controllerscriptenginelegacy.cpp @@ -99,6 +99,15 @@ void ControllerScriptEngineLegacy::setScriptFiles( m_scriptFiles = scripts; } +void ControllerScriptEngineLegacy::setSettings( + const QList>& settings) { + m_settings.clear(); + for (const auto& pSetting : qAsConst(settings)) { + m_settings.append(std::tuple( + pSetting->variableName(), pSetting->value())); + } +} + bool ControllerScriptEngineLegacy::initialize() { if (!ControllerScriptEngineBase::initialize()) { return false; @@ -125,7 +134,7 @@ bool ControllerScriptEngineLegacy::initialize() { new ControllerScriptInterfaceLegacy(this, m_logger); QJSValue settingsObject = m_pJSEngine->newObject(); - for (auto setting : m_settings) { + for (const auto& setting : qAsConst(m_settings)) { settingsObject.setProperty(std::get<0>(setting), std::get<1>(setting)); } engineGlobalObject.setProperty( diff --git a/src/controllers/scripting/legacy/controllerscriptenginelegacy.h b/src/controllers/scripting/legacy/controllerscriptenginelegacy.h index e0392a819bfd..7cc2d94bf262 100644 --- a/src/controllers/scripting/legacy/controllerscriptenginelegacy.h +++ b/src/controllers/scripting/legacy/controllerscriptenginelegacy.h @@ -28,14 +28,15 @@ class ControllerScriptEngineLegacy : public ControllerScriptEngineBase { public slots: void setScriptFiles(const QList& scripts); - inline void setSettings( - const QList>& settings) { - m_settings.clear(); - for (auto pSetting : settings) { - m_settings.append(std::tuple( - pSetting->variableName(), pSetting->value())); - } - } + + /// @brief Set the list of customizable settings and their currently set + /// value, ready to be used. This method will generate a JSValue from their + /// current state, meaning that any later mutation won't be used, and this + /// method should be called again + /// @param settings The list of settings in a valid state (initialized and + /// restored) + void setSettings( + const QList>& settings); private: bool evaluateScriptFile(const QFileInfo& scriptFile); diff --git a/src/test/controller_mapping_settings_test.cpp b/src/test/controller_mapping_settings_test.cpp new file mode 100644 index 000000000000..48214c51ec03 --- /dev/null +++ b/src/test/controller_mapping_settings_test.cpp @@ -0,0 +1,214 @@ + +#include + +#include + +// FIXME (acolombier): here we need the CPP file so the templated methods gets +// built. AFAIK, there is an alternate solution which is to include the cpp file +// in the CMake build list +#include "controllers/legacycontrollersettings.cpp" +#include "test/mixxxtest.h" + +class LegacyControllerMappingSettingsTest : public MixxxTest { +}; + +const char* const kValidBoolean = + ""; + +const char* const kValidInteger = + ""; + +// This setting has purposfully no custom "label" and description +const char* const kValidDouble = + " @@ -206,6 +206,9 @@ Qt::Vertical + + QSizePolicy::Expanding + 20 @@ -430,6 +433,12 @@ + + + 0 + 0 + + Mapping settings diff --git a/src/controllers/legacycontrollermappingfilehandler.cpp b/src/controllers/legacycontrollermappingfilehandler.cpp index a491149ae468..b476fcda2bb4 100644 --- a/src/controllers/legacycontrollermappingfilehandler.cpp +++ b/src/controllers/legacycontrollermappingfilehandler.cpp @@ -136,7 +136,7 @@ void LegacyControllerMappingFileHandler::parseMappingSettingsElement( for (QDomElement element = current.firstChildElement(); !element.isNull(); element = element.nextSiblingElement()) { - const QString& tagName = element.tagName(); + const QString& tagName = element.tagName().toLower(); if (tagName == "option") { std::shared_ptr pSetting( LegacyControllerSettingBuilder::build(element)); @@ -151,8 +151,15 @@ void LegacyControllerMappingFileHandler::parseMappingSettingsElement( layout->addItem(pSetting); mapping->addSetting(pSetting); } else if (tagName == "row") { + LegacyControllerSettingsLayoutContainer::Disposition orientation = + element.attribute("orientation").trimmed().toLower() == + "vertical" + ? LegacyControllerSettingsLayoutContainer::VERTICAL + : LegacyControllerSettingsLayoutContainer::HORIZONTAL; std::unique_ptr row = - std::make_unique(); + std::make_unique( + LegacyControllerSettingsLayoutContainer::HORIZONTAL, + orientation); parseMappingSettingsElement(element, mapping, row); layout->addItem(std::move(row)); } else if (tagName == "group") { diff --git a/src/controllers/legacycontrollersettings.cpp b/src/controllers/legacycontrollersettings.cpp index 1772e2538a86..f92e57951e45 100644 --- a/src/controllers/legacycontrollersettings.cpp +++ b/src/controllers/legacycontrollersettings.cpp @@ -28,11 +28,31 @@ AbstractLegacyControllerSetting::AbstractLegacyControllerSetting(const QDomEleme } } -QWidget* AbstractLegacyControllerSetting::buildWidget(QWidget* parent) { - QWidget* pRoot = new QWidget(parent); +QWidget* AbstractLegacyControllerSetting::buildWidget(QWidget* pParent, + LegacyControllerSettingsLayoutContainer::Disposition orientation) { + QWidget* pRoot = new QWidget(pParent); QBoxLayout* pLayout = new QBoxLayout(QBoxLayout::LeftToRight); pLayout->setContentsMargins(0, 0, 0, 0); + if (orientation == LegacyControllerSettingsLayoutContainer::VERTICAL) { + auto* pSettingsContainer = dynamic_cast(pParent); + if (pSettingsContainer) { + connect(pSettingsContainer, + &WLegacyControllerSettingsContainer::orientationChanged, + this, + [pLayout, pParent]( + LegacyControllerSettingsLayoutContainer::Disposition + disposition) { + pLayout->setDirection(disposition == + LegacyControllerSettingsLayoutContainer:: + HORIZONTAL + ? QBoxLayout::TopToBottom + : QBoxLayout::LeftToRight); + pParent->layout()->invalidate(); + }); + } + } + QLabel* pLabelWidget = new QLabel(pRoot); pLabelWidget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum); pLabelWidget->setText(label()); @@ -60,9 +80,10 @@ LegacyControllerBooleanSetting::LegacyControllerBooleanSetting( m_dirtyValue = m_defaultValue; } -QWidget* LegacyControllerBooleanSetting::buildWidget(QWidget* parent) { +QWidget* LegacyControllerBooleanSetting::buildWidget( + QWidget* parent, LegacyControllerSettingsLayoutContainer::Disposition) { QWidget* pRoot = new QWidget(parent); - QBoxLayout* pLayout = new QBoxLayout(QBoxLayout::LeftToRight); + QBoxLayout* pLayout = new QHBoxLayout(); pLayout->setContentsMargins(0, 0, 0, 0); QLabel* pLabelWidget = new QLabel(pRoot); @@ -143,7 +164,7 @@ QWidget* LegacyControllerNumberSetting::buildInputWidget(QWidget* parent) { InputWidget* spinBox = new InputWidget(parent); - spinBox->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Fixed); + spinBox->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum); spinBox->setRange(this->m_minValue, this->m_maxValue); spinBox->setSingleStep(this->m_stepValue); diff --git a/src/controllers/legacycontrollersettings.h b/src/controllers/legacycontrollersettings.h index 01c7bebe2960..1c0971a8ba34 100644 --- a/src/controllers/legacycontrollersettings.h +++ b/src/controllers/legacycontrollersettings.h @@ -16,6 +16,7 @@ #include #include "controllers/legacycontrollersettingsfactory.h" +#include "controllers/legacycontrollersettingslayout.h" /// @brief The abstract controller setting. Any type of setting will have to /// implement this base class @@ -29,7 +30,9 @@ class AbstractLegacyControllerSetting : public QObject { /// @param parent The parent widget for which this widget is being created. /// The parent widget will own the newly created widget /// @return a new widget - virtual QWidget* buildWidget(QWidget* parent); + virtual QWidget* buildWidget(QWidget* parent, + LegacyControllerSettingsLayoutContainer::Disposition orientation = + LegacyControllerSettingsLayoutContainer::HORIZONTAL); /// @brief Build a JSValue with the current setting value. The JSValue /// variant will use the appropriate type @@ -121,7 +124,10 @@ class LegacyControllerBooleanSetting virtual ~LegacyControllerBooleanSetting() = default; - QWidget* buildWidget(QWidget* parent) override; + QWidget* buildWidget(QWidget* parent, + LegacyControllerSettingsLayoutContainer::Disposition orientation = + LegacyControllerSettingsLayoutContainer::HORIZONTAL) + override; QJSValue value() const override { return QJSValue(m_currentValue); diff --git a/src/controllers/legacycontrollersettingslayout.cpp b/src/controllers/legacycontrollersettingslayout.cpp index 01cfb6f26170..52a78079422f 100644 --- a/src/controllers/legacycontrollersettingslayout.cpp +++ b/src/controllers/legacycontrollersettingslayout.cpp @@ -4,42 +4,22 @@ #include #include #include +#include #include #include #include #include +#include "controllers/legacycontrollersettings.h" + void LegacyControllerSettingsLayoutContainer::addItem( std::shared_ptr setting) { - m_elements.push_back(std::make_unique(setting)); + m_elements.push_back(std::make_unique( + setting, m_widgetOrientation)); } QBoxLayout* LegacyControllerSettingsLayoutContainer::buildLayout(QWidget* pParent) const { - QBoxLayout* pLayout = nullptr; - - /// Find the active screen. If its size is too small, we force vertical orientation - QScreen* pActive = nullptr; - QWidget* pWidget = pParent; - - while (pWidget) { - auto* pW = pWidget->windowHandle(); - if (pW != nullptr) { - pActive = pW->screen(); - break; - } else { - pWidget = pWidget->parentWidget(); - } - } - - if (m_disposition == VERTICAL || - (pActive != nullptr && - pActive->availableSize().width() < - MIN_SCREEN_SIZE_FOR_CONTROLLER_SETTING_ROW)) { - pLayout = new QVBoxLayout(); - } else { - pLayout = new QHBoxLayout(); - pLayout->setSpacing(16); - } + QBoxLayout* pLayout = new QBoxLayout(QBoxLayout::TopToBottom); pParent->setLayout(pLayout); @@ -47,20 +27,13 @@ QBoxLayout* LegacyControllerSettingsLayoutContainer::buildLayout(QWidget* pParen } QWidget* LegacyControllerSettingsLayoutContainer::build(QWidget* pParent) { - QWidget* pContainer = new QWidget(pParent); + QWidget* pContainer = new WLegacyControllerSettingsContainer(m_disposition, pParent); QBoxLayout* pLayout = buildLayout(pContainer); pLayout->setContentsMargins(0, 0, 0, 0); for (auto& element : m_elements) { - auto* pWidget = element->build(pContainer); - if (pLayout->direction() == QBoxLayout::LeftToRight) { - auto* pLayout = dynamic_cast(pWidget->layout()); - if (pLayout != nullptr) { - pLayout->setDirection(QBoxLayout::TopToBottom); - } - } - pLayout->addWidget(pWidget); + pLayout->addWidget(element->build(pContainer)); } return pContainer; @@ -76,3 +49,34 @@ QWidget* LegacyControllerSettingsGroup::build(QWidget* pParent) { return pContainer; } + +QWidget* LegacyControllerSettingsLayoutItem::build(QWidget* parent) { + VERIFY_OR_DEBUG_ASSERT(m_setting.get() != nullptr) { + return nullptr; + } + return m_setting->buildWidget(parent, m_prefferedOrientation); +} + +void WLegacyControllerSettingsContainer::resizeEvent(QResizeEvent* event) { + if (m_prefferedOrientation == LegacyControllerSettingsLayoutContainer::VERTICAL) { + return; + } + + auto* pLayout = dynamic_cast(layout()); + if (pLayout == nullptr) { + return; + } + + if (event->size().width() < MIN_SCREEN_SIZE_FOR_CONTROLLER_SETTING_ROW && + pLayout->direction() == QBoxLayout::LeftToRight) { + pLayout->setDirection(QBoxLayout::TopToBottom); + pLayout->setSpacing(6); + emit orientationChanged(LegacyControllerSettingsLayoutContainer::VERTICAL); + } else if (event->size().width() >= + MIN_SCREEN_SIZE_FOR_CONTROLLER_SETTING_ROW && + pLayout->direction() == QBoxLayout::TopToBottom) { + pLayout->setDirection(QBoxLayout::LeftToRight); + pLayout->setSpacing(16); + emit orientationChanged(LegacyControllerSettingsLayoutContainer::HORIZONTAL); + } +} diff --git a/src/controllers/legacycontrollersettingslayout.h b/src/controllers/legacycontrollersettingslayout.h index 3ce83cb596cb..d1b6387a0fa0 100644 --- a/src/controllers/legacycontrollersettingslayout.h +++ b/src/controllers/legacycontrollersettingslayout.h @@ -2,6 +2,7 @@ #include +#include #include #include #include @@ -13,11 +14,12 @@ #include #include -#include "controllers/legacycontrollersettings.h" #include "defs_urls.h" #include "preferences/usersettings.h" -#define MIN_SCREEN_SIZE_FOR_CONTROLLER_SETTING_ROW 1280 +#define MIN_SCREEN_SIZE_FOR_CONTROLLER_SETTING_ROW 960 + +class AbstractLegacyControllerSetting; class LegacyControllerSettingsLayoutElement { public: @@ -37,8 +39,12 @@ class LegacyControllerSettingsLayoutContainer : public LegacyControllerSettingsL VERTICAL, }; - LegacyControllerSettingsLayoutContainer(Disposition disposition = HORIZONTAL) - : LegacyControllerSettingsLayoutElement(), m_disposition(disposition) { + LegacyControllerSettingsLayoutContainer( + Disposition disposition = HORIZONTAL, + Disposition widgetOrientation = HORIZONTAL) + : LegacyControllerSettingsLayoutElement(), + m_disposition(disposition), + m_widgetOrientation(widgetOrientation) { } LegacyControllerSettingsLayoutContainer(const LegacyControllerSettingsLayoutContainer& other) { m_elements.reserve(other.m_elements.size()); @@ -62,6 +68,7 @@ class LegacyControllerSettingsLayoutContainer : public LegacyControllerSettingsL QBoxLayout* buildLayout(QWidget* parent) const; Disposition m_disposition; + Disposition m_widgetOrientation; std::vector> m_elements; }; @@ -80,15 +87,20 @@ class LegacyControllerSettingsGroup : public LegacyControllerSettingsLayoutConta } QWidget* build(QWidget* parent) override; - + private: QString m_label; }; class LegacyControllerSettingsLayoutItem : public LegacyControllerSettingsLayoutElement { public: - LegacyControllerSettingsLayoutItem(std::shared_ptr setting) - : LegacyControllerSettingsLayoutElement(), m_setting(setting) { + LegacyControllerSettingsLayoutItem( + std::shared_ptr setting, + LegacyControllerSettingsLayoutContainer::Disposition orientation = + LegacyControllerSettingsGroup::HORIZONTAL) + : LegacyControllerSettingsLayoutElement(), + m_setting(setting), + m_prefferedOrientation(orientation) { } virtual ~LegacyControllerSettingsLayoutItem() = default; @@ -96,13 +108,29 @@ class LegacyControllerSettingsLayoutItem : public LegacyControllerSettingsLayout return std::make_unique(*this); } - QWidget* build(QWidget* parent) override { - VERIFY_OR_DEBUG_ASSERT(m_setting.get() != nullptr) { - return nullptr; - } - return m_setting->buildWidget(parent); - } + QWidget* build(QWidget* parent) override; private: std::shared_ptr m_setting; + LegacyControllerSettingsLayoutContainer::Disposition m_prefferedOrientation; +}; + +class WLegacyControllerSettingsContainer : public QWidget { + Q_OBJECT + public: + WLegacyControllerSettingsContainer( + LegacyControllerSettingsLayoutContainer::Disposition + prefferedOrientation, + QWidget* parent) + : QWidget(parent), m_prefferedOrientation(prefferedOrientation) { + } + + protected: + void resizeEvent(QResizeEvent* event); + + signals: + void orientationChanged(LegacyControllerSettingsLayoutContainer::Disposition); + + private: + LegacyControllerSettingsLayoutContainer::Disposition m_prefferedOrientation; }; From d8a925fd61a41184a6ebae59327212d5cf788a70 Mon Sep 17 00:00:00 2001 From: Antoine C Date: Tue, 28 Feb 2023 23:38:01 +0000 Subject: [PATCH 06/17] feat(Controller): adding tests for controller settings --- src/controllers/dlgprefcontrollerdlg.ui | 2 +- src/controllers/legacycontrollermapping.cpp | 2 +- src/controllers/legacycontrollermapping.h | 21 +- .../legacycontrollermappingfilehandler.cpp | 36 ++- .../legacycontrollermappingfilehandler.h | 24 +- src/controllers/legacycontrollersettings.cpp | 60 ++-- src/controllers/legacycontrollersettings.h | 11 +- .../legacycontrollersettingslayout.cpp | 16 +- .../legacycontrollersettingslayout.h | 14 +- ...legacymidicontrollermappingfilehandler.cpp | 2 +- src/test/controller_mapping_settings_test.cpp | 298 +++++++++++++++++- 11 files changed, 409 insertions(+), 77 deletions(-) diff --git a/src/controllers/dlgprefcontrollerdlg.ui b/src/controllers/dlgprefcontrollerdlg.ui index ba82eb686496..9b296011d893 100644 --- a/src/controllers/dlgprefcontrollerdlg.ui +++ b/src/controllers/dlgprefcontrollerdlg.ui @@ -7,7 +7,7 @@ 0 0 507 - 497 + 437 diff --git a/src/controllers/legacycontrollermapping.cpp b/src/controllers/legacycontrollermapping.cpp index 1c6863e040e0..cd501f6ba382 100644 --- a/src/controllers/legacycontrollermapping.cpp +++ b/src/controllers/legacycontrollermapping.cpp @@ -23,7 +23,7 @@ void LegacyControllerMapping::restoreSettings(const QFileInfo& mappingFile, pConfig->remove(key); continue; } - auto& pSetting = availableSettings.at(availableSettingKeys.indexOf(key.item)); + const auto& pSetting = availableSettings.at(availableSettingKeys.indexOf(key.item)); QString value = pConfig->getValueString(key); if (!pSetting->valid()) { qWarning() << "The setting" << pSetting->variableName() diff --git a/src/controllers/legacycontrollermapping.h b/src/controllers/legacycontrollermapping.h index a0934281e7bd..a989fe5b8a8d 100644 --- a/src/controllers/legacycontrollermapping.h +++ b/src/controllers/legacycontrollermapping.h @@ -18,7 +18,6 @@ #include "defs_urls.h" #include "preferences/usersettings.h" -// TODO (acolombier) is it okay to keep it as it? Or shall we generate a UUID from that pair? #define CONTROLLER_SETTINGS_PREFERENCE_GROUP_KEY "[ControllerSettings_%1_%2]" /// This class represents a controller mapping, containing the data elements that @@ -83,16 +82,22 @@ class LegacyControllerMapping { /// Adds a setting option to the list of setting option for this mapping. /// The option added must be a valid option. /// @param option The option to add - void addSetting(std::shared_ptr option) { - // if (m_settings.contains(option->variableName())){ - // qWarning() << QString("Mapping setting duplication detected. - // Keeping the first version of '%1'.").arg(option->variableName()); - // return; - // } + /// @return whether or not the setting was added successfully. + bool addSetting(std::shared_ptr option) { VERIFY_OR_DEBUG_ASSERT(option->valid()) { - return; + return false; + } + for (const auto& setting : qAsConst(m_settings)) { + if (*setting == *option) { + qWarning() << "Mapping setting duplication detected for " + "setting with name" + << option->variableName() + << ". Keeping the first occurrence."; + return false; + } } m_settings.append(option); + return true; } /// @brief Set a setting layout as they should be perceived when edited in diff --git a/src/controllers/legacycontrollermappingfilehandler.cpp b/src/controllers/legacycontrollermappingfilehandler.cpp index b476fcda2bb4..34b66017cc40 100644 --- a/src/controllers/legacycontrollermappingfilehandler.cpp +++ b/src/controllers/legacycontrollermappingfilehandler.cpp @@ -66,6 +66,9 @@ std::shared_ptr LegacyControllerMappingFileHandler::loa std::shared_ptr pMapping = pHandler->load( mappingFile.absoluteFilePath(), systemMappingsPath); + + // FIXME(acolombier): is `delete pHandler;` not missing? + if (pMapping) { pMapping->setDirty(false); } @@ -129,10 +132,9 @@ void LegacyControllerMappingFileHandler::parseMappingSettings( void LegacyControllerMappingFileHandler::parseMappingSettingsElement( const QDomElement& current, - std::shared_ptr mapping, + std::shared_ptr pMapping, const std::unique_ptr& layout) const { - // TODO (acolombier) Add test for the parser for (QDomElement element = current.firstChildElement(); !element.isNull(); element = element.nextSiblingElement()) { @@ -141,15 +143,25 @@ void LegacyControllerMappingFileHandler::parseMappingSettingsElement( std::shared_ptr pSetting( LegacyControllerSettingBuilder::build(element)); if (pSetting.get() == nullptr) { - qDebug() << "Could not parse the unknown controller setting. Ignoring it."; + qDebug() << "Ignoring unsupported controller setting in file" + << pMapping->filePath() << "at line" + << element.lineNumber() << "."; continue; } if (!pSetting->valid()) { - qDebug() << "The parsed setting appears to be invalid. Discarding it."; + qDebug() << "The parsed setting in file" << pMapping->filePath() + << "at line" << element.lineNumber() + << "appears to be invalid. It will be ignored."; + continue; + } + if (pMapping->addSetting(pSetting)) { + layout->addItem(pSetting); + } else { + qDebug() << "The parsed setting in file" << pMapping->filePath() + << "at line" << element.lineNumber() + << "couldn't be added. Its layout information will also be ignored."; continue; } - layout->addItem(pSetting); - mapping->addSetting(pSetting); } else if (tagName == "row") { LegacyControllerSettingsLayoutContainer::Disposition orientation = element.attribute("orientation").trimmed().toLower() == @@ -160,17 +172,19 @@ void LegacyControllerMappingFileHandler::parseMappingSettingsElement( std::make_unique( LegacyControllerSettingsLayoutContainer::HORIZONTAL, orientation); - parseMappingSettingsElement(element, mapping, row); + parseMappingSettingsElement(element, pMapping, row); layout->addItem(std::move(row)); } else if (tagName == "group") { std::unique_ptr group = std::make_unique( element.attribute("label")); - parseMappingSettingsElement(element, mapping, group); + parseMappingSettingsElement(element, pMapping, group); layout->addItem(std::move(group)); } else { - qDebug() << "Unsupported tag" << tagName - << "for controller layout settings. Discarding it."; + qDebug() << "Ignoring unsupported tag" << tagName + << "in file" << pMapping->filePath() + << "on line" << element.lineNumber() + << "for controller layout settings. Check the documentation supported tags."; continue; } } @@ -261,7 +275,7 @@ QDomDocument LegacyControllerMappingFileHandler::buildRootWithScripts( QString blank = "\n" "\n" - "\n"; + "\n"; doc.setContent(blank); QDomElement rootNode = doc.documentElement(); diff --git a/src/controllers/legacycontrollermappingfilehandler.h b/src/controllers/legacycontrollermappingfilehandler.h index 8daf2f21602a..de7186532cab 100644 --- a/src/controllers/legacycontrollermappingfilehandler.h +++ b/src/controllers/legacycontrollermappingfilehandler.h @@ -47,17 +47,6 @@ class LegacyControllerMappingFileHandler { void parseMappingSettings(const QDomElement& root, std::shared_ptr mapping) const; - /// @brief Recursively parse setting definition and layout information - /// within a setting node - /// @param current The setting node (MixxxControllerPreset.settings) or any - /// children nodes - /// @param mapping The mapping object to populate with the gathered data - /// @param layout The currently active layout, on which new setting item - /// (leaf) should be attached - void parseMappingSettingsElement(const QDomElement& current, - std::shared_ptr mapping, - const std::unique_ptr& layout) const; - /// Adds script files from XML to the LegacyControllerMapping. /// /// This function parses the supplied QDomElement structure, finds the @@ -78,8 +67,21 @@ class LegacyControllerMappingFileHandler { bool writeDocument(const QDomDocument& root, const QString& fileName) const; private: + /// @brief Recursively parse setting definition and layout information + /// within a setting node + /// @param current The setting node (MixxxControllerPreset.settings) or any + /// children nodes + /// @param mapping The mapping object to populate with the gathered data + /// @param layout The currently active layout, on which new setting item + /// (leaf) should be attached + void parseMappingSettingsElement(const QDomElement& current, + std::shared_ptr mapping, + const std::unique_ptr& layout) const; + // Sub-classes implement this. virtual std::shared_ptr load(const QDomElement& root, const QString& filePath, const QDir& systemMappingPath) = 0; + + friend class LegacyControllerMappingSettingsTest_parseSettingBlock_Test; }; diff --git a/src/controllers/legacycontrollersettings.cpp b/src/controllers/legacycontrollersettings.cpp index f92e57951e45..c6eb2fda2f40 100644 --- a/src/controllers/legacycontrollersettings.cpp +++ b/src/controllers/legacycontrollersettings.cpp @@ -6,7 +6,7 @@ #include #include -// TODO (acolombier): remove "new" where possible and use parented_ptr +#include "util/parented_ptr.h" LegacyControllerSettingBuilder* LegacyControllerSettingBuilder::__self = nullptr; @@ -30,8 +30,9 @@ AbstractLegacyControllerSetting::AbstractLegacyControllerSetting(const QDomEleme QWidget* AbstractLegacyControllerSetting::buildWidget(QWidget* pParent, LegacyControllerSettingsLayoutContainer::Disposition orientation) { - QWidget* pRoot = new QWidget(pParent); + auto pRoot = make_parented(pParent); QBoxLayout* pLayout = new QBoxLayout(QBoxLayout::LeftToRight); + pLayout->setContentsMargins(0, 0, 0, 0); if (orientation == LegacyControllerSettingsLayoutContainer::VERTICAL) { @@ -53,7 +54,7 @@ QWidget* AbstractLegacyControllerSetting::buildWidget(QWidget* pParent, } } - QLabel* pLabelWidget = new QLabel(pRoot); + auto pLabelWidget = make_parented(pRoot); pLabelWidget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum); pLabelWidget->setText(label()); @@ -81,40 +82,27 @@ LegacyControllerBooleanSetting::LegacyControllerBooleanSetting( } QWidget* LegacyControllerBooleanSetting::buildWidget( - QWidget* parent, LegacyControllerSettingsLayoutContainer::Disposition) { - QWidget* pRoot = new QWidget(parent); - QBoxLayout* pLayout = new QHBoxLayout(); - pLayout->setContentsMargins(0, 0, 0, 0); - - QLabel* pLabelWidget = new QLabel(pRoot); - pLabelWidget->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum); - pLabelWidget->setText(label()); - - if (!description().isEmpty()) { - pRoot->setToolTip(QString("

%1

").arg(description())); - } - - pLayout->addWidget(buildInputWidget(pRoot)); - pLayout->addWidget(pLabelWidget); - - pRoot->setLayout(pLayout); - - return pRoot; + QWidget* pParent, LegacyControllerSettingsLayoutContainer::Disposition) { + return buildInputWidget(pParent); } -QWidget* LegacyControllerBooleanSetting::buildInputWidget(QWidget* parent) { - QCheckBox* checkBox = new QCheckBox("", parent); - checkBox->setSizePolicy(QSizePolicy::Maximum, QSizePolicy::Fixed); +QWidget* LegacyControllerBooleanSetting::buildInputWidget(QWidget* pParent) { + auto pCheckBox = make_parented(label(), pParent); + pCheckBox->setSizePolicy(QSizePolicy::Maximum, QSizePolicy::Fixed); if (m_currentValue) { - checkBox->setCheckState(Qt::Checked); + pCheckBox->setCheckState(Qt::Checked); + } + + if (!description().isEmpty()) { + pCheckBox->setToolTip(QString("

%1

").arg(description())); } - connect(checkBox, &QCheckBox::stateChanged, this, [this](int state) { + connect(pCheckBox, &QCheckBox::stateChanged, this, [this](int state) { m_dirtyValue = state == Qt::Checked; emit changed(); }); - return checkBox; + return pCheckBox; } bool LegacyControllerBooleanSetting::match(const QDomElement& element) { @@ -162,8 +150,8 @@ template::buildInputWidget(QWidget* parent) { - InputWidget* spinBox = new InputWidget(parent); + InputWidget>::buildInputWidget(QWidget* pParent) { + auto spinBox = make_parented(pParent); spinBox->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum); spinBox->setRange(this->m_minValue, this->m_maxValue); @@ -211,12 +199,12 @@ LegacyControllerRealSetting::LegacyControllerRealSetting(const QDomElement& elem } } -QWidget* LegacyControllerRealSetting::buildInputWidget(QWidget* parent) { +QWidget* LegacyControllerRealSetting::buildInputWidget(QWidget* pParent) { QDoubleSpinBox* spinBox = dynamic_cast( - LegacyControllerNumberSetting::buildInputWidget(parent)); + LegacyControllerNumberSetting::buildInputWidget(pParent)); VERIFY_OR_DEBUG_ASSERT(spinBox != nullptr) { - qWarning() << "Unable to set precision on the controller setting " - "input: tt does not appear to be a valid QDoubleSpinBox"; + qWarning() << "Unable to set precision on the input widget " + "input: it does not appear to be a valid QDoubleSpinBox"; return spinBox; } spinBox->setDecimals(m_precisionValue); @@ -271,8 +259,8 @@ void LegacyControllerEnumSetting::parse(const QString& in, bool* ok) { } } -QWidget* LegacyControllerEnumSetting::buildInputWidget(QWidget* parent) { - QComboBox* comboBox = new QComboBox(parent); +QWidget* LegacyControllerEnumSetting::buildInputWidget(QWidget* pParent) { + auto comboBox = make_parented(pParent); for (const auto& value : qAsConst(m_options)) { comboBox->addItem(std::get<1>(value)); diff --git a/src/controllers/legacycontrollersettings.h b/src/controllers/legacycontrollersettings.h index 1c0971a8ba34..bf3ab316945c 100644 --- a/src/controllers/legacycontrollersettings.h +++ b/src/controllers/legacycontrollersettings.h @@ -93,6 +93,10 @@ class AbstractLegacyControllerSetting : public QObject { return m_description; } + bool operator==(const AbstractLegacyControllerSetting& other) const noexcept { + return variableName() == other.variableName(); + } + protected: AbstractLegacyControllerSetting(const QString& variableName, const QString& label, @@ -278,6 +282,9 @@ class LegacyControllerNumberSetting SettingType m_stepValue; SettingType m_dirtyValue; + + friend class LegacyControllerMappingSettingsTest_integerSettingEditing_Test; + friend class LegacyControllerMappingSettingsTest_doubleSettingEditing_Test; }; template @@ -332,7 +339,7 @@ class LegacyControllerEnumSetting } QString stringify() const override { - return std::get<0>(m_options.value(m_currentValue)); + return std::get<0>(m_options.value((int)m_currentValue)); } void parse(const QString& in, bool* ok) override; bool isDefault() const override { @@ -386,4 +393,6 @@ class LegacyControllerEnumSetting size_t m_defaultValue; size_t m_dirtyValue; + + friend class LegacyControllerMappingSettingsTest_enumSettingEditing_Test; }; diff --git a/src/controllers/legacycontrollersettingslayout.cpp b/src/controllers/legacycontrollersettingslayout.cpp index 52a78079422f..8b2ff08e4d54 100644 --- a/src/controllers/legacycontrollersettingslayout.cpp +++ b/src/controllers/legacycontrollersettingslayout.cpp @@ -11,6 +11,7 @@ #include #include "controllers/legacycontrollersettings.h" +#include "util/parented_ptr.h" void LegacyControllerSettingsLayoutContainer::addItem( std::shared_ptr setting) { @@ -19,7 +20,7 @@ void LegacyControllerSettingsLayoutContainer::addItem( } QBoxLayout* LegacyControllerSettingsLayoutContainer::buildLayout(QWidget* pParent) const { - QBoxLayout* pLayout = new QBoxLayout(QBoxLayout::TopToBottom); + auto pLayout = make_parented(QBoxLayout::TopToBottom); pParent->setLayout(pLayout); @@ -27,20 +28,27 @@ QBoxLayout* LegacyControllerSettingsLayoutContainer::buildLayout(QWidget* pParen } QWidget* LegacyControllerSettingsLayoutContainer::build(QWidget* pParent) { - QWidget* pContainer = new WLegacyControllerSettingsContainer(m_disposition, pParent); + auto pContainer = make_parented(m_disposition, pParent); QBoxLayout* pLayout = buildLayout(pContainer); pLayout->setContentsMargins(0, 0, 0, 0); + auto& lastElement = m_elements.back(); for (auto& element : m_elements) { - pLayout->addWidget(element->build(pContainer)); + auto* pWidget = element->build(pContainer); + pWidget->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Preferred); + pLayout->addWidget(pWidget); + if (element != lastElement) { + pLayout->addItem(new QSpacerItem( + 10, 10, QSizePolicy::Expanding, QSizePolicy::Fixed)); + } } return pContainer; } QWidget* LegacyControllerSettingsGroup::build(QWidget* pParent) { - QWidget* pContainer = new QGroupBox(m_label, pParent); + auto pContainer = make_parented(m_label, pParent); QBoxLayout* pLayout = buildLayout(pContainer); for (auto& element : m_elements) { diff --git a/src/controllers/legacycontrollersettingslayout.h b/src/controllers/legacycontrollersettingslayout.h index d1b6387a0fa0..db9cb63eca64 100644 --- a/src/controllers/legacycontrollersettingslayout.h +++ b/src/controllers/legacycontrollersettingslayout.h @@ -21,6 +21,7 @@ class AbstractLegacyControllerSetting; +/// @brief Layout information used for controller setting when rendered in the Preference Dialog class LegacyControllerSettingsLayoutElement { public: LegacyControllerSettingsLayoutElement() { @@ -32,8 +33,14 @@ class LegacyControllerSettingsLayoutElement { virtual QWidget* build(QWidget* parent) = 0; }; +/// @brief This layout element can hold others element. It is also the one used +/// to represent a `row` in the settings class LegacyControllerSettingsLayoutContainer : public LegacyControllerSettingsLayoutElement { public: + /// @brief This is a simplified representation of disposition orientation. This used to + /// define how a container orients its children. This is also used by layout + /// items to decide how the label should be rendered alongside the input + /// widget enum Disposition { HORIZONTAL = 0, VERTICAL, @@ -57,6 +64,11 @@ class LegacyControllerSettingsLayoutContainer : public LegacyControllerSettingsL return std::make_unique(*this); } + /// @brief This helper method allows to add a LegacyControllerSetting + /// directly, without to have it to wrap within an item object. This is + /// helpful as the item that will be create to wrap will be initialised with + /// the right parameters + /// @param setting The controller setting to add to the layout container void addItem(std::shared_ptr setting); void addItem(std::unique_ptr&& container) { m_elements.push_back(std::move(container)); @@ -87,7 +99,7 @@ class LegacyControllerSettingsGroup : public LegacyControllerSettingsLayoutConta } QWidget* build(QWidget* parent) override; - + private: QString m_label; }; diff --git a/src/controllers/midi/legacymidicontrollermappingfilehandler.cpp b/src/controllers/midi/legacymidicontrollermappingfilehandler.cpp index 48d97d2ea830..4735a6a32553 100644 --- a/src/controllers/midi/legacymidicontrollermappingfilehandler.cpp +++ b/src/controllers/midi/legacymidicontrollermappingfilehandler.cpp @@ -11,7 +11,7 @@ std::shared_ptr LegacyMidiControllerMappingFileHandler::load(const QDomElement& root, const QString& filePath, const QDir& systemMappingsPath) { - // TODO (acolombier): support for controller settings + // TODO (XXX): support for controller settings if (root.isNull()) { return nullptr; } diff --git a/src/test/controller_mapping_settings_test.cpp b/src/test/controller_mapping_settings_test.cpp index 48214c51ec03..3d36d136d792 100644 --- a/src/test/controller_mapping_settings_test.cpp +++ b/src/test/controller_mapping_settings_test.cpp @@ -6,6 +6,8 @@ // FIXME (acolombier): here we need the CPP file so the templated methods gets // built. AFAIK, there is an alternate solution which is to include the cpp file // in the CMake build list +#include "controllers/legacycontrollermapping.h" +#include "controllers/legacycontrollermappingfilehandler.h" #include "controllers/legacycontrollersettings.cpp" #include "test/mixxxtest.h" @@ -27,6 +29,11 @@ const char* const kValidDouble = ""; +const char* const kValidEnumOption = "%2"; + TEST_F(LegacyControllerMappingSettingsTest, booleanSettingParsing) { QDomDocument doc; doc.setContent(QString(kValidBoolean).arg("false").toLatin1()); @@ -79,6 +86,8 @@ TEST_F(LegacyControllerMappingSettingsTest, booleanSettingEditing) { EXPECT_TRUE(ok); EXPECT_FALSE(setting->isDirty()); EXPECT_EQ(setting->stringify(), "true"); + EXPECT_TRUE(setting->value().isBool()); + EXPECT_TRUE(setting->value().toBool()); EXPECT_FALSE(setting->isDefault()); setting->parse("1", &ok); EXPECT_TRUE(ok); @@ -89,9 +98,14 @@ TEST_F(LegacyControllerMappingSettingsTest, booleanSettingEditing) { setting->parse("TRUE", &ok); EXPECT_TRUE(ok); EXPECT_EQ(setting->stringify(), "true"); + EXPECT_TRUE(setting->value().isBool()); + EXPECT_TRUE(setting->value().toBool()); setting->reset(); EXPECT_EQ(setting->stringify(), "false"); EXPECT_TRUE(setting->isDefault()); + + EXPECT_TRUE(setting->value().isBool()); + EXPECT_FALSE(setting->value().toBool()); } TEST_F(LegacyControllerMappingSettingsTest, integerSettingParsing) { @@ -151,7 +165,46 @@ TEST_F(LegacyControllerMappingSettingsTest, integerSettingParsing) { } TEST_F(LegacyControllerMappingSettingsTest, integerSettingEditing) { - // TODO (acolombier) Add test for setting edition + QDomDocument doc; + doc.setContent( + QByteArray("