use std::shared_ptr for LegacyControllerMapping

src/controllers/bulk/bulkcontroller.cpp

@@ -98,10 +98,22 @@ QString BulkController::mappingExtension() {
     return BULK_MAPPING_EXTENSION;
 }
 
-void BulkController::setMapping(LegacyControllerMapping* pMapping) {
-    auto pHidMapping = dynamic_cast<LegacyHidControllerMapping*>(pMapping);
-    DEBUG_ASSERT(pHidMapping);
-    m_mapping = *pHidMapping;
+void BulkController::setMapping(std::shared_ptr<LegacyControllerMapping> pMapping) {
+    VERIFY_OR_DEBUG_ASSERT(pMapping.use_count() == 1) {
+        return;
+    }
+    auto pDowncastedMapping = std::dynamic_pointer_cast<LegacyHidControllerMapping>(pMapping);
+    VERIFY_OR_DEBUG_ASSERT(pDowncastedMapping) {
+        return;
+    }
+    m_pMapping = pDowncastedMapping;
+}
+
+std::shared_ptr<LegacyControllerMapping> BulkController::cloneMapping() {
+    if (!m_pMapping) {
+        return nullptr;
+    }
+    return m_pMapping->clone();
 }
 
 bool BulkController::matchMapping(const MappingInfo& mapping) {

src/controllers/bulk/bulkcontroller.h

@@ -44,16 +44,14 @@ class BulkController : public Controller {
 
     QString mappingExtension() override;
 
-    LegacyControllerMappingPointer getMapping() const override {
-        LegacyHidControllerMapping* pClone = new LegacyHidControllerMapping();
-        *pClone = m_mapping;
-        return LegacyControllerMappingPointer(pClone);
-    }
-
-    void setMapping(LegacyControllerMapping* pMapping) override;
+    virtual std::shared_ptr<LegacyControllerMapping> cloneMapping() override;
+    void setMapping(std::shared_ptr<LegacyControllerMapping> pMapping) override;
 
     bool isMappable() const override {
-        return m_mapping.isMappable();
+        if (!m_pMapping) {
+            return false;
+        }
+        return m_pMapping->isMappable();
     }
 
     bool matchMapping(const MappingInfo& mapping) override;
@@ -70,12 +68,6 @@ class BulkController : public Controller {
     // 0x0.
     void sendBytes(const QByteArray& data) override;
 
-    // Returns a pointer to the currently loaded controller mapping. For internal
-    // use only.
-    LegacyControllerMapping* mapping() override {
-        return &m_mapping;
-    }
-
     bool matchProductInfo(const ProductInfo& product);
 
     libusb_context* m_context;
@@ -92,5 +84,5 @@ class BulkController : public Controller {
 
     QString m_sUID;
     BulkReader* m_pReader;
-    LegacyHidControllerMapping m_mapping;
+    std::shared_ptr<LegacyHidControllerMapping> m_pMapping;
 };

src/controllers/controller.cpp

@@ -49,7 +49,7 @@ void Controller::stopEngine() {
 bool Controller::applyMapping() {
     qDebug() << "Applying controller mapping...";
 
-    const LegacyControllerMapping* pMapping = mapping();
+    const std::shared_ptr<LegacyControllerMapping> pMapping = cloneMapping();
 
     // Load the script code into the engine
     if (!m_pScriptEngineLegacy) {

src/controllers/controller.h

@@ -30,10 +30,10 @@ class Controller : public QObject {
     /// the controller (type.)
     virtual QString mappingExtension() = 0;
 
-    virtual void setMapping(LegacyControllerMapping* pMapping) = 0;
-
-    // Returns a clone of the Controller's loaded mapping.
-    virtual LegacyControllerMappingPointer getMapping() const = 0;
+    virtual std::shared_ptr<LegacyControllerMapping> cloneMapping() = 0;
+    /// WARNING: LegacyControllerMapping is not thread safe!
+    /// Clone the mapping before passing to setMapping for use in the controller polling thread.
+    virtual void setMapping(std::shared_ptr<LegacyControllerMapping> pMapping) = 0;
 
     inline bool isOpen() const {
         return m_bIsOpen;
@@ -58,10 +58,6 @@ class Controller : public QObject {
     virtual bool matchMapping(const MappingInfo& mapping) = 0;
 
   signals:
-    // Emitted when a new mapping is loaded. pMapping is a /clone/ of the loaded
-    // mapping, not a pointer to the mapping itself.
-    void mappingLoaded(LegacyControllerMappingPointer pMapping);
-
     /// Emitted when the controller is opened or closed.
     void openChanged(bool bOpen);
 
@@ -137,9 +133,6 @@ class Controller : public QObject {
     }
 
   private:
-    // Returns a pointer to the currently loaded controller mapping. For internal
-    // use only.
-    virtual LegacyControllerMapping* mapping() = 0;
     ControllerScriptEngineLegacy* m_pScriptEngineLegacy;
 
     // Verbose and unique device name suitable for display.

src/controllers/controllermanager.cpp

@@ -87,7 +87,8 @@ ControllerManager::ControllerManager(UserSettingsPointer pConfig)
           m_pControllerLearningEventFilter(new ControllerLearningEventFilter()),
           m_pollTimer(this),
           m_skipPoll(false) {
-    qRegisterMetaType<LegacyControllerMappingPointer>("LegacyControllerMappingPointer");
+    qRegisterMetaType<std::shared_ptr<LegacyControllerMapping>>(
+            "std::shared_ptr<LegacyControllerMapping>");
 
     // Create controller mapping paths in the user's home directory.
     QString userMappings = userMappingsPath(m_pConfig);
@@ -262,8 +263,9 @@ void ControllerManager::slotSetUpDevices() {
             continue;
         }
 
-        LegacyControllerMappingPointer pMapping = LegacyControllerMappingFileHandler::loadMapping(
-                mappingFile, resourceMappingsPath(m_pConfig));
+        std::shared_ptr<LegacyControllerMapping> pMapping =
+                LegacyControllerMappingFileHandler::loadMapping(
+                        mappingFile, resourceMappingsPath(m_pConfig));
 
         if (!pMapping) {
             continue;
@@ -397,7 +399,7 @@ void ControllerManager::closeController(Controller* pController) {
 }
 
 void ControllerManager::slotApplyMapping(Controller* pController,
-        LegacyControllerMappingPointer pMapping,
+        std::shared_ptr<LegacyControllerMapping> pMapping,
         bool bEnabled) {
     VERIFY_OR_DEBUG_ASSERT(pController) {
         qWarning() << "slotApplyMapping got invalid controller!";

src/controllers/controllermanager.h

@@ -51,7 +51,7 @@ class ControllerManager : public QObject {
     void updateControllerList();
 
     void slotApplyMapping(Controller* pController,
-            LegacyControllerMappingPointer pMapping,
+            std::shared_ptr<LegacyControllerMapping> pMapping,
             bool bEnabled);
     void openController(Controller* pController);
     void closeController(Controller* pController);

src/controllers/controllermappingtablemodel.cpp

@@ -11,8 +11,8 @@ ControllerMappingTableModel::~ControllerMappingTableModel() {
 
 }
 
-void ControllerMappingTableModel::setMapping(LegacyControllerMappingPointer pMapping) {
-    m_pMidiMapping = dynamic_cast<LegacyMidiControllerMapping*>(pMapping.data());
+void ControllerMappingTableModel::setMapping(std::shared_ptr<LegacyControllerMapping> pMapping) {
+    m_pMidiMapping = std::dynamic_pointer_cast<LegacyMidiControllerMapping>(pMapping);
     // Only legacy MIDI mappings are supported
     // TODO: prevent calling this code for unsupported mapping types?
     if (!m_pMidiMapping) {

src/controllers/controllermappingtablemodel.h

@@ -17,7 +17,7 @@ class ControllerMappingTableModel : public QAbstractTableModel {
     ControllerMappingTableModel(QObject* pParent);
     ~ControllerMappingTableModel() override;
 
-    void setMapping(LegacyControllerMappingPointer pMapping);
+    void setMapping(std::shared_ptr<LegacyControllerMapping> pMapping);
 
     // Revert changes made since the last apply.
     virtual void cancel();
@@ -38,5 +38,5 @@ class ControllerMappingTableModel : public QAbstractTableModel {
     virtual void onMappingLoaded() = 0;
 
     QVector<QHash<int, QVariant> > m_headerInfo;
-    LegacyMidiControllerMapping* m_pMidiMapping;
+    std::shared_ptr<LegacyMidiControllerMapping> m_pMidiMapping;
 };

src/controllers/dlgprefcontroller.cpp

@@ -46,13 +46,7 @@ DlgPrefController::DlgPrefController(
     initTableView(m_ui.m_pInputMappingTableView);
     initTableView(m_ui.m_pOutputMappingTableView);
 
-    connect(m_pController, &Controller::mappingLoaded, this, &DlgPrefController::slotShowMapping);
-    // TODO(rryan): Eh, this really isn't thread safe but it's the way it's been
-    // since 1.11.0. We shouldn't be calling Controller methods because it lives
-    // in a different thread. Booleans (like isOpen()) are fine but a complex
-    // object like a mapping involves QHash's and other data structures that
-    // really don't like concurrent access.
-    LegacyControllerMappingPointer pMapping = m_pController->getMapping();
+    std::shared_ptr<LegacyControllerMapping> pMapping = m_pController->cloneMapping();
     slotShowMapping(pMapping);
 
     m_ui.labelDeviceName->setText(m_pController->getName());
@@ -206,7 +200,7 @@ void DlgPrefController::midiInputMappingsLearned(
 }
 
 QString DlgPrefController::mappingShortName(
-        const LegacyControllerMappingPointer pMapping) const {
+        const std::shared_ptr<LegacyControllerMapping> pMapping) const {
     QString mappingName = tr("None");
     if (pMapping) {
         QString name = pMapping->name();
@@ -224,7 +218,7 @@ QString DlgPrefController::mappingShortName(
 }
 
 QString DlgPrefController::mappingName(
-        const LegacyControllerMappingPointer pMapping) const {
+        const std::shared_ptr<LegacyControllerMapping> pMapping) const {
     if (pMapping) {
         QString name = pMapping->name();
         if (name.length() > 0) {
@@ -235,7 +229,7 @@ QString DlgPrefController::mappingName(
 }
 
 QString DlgPrefController::mappingDescription(
-        const LegacyControllerMappingPointer pMapping) const {
+        const std::shared_ptr<LegacyControllerMapping> pMapping) const {
     if (pMapping) {
         QString description = pMapping->description();
         if (description.length() > 0) {
@@ -246,7 +240,7 @@ QString DlgPrefController::mappingDescription(
 }
 
 QString DlgPrefController::mappingAuthor(
-        const LegacyControllerMappingPointer pMapping) const {
+        const std::shared_ptr<LegacyControllerMapping> pMapping) const {
     if (pMapping) {
         QString author = pMapping->author();
         if (author.length() > 0) {
@@ -257,7 +251,7 @@ QString DlgPrefController::mappingAuthor(
 }
 
 QString DlgPrefController::mappingSupportLinks(
-        const LegacyControllerMappingPointer pMapping) const {
+        const std::shared_ptr<LegacyControllerMapping> pMapping) const {
     if (!pMapping) {
         return QString();
     }
@@ -299,7 +293,7 @@ QString DlgPrefController::mappingSupportLinks(
 }
 
 QString DlgPrefController::mappingFileLinks(
-        const LegacyControllerMappingPointer pMapping) const {
+        const std::shared_ptr<LegacyControllerMapping> pMapping) const {
     if (!pMapping) {
         return QString();
     }
@@ -526,8 +520,9 @@ void DlgPrefController::slotMappingSelected(int chosenIndex) {
         }
     }
 
-    LegacyControllerMappingPointer pMapping = LegacyControllerMappingFileHandler::loadMapping(
-            mappingPath, QDir(resourceMappingsPath(m_pConfig)));
+    std::shared_ptr<LegacyControllerMapping> pMapping =
+            LegacyControllerMappingFileHandler::loadMapping(
+                    mappingPath, QDir(resourceMappingsPath(m_pConfig)));
 
     if (pMapping) {
         DEBUG_ASSERT(!pMapping->isDirty());
@@ -679,21 +674,21 @@ void DlgPrefController::initTableView(QTableView* pTable) {
     pTable->setAlternatingRowColors(true);
 }
 
-void DlgPrefController::slotShowMapping(LegacyControllerMappingPointer mapping) {
-    m_ui.labelLoadedMapping->setText(mappingName(mapping));
-    m_ui.labelLoadedMappingDescription->setText(mappingDescription(mapping));
-    m_ui.labelLoadedMappingAuthor->setText(mappingAuthor(mapping));
-    m_ui.labelLoadedMappingSupportLinks->setText(mappingSupportLinks(mapping));
-    m_ui.labelLoadedMappingScriptFileLinks->setText(mappingFileLinks(mapping));
+void DlgPrefController::slotShowMapping(std::shared_ptr<LegacyControllerMapping> pMapping) {
+    m_ui.labelLoadedMapping->setText(mappingName(pMapping));
+    m_ui.labelLoadedMappingDescription->setText(mappingDescription(pMapping));
+    m_ui.labelLoadedMappingAuthor->setText(mappingAuthor(pMapping));
+    m_ui.labelLoadedMappingSupportLinks->setText(mappingSupportLinks(pMapping));
+    m_ui.labelLoadedMappingScriptFileLinks->setText(mappingFileLinks(pMapping));
 
     // 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.
-    m_pMapping = mapping;
+    m_pMapping = pMapping;
 
     ControllerInputMappingTableModel* pInputModel =
             new ControllerInputMappingTableModel(this);
-    pInputModel->setMapping(mapping);
+    pInputModel->setMapping(pMapping);
 
     QSortFilterProxyModel* pInputProxyModel = new QSortFilterProxyModel(this);
     pInputProxyModel->setSortRole(Qt::UserRole);
@@ -717,7 +712,7 @@ void DlgPrefController::slotShowMapping(LegacyControllerMappingPointer mapping)
 
     ControllerOutputMappingTableModel* pOutputModel =
             new ControllerOutputMappingTableModel(this);
-    pOutputModel->setMapping(mapping);
+    pOutputModel->setMapping(pMapping);
 
     QSortFilterProxyModel* pOutputProxyModel = new QSortFilterProxyModel(this);
     pOutputProxyModel->setSortRole(Qt::UserRole);

src/controllers/dlgprefcontroller.h

@@ -40,7 +40,7 @@ class DlgPrefController : public DlgPreferencePage {
 
   signals:
     void applyMapping(Controller* pController,
-            LegacyControllerMappingPointer pMapping,
+            std::shared_ptr<LegacyControllerMapping> pMapping,
             bool bEnabled);
     void mappingStarted();
     void mappingEnded();
@@ -50,7 +50,7 @@ class DlgPrefController : public DlgPreferencePage {
     void slotMappingSelected(int index);
     /// Used to selected the current mapping in the combobox and display the
     /// mapping information.
-    void slotShowMapping(LegacyControllerMappingPointer mapping);
+    void slotShowMapping(std::shared_ptr<LegacyControllerMapping> mapping);
 
     // Input mappings
     void addInputMapping();
@@ -66,12 +66,12 @@ class DlgPrefController : public DlgPreferencePage {
     void midiInputMappingsLearned(const MidiInputMappings& mappings);
 
   private:
-    QString mappingShortName(const LegacyControllerMappingPointer pMapping) const;
-    QString mappingName(const LegacyControllerMappingPointer pMapping) const;
-    QString mappingAuthor(const LegacyControllerMappingPointer pMapping) const;
-    QString mappingDescription(const LegacyControllerMappingPointer pMapping) const;
-    QString mappingSupportLinks(const LegacyControllerMappingPointer pMapping) const;
-    QString mappingFileLinks(const LegacyControllerMappingPointer pMapping) const;
+    QString mappingShortName(const std::shared_ptr<LegacyControllerMapping> pMapping) const;
+    QString mappingName(const std::shared_ptr<LegacyControllerMapping> pMapping) const;
+    QString mappingAuthor(const std::shared_ptr<LegacyControllerMapping> pMapping) const;
+    QString mappingDescription(const std::shared_ptr<LegacyControllerMapping> pMapping) const;
+    QString mappingSupportLinks(const std::shared_ptr<LegacyControllerMapping> pMapping) const;
+    QString mappingFileLinks(const std::shared_ptr<LegacyControllerMapping> pMapping) const;
     void applyMappingChanges();
     void saveMapping();
     void initTableView(QTableView* pTable);
@@ -111,7 +111,7 @@ class DlgPrefController : public DlgPreferencePage {
     std::shared_ptr<ControllerManager> m_pControllerManager;
     Controller* m_pController;
     DlgControllerLearning* m_pDlgControllerLearning;
-    LegacyControllerMappingPointer m_pMapping;
+    std::shared_ptr<LegacyControllerMapping> m_pMapping;
     QMap<QString, bool> m_pOverwriteMappings;
     ControllerInputMappingTableModel* m_pInputTableModel;
     QSortFilterProxyModel* m_pInputProxyModel;

src/controllers/hid/hidcontroller.cpp

@@ -38,10 +38,22 @@ QString HidController::mappingExtension() {
     return HID_MAPPING_EXTENSION;
 }
 
-void HidController::setMapping(LegacyControllerMapping* pMapping) {
-    auto pHidMapping = dynamic_cast<LegacyHidControllerMapping*>(pMapping);
-    DEBUG_ASSERT(pHidMapping);
-    m_mapping = *pHidMapping;
+void HidController::setMapping(std::shared_ptr<LegacyControllerMapping> pMapping) {
+    VERIFY_OR_DEBUG_ASSERT(pMapping.use_count() == 1) {
+        return;
+    }
+    auto pDowncastedMapping = std::dynamic_pointer_cast<LegacyHidControllerMapping>(pMapping);
+    VERIFY_OR_DEBUG_ASSERT(pDowncastedMapping) {
+        return;
+    }
+    m_pMapping = pDowncastedMapping;
+}
+
+std::shared_ptr<LegacyControllerMapping> HidController::cloneMapping() {
+    if (!m_pMapping) {
+        return nullptr;
+    }
+    return m_pMapping->clone();
 }
 
 bool HidController::matchMapping(const MappingInfo& mapping) {