diff --git a/src/blocks/scratch3_procedures.js b/src/blocks/scratch3_procedures.js index 35ed6921d..3eec20b64 100644 --- a/src/blocks/scratch3_procedures.js +++ b/src/blocks/scratch3_procedures.js @@ -24,34 +24,32 @@ class Scratch3ProcedureBlocks { } call (args, util) { - if (!util.stackFrame.executed) { - const procedureCode = args.mutation.proccode; - const paramNamesIdsAndDefaults = util.getProcedureParamNamesIdsAndDefaults(procedureCode); + const procedureCode = args.mutation.proccode; + const paramNamesIdsAndDefaults = util.getProcedureParamNamesIdsAndDefaults(procedureCode); - // If null, procedure could not be found, which can happen if custom - // block is dragged between sprites without the definition. - // Match Scratch 2.0 behavior and noop. - if (paramNamesIdsAndDefaults === null) { - return; - } - - 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]]); - } else { - util.pushParam(paramNames[i], paramDefaults[i]); - } - } - - util.stackFrame.executed = true; - util.startProcedure(procedureCode); + // If null, procedure could not be found, which can happen if custom + // block is dragged between sprites without the definition. + // Match Scratch 2.0 behavior and noop. + if (paramNamesIdsAndDefaults === null) { + return; } + + const [paramNames, paramIds, paramDefaults] = paramNamesIdsAndDefaults; + + util.startProcedure(procedureCode); + + // 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]]); + } else { + util.pushParam(paramNames[i], paramDefaults[i]); + } + } + } argumentReporterStringNumber (args, util) { diff --git a/src/engine/block-utility.js b/src/engine/block-utility.js index cfe18a227..d7f9ef17b 100644 --- a/src/engine/block-utility.js +++ b/src/engine/block-utility.js @@ -60,11 +60,7 @@ class BlockUtility { * @type {object} */ get stackFrame () { - const frame = this.thread.peekStackFrame(); - if (frame.executionContext === null) { - frame.executionContext = {}; - } - return frame.executionContext; + return this.thread.getExecutionContext(); } /** diff --git a/src/engine/execute.js b/src/engine/execute.js index 7c5bc1ef8..4825ad6ab 100644 --- a/src/engine/execute.js +++ b/src/engine/execute.js @@ -385,7 +385,6 @@ const execute = function (sequencer, thread) { // 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 blockCached = BlocksExecuteCache.getCached(blockContainer, currentBlockId, BlockCached); @@ -404,8 +403,8 @@ const execute = function (sequencer, thread) { const length = ops.length; let i = 0; - if (currentStackFrame.reported !== null) { - const reported = currentStackFrame.reported; + if (thread.reported !== null) { + const reported = thread.reported; // Reinstate all the previous values. for (; i < reported.length; i++) { const {opCached: oldOpCached, inputValue} = reported[i]; @@ -441,7 +440,7 @@ const execute = function (sequencer, thread) { } // The reporting block must exist and must be the next one in the sequence of operations. - if (thread.justReported !== null && ops[i] && ops[i].id === currentStackFrame.reporting) { + if (thread.justReported !== null && ops[i] && ops[i].id === thread.reportingBlockId) { const opCached = ops[i]; const inputValue = thread.justReported; @@ -462,8 +461,8 @@ const execute = function (sequencer, thread) { i += 1; } - currentStackFrame.reporting = null; - currentStackFrame.reported = null; + thread.reportingBlockId = null; + thread.reported = null; } for (; i < length; i++) { @@ -518,8 +517,8 @@ const execute = function (sequencer, thread) { // operation if it is promise waiting will set its parent value at // that time. thread.justReported = null; - currentStackFrame.reporting = ops[i].id; - currentStackFrame.reported = ops.slice(0, i).map(reportedCached => { + thread.reportingBlockId = ops[i].id; + thread.reported = ops.slice(0, i).map(reportedCached => { const inputName = reportedCached._parentKey; const reportedValues = reportedCached._parentValues; diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 47a4c75af..3dd02fe02 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -1490,7 +1490,7 @@ class Runtime extends EventEmitter { isActiveThread (thread) { return ( ( - thread.stack.length > 0 && + thread.stackFrame !== null && thread.status !== Thread.STATUS_DONE) && this.threads.indexOf(thread) > -1); } @@ -1667,11 +1667,20 @@ class Runtime extends EventEmitter { // Start the thread with this top block. newThreads.push(this._pushThread(topBlockId, target)); }, optTarget); - // For compatibility with Scratch 2, edge triggered hats need to be processed before - // threads are stepped. See ScratchRuntime.as for original implementation + // For compatibility with Scratch 2, edge triggered hats need to be + // processed before threads are stepped. See ScratchRuntime.as for + // original implementation. + // + // TODO: Move the execute call to sequencer. Maybe in a method call + // stepHat or stepOne. newThreads.forEach(thread => { execute(this.sequencer, thread); - thread.goToNextBlock(); + if (thread.status !== Thread.STATUS_DONE) { + thread.goToNextBlock(); + if (thread.stackFrame === null) { + this.sequencer.retireThread(thread); + } + } }); return newThreads; } diff --git a/src/engine/sequencer.js b/src/engine/sequencer.js index 54d4870a5..00e67302f 100644 --- a/src/engine/sequencer.js +++ b/src/engine/sequencer.js @@ -103,7 +103,7 @@ class Sequencer { for (let i = 0; i < threads.length; i++) { const activeThread = this.activeThread = threads[i]; // Check if the thread is done so it is not executed. - if (activeThread.stack.length === 0 || + if (activeThread.stackFrame === null || activeThread.status === Thread.STATUS_DONE) { // Finished with this thread. stoppedThread = true; @@ -137,7 +137,7 @@ class Sequencer { } // Check if the thread completed while it just stepped to make // sure we remove it before the next iteration of all threads. - if (activeThread.stack.length === 0 || + if (activeThread.stackFrame === null || activeThread.status === Thread.STATUS_DONE) { // Finished with this thread. stoppedThread = true; @@ -156,7 +156,7 @@ class Sequencer { let nextActiveThread = 0; for (let i = 0; i < this.runtime.threads.length; i++) { const thread = this.runtime.threads[i]; - if (thread.stack.length !== 0 && + if (thread.stackFrame !== null && thread.status !== Thread.STATUS_DONE) { this.runtime.threads[nextActiveThread] = thread; nextActiveThread++; @@ -184,7 +184,7 @@ class Sequencer { thread.popStack(); // Did the null follow a hat block? - if (thread.stack.length === 0) { + if (thread.peekStackFrame() === null) { thread.status = Thread.STATUS_DONE; return; } @@ -248,7 +248,7 @@ class Sequencer { while (!thread.peekStack()) { thread.popStack(); - if (thread.stack.length === 0) { + if (thread.stackFrame === null) { // No more stack to run! thread.status = Thread.STATUS_DONE; return; @@ -299,7 +299,11 @@ class Sequencer { currentBlockId, branchNum ); - thread.peekStackFrame().isLoop = isLoop; + if (isLoop) { + const stackFrame = thread.peekStackFrame(); + stackFrame.needsReset = true; + stackFrame.isLoop = true; + } if (branchId) { // Push branch ID to the thread's stack. thread.pushStack(branchId); @@ -361,7 +365,9 @@ class Sequencer { */ retireThread (thread) { thread.stack = []; - thread.stackFrame = []; + thread.pointer = null; + thread.stackFrames = []; + thread.stackFrame = null; thread.requestScriptGlowInFrame = false; thread.status = Thread.STATUS_DONE; } diff --git a/src/engine/thread.js b/src/engine/thread.js index e381dd9bf..a193e64f8 100644 --- a/src/engine/thread.js +++ b/src/engine/thread.js @@ -5,14 +5,23 @@ const _stackFrameFreeList = []; /** - * A frame used for each level of the stack. A general purpose - * place to store a bunch of execution context and parameters - * @param {boolean} warpMode Whether this level of the stack is warping + * Default params object for stack frames outside of a procedure. + * + * StackFrame.params uses a null prototype object. It does not have Object + * methods like hasOwnProperty. With a null prototype + * `typeof params[key] !== 'undefined'` has similar behaviour to hasOwnProperty. + * @type {object} + */ +const defaultParams = Object.create(null); + +/** + * A frame used for each level of the stack. A general purpose place to store a + * bunch of execution contexts and parameters. * @constructor * @private */ class _StackFrame { - constructor (warpMode) { + constructor () { /** * Whether this level of the stack is a loop. * @type {boolean} @@ -20,90 +29,62 @@ class _StackFrame { this.isLoop = false; /** - * Whether this level is in warp mode. Is set by some legacy blocks and - * "turbo mode" + * Whether this level is in warp mode. Set to true by the sequencer for + * some procedures. + * + * After being set to true at the beginning of a procedure a thread + * will be in warpMode until it pops a stack frame to reveal one that + * is not in warpMode. Either this value is always false for a stack + * frame or always true after a procedure sets it. * @type {boolean} */ - this.warpMode = warpMode; - - /** - * Reported value from just executed block. - * @type {Any} - */ - this.justReported = null; - - /** - * The active block that is waiting on a promise. - * @type {string} - */ - this.reporting = ''; - - /** - * Persists reported inputs during async block. - * @type {Object} - */ - this.reported = null; - - /** - * Name of waiting reporter. - * @type {string} - */ - this.waitingReporter = null; + this.warpMode = false; /** * Procedure parameters. + * + * After being set by a procedure these values do not change and they + * will be copied to deeper stack frames. * @type {Object} */ - this.params = null; + this.params = defaultParams; /** * A context passed to block implementations. * @type {Object} */ - this.executionContext = null; + this.executionContext = {}; + + /** + * Has this frame changed and need a reset? + * @type {boolean} + */ + this.needsReset = false; } /** - * Reset all properties of the frame to pristine null and false states. - * Used to recycle. + * Reset some properties of the frame to default values. Used to recycle. * @return {_StackFrame} this */ reset () { - this.isLoop = false; - this.warpMode = false; - this.justReported = null; - this.reported = null; - this.waitingReporter = null; - this.params = null; - this.executionContext = null; + this.executionContext = {}; + this.needsReset = false; return this; } - /** - * Reuse an active stack frame in the stack. - * @param {?boolean} warpMode defaults to current warpMode - * @returns {_StackFrame} this - */ - reuse (warpMode = this.warpMode) { - this.reset(); - this.warpMode = Boolean(warpMode); - return this; - } - /** * Create or recycle a stack frame object. - * @param {boolean} warpMode Enable warpMode on this frame. - * @returns {_StackFrame} The clean stack frame with correct warpMode setting. + * @param {_StackFrame} parent Parent frame to copy "immutable" values. + * @returns {_StackFrame} The clean stack frame with correct warpMode + * setting. */ - static create (warpMode) { - const stackFrame = _stackFrameFreeList.pop(); - if (typeof stackFrame !== 'undefined') { - stackFrame.warpMode = Boolean(warpMode); - return stackFrame; - } - return new _StackFrame(warpMode); + static create (parent) { + const stackFrame = _stackFrameFreeList.pop() || new _StackFrame(); + stackFrame.warpMode = parent.warpMode; + stackFrame.params = parent.params; + return stackFrame; } /** @@ -111,12 +92,22 @@ class _StackFrame { * @param {_StackFrame} stackFrame The frame to reset and recycle. */ static release (stackFrame) { - if (typeof stackFrame !== 'undefined') { - _stackFrameFreeList.push(stackFrame.reset()); + if (stackFrame !== null) { + _stackFrameFreeList.push( + stackFrame.needsReset ? stackFrame.reset() : stackFrame + ); } } } +/** + * The initial stack frame for all threads. A call to pushStack will create the + * first to be used frame for a thread. That first frame will use the initial + * values from initialStackFrame. + * @type {_StackFrame} + */ +const initialStackFrame = new _StackFrame(); + /** * A thread is a running stack context and all the metadata needed. * @param {?string} firstBlock First block to execute in the thread. @@ -137,12 +128,25 @@ class Thread { */ this.stack = []; + /** + * The "instruction" pointer the thread is currently at. This + * determines what block is executed. + * @type {string} + */ + this.pointer = null; + /** * Stack frames for the thread. Store metadata for the executing blocks. * @type {Array.<_StackFrame>} */ this.stackFrames = []; + /** + * The current stack frame that goes along with the pointer. + * @type {_StackFrame} + */ + this.stackFrame = null; + /** * Status of the thread, one of three states (below) * @type {number} @@ -186,7 +190,25 @@ class Thread { */ this.warpTimer = null; + /** + * The value just reported by a promise. + * @type {*} + */ this.justReported = null; + + /** + * The id of the block that we will report the promise resolved value + * for. + * @type {string} + */ + this.reportingBlockId = null; + + /** + * The already reported values in a sequence of blocks to restore when + * the awaited promise resolves. + * @type {Array.<*>} + */ + this.reported = null; } /** @@ -239,12 +261,16 @@ class Thread { * @param {string} blockId Block ID to push to stack. */ pushStack (blockId) { - this.stack.push(blockId); - // Push an empty stack frame, if we need one. - // Might not, if we just popped the stack. - if (this.stack.length > this.stackFrames.length) { - const parent = this.stackFrames[this.stackFrames.length - 1]; - this.stackFrames.push(_StackFrame.create(typeof parent !== 'undefined' && parent.warpMode)); + if (this.stackFrame === null) { + this.pointer = blockId; + this.stackFrame = _StackFrame.create(initialStackFrame); + } else { + this.stack.push(this.pointer); + this.pointer = blockId; + + const parent = this.stackFrame; + this.stackFrames.push(parent); + this.stackFrame = _StackFrame.create(parent); } } @@ -254,21 +280,27 @@ class Thread { * @param {string} blockId Block ID to push to stack. */ reuseStackForNextBlock (blockId) { - this.stack[this.stack.length - 1] = blockId; - this.stackFrames[this.stackFrames.length - 1].reuse(); + this.pointer = blockId; + if (this.stackFrame.needsReset) this.stackFrame.reset(); } /** - * Pop last block on the stack and its stack frame. - * @return {string} Block ID popped from the stack. + * Move the instruction pointer to the last value before this stack of + * blocks was pushed and executed. + * @return {?string} Block ID popped from the stack. */ popStack () { - _StackFrame.release(this.stackFrames.pop()); - return this.stack.pop(); + const lastPointer = this.pointer; + this.pointer = this.stack.pop() || null; + _StackFrame.release(this.stackFrame); + this.stackFrame = this.stackFrames.pop() || null; + return lastPointer; } /** - * Pop back down the stack frame until we hit a procedure call or the stack frame is emptied + * Move the instruction pointer to the last procedure call block and resume + * execution there or to the end of this thread and stop executing this + * thread. */ stopThisScript () { let blockID = this.peekStack(); @@ -277,11 +309,10 @@ class Thread { if (typeof block !== 'undefined' && block.opcode === 'procedures_call') { break; } - this.popStack(); - blockID = this.peekStack(); + blockID = this.popStack(); } - if (this.stack.length === 0) { + if (this.stackFrame === null) { // Clean up! this.requestScriptGlowInFrame = false; this.status = Thread.STATUS_DONE; @@ -293,7 +324,7 @@ class Thread { * @return {?string} Block ID on top of stack. */ peekStack () { - return this.stack.length > 0 ? this.stack[this.stack.length - 1] : null; + return this.pointer; } @@ -302,7 +333,7 @@ class Thread { * @return {?object} Last stack frame stored on this thread. */ peekStackFrame () { - return this.stackFrames.length > 0 ? this.stackFrames[this.stackFrames.length - 1] : null; + return this.stackFrame; } /** @@ -310,7 +341,7 @@ class Thread { * @return {?object} Second to last stack frame stored on this thread. */ peekParentStackFrame () { - return this.stackFrames.length > 1 ? this.stackFrames[this.stackFrames.length - 2] : null; + return this.stackFrames.length > 0 ? this.stackFrames[this.stackFrames.length - 1] : null; } /** @@ -321,14 +352,22 @@ class Thread { this.justReported = typeof value === 'undefined' ? null : value; } + /** + * Return an execution context for a block to use. + * @returns {object} the execution context + */ + getExecutionContext () { + const frame = this.stackFrame; + frame.needsReset = true; + return frame.executionContext; + } + /** * Initialize procedure parameters on this stack frame. */ initParams () { - const stackFrame = this.peekStackFrame(); - if (stackFrame.params === null) { - stackFrame.params = {}; - } + const stackFrame = this.stackFrame; + stackFrame.params = Object.create(null); } /** @@ -338,7 +377,7 @@ class Thread { * @param {*} value Value to set for parameter. */ pushParam (paramName, value) { - const stackFrame = this.peekStackFrame(); + const stackFrame = this.stackFrame; stackFrame.params[paramName] = value; } @@ -348,15 +387,9 @@ class Thread { * @return {*} value Value for parameter. */ getParam (paramName) { - for (let i = this.stackFrames.length - 1; i >= 0; i--) { - const frame = this.stackFrames[i]; - if (frame.params === null) { - continue; - } - if (frame.params.hasOwnProperty(paramName)) { - return frame.params[paramName]; - } - return null; + const stackFrame = this.stackFrame; + if (typeof stackFrame.params[paramName] !== 'undefined') { + return stackFrame.params[paramName]; } return null; } @@ -376,26 +409,26 @@ class Thread { * where execution proceeds from one block to the next. */ goToNextBlock () { - const nextBlockId = this.target.blocks.getNextBlock(this.peekStack()); + const nextBlockId = this.target.blocks.getNextBlock(this.pointer); this.reuseStackForNextBlock(nextBlockId); } /** - * Attempt to determine whether a procedure call is recursive, - * by examining the stack. + * Attempt to determine whether a procedure call is recursive, by examining + * the stack. * @param {!string} procedureCode Procedure code of procedure being called. * @return {boolean} True if the call appears recursive. */ isRecursiveCall (procedureCode) { - let callCount = 5; // Max number of enclosing procedure calls to examine. - const sp = this.stack.length - 1; - for (let i = sp - 1; i >= 0; i--) { + const stackHeight = this.stack.length; + // Limit the number of stack levels that are examined for procedures. + const stackBottom = Math.max(stackHeight - 5, 0); + for (let i = stackHeight - 1; i >= stackBottom; i--) { const block = this.target.blocks.getBlock(this.stack[i]); if (block.opcode === 'procedures_call' && block.mutation.proccode === procedureCode) { return true; } - if (--callCount < 0) return false; } return false; } diff --git a/test/unit/engine_sequencer.js b/test/unit/engine_sequencer.js index a383001fe..2e0657382 100644 --- a/test/unit/engine_sequencer.js +++ b/test/unit/engine_sequencer.js @@ -137,7 +137,7 @@ test('retireThread', t => { const r = new Runtime(); const s = new Sequencer(r); const th = generateThread(r); - t.strictEquals(th.stack.length, 12); + t.strictEquals(th.stack.length, 11); s.retireThread(th); t.strictEquals(th.stack.length, 0); t.strictEquals(th.status, Thread.STATUS_DONE); diff --git a/test/unit/engine_thread.js b/test/unit/engine_thread.js index 36206eb99..1d0525ddd 100644 --- a/test/unit/engine_thread.js +++ b/test/unit/engine_thread.js @@ -40,7 +40,7 @@ test('popStack', t => { const th = new Thread('arbitraryString'); th.pushStack('arbitraryString'); t.strictEquals(th.popStack(), 'arbitraryString'); - t.strictEquals(th.popStack(), undefined); + t.strictEquals(th.popStack(), null); t.end(); });