Merge pull request #1839 from mzgoddard/fix-missing-no-param

fix: initialize stack frame params for all procedures

src/blocks/scratch3_procedures.js

@@ -38,6 +38,10 @@ class Scratch3ProcedureBlocks {
 
             const [paramNames, paramIds, paramDefaults] = paramNamesIdsAndDefaults;
 
+            // Initialize params for the current stackFrame to {}, even if the procedure does
+            // not take any arguments. This is so that `getParam` down the line does not look
+            // at earlier stack frames for the values of a given parameter (#1729)
+            util.initParams();
             for (let i = 0; i < paramIds.length; i++) {
                 if (args.hasOwnProperty(paramIds[i])) {
                     util.pushParam(paramNames[i], args[paramIds[i]]);

src/engine/block-utility.js

@@ -170,6 +170,13 @@ class BlockUtility {
         return this.thread.target.blocks.getProcedureParamNamesIdsAndDefaults(procedureCode);
     }
 
+    /**
+     * Initialize procedure parameters in the thread before pushing parameters.
+     */
+    initParams () {
+        this.thread.initParams();
+    }
+
     /**
      * Store a procedure parameter value by its name.
      * @param {string} paramName The procedure's parameter name.

src/engine/thread.js

@@ -321,6 +321,16 @@ class Thread {
         this.justReported = typeof value === 'undefined' ? null : value;
     }
 
+    /**
+     * Initialize procedure parameters on this stack frame.
+     */
+    initParams () {
+        const stackFrame = this.peekStackFrame();
+        if (stackFrame.params === null) {
+            stackFrame.params = {};
+        }
+    }
+
     /**
      * Add a parameter to the stack frame.
      * Use when calling a procedure with parameter values.
@@ -329,9 +339,6 @@ class Thread {
      */
     pushParam (paramName, value) {
         const stackFrame = this.peekStackFrame();
-        if (stackFrame.params === null) {
-            stackFrame.params = {};
-        }
         stackFrame.params[paramName] = value;
     }
 

test/unit/engine_thread.js

@@ -5,7 +5,7 @@ const Sprite = require('../../src/sprites/sprite');
 
 test('spec', t => {
     t.type(Thread, 'function');
-    
+
     const th = new Thread('arbitraryString');
     t.type(th, 'object');
     t.ok(th instanceof Thread);
@@ -17,20 +17,21 @@ test('spec', t => {
     t.type(th.peekStackFrame, 'function');
     t.type(th.peekParentStackFrame, 'function');
     t.type(th.pushReportedValue, 'function');
+    t.type(th.initParams, 'function');
     t.type(th.pushParam, 'function');
     t.type(th.peekStack, 'function');
     t.type(th.getParam, 'function');
     t.type(th.atStackTop, 'function');
     t.type(th.goToNextBlock, 'function');
     t.type(th.isRecursiveCall, 'function');
-    
+
     t.end();
 });
 
 test('pushStack', t => {
     const th = new Thread('arbitraryString');
     th.pushStack('arbitraryString');
-    
+
     t.end();
 });
 
@@ -39,7 +40,7 @@ test('popStack', t => {
     th.pushStack('arbitraryString');
     t.strictEquals(th.popStack(), 'arbitraryString');
     t.strictEquals(th.popStack(), undefined);
-    
+
     t.end();
 });
 
@@ -50,7 +51,7 @@ test('atStackTop', t => {
     t.strictEquals(th.atStackTop(), false);
     th.popStack();
     t.strictEquals(th.atStackTop(), true);
-    
+
     t.end();
 });
 
@@ -59,7 +60,7 @@ test('reuseStackForNextBlock', t => {
     th.pushStack('arbitraryString');
     th.reuseStackForNextBlock('secondString');
     t.strictEquals(th.popStack(), 'secondString');
-    
+
     t.end();
 });
 
@@ -69,7 +70,7 @@ test('peekStackFrame', t => {
     t.strictEquals(th.peekStackFrame().warpMode, false);
     th.popStack();
     t.strictEquals(th.peekStackFrame(), null);
-    
+
     t.end();
 });
 
@@ -80,7 +81,7 @@ test('peekParentStackFrame', t => {
     t.strictEquals(th.peekParentStackFrame(), null);
     th.pushStack('secondString');
     t.strictEquals(th.peekParentStackFrame().warpMode, true);
-    
+
     t.end();
 });
 
@@ -100,13 +101,14 @@ test('peekStack', t => {
     t.strictEquals(th.peekStack(), 'arbitraryString');
     th.popStack();
     t.strictEquals(th.peekStack(), null);
-    
+
     t.end();
 });
 
 test('PushGetParam', t => {
     const th = new Thread('arbitraryString');
     th.pushStack('arbitraryString');
+    th.initParams();
     th.pushParam('testParam', 'testValue');
     t.strictEquals(th.peekStackFrame().params.testParam, 'testValue');
     t.strictEquals(th.getParam('testParam'), 'testValue');
@@ -149,12 +151,12 @@ test('goToNextBlock', t => {
         x: 0,
         y: 0
     };
-    
+
     rt.blocks.createBlock(block1);
     rt.blocks.createBlock(block2);
     rt.blocks.createBlock(block2);
     th.target = rt;
-    
+
     t.strictEquals(th.peekStack(), null);
     th.pushStack('secondString');
     t.strictEquals(th.peekStack(), 'secondString');
@@ -167,7 +169,7 @@ test('goToNextBlock', t => {
     t.strictEquals(th.peekStack(), 'secondString');
     th.goToNextBlock();
     t.strictEquals(th.peekStack(), null);
-    
+
     t.end();
 });
 
@@ -204,11 +206,11 @@ test('stopThisScript', t => {
         x: 0,
         y: 0
     };
-    
+
     rt.blocks.createBlock(block1);
     rt.blocks.createBlock(block2);
     th.target = rt;
-    
+
     th.stopThisScript();
     t.strictEquals(th.peekStack(), null);
     th.pushStack('arbitraryString');
@@ -219,7 +221,7 @@ test('stopThisScript', t => {
     th.pushStack('secondString');
     th.stopThisScript();
     t.strictEquals(th.peekStack(), 'secondString');
-    
+
     t.end();
 });
 
@@ -256,11 +258,11 @@ test('isRecursiveCall', t => {
         x: 0,
         y: 0
     };
-    
+
     rt.blocks.createBlock(block1);
     rt.blocks.createBlock(block2);
     th.target = rt;
-    
+
     t.strictEquals(th.isRecursiveCall('fakeCode'), false);
     th.pushStack('secondString');
     t.strictEquals(th.isRecursiveCall('fakeCode'), false);
@@ -274,6 +276,6 @@ test('isRecursiveCall', t => {
     t.strictEquals(th.isRecursiveCall('fakeCode'), false);
     th.popStack();
     t.strictEquals(th.isRecursiveCall('fakeCode'), false);
-    
+
     t.end();
 });