Merge branch 'master' of github.com:mixxxdj/mixxx into migrate-to-QJS…

…Engine

src/controllers/dlgprefcontroller.h

@@ -34,12 +34,12 @@ class DlgPrefController : public DlgPreferencePage {
     DlgPreferencePage::State state() override;
 
   public slots:
-    // Called when we should apply / save our changes.
-    void slotApply() override;
-    // Called when we should cancel the changes made.
-    void slotCancel() override;
-    // Called when preference dialog (not this dialog) is displayed.
+    /// Called when the preference dialog (not this page) is shown to the user.
     void slotUpdate() override;
+    /// Called when the user clicks the global "Apply" button.
+    void slotApply() override;
+    /// Called when the user clicks the global "Reset to Defaults" button.
+    void slotResetToDefaults() override;
 
   signals:
     void applyPreset(Controller* pController, ControllerPresetPointer pPreset, bool bEnabled);

src/controllers/engine/controllerengine.cpp

@@ -65,25 +65,25 @@ ControllerEngine::~ControllerEngine() {
         m_scratchFilters[i] = nullptr;
     }
 
-    // Delete the script engine, first clearing the pointer so that
-    // other threads will not get the dead pointer after we delete it.
-    if (m_pScriptEngine != nullptr) {
-        QJSEngine* engine = m_pScriptEngine;
-        m_pScriptEngine = nullptr;
-        engine->deleteLater();
-    }
+    uninitializeScriptEngine();
 }
 
 /* -------- ------------------------------------------------------
 Purpose: Calls the same method on a list of JS Objects
 Input:   -
 Output:  -
 -------- ------------------------------------------------------ */
-void ControllerEngine::callFunctionOnObjects(QList<QString> scriptFunctionPrefixes,
+bool ControllerEngine::callFunctionOnObjects(QList<QString> scriptFunctionPrefixes,
         const QString& function,
-        QJSValueList args) {
+        QJSValueList args,
+        bool bFatalError) {
+    VERIFY_OR_DEBUG_ASSERT(m_pScriptEngine) {
+        return false;
+    }
+
     const QJSValue global = m_pScriptEngine->globalObject();
 
+    bool success = true;
     for (const QString& prefixName : scriptFunctionPrefixes) {
         QJSValue prefix = global.property(prefixName);
         if (!prefix.isObject()) {
@@ -99,9 +99,11 @@ void ControllerEngine::callFunctionOnObjects(QList<QString> scriptFunctionPrefix
         controllerDebug("ControllerEngine: Executing" << prefixName << "." << function);
         QJSValue result = init.callWithInstance(prefix, args);
         if (result.isError()) {
-            showScriptExceptionDialog(result);
+            showScriptExceptionDialog(result, bFatalError);
+            success = false;
         }
     }
+    return success;
 }
 
 QJSValue ControllerEngine::byteArrayToScriptValue(const QByteArray byteArray) {
@@ -130,6 +132,12 @@ Output:  QJSValue of JS snippet wrapped in an anonymous function
 ------------------------------------------------------------------- */
 QJSValue ControllerEngine::wrapFunctionCode(const QString& codeSnippet,
         int numberOfArgs) {
+    // This function is called from outside the controller engine, so we can't
+    // use VERIFY_OR_DEBUG_ASSERT here
+    if (m_pScriptEngine == nullptr) {
+        return QJSValue();
+    }
+
     QJSValue wrappedFunction;
 
     auto i = m_scriptWrappedFunctionCache.constFind(codeSnippet);
@@ -160,7 +168,7 @@ Input:   -
 Output:  -
 -------- ------------------------------------------------------ */
 void ControllerEngine::gracefulShutdown() {
-    if (m_pEngine == nullptr) {
+    if (m_pScriptEngine == nullptr) {
         return;
     }
 
@@ -203,8 +211,9 @@ void ControllerEngine::gracefulShutdown() {
 }
 
 void ControllerEngine::initializeScriptEngine() {
-    // Clear any errors from previous script engine usages
-    m_scriptErrors.clear();
+    VERIFY_OR_DEBUG_ASSERT(!m_pScriptEngine) {
+        return;
+    }
 
     // Create the Script Engine
     m_pScriptEngine = new QJSEngine(this);
@@ -235,9 +244,9 @@ void ControllerEngine::initializeScriptEngine() {
 void ControllerEngine::uninitializeScriptEngine() {
     // Delete the script engine, first clearing the pointer so that
     // other threads will not get the dead pointer after we delete it.
-    if (m_pEngine != nullptr) {
-        QScriptEngine* engine = m_pEngine;
-        m_pEngine = nullptr;
+    if (m_pScriptEngine != nullptr) {
+        QJSEngine* engine = m_pScriptEngine;
+        m_pScriptEngine = nullptr;
         engine->deleteLater();
     }
 }
@@ -259,8 +268,7 @@ bool ControllerEngine::loadScriptFiles(const QList<ControllerPreset::ScriptFileI
 
     connect(&m_scriptWatcher, &QFileSystemWatcher::fileChanged, this, &ControllerEngine::scriptHasChanged);
 
-    bool success = result && m_scriptErrors.isEmpty();
-    if (!success) {
+    if (!scriptsEvaluatedCorrectly) {
         gracefulShutdown();
         uninitializeScriptEngine();
     }
@@ -280,14 +288,7 @@ void ControllerEngine::reloadScripts() {
     ControllerPresetPointer pPreset = m_pController->getPreset();
 
     gracefulShutdown();
-
-    // Delete the script engine, first clearing the pointer so that
-    // other threads will not get the dead pointer after we delete it.
-    if (m_pScriptEngine != nullptr) {
-        QJSEngine* engine = m_pScriptEngine;
-        m_pScriptEngine = nullptr;
-        engine->deleteLater();
-    }
+    uninitializeScriptEngine();
 
     initializeScriptEngine();
     if (!loadScriptFiles(m_lastScriptFiles)) {
@@ -318,18 +319,20 @@ void ControllerEngine::initializeScripts(const QList<ControllerPreset::ScriptFil
     args << QJSValue(ControllerDebug::enabled());
 
     // Call the init method for all the prefixes.
-    callFunctionOnObjects(m_scriptFunctionPrefixes, "init", args);
+    bool success = callFunctionOnObjects(m_scriptFunctionPrefixes, "init", args, true);
 
     // We failed to initialize the controller scripts, shutdown the script
     // engine to avoid error popups on every button press or slider move
-    if (checkException(true)) {
+    if (!success) {
         gracefulShutdown();
         uninitializeScriptEngine();
     }
 }
 
 bool ControllerEngine::executeFunction(QJSValue functionObject, QJSValueList args) {
-    VERIFY_OR_DEBUG_ASSERT(!(m_pScriptEngine == nullptr)) {
+    // This function is called from outside the controller engine, so we can't
+    // use VERIFY_OR_DEBUG_ASSERT here
+    if (!m_pScriptEngine) {
         return false;
     }
 
@@ -357,7 +360,9 @@ bool ControllerEngine::executeFunction(QJSValue functionObject, QJSValueList arg
 }
 
 bool ControllerEngine::executeFunction(QJSValue functionObject, const QByteArray data) {
-    VERIFY_OR_DEBUG_ASSERT(!(m_pScriptEngine == nullptr)) {
+    // This function is called from outside the controller engine, so we can't
+    // use VERIFY_OR_DEBUG_ASSERT here
+    if (!m_pScriptEngine) {
         return false;
     }
     QJSValueList args;
@@ -379,14 +384,14 @@ void ControllerEngine::throwJSError(const QString& message) {
     QString errorText = tr("Uncaught exception: %1").arg(message);
     qWarning() << "ControllerEngine:" << errorText;
     if (!m_bDisplayingExceptionDialog) {
-        scriptErrorDialog(errorText);
+        scriptErrorDialog(errorText, errorText);
     }
 #else
     m_pScriptEngine->throwError(message);
 #endif
 }
 
-void ControllerEngine::showScriptExceptionDialog(QJSValue evaluationResult) {
+void ControllerEngine::showScriptExceptionDialog(QJSValue evaluationResult, bool bFatalError) {
     VERIFY_OR_DEBUG_ASSERT(evaluationResult.isError()) {
         return;
     }
@@ -396,21 +401,26 @@ void ControllerEngine::showScriptExceptionDialog(QJSValue evaluationResult) {
     QString backtrace = evaluationResult.property("stack").toString();
     QString filename = evaluationResult.property("fileName").toString();
 
-    QString errorText = tr("Uncaught exception at line %1 in file %2: %3")
-                                .arg(line, (filename.isEmpty() ? "" : filename), errorMessage);
+    QString errorText;
+    if (filename.isEmpty()) {
+        errorText = QString("Uncaught exception at line %1 in passed code.").arg(line);
+    } else {
+        errorText = QString("Uncaught exception at line %1 in file %2.").arg(line, filename);
+    }
 
-    if (filename.isEmpty())
-        errorText = tr("Uncaught exception at line %1 in passed code: %2")
-                            .arg(line, errorMessage);
+    errorText += QStringLiteral("\n\nException:\n  ") + errorMessage;
 
-    QString detailedError = ControllerDebug::enabled() ? QString("%1\nBacktrace:\n%2")
-                                                                 .arg(errorText, backtrace)
-                                                       : errorText;
+    // Do not include backtrace in dialog key because it might contain midi
+    // slider values that will differ most of the time. This would break
+    // the "Ignore" feature of the error dialog.
+    QString key = errorText;
+    qWarning() << "ControllerEngine:" << errorText;
 
-    qWarning() << "ControllerEngine:" << detailedError;
+    // Add backtrace to the error details
+    errorText += QStringLiteral("\n\nBacktrace:\n") + backtrace;
 
     if (!m_bDisplayingExceptionDialog) {
-        scriptErrorDialog(detailedError);
+        scriptErrorDialog(errorText, key, bFatalError);
     }
 }
 
@@ -420,22 +430,55 @@ void ControllerEngine::showScriptExceptionDialog(QJSValue evaluationResult) {
     Input:   Detailed error string
     Output:  -
     -------- ------------------------------------------------------ */
-void ControllerEngine::scriptErrorDialog(const QString& detailedError) {
-    ErrorDialogProperties* props = ErrorDialogHandler::instance()->newDialogProperties();
+void ControllerEngine::scriptErrorDialog(
+        const QString& detailedError, const QString& key, bool bFatalError) {
+    if (m_bTesting) {
+        return;
+    }
+
+    ErrorDialogProperties* props =
+            ErrorDialogHandler::instance()->newDialogProperties();
+
+    QString additionalErrorText;
+    if (bFatalError) {
+        additionalErrorText =
+                tr("The functionality provided by this controller mapping will "
+                   "be disabled until the issue has been resolved.");
+    } else {
+        additionalErrorText =
+                tr("You can ignore this error for this session but "
+                   "you may experience erratic behavior.") +
+                QString("<br>") +
+                tr("Try to recover by resetting your controller.");
+    }
+
     props->setType(DLG_WARNING);
-    props->setTitle(tr("Controller script error"));
-    props->setText(tr("The mapping for the %1 controller is not working properly.").arg(m_pController->getName()));
-    props->setInfoText("<html>" + tr("The script code needs to be fixed.") +
-            "<p>" + tr("For now, you can: Ignore this error for this session but you may experience erratic behavior.") +
-            "<br>" + tr("Try to recover by resetting your controller.") + "</p>" + "</html>");
-    props->setDetails(detailedError);
-    props->setKey(detailedError); // To prevent multiple windows for the same error
+    props->setTitle(tr("Controller Preset Error"));
+    props->setText(QString(tr("The preset for your controller \"%1\" is not "
+                              "working properly."))
+                           .arg(m_pController->getName()));
+    props->setInfoText(QStringLiteral("<html>") +
+            tr("The script code needs to be fixed.") + QStringLiteral("<p>") +
+            additionalErrorText + QStringLiteral("</p></html>"));
+
+    // Add "Details" text and set monospace font since they may contain
+    // backtraces and code.
+    props->setDetails(detailedError, true);
+
+    // To prevent multiple windows for the same error
+    props->setKey(key);
 
     // Allow user to suppress further notifications about this particular error
-    props->addButton(QMessageBox::Ignore);
-    props->addButton(QMessageBox::Retry);
-    props->setDefaultButton(QMessageBox::Ignore);
-    props->setEscapeButton(QMessageBox::Ignore);
+    if (!bFatalError) {
+        props->addButton(QMessageBox::Ignore);
+        props->addButton(QMessageBox::Retry);
+        props->setDefaultButton(QMessageBox::Ignore);
+        props->setEscapeButton(QMessageBox::Ignore);
+    } else {
+        props->addButton(QMessageBox::Close);
+        props->setDefaultButton(QMessageBox::Close);
+        props->setEscapeButton(QMessageBox::Close);
+    }
     props->setModal(false);
 
     if (ErrorDialogHandler::instance()->requestErrorDialog(props)) {
@@ -863,7 +906,7 @@ void ControllerEngine::trigger(QString group, QString name) {
    Output:  false if the script file has errors or doesn't exist
    -------- ------------------------------------------------------ */
 bool ControllerEngine::evaluateScriptFile(const QFileInfo& scriptFile) {
-    VERIFY_OR_DEBUG_ASSERT(!(m_pScriptEngine == nullptr)) {
+    VERIFY_OR_DEBUG_ASSERT(m_pScriptEngine) {
         return false;
     }
 
@@ -884,9 +927,20 @@ bool ControllerEngine::evaluateScriptFile(const QFileInfo& scriptFile) {
         // Set up error dialog
         ErrorDialogProperties* props = ErrorDialogHandler::instance()->newDialogProperties();
         props->setType(DLG_WARNING);
-        props->setTitle("Controller script file problem");
-        props->setText(QString("There was a problem opening the controller script file %1.").arg(filename));
-        props->setInfoText(input.errorString());
+        props->setTitle(tr("Controller Mapping File Problem"));
+        props->setText(tr("The mapping for controller \"%1\" cannot be opened.")
+                               .arg(m_pController->getName()));
+        props->setInfoText(
+                tr("The functionality provided by this controller mapping will "
+                   "be disabled until the issue has been resolved."));
+
+        // We usually don't translate the details field, but the cause of
+        // this problem lies in the user's system (e.g. a permission
+        // issue). Translating this will help users to fix the issue even
+        // when they don't speak english.
+        props->setDetails(tr("File:") + QStringLiteral(" ") + filename +
+                QStringLiteral("\n") + tr("Error:") + QStringLiteral(" ") +
+                input.errorString());
 
         // Ask above layer to display the dialog & handle user response
         ErrorDialogHandler::instance()->requestErrorDialog(props);
@@ -901,7 +955,7 @@ bool ControllerEngine::evaluateScriptFile(const QFileInfo& scriptFile) {
     // Evaluate the code
     QJSValue scriptFunction = evaluateCodeString(scriptCode, filename);
     if (scriptFunction.isError()) {
-        showScriptExceptionDialog(scriptFunction);
+        showScriptExceptionDialog(scriptFunction, true);
         return false;
     }
 
@@ -1023,7 +1077,7 @@ bool ControllerEngine::isDeckPlaying(const QString& group) {
 
     if (pPlay == nullptr) {
         QString error = QString("Could not getControlObjectScript()");
-        scriptErrorDialog(error);
+        scriptErrorDialog(error, error);
         return false;
     }
 

src/controllers/engine/controllerengine.h

@@ -150,23 +150,24 @@ class ControllerEngine : public QObject {
     void gracefulShutdown();
     void scriptHasChanged(const QString&);
 
-  signals:
-    void initialized();
-
   private slots:
     void errorDialogButton(const QString& key, QMessageBox::StandardButton button);
 
   private:
     bool evaluateScriptFile(const QFileInfo& scriptFile);
     void initializeScriptEngine();
+    void uninitializeScriptEngine();
     void reloadScripts();
 
     void scriptErrorDialog(const QString& detailedError, const QString& key, bool bFatal = false);
     void generateScriptFunctions(const QString& code);
     // Stops and removes all timers (for shutdown).
     void stopAllTimers();
 
-    void callFunctionOnObjects(QList<QString>, const QString&, QJSValueList args = QJSValueList());
+    bool callFunctionOnObjects(QList<QString>,
+            const QString&,
+            QJSValueList args = QJSValueList(),
+            bool bFatalError = false);
     // Convert a byteArray to a JS typed array over an ArrayBuffer
     QJSValue byteArrayToScriptValue(const QByteArray byteArray);
     QJSValue evaluateCodeString(const QString& program, const QString& fileName = QString(), int lineNumber = 1);
@@ -175,7 +176,7 @@ class ControllerEngine : public QObject {
 
     // Shows a UI dialog notifying of a script evaluation error.
     // Precondition: QJSValue.isError() == true
-    void showScriptExceptionDialog(QJSValue evaluationResult);
+    void showScriptExceptionDialog(QJSValue evaluationResult, bool bFatal = false);
     bool m_bDisplayingExceptionDialog;
     QJSEngine* m_pScriptEngine;