Cleaner LUA string->function helpers (#4561)

Cleaner helpers to go string->function
Unit test of that in all the error cases
Apply to the formula modulator
Apply to the wavetable script prototype

Addresses #2048
Addresses #4539

CMakeLists.txt

@@ -878,6 +878,8 @@ if( BUILD_SURGE_XT )
           JUCE_WASAPI=1
           JUCE_DIRECTSOUND=1
 
+          JUCE_CATCH_UNHANDLED_EXCEPTIONS=$<IF:$<CONFIG:DEBUG>,1,0>
+
           TARGET_JUCE_SYNTH=1
           TARGET_HEADLESS=1
           TARGET_JUCE_UI=1

src/common/LuaSupport.cpp

@@ -17,8 +17,69 @@
 #include <iostream>
 #include <string>
 #include <vector>
+#include <sstream>
+#include <cstring>
 #include "basic_dsp.h"
 
+bool Surge::LuaSupport::parseStringDefiningFunction(lua_State *L, const std::string &definition,
+                                                    const std::string &functionName,
+                                                    std::string &errorMessage)
+{
+    const char *lua_script = definition.c_str();
+    auto lerr = luaL_loadbuffer(L, lua_script, strlen(lua_script), "lua-script");
+    if (lerr != LUA_OK)
+    {
+        if (lerr == LUA_ERRSYNTAX)
+        {
+            std::ostringstream oss;
+            oss << "Lua Syntax Error: " << lua_tostring(L, -1);
+            errorMessage = oss.str();
+        }
+        else
+        {
+            std::ostringstream oss;
+            oss << "Lua Unknown Error: " << lua_tostring(L, -1);
+            errorMessage = oss.str();
+        }
+        lua_pop(L, 1);
+        lua_pushnil(L);
+        return false;
+    }
+
+    lerr = lua_pcall(L, 0, 0, 0);
+    if (lerr != LUA_OK)
+    {
+        // FIXME obviously
+        std::ostringstream oss;
+        oss << "Lua Evaluation Error: " << lua_tostring(L, -1);
+        errorMessage = oss.str();
+        lua_pop(L, 1);
+        lua_pushnil(L);
+        return false;
+    }
+
+    lua_getglobal(L, functionName.c_str());
+    if (lua_isfunction(L, -1))
+        return true;
+
+    if (lua_isnil(L, -1))
+    {
+        std::ostringstream oss;
+        oss << "Resolving global name '" << functionName << "' after parse returned nil."
+            << " Did you define the function?";
+        errorMessage = oss.str();
+        return false;
+    }
+
+    std::ostringstream oss;
+    oss << "After trying to find function '" << functionName << "' found non-function type '"
+        << lua_typename(L, lua_type(L, -1)) << "'";
+    errorMessage = oss.str();
+    lua_pop(L, 1);
+    lua_pushnil(L);
+    return false;
+}
+
 int lua_limitRange(lua_State *L)
 {
     auto x = luaL_checknumber(L, -3);

src/common/LuaSupport.h

@@ -43,6 +43,17 @@ namespace LuaSupport
 {
 #if HAS_LUAJIT
 
+/*
+ * Given a string which is supposed to be valid lua defining a function
+ * with a name, parse the string, look for the function, and if the parse
+ * and stuff has no errors, return true with the function on the top of the
+ * stack, oterhwise return false with nil on top of the stack. So increases
+ * stack by 1.
+ */
+
+bool parseStringDefiningFunction(lua_State *s, const std::string &definition,
+                                 const std::string &functionName, std::string &errorMessage);
+
 /*
  * Call this function with the top of your stack being a
  * lua_function and the function will get wrapped in the standard

src/common/dsp/WavetableScriptEvaluator.cpp

@@ -38,61 +38,49 @@ std::vector<float> evaluateScriptAtFrame(const std::string &eqn, int resolution,
     auto values = std::vector<float>();
 
     auto wg = Surge::LuaSupport::SGLD("WavetableScript::evaluate", L);
-    int load_stat = luaL_loadbuffer(L, eqn.c_str(), eqn.length(), "wtScript");
-    if (load_stat != LUA_OK)
+    std::string emsg;
+    auto res = Surge::LuaSupport::parseStringDefiningFunction(L, eqn.c_str(), "generate", emsg);
+    if (res)
     {
-        std::cout << "FIXME: Error Handling " << lua_error(L) << std::endl;
-        return values;
-    }
-    auto res = lua_pcall(L, 0, 0, 0);
-    if (res == LUA_OK)
-    {
-        lua_getglobal(L, "generate");
-        if (lua_isfunction(L, -1))
+        Surge::LuaSupport::setSurgeFunctionEnvironment(L);
+        /*
+         * Alright so we want the stack to be an array of 0...1 and a frame
+         * Right now the stack is just our generation function so
+         */
+        lua_createtable(L, resolution, 0);
+        double dp = 1.0 / (resolution - 1);
+        for (auto i = 0; i < resolution; ++i)
         {
-            Surge::LuaSupport::setSurgeFunctionEnvironment(L);
-            /*
-             * Alright so we want the stack to be an array of 0...1 and a frame
-             * Right now the stack is just our generation function so
-             */
-            lua_createtable(L, resolution, 0);
-            double dp = 1.0 / (resolution - 1);
-            for (auto i = 0; i < resolution; ++i)
-            {
-                lua_pushinteger(L, i + 1); // lua has a 1 based convention
-                lua_pushnumber(L, i * dp);
-                lua_settable(L, -3);
-            }
-            lua_pushinteger(L, frame);
-            auto pcr = lua_pcall(L, 2, 1, 0);
-            if (pcr == LUA_OK)
+            lua_pushinteger(L, i + 1); // lua has a 1 based convention
+            lua_pushnumber(L, i * dp);
+            lua_settable(L, -3);
+        }
+        lua_pushinteger(L, frame);
+        auto pcr = lua_pcall(L, 2, 1, 0);
+        if (pcr == LUA_OK)
+        {
+            if (lua_istable(L, -1))
             {
-                if (lua_istable(L, -1))
+                for (auto i = 0; i < resolution; ++i)
                 {
-                    for (auto i = 0; i < resolution; ++i)
+                    lua_pushinteger(L, i + 1);
+                    lua_gettable(L, -2);
+                    if (lua_isnumber(L, -1))
+                    {
+                        values.push_back(lua_tonumber(L, -1));
+                    }
+                    else
                     {
-                        lua_pushinteger(L, i + 1);
-                        lua_gettable(L, -2);
-                        if (lua_isnumber(L, -1))
-                        {
-                            values.push_back(lua_tonumber(L, -1));
-                        }
-                        else
-                        {
-                            values.push_back(0.f);
-                        }
-                        lua_pop(L, 1);
+                        values.push_back(0.f);
                     }
+                    lua_pop(L, 1);
                 }
             }
-            else
-            {
-                std::cout << "FIXME : Error " << std::endl;
-            }
-        }
-        else
-        {
         }
+    }
+    else
+    {
+        std::cout << emsg << std::endl;
         lua_pop(L, 1);
     }
     return values;

src/common/dsp/modulators/FormulaModulationHelper.cpp

@@ -84,59 +84,39 @@ bool prepareForEvaluation(FormulaModulatorStorage *fs, EvaluatorState &s, bool i
     if (hasString)
     {
         snprintf(s.funcName, TXT_SIZE, "%s", pvf.c_str());
-        s.isvalid = true;
+        // CHECK that I can actually get the function here
+        lua_getglobal(s.L, s.funcName);
+        s.isvalid = lua_isfunction(s.L, -1);
+        lua_pop(s.L, 1);
     }
     else
     {
-        const char *lua_script = fs->formulaString.c_str();
-        int load_stat = luaL_loadbuffer(s.L, lua_script, strlen(lua_script), "processScript");
-        if (load_stat != LUA_OK)
-        {
-            std::ostringstream oss;
-            oss << "Error parsing user script: " << lua_error(s.L);
-            s.adderror(oss.str());
-            std::cout << "FIXME : Do I need to pop here?" << std::endl;
-            return false;
-        }
-        auto res = lua_pcall(s.L, 0, 0, 0); // FIXME error
+        std::string emsg;
+        bool res =
+            Surge::LuaSupport::parseStringDefiningFunction(s.L, fs->formulaString, "process", emsg);
 
-        if (res == LUA_OK)
+        if (res)
         {
-            // great now get the modfunc
-            lua_getglobal(s.L, "process");
-            if (lua_isfunction(s.L, -1))
-            {
-                // Great - rename it and nuke process
-                lua_setglobal(s.L, s.funcName);
-                lua_pushnil(s.L);
-                lua_setglobal(s.L, "process");
+            // Great - rename it and nuke process
+            lua_setglobal(s.L, s.funcName);
+            lua_pushnil(s.L);
+            lua_setglobal(s.L, "process");
 
-                // Then get it and set its env
-                lua_getglobal(s.L, s.funcName);
-                Surge::LuaSupport::setSurgeFunctionEnvironment(s.L);
-                lua_pop(s.L, 1);
+            // Then get it and set its env
+            lua_getglobal(s.L, s.funcName);
+            Surge::LuaSupport::setSurgeFunctionEnvironment(s.L);
+            lua_pop(s.L, 1);
 
-                s.isvalid = true;
-            }
-            else
-            {
-                // FIXME error
-                s.adderror("After parsing formula, no function 'process' present. You must define "
-                           "a function called 'process' in your LUA.");
-                // Pop whatever I got
-                lua_pop(s.L, 1);
-                s.isvalid = false;
-            }
+            s.isvalid = true;
         }
         else
         {
-            std::ostringstream oss;
-            oss << "LUA Raised an error parsing formula: " << lua_tostring(s.L, -1);
-            s.adderror(oss.str());
+            s.adderror(emsg);
+            lua_pop(s.L, 1);
         }
 
         // this happens here because we did parse it at least. Don't parse again until it is changed
-        lua_pushstring(s.L, lua_script);
+        lua_pushstring(s.L, fs->formulaString.c_str());
         lua_setglobal(s.L, pvn.c_str());
     }
 
@@ -230,7 +210,7 @@ float valueAt(int phaseIntPart, float phaseFracPart, FormulaModulatorStorage *fs
      * values; then call my function; then update my global
      */
     lua_getglobal(s->L, s->funcName);
-    if (lua_isnil(s->L, -1))
+    if (!lua_isfunction(s->L, -1))
     {
         s->isvalid = false;
         lua_pop(s->L, 1);