Revert "Push reported"

src/engine/blocks-execute-cache.js

@@ -1,19 +0,0 @@
-/**
- * @fileoverview
- * Access point for private method shared between blocks.js and execute.js for
- * caching execute information.
- */
-
-/**
- * A private method shared with execute to build an object containing the block
- * information execute needs and that is reset when other cached Blocks info is
- * reset.
- * @param {Blocks} blocks Blocks containing the expected blockId
- * @param {string} blockId blockId for the desired execute cache
- */
-exports.getCached = function () {
-    throw new Error('blocks.js has not initialized BlocksExecuteCache');
-};
-
-// Call after the default throwing getCached is assigned for Blocks to replace.
-require('./blocks');

src/engine/blocks.js

@@ -4,7 +4,6 @@ const xmlEscape = require('../util/xml-escape');
 const MonitorRecord = require('./monitor-record');
 const Clone = require('../util/clone');
 const {Map} = require('immutable');
-const BlocksExecuteCache = require('./blocks-execute-cache');
 
 /**
  * @fileoverview
@@ -48,14 +47,7 @@ class Blocks {
              * Cache procedure definitions by block id
              * @type {object.<string, ?string>}
              */
-            procedureDefinitions: {},
+            procedureDefinitions: {}
-
-            /**
-             * A cache for execute to use and store on. Only available to
-             * execute.
-             * @type {object.<string, object>}
-             */
-            _executeCached: {}
         };
 
     }
@@ -351,7 +343,6 @@ class Blocks {
         this._cache.inputs = {};
         this._cache.procedureParamNames = {};
         this._cache.procedureDefinitions = {};
-        this._cache._executeCached = {};
     }
 
     /**
@@ -737,31 +728,4 @@ class Blocks {
     }
 }
 
-/**
- * A private method shared with execute to build an object containing the block
- * information execute needs and that is reset when other cached Blocks info is
- * reset.
- * @param {Blocks} blocks Blocks containing the expected blockId
- * @param {string} blockId blockId for the desired execute cache
- * @return {object} execute cache object
- */
-BlocksExecuteCache.getCached = function (blocks, blockId) {
-    const block = blocks.getBlock(blockId);
-    if (typeof block === 'undefined') return null;
-    let cached = blocks._cache._executeCached[blockId];
-    if (typeof cached !== 'undefined') {
-        return cached;
-    }
-
-    cached = {
-        _initialized: false,
-        opcode: blocks.getOpcode(block),
-        fields: blocks.getFields(block),
-        inputs: blocks.getInputs(block),
-        mutation: blocks.getMutation(block)
-    };
-    blocks._cache._executeCached[blockId] = cached;
-    return cached;
-};
-
 module.exports = Blocks;

src/engine/execute.js

@@ -1,5 +1,4 @@
 const BlockUtility = require('./block-utility');
-const BlocksExecuteCache = require('./blocks-execute-cache');
 const log = require('../util/log');
 const Thread = require('./thread');
 const {Map} = require('immutable');
@@ -99,27 +98,32 @@ const handleReport = function (
     }
 };
 
-/**
- * A convenienve constant to hide that the recursiveCall argument to execute is
- * a boolean trap.
- * @const {boolean}
- */
-const RECURSIVE = true;
-
 /**
  * Execute a block.
  * @param {!Sequencer} sequencer Which sequencer is executing.
  * @param {!Thread} thread Thread which to read and execute.
- * @param {boolean} recursiveCall is execute called from another execute call?
  */
-const execute = function (sequencer, thread, recursiveCall) {
+const execute = function (sequencer, thread) {
     const runtime = sequencer.runtime;
+    const target = thread.target;
+
+    // Stop if block or target no longer exists.
+    if (target === null) {
+        // No block found: stop the thread; script no longer exists.
+        sequencer.retireThread(thread);
+        return;
+    }
 
     // Current block to execute is the one on the top of the stack.
     const currentBlockId = thread.peekStack();
     const currentStackFrame = thread.peekStackFrame();
 
-    let blockContainer = thread.blockContainer;
+    let blockContainer;
+    if (thread.updateMonitor) {
+        blockContainer = runtime.monitorBlocks;
+    } else {
+        blockContainer = target.blocks;
+    }
     let block = blockContainer.getBlock(currentBlockId);
     if (typeof block === 'undefined') {
         blockContainer = runtime.flyoutBlocks;
@@ -132,83 +136,33 @@ const execute = function (sequencer, thread, recursiveCall) {
         }
     }
 
-    const blockCached = BlocksExecuteCache.getCached(blockContainer, currentBlockId);
+    const opcode = blockContainer.getOpcode(block);
-    if (blockCached._initialized !== true) {
+    const fields = blockContainer.getFields(block);
-        const {opcode, fields, inputs} = blockCached;
+    const inputs = blockContainer.getInputs(block);
+    const blockFunction = runtime.getOpcodeFunction(opcode);
+    const isHat = runtime.getIsHat(opcode);
 
-        // Assign opcode isHat and blockFunction data to avoid dynamic lookups.
-        blockCached._isHat = runtime.getIsHat(opcode);
-        blockCached._blockFunction = runtime.getOpcodeFunction(opcode);
-        blockCached._definedBlockFunction = typeof blockCached._blockFunction !== 'undefined';
 
-        const fieldKeys = Object.keys(fields);
+    if (!opcode) {
-
+        log.warn(`Could not get opcode for block: ${currentBlockId}`);
-        // Store the current shadow value if there is a shadow value.
+        return;
-        blockCached._isShadowBlock = fieldKeys.length === 1 && Object.keys(inputs).length === 0;
-        blockCached._shadowValue = fieldKeys.length === 1 && fields[fieldKeys[0]].value;
-
-        // Store a fields copy. If fields is a VARIABLE, LIST, or
-        // BROADCAST_OPTION, store the created values so fields assignment to
-        // argValues does not iterate over fields.
-        blockCached._fields = Object.assign({}, blockCached.fields);
-        blockCached._isFieldVariable = fieldKeys.length === 1 && fieldKeys.includes('VARIABLE');
-        blockCached._fieldVariable = blockCached._isFieldVariable ?
-            {
-                id: fields.VARIABLE.id,
-                name: fields.VARIABLE.value
-            } :
-            null;
-        blockCached._isFieldList = fieldKeys.length === 1 && fieldKeys.includes('LIST');
-        blockCached._fieldList = blockCached._isFieldList ?
-            {
-                id: fields.LIST.id,
-                name: fields.LIST.value
-            } :
-            null;
-        blockCached._isFieldBroadcastOption = fieldKeys.length === 1 && fieldKeys.includes('BROADCAST_OPTION');
-        blockCached._fieldBroadcastOption = blockCached._isFieldBroadcastOption ?
-            {
-                id: fields.BROADCAST_OPTION.id,
-                name: fields.BROADCAST_OPTION.value
-            } :
-            null;
-        blockCached._isFieldKnown = blockCached._isFieldVariable ||
-            blockCached._isFieldList || blockCached._isFieldBroadcastOption;
-
-        // Store a modified inputs. This assures the keys are its own properties
-        // and that custom_block will not be evaluated.
-        blockCached._inputs = Object.assign({}, blockCached.inputs);
-        delete blockCached._inputs.custom_block;
-
-        blockCached._initialized = true;
     }
 
-    const opcode = blockCached.opcode;
-    const fields = blockCached._fields;
-    const inputs = blockCached._inputs;
-    const mutation = blockCached.mutation;
-    const blockFunction = blockCached._blockFunction;
-    const isHat = blockCached._isHat;
-
     // Hats and single-field shadows are implemented slightly differently
     // from regular blocks.
     // For hats: if they have an associated block function,
     // it's treated as a predicate; if not, execution will proceed as a no-op.
     // For single-field shadows: If the block has a single field, and no inputs,
     // immediately return the value of the field.
-    if (!blockCached._definedBlockFunction) {
+    if (typeof blockFunction === 'undefined') {
-        if (!opcode) {
+        if (isHat) {
-            log.warn(`Could not get opcode for block: ${currentBlockId}`);
-            return;
-        }
-
-        if (recursiveCall === RECURSIVE && blockCached._isShadowBlock) {
-            // One field and no inputs - treat as arg.
-            thread.pushReportedValue(blockCached._shadowValue);
-            thread.status = Thread.STATUS_RUNNING;
-        } else if (isHat) {
             // Skip through the block (hat with no predicate).
             return;
+        }
+        const keys = Object.keys(fields);
+        if (keys.length === 1 && Object.keys(inputs).length === 0) {
+            // One field and no inputs - treat as arg.
+            handleReport(fields[keys[0]].value, sequencer, thread, currentBlockId, opcode, isHat);
         } else {
             log.warn(`Could not get implementation for opcode: ${opcode}`);
         }
@@ -220,22 +174,24 @@ const execute = function (sequencer, thread, recursiveCall) {
     const argValues = {};
 
     // Add all fields on this block to the argValues.
-    if (blockCached._isFieldKnown) {
+    for (const fieldName in fields) {
-        if (blockCached._isFieldVariable) {
+        if (!fields.hasOwnProperty(fieldName)) continue;
-            argValues.VARIABLE = blockCached._fieldVariable;
+        if (fieldName === 'VARIABLE' || fieldName === 'LIST' ||
-        } else if (blockCached._isFieldList) {
+            fieldName === 'BROADCAST_OPTION') {
-            argValues.LIST = blockCached._fieldList;
+            argValues[fieldName] = {
-        } else if (blockCached._isFieldBroadcastOption) {
+                id: fields[fieldName].id,
-            argValues.BROADCAST_OPTION = blockCached._fieldBroadcastOption;
+                name: fields[fieldName].value
-        }
+            };
-    } else {
+        } else {
-        for (const fieldName in fields) {
             argValues[fieldName] = fields[fieldName].value;
         }
     }
 
     // Recursively evaluate input blocks.
     for (const inputName in inputs) {
+        if (!inputs.hasOwnProperty(inputName)) continue;
+        // Do not evaluate the internal custom command block within definition
+        if (inputName === 'custom_block') continue;
         const input = inputs[inputName];
         const inputBlockId = input.block;
         // Is there no value for this input waiting in the stack frame?
@@ -246,16 +202,8 @@ const execute = function (sequencer, thread, recursiveCall) {
             // Save name of input for `Thread.pushReportedValue`.
             currentStackFrame.waitingReporter = inputName;
             // Actually execute the block.
-            execute(sequencer, thread, RECURSIVE);
+            execute(sequencer, thread);
             if (thread.status === Thread.STATUS_PROMISE_WAIT) {
-                for (const _inputName in inputs) {
-                    if (_inputName === inputName) break;
-                    if (_inputName === 'BROADCAST_INPUT') {
-                        currentStackFrame.reported[_inputName] = argValues[_inputName].name;
-                    } else {
-                        currentStackFrame.reported[_inputName] = argValues[_inputName];
-                    }
-                }
                 return;
             }
 
@@ -264,23 +212,7 @@ const execute = function (sequencer, thread, recursiveCall) {
             currentStackFrame.waitingReporter = null;
             thread.popStack();
         }
-        let inputValue;
+        const inputValue = currentStackFrame.reported[inputName];
-        if (
-            currentStackFrame.waitingReporter === null
-        ) {
-            inputValue = currentStackFrame.justReported;
-        } else if (currentStackFrame.waitingReporter === inputName) {
-            inputValue = currentStackFrame.justReported;
-            currentStackFrame.waitingReporter = null;
-            // If we've gotten this far, all of the input blocks are evaluated,
-            // and `argValues` is fully populated. So, execute the block
-            // primitive. First, clear `currentStackFrame.reported`, so any
-            // subsequent execution (e.g., on return from a branch) gets fresh
-            // inputs.
-            currentStackFrame.reported = {};
-        } else if (typeof currentStackFrame.reported[inputName] !== 'undefined') {
-            inputValue = currentStackFrame.reported[inputName];
-        }
         if (inputName === 'BROADCAST_INPUT') {
             const broadcastInput = inputs[inputName];
             // Check if something is plugged into the broadcast block, or
@@ -307,7 +239,16 @@ const execute = function (sequencer, thread, recursiveCall) {
     }
 
     // Add any mutation to args (e.g., for procedures).
-    argValues.mutation = mutation;
+    const mutation = blockContainer.getMutation(block);
+    if (mutation !== null) {
+        argValues.mutation = mutation;
+    }
+
+    // If we've gotten this far, all of the input blocks are evaluated,
+    // and `argValues` is fully populated. So, execute the block primitive.
+    // First, clear `currentStackFrame.reported`, so any subsequent execution
+    // (e.g., on return from a branch) gets fresh inputs.
+    currentStackFrame.reported = {};
 
     let primitiveReportedValue = null;
     blockUtility.sequencer = sequencer;
@@ -330,7 +271,7 @@ const execute = function (sequencer, thread, recursiveCall) {
         runtime.profiler.records.push(runtime.profiler.STOP, performance.now());
     }
 
-    if (recursiveCall !== RECURSIVE && typeof primitiveReportedValue === 'undefined') {
+    if (typeof primitiveReportedValue === 'undefined') {
         // No value reported - potentially a command block.
         // Edge-activated hats don't request a glow; all commands do.
         thread.requestScriptGlowInFrame = true;
@@ -377,11 +318,7 @@ const execute = function (sequencer, thread, recursiveCall) {
             thread.popStack();
         });
     } else if (thread.status === Thread.STATUS_RUNNING) {
-        if (recursiveCall === RECURSIVE) {
+        handleReport(primitiveReportedValue, sequencer, thread, currentBlockId, opcode, isHat);
-            thread.pushReportedValue(primitiveReportedValue);
-        } else {
-            handleReport(primitiveReportedValue, sequencer, thread, currentBlockId, opcode, isHat);
-        }
     }
 };
 

src/engine/runtime.js

@@ -861,9 +861,6 @@ class Runtime extends EventEmitter {
         thread.target = target;
         thread.stackClick = opts.stackClick;
         thread.updateMonitor = opts.updateMonitor;
-        thread.blockContainer = opts.updateMonitor ?
-            this.monitorBlocks :
-            target.blocks;
 
         thread.pushStack(id);
         this.threads.push(thread);
@@ -893,7 +890,6 @@ class Runtime extends EventEmitter {
         newThread.target = thread.target;
         newThread.stackClick = thread.stackClick;
         newThread.updateMonitor = thread.updateMonitor;
-        newThread.blockContainer = thread.blockContainer;
         newThread.pushStack(thread.topBlock);
         const i = this.threads.indexOf(thread);
         if (i > -1) {

src/engine/sequencer.js

@@ -197,11 +197,7 @@ class Sequencer {
                 this.runtime.profiler.records.push(
                     this.runtime.profiler.START, executeProfilerId, null, performance.now());
             }
-            if (thread.target === null) {
+            execute(this, thread);
-                this.retireThread(thread);
-            } else {
-                execute(this, thread);
-            }
             if (this.runtime.profiler !== null) {
                 // this.runtime.profiler.stop();
                 this.runtime.profiler.records.push(this.runtime.profiler.STOP, performance.now());

src/engine/thread.js

@@ -42,12 +42,6 @@ class Thread {
          */
         this.target = null;
 
-        /**
-         * The Blocks this thread will execute.
-         * @type {Blocks}
-         */
-        this.blockContainer = null;
-
         /**
          * Whether the thread requests its script to glow during this frame.
          * @type {boolean}
@@ -130,8 +124,7 @@ class Thread {
             this.stackFrames.push({
                 isLoop: false, // Whether this level of the stack is a loop.
                 warpMode: warpMode, // Whether this level is in warp mode.
-                justReported: null, // Reported value from just executed block.
+                reported: {}, // Collects reported input values.
-                reported: {}, // Persists reported inputs during async block.
                 waitingReporter: null, // Name of waiting reporter.
                 params: {}, // Procedure parameters.
                 executionContext: {} // A context passed to block implementations.
@@ -216,8 +209,9 @@ class Thread {
      */
     pushReportedValue (value) {
         const parentStackFrame = this.peekParentStackFrame();
-        if (parentStackFrame !== null) {
+        if (parentStackFrame) {
-            parentStackFrame.justReported = value;
+            const waitingReporter = parentStackFrame.waitingReporter;
+            parentStackFrame.reported[waitingReporter] = value;
         }
     }
 

test/unit/engine_sequencer.js

@@ -91,7 +91,6 @@ const generateThread = function (runtime) {
     rt.blocks.createBlock(generateBlock(next));
     th.pushStack(next);
     th.target = rt;
-    th.blockContainer = rt.blocks;
     
     runtime.threads.push(th);
 

test/unit/engine_thread.js

@@ -89,7 +89,7 @@ test('pushReportedValue', t => {
     th.pushStack('arbitraryString');
     th.pushStack('secondString');
     th.pushReportedValue('value');
-    t.strictEquals(th.peekParentStackFrame().justReported, 'value');
+    t.strictEquals(th.peekParentStackFrame().reported.null, 'value');
     
     t.end();
 });