make custom functions in a scope aware of all other custom functions

In a setup with functions:

    f1() = f2(3)

    f2(x) = 1+random(1..x)

Then a variable `q = f1()` seemed to depend on a variable `f2` because
the custom findvars method for `f1` didn't know about `f2` when it was
made.

This commit changes `Numbas.jme.makeFunctions` so that first of
all a temporary scope is made where all the custom functions are
defined, and then the real definitions of those functions are made in
the scope that will be used.

runtime/scripts/jme-variables.js

@@ -167,15 +167,22 @@ jme.variables = /** @lends Numbas.jme.variables */ {
      */
     makeFunctions: function(tmpFunctions,scope,withEnv)
     {
+        var tmpscope = new jme.Scope(scope);
+
         scope = new jme.Scope(scope);
-        var functions = scope.functions;
-        var tmpFunctions2 = [];
-        for(var i=0;i<tmpFunctions.length;i++)
-        {
-            var cfn = jme.variables.makeFunction(tmpFunctions[i],scope,withEnv);
+
+        // Make the list of functions twice, so that on the second pass, the scope knows about every function we'll be defining.
+
+        tmpFunctions.forEach(function(def) {
+            var cfn = jme.variables.makeFunction(def,tmpscope,withEnv);
+            tmpscope.addFunction(cfn);
+        });
+
+        tmpFunctions.forEach(function(def) {
+            var cfn = jme.variables.makeFunction(def,tmpscope,withEnv);
             scope.addFunction(cfn);
-        }
-        return functions;
+        });
+        return scope.functions;
     },
     /** Evaluate a variable, evaluating all its dependencies first.
      *

tests/jme-runtime.js

@@ -14824,7 +14824,10 @@ newBuiltin('image',[TString, '[number]', '[number]'],THTML,null, {
         }
         var subber = new jme.variables.DOMcontentsubber(scope);
         var element = subber.subvars(img);
-        element.setAttribute('data-interactive', 'false');
+
+        // The subber replaces SVG images with <object> tags which have an event listener for when the content loads, so they must be considered interactive.
+        element.setAttribute('data-interactive', element.tagName.toLowerCase() == 'object');
+
         return new THTML(element);
     }
 });
@@ -20089,15 +20092,22 @@ jme.variables = /** @lends Numbas.jme.variables */ {
      */
     makeFunctions: function(tmpFunctions,scope,withEnv)
     {
+        var tmpscope = new jme.Scope(scope);
+
         scope = new jme.Scope(scope);
-        var functions = scope.functions;
-        var tmpFunctions2 = [];
-        for(var i=0;i<tmpFunctions.length;i++)
-        {
-            var cfn = jme.variables.makeFunction(tmpFunctions[i],scope,withEnv);
+
+        // Make the list of functions twice, so that on the second pass, the scope knows about every function we'll be defining.
+
+        tmpFunctions.forEach(function(def) {
+            var cfn = jme.variables.makeFunction(def,tmpscope,withEnv);
+            tmpscope.addFunction(cfn);
+        });
+
+        tmpFunctions.forEach(function(def) {
+            var cfn = jme.variables.makeFunction(def,tmpscope,withEnv);
             scope.addFunction(cfn);
-        }
-        return functions;
+        });
+        return scope.functions;
     },
     /** Evaluate a variable, evaluating all its dependencies first.
      *
@@ -20725,8 +20735,8 @@ DOMcontentsubber.prototype = {
         } else if(element.hasAttribute('nosubvars')) {
             return element;
         } else if(tagName=='img') {
-            if(element.getAttribute('src').match(/.svg$/i)) {
-                element.parentElement
+            const url = new URL(element.getAttribute('src'), window.location);
+            if(url.pathname.match(/\.svg$/i)) {
                 var object = element.ownerDocument.createElement('object');
                 for(var i=0;i<element.attributes.length;i++) {
                     var attr = element.attributes[i];

tests/numbas-runtime.js

@@ -14160,7 +14160,10 @@ newBuiltin('image',[TString, '[number]', '[number]'],THTML,null, {
         }
         var subber = new jme.variables.DOMcontentsubber(scope);
         var element = subber.subvars(img);
-        element.setAttribute('data-interactive', 'false');
+
+        // The subber replaces SVG images with <object> tags which have an event listener for when the content loads, so they must be considered interactive.
+        element.setAttribute('data-interactive', element.tagName.toLowerCase() == 'object');
+
         return new THTML(element);
     }
 });
@@ -19425,15 +19428,22 @@ jme.variables = /** @lends Numbas.jme.variables */ {
      */
     makeFunctions: function(tmpFunctions,scope,withEnv)
     {
+        var tmpscope = new jme.Scope(scope);
+
         scope = new jme.Scope(scope);
-        var functions = scope.functions;
-        var tmpFunctions2 = [];
-        for(var i=0;i<tmpFunctions.length;i++)
-        {
-            var cfn = jme.variables.makeFunction(tmpFunctions[i],scope,withEnv);
+
+        // Make the list of functions twice, so that on the second pass, the scope knows about every function we'll be defining.
+
+        tmpFunctions.forEach(function(def) {
+            var cfn = jme.variables.makeFunction(def,tmpscope,withEnv);
+            tmpscope.addFunction(cfn);
+        });
+
+        tmpFunctions.forEach(function(def) {
+            var cfn = jme.variables.makeFunction(def,tmpscope,withEnv);
             scope.addFunction(cfn);
-        }
-        return functions;
+        });
+        return scope.functions;
     },
     /** Evaluate a variable, evaluating all its dependencies first.
      *
@@ -20061,8 +20071,8 @@ DOMcontentsubber.prototype = {
         } else if(element.hasAttribute('nosubvars')) {
             return element;
         } else if(tagName=='img') {
-            if(element.getAttribute('src').match(/.svg$/i)) {
-                element.parentElement
+            const url = new URL(element.getAttribute('src'), window.location);
+            if(url.pathname.match(/\.svg$/i)) {
                 var object = element.ownerDocument.createElement('object');
                 for(var i=0;i<element.attributes.length;i++) {
                     var attr = element.attributes[i];

tests/parts/part-tests.mjs

@@ -1532,6 +1532,33 @@ return new Promise(resolve => {
         }
     );
 
+    QUnit.module('Variables');
+    question_test(
+        'Custom functions know about other custom functions when finding free variables',
+        {
+            functions: {
+                "f1": {
+                    parameters: [],
+                    type: 'number',
+                    language: 'jme',
+                    definition: 'f2(3)+y',
+                },
+                "f2": {
+                    parameters: [
+                        ['x', 'number']
+                    ],
+                    type: 'number',
+                    language: 'jme',
+                    definition: '1+x+z'
+                }
+            }
+        },
+        async function(assert,q) {
+            const f1_vars = Numbas.jme.findvars(Numbas.jme.compile('f1()'), {}, q.getScope());
+            assert.deepEqual(f1_vars, ['y','z'], 'f1() has free variables y, from the definition of f1, and z, from the definition of f2. f2 is not a free variable.');
+        }
+    );
+
     /** 
      *  Capture signals and events from Numbas.schedule.EventBox/SignalBox objects.
      *  If `owner` is not given, then the prototype trigger method is replaced, catching signals/events on all objects.