fix(controllers): engine.beginTimer(ms, string) would evaluate the code immediately instead of after ms

res/controllers/Hercules-DJ-Control-MP3-hid-scripts.js

@@ -142,7 +142,7 @@ HerculesMP3Hid.init = function() {
 
             if (v < 30) v = 30;
             if (HerculesMP3Hid.track_timer) engine.stopTimer(HerculesMP3Hid.track_timer);
-            HerculesMP3Hid.track_timer = engine.beginTimer(parseInt(5120 / v), 'HerculesMP3Hid.scroll_tracks_joystick');
+            HerculesMP3Hid.track_timer = engine.beginTimer(parseInt(5120 / v), HerculesMP3Hid.scroll_tracks_joystick);
         }
     });
     */
@@ -338,7 +338,9 @@ HerculesMP3Hid.scroll_tracks = function(g, e, v) {
     if (v > 0) {
         engine.setValue("[Playlist]", e == "track_next_a" ? "SelectNextTrack" : "SelectPrevTrack", 1);
         if (!HerculesMP3Hid.scroll_timer) {
-            HerculesMP3Hid.scroll_timer = engine.beginTimer(150, 'HerculesMP3Hid.scroll_tracks("[Playlist]","' + e + '",' + v + ')');
+            HerculesMP3Hid.scroll_timer = engine.beginTimer(150, function () {
+                HerculesMP3Hid.scroll_tracks("[Playlist]", e, v);
+            });
         }
     }
     else {

src/controllers/scripting/controllerscriptenginebase.cpp

@@ -77,16 +77,12 @@ bool ControllerScriptEngineBase::executeFunction(
         return false;
     }
 
-    if (functionObject.isError()) {
-        qDebug() << "ControllerScriptHandlerBase::executeFunction:"
-                 << functionObject.toString();
-        return false;
-    }
-
-    // If it's not a function, we're done.
-    if (!functionObject.isCallable()) {
-        qDebug() << "ControllerScriptHandlerBase::executeFunction:"
-                 << functionObject.toVariant() << "Not a function";
+    const bool isError = functionObject.isError();
+    const bool isCallable = functionObject.isCallable();
+    if (isError || !isCallable) {
+        logOrThrowError((isError ? QStringLiteral("\"%1\" resulted in an error")
+                                 : QStringLiteral("\"%1\" is not callable"))
+                                .arg(functionObject.toString()));
         return false;
     }
 
@@ -129,6 +125,14 @@ void ControllerScriptEngineBase::showScriptExceptionDialog(
     }
 }
 
+void ControllerScriptEngineBase::logOrThrowError(const QString& errorMessage) {
+    if (m_bAbortOnWarning) {
+        throwJSError(errorMessage);
+    } else {
+        qCWarning(m_logger) << errorMessage;
+    }
+}
+
 void ControllerScriptEngineBase::scriptErrorDialog(
         const QString& detailedError, const QString& key, bool bFatalError) {
     if (m_bTesting) {

src/controllers/scripting/controllerscriptenginebase.h

@@ -48,6 +48,7 @@ class ControllerScriptEngineBase : public QObject {
     virtual void shutdown();
 
     void scriptErrorDialog(const QString& detailedError, const QString& key, bool bFatal = false);
+    void logOrThrowError(const QString& errorMessage);
 
     bool m_bDisplayingExceptionDialog;
     std::shared_ptr<QJSEngine> m_pJSEngine;

src/controllers/scripting/legacy/controllerscriptinterfacelegacy.cpp

@@ -110,7 +110,9 @@ ControlObjectScript* ControllerScriptInterfaceLegacy::getControlObjectScript(
 double ControllerScriptInterfaceLegacy::getValue(const QString& group, const QString& name) {
     ControlObjectScript* coScript = getControlObjectScript(group, name);
     if (coScript == nullptr) {
-        logOrThrowError(QStringLiteral("Unknown control (%1, %2) returning 0.0").arg(group, name));
+        m_pScriptEngineLegacy->logOrThrowError(
+                QStringLiteral("Unknown control (%1, %2) returning 0.0")
+                        .arg(group, name));
         return 0.0;
     }
     return coScript->get();
@@ -119,9 +121,9 @@ double ControllerScriptInterfaceLegacy::getValue(const QString& group, const QSt
 void ControllerScriptInterfaceLegacy::setValue(
         const QString& group, const QString& name, double newValue) {
     if (util_isnan(newValue)) {
-        logOrThrowError(QStringLiteral(
+        m_pScriptEngineLegacy->logOrThrowError(QStringLiteral(
                 "Script tried setting (%1, %2) to NotANumber (NaN)")
-                                .arg(group, name));
+                                                       .arg(group, name));
         return;
     }
 
@@ -141,7 +143,9 @@ void ControllerScriptInterfaceLegacy::setValue(
 double ControllerScriptInterfaceLegacy::getParameter(const QString& group, const QString& name) {
     ControlObjectScript* coScript = getControlObjectScript(group, name);
     if (coScript == nullptr) {
-        logOrThrowError(QStringLiteral("Unknown control (%1, %2) returning 0.0").arg(group, name));
+        m_pScriptEngineLegacy->logOrThrowError(
+                QStringLiteral("Unknown control (%1, %2) returning 0.0")
+                        .arg(group, name));
         return 0.0;
     }
     return coScript->getParameter();
@@ -150,9 +154,9 @@ double ControllerScriptInterfaceLegacy::getParameter(const QString& group, const
 void ControllerScriptInterfaceLegacy::setParameter(
         const QString& group, const QString& name, double newParameter) {
     if (util_isnan(newParameter)) {
-        logOrThrowError(QStringLiteral(
+        m_pScriptEngineLegacy->logOrThrowError(QStringLiteral(
                 "Script tried setting (%1, %2) to NotANumber (NaN)")
-                                .arg(group, name));
+                                                       .arg(group, name));
         return;
     }
 
@@ -170,16 +174,18 @@ void ControllerScriptInterfaceLegacy::setParameter(
 double ControllerScriptInterfaceLegacy::getParameterForValue(
         const QString& group, const QString& name, double value) {
     if (util_isnan(value)) {
-        logOrThrowError(QStringLiteral(
+        m_pScriptEngineLegacy->logOrThrowError(QStringLiteral(
                 "Script tried setting (%1, %2) to NotANumber (NaN)")
-                                .arg(group, name));
+                                                       .arg(group, name));
         return 0.0;
     }
 
     ControlObjectScript* coScript = getControlObjectScript(group, name);
 
     if (coScript == nullptr) {
-        logOrThrowError(QStringLiteral("Unknown control (%1, %2) returning 0.0").arg(group, name));
+        m_pScriptEngineLegacy->logOrThrowError(
+                QStringLiteral("Unknown control (%1, %2) returning 0.0")
+                        .arg(group, name));
         return 0.0;
     }
 
@@ -197,7 +203,9 @@ double ControllerScriptInterfaceLegacy::getDefaultValue(const QString& group, co
     ControlObjectScript* coScript = getControlObjectScript(group, name);
 
     if (coScript == nullptr) {
-        logOrThrowError(QStringLiteral("Unknown control (%1, %2) returning 0.0").arg(group, name));
+        m_pScriptEngineLegacy->logOrThrowError(
+                QStringLiteral("Unknown control (%1, %2) returning 0.0")
+                        .arg(group, name));
         return 0.0;
     }
 
@@ -209,7 +217,9 @@ double ControllerScriptInterfaceLegacy::getDefaultParameter(
     ControlObjectScript* coScript = getControlObjectScript(group, name);
 
     if (coScript == nullptr) {
-        logOrThrowError(QStringLiteral("Unknown control (%1, %2) returning 0.0").arg(group, name));
+        m_pScriptEngineLegacy->logOrThrowError(
+                QStringLiteral("Unknown control (%1, %2) returning 0.0")
+                        .arg(group, name));
         return 0.0;
     }
 
@@ -238,7 +248,7 @@ QJSValue ControllerScriptInterfaceLegacy::makeConnectionInternal(
         // The test setups do not run all of Mixxx, so ControlObjects not
         // existing during tests is okay.
         if (!m_pScriptEngineLegacy->isTesting()) {
-            logOrThrowError(
+            m_pScriptEngineLegacy->logOrThrowError(
                     QStringLiteral("script tried to connect to ControlObject "
                                    "(%1, %2) which is non-existent.")
                             .arg(group, name));
@@ -247,10 +257,10 @@ QJSValue ControllerScriptInterfaceLegacy::makeConnectionInternal(
     }
 
     if (!callback.isCallable()) {
-        logOrThrowError(QStringLiteral(
+        m_pScriptEngineLegacy->logOrThrowError(QStringLiteral(
                 "Tried to connect (%1, %2) to an invalid callback. Make sure "
                 "that your code contains no syntax errors.")
-                                .arg(group, name));
+                                                       .arg(group, name));
         return QJSValue();
     }
 
@@ -291,10 +301,10 @@ void ControllerScriptInterfaceLegacy::triggerScriptConnection(
     ControlObjectScript* coScript =
             getControlObjectScript(connection.key.group, connection.key.item);
     if (coScript == nullptr) {
-        logOrThrowError(QStringLiteral(
+        m_pScriptEngineLegacy->logOrThrowError(QStringLiteral(
                 "Script tried to trigger (%1, %2) which is non-existent.")
-                                .arg(connection.key.group,
-                                        connection.key.item));
+                                                       .arg(connection.key.group,
+                                                               connection.key.item));
         return;
     }
 
@@ -313,10 +323,10 @@ QJSValue ControllerScriptInterfaceLegacy::connectControl(const QString& group,
         const QString& name,
         const QJSValue& passedCallback,
         bool disconnect) {
-    logOrThrowError(QStringLiteral(
+    m_pScriptEngineLegacy->logOrThrowError(QStringLiteral(
             "Script tried to connect to (%1, %2) using `connectControl` which "
             "is deprecated. Use `makeConnection` instead!")
-                            .arg(group, name));
+                                                   .arg(group, name));
 
     // The passedCallback may or may not actually be a function, so when
     // the actual callback function is found, store it in this variable.
@@ -435,33 +445,31 @@ QJSValue ControllerScriptInterfaceLegacy::connectControl(const QString& group,
 void ControllerScriptInterfaceLegacy::trigger(const QString& group, const QString& name) {
     ControlObjectScript* coScript = getControlObjectScript(group, name);
     if (coScript == nullptr) {
-        logOrThrowError(QStringLiteral(
+        m_pScriptEngineLegacy->logOrThrowError(QStringLiteral(
                 "Script tried to trigger (%1, %2) which is non-existent.")
-                                .arg(group, name));
+                                                       .arg(group, name));
         return;
     }
     coScript->emitValueChanged();
 }
 
-void ControllerScriptInterfaceLegacy::logOrThrowError(const QString& errorMessage) const {
-    if (m_pScriptEngineLegacy->willAbortOnWarning()) {
-        m_pScriptEngineLegacy->throwJSError(errorMessage);
-    } else {
-        qCWarning(m_logger) << errorMessage;
-    }
-}
-
 void ControllerScriptInterfaceLegacy::log(const QString& message) {
-    logOrThrowError(QStringLiteral("`engine.log` is deprecated. Use `console.log` instead!"));
+    m_pScriptEngineLegacy->logOrThrowError(QStringLiteral(
+            "`engine.log` is deprecated. Use `console.log` instead!"));
     qCDebug(m_logger) << message;
 }
 int ControllerScriptInterfaceLegacy::beginTimer(
         int intervalMillis, QJSValue timerCallback, bool oneShot) {
     if (timerCallback.isString()) {
-        logOrThrowError(
+        m_pScriptEngineLegacy->logOrThrowError(
                 QStringLiteral("passed a string to `engine.beginTimer`, please "
                                "pass a function instead!"));
-        timerCallback = m_pScriptEngineLegacy->jsEngine()->evaluate(timerCallback.toString());
+        // wrap the code in a function to make the evaluation lazy.
+        // otherwise the code would be evaluated immediately instead of after
+        // the timer which is obviously undesired and could also cause
+        // issues when used recursively.
+        timerCallback = m_pScriptEngineLegacy->jsEngine()->evaluate(
+                QStringLiteral("()=>%1").arg(timerCallback.toString()));
     } else if (!timerCallback.isCallable()) {
         QString sErrorMessage(
                 "Invalid timer callback provided to engine.beginTimer. Valid "
@@ -489,7 +497,7 @@ int ControllerScriptInterfaceLegacy::beginTimer(
     info.oneShot = oneShot;
     m_timers[timerId] = info;
     if (timerId == 0) {
-        logOrThrowError(QStringLiteral("Script timer could not be created"));
+        m_pScriptEngineLegacy->logOrThrowError(QStringLiteral("Script timer could not be created"));
     } else if (oneShot) {
         qCDebug(m_logger) << "Starting one-shot timer:" << timerId;
     } else {
@@ -500,9 +508,9 @@ int ControllerScriptInterfaceLegacy::beginTimer(
 
 void ControllerScriptInterfaceLegacy::stopTimer(int timerId) {
     if (!m_timers.contains(timerId)) {
-        logOrThrowError(QStringLiteral(
+        m_pScriptEngineLegacy->logOrThrowError(QStringLiteral(
                 "Tried to kill Timer \"%1\" that does not exists")
-                                .arg(timerId));
+                                                       .arg(timerId));
         return;
     }
     qCDebug(m_logger) << "Killing timer:" << timerId;

src/controllers/scripting/legacy/controllerscriptinterfacelegacy.h

@@ -83,7 +83,6 @@ class ControllerScriptInterfaceLegacy : public QObject {
             bool skipSuperseded = false);
     QHash<ConfigKey, ControlObjectScript*> m_controlCache;
     ControlObjectScript* getControlObjectScript(const QString& group, const QString& name);
-    void logOrThrowError(const QString& errorMessage) const;
 
     SoftTakeoverCtrl m_st;