diff --git a/src/engine/blocks-runtime-cache.js b/src/engine/blocks-runtime-cache.js deleted file mode 100644 index cc30e8345..000000000 --- a/src/engine/blocks-runtime-cache.js +++ /dev/null @@ -1,78 +0,0 @@ -/** - * @fileoverview - * The BlocksRuntimeCache caches data about the top block of scripts so that - * Runtime can iterate a targeted opcode and iterate the returned set faster. - * Many top blocks need to match fields as well as opcode, since that matching - * compares strings in uppercase we can go ahead and uppercase the cached value - * so we don't need to in the future. - */ - -/** - * A set of cached data about the top block of a script. - * @param {Blocks} container - Container holding the block and related data - * @param {string} blockId - Id for whose block data is cached in this instance - */ -class RuntimeScriptCache { - constructor (container, blockId) { - /** - * Container with block data for blockId. - * @type {Blocks} - */ - this.container = container; - - /** - * ID for block this instance caches. - * @type {string} - */ - this.blockId = blockId; - - const block = container.getBlock(blockId); - const fields = container.getFields(block); - - /** - * Formatted fields or fields of input blocks ready for comparison in - * runtime. - * - * This is a clone of parts of the targeted blocks. Changes to these - * clones are limited to copies under RuntimeScriptCache and will not - * appear in the original blocks in their container. This copy is - * modified changing the case of strings to uppercase. These uppercase - * values will be compared later by the VM. - * @type {object} - */ - this.fieldsOfInputs = Object.assign({}, fields); - if (Object.keys(fields).length === 0) { - const inputs = container.getInputs(block); - for (const input in inputs) { - if (!inputs.hasOwnProperty(input)) continue; - const id = inputs[input].block; - const inputBlock = container.getBlock(id); - const inputFields = container.getFields(inputBlock); - Object.assign(this.fieldsOfInputs, inputFields); - } - } - for (const key in this.fieldsOfInputs) { - const field = this.fieldsOfInputs[key] = Object.assign({}, this.fieldsOfInputs[key]); - if (field.value.toUpperCase) { - field.value = field.value.toUpperCase(); - } - } - } -} - -/** - * Get an array of scripts from a block container prefiltered to match opcode. - * @param {Blocks} container - Container of blocks - * @param {string} opcode - Opcode to filter top blocks by - */ -exports.getScripts = function () { - throw new Error('blocks.js has not initialized BlocksRuntimeCache'); -}; - -/** - * Exposed RuntimeScriptCache class used by integration in blocks.js. - * @private - */ -exports._RuntimeScriptCache = RuntimeScriptCache; - -require('./blocks'); diff --git a/src/engine/blocks.js b/src/engine/blocks.js index 8f427ec44..db0d5a5bc 100644 --- a/src/engine/blocks.js +++ b/src/engine/blocks.js @@ -5,7 +5,6 @@ const MonitorRecord = require('./monitor-record'); const Clone = require('../util/clone'); const {Map} = require('immutable'); const BlocksExecuteCache = require('./blocks-execute-cache'); -const BlocksRuntimeCache = require('./blocks-runtime-cache'); const log = require('../util/log'); const Variable = require('./variable'); const getMonitorIdForBlockWithArgs = require('../util/get-monitor-id'); @@ -75,13 +74,7 @@ class Blocks { * actively monitored. * @type {Array<{blockId: string, target: Target}>} */ - _monitored: null, - - /** - * A cache of hat opcodes to collection of theads to execute. - * @type {object.} - */ - scripts: {} + _monitored: null }; /** @@ -516,7 +509,6 @@ class Blocks { this._cache.procedureDefinitions = {}; this._cache._executeCached = {}; this._cache._monitored = null; - this._cache.scripts = {}; } /** @@ -1223,35 +1215,4 @@ BlocksExecuteCache.getCached = function (blocks, blockId, CacheType) { return cached; }; -/** - * Cache class constructor for runtime. Used to consider what threads should - * start based on hat data. - * @type {function} - */ -const RuntimeScriptCache = BlocksRuntimeCache._RuntimeScriptCache; - -/** - * Get an array of scripts from a block container prefiltered to match opcode. - * @param {Blocks} blocks - Container of blocks - * @param {string} opcode - Opcode to filter top blocks by - * @returns {Array.} - Array of RuntimeScriptCache cache - * objects - */ -BlocksRuntimeCache.getScripts = function (blocks, opcode) { - let scripts = blocks._cache.scripts[opcode]; - if (!scripts) { - scripts = blocks._cache.scripts[opcode] = []; - - const allScripts = blocks._scripts; - for (let i = 0; i < allScripts.length; i++) { - const topBlockId = allScripts[i]; - const block = blocks.getBlock(topBlockId); - if (block.opcode === opcode) { - scripts.push(new RuntimeScriptCache(blocks, topBlockId)); - } - } - } - return scripts; -}; - module.exports = Blocks; diff --git a/src/engine/runtime.js b/src/engine/runtime.js index eebda471e..204cd4633 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -4,7 +4,6 @@ const escapeHtml = require('escape-html'); const ArgumentType = require('../extension-support/argument-type'); const Blocks = require('./blocks'); -const BlocksRuntimeCache = require('./blocks-runtime-cache'); const BlockType = require('../extension-support/block-type'); const Profiler = require('./profiler'); const Sequencer = require('./sequencer'); @@ -1435,11 +1434,16 @@ class Runtime extends EventEmitter { * @return {!Thread} The newly created thread. */ _pushThread (id, target, opts) { + opts = Object.assign({ + stackClick: false, + updateMonitor: false + }, opts); + const thread = new Thread(id); thread.target = target; - thread.stackClick = Boolean(opts && opts.stackClick); - thread.updateMonitor = Boolean(opts && opts.updateMonitor); - thread.blockContainer = thread.updateMonitor ? + thread.stackClick = opts.stackClick; + thread.updateMonitor = opts.updateMonitor; + thread.blockContainer = opts.updateMonitor ? this.monitorBlocks : target.blocks; @@ -1582,20 +1586,6 @@ class Runtime extends EventEmitter { } } - allScriptsByOpcodeDo (opcode, f, optTarget) { - let targets = this.executableTargets; - if (optTarget) { - targets = [optTarget]; - } - for (let t = targets.length - 1; t >= 0; t--) { - const target = targets[t]; - const scripts = BlocksRuntimeCache.getScripts(target.blocks, opcode); - for (let j = 0; j < scripts.length; j++) { - f(scripts[j], target); - } - } - } - /** * Start all relevant hats. * @param {!string} requestedHatOpcode Opcode of hats to start. @@ -1620,52 +1610,71 @@ class Runtime extends EventEmitter { } // Consider all scripts, looking for hats with opcode `requestedHatOpcode`. - this.allScriptsByOpcodeDo(requestedHatOpcode, (script, target) => { - const { - blockId: topBlockId, - fieldsOfInputs: hatFields - } = script; + this.allScriptsDo((topBlockId, target) => { + const blocks = target.blocks; + const block = blocks.getBlock(topBlockId); + const potentialHatOpcode = block.opcode; + if (potentialHatOpcode !== requestedHatOpcode) { + // Not the right hat. + return; + } // Match any requested fields. // For example: ensures that broadcasts match. // This needs to happen before the block is evaluated // (i.e., before the predicate can be run) because "broadcast and wait" // needs to have a precise collection of started threads. - for (const matchField in optMatchFields) { - if (hatFields[matchField].value !== optMatchFields[matchField]) { - // Field mismatch. - return; + let hatFields = blocks.getFields(block); + + // If no fields are present, check inputs (horizontal blocks) + if (Object.keys(hatFields).length === 0) { + hatFields = {}; // don't overwrite the block's actual fields list + const hatInputs = blocks.getInputs(block); + for (const input in hatInputs) { + if (!hatInputs.hasOwnProperty(input)) continue; + const id = hatInputs[input].block; + const inpBlock = blocks.getBlock(id); + const fields = blocks.getFields(inpBlock); + Object.assign(hatFields, fields); + } + } + + if (optMatchFields) { + for (const matchField in optMatchFields) { + if (hatFields[matchField].value.toUpperCase() !== + optMatchFields[matchField]) { + // Field mismatch. + return; + } } } if (hatMeta.restartExistingThreads) { // If `restartExistingThreads` is true, we should stop // any existing threads starting with the top block. - for (let i = 0; i < this.threads.length; i++) { - if (this.threads[i].target === target && - this.threads[i].topBlock === topBlockId && - // stack click threads and hat threads can coexist - !this.threads[i].stackClick) { - newThreads.push(this._restartThread(this.threads[i])); + for (let i = 0; i < instance.threads.length; i++) { + if (instance.threads[i].topBlock === topBlockId && + !instance.threads[i].stackClick && // stack click threads and hat threads can coexist + instance.threads[i].target === target) { + newThreads.push(instance._restartThread(instance.threads[i])); return; } } } else { // If `restartExistingThreads` is false, we should // give up if any threads with the top block are running. - for (let j = 0; j < this.threads.length; j++) { - if (this.threads[j].target === target && - this.threads[j].topBlock === topBlockId && - // stack click threads and hat threads can coexist - !this.threads[j].stackClick && - this.threads[j].status !== Thread.STATUS_DONE) { + for (let j = 0; j < instance.threads.length; j++) { + if (instance.threads[j].topBlock === topBlockId && + instance.threads[j].target === target && + !instance.threads[j].stackClick && // stack click threads and hat threads can coexist + instance.threads[j].status !== Thread.STATUS_DONE) { // Some thread is already running. return; } } } // Start the thread with this top block. - newThreads.push(this._pushThread(topBlockId, target)); + newThreads.push(instance._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 diff --git a/test/integration/hat-threads-run-every-frame.js b/test/integration/hat-threads-run-every-frame.js index 9eaf9771f..f7a519b34 100644 --- a/test/integration/hat-threads-run-every-frame.js +++ b/test/integration/hat-threads-run-every-frame.js @@ -64,52 +64,6 @@ test('edge activated hat thread runs once every frame', t => { }); }); -/** - * When a hat is added it should run in the next frame. Any block related - * caching should be reset. - */ -test('edge activated hat thread runs after being added to previously executed target', t => { - const vm = new VirtualMachine(); - vm.attachStorage(makeTestStorage()); - - // Start VM, load project, and run - t.doesNotThrow(() => { - // Note: don't run vm.start(), we handle calling _step() manually in this test - vm.runtime.currentStepTime = Runtime.THREAD_STEP_INTERVAL; - vm.clear(); - vm.setCompatibilityMode(false); - vm.setTurboMode(false); - - vm.loadProject(project).then(() => { - t.equal(vm.runtime.threads.length, 0); - - vm.runtime._step(); - let threads = vm.runtime._lastStepDoneThreads; - t.equal(vm.runtime.threads.length, 0); - t.equal(threads.length, 1); - checkIsHatThread(t, vm, threads[0]); - t.assert(threads[0].status === Thread.STATUS_DONE); - - // Add a second hat that should create a second thread - const hatBlock = threads[0].target.blocks.getBlock(threads[0].topBlock); - threads[0].target.blocks.createBlock(Object.assign( - {}, hatBlock, {id: 'hatblock2', next: null} - )); - - // Check that the hat thread is added again when another step is taken - vm.runtime._step(); - threads = vm.runtime._lastStepDoneThreads; - t.equal(vm.runtime.threads.length, 0); - t.equal(threads.length, 2); - checkIsHatThread(t, vm, threads[0]); - checkIsHatThread(t, vm, threads[1]); - t.assert(threads[0].status === Thread.STATUS_DONE); - t.assert(threads[1].status === Thread.STATUS_DONE); - t.end(); - }); - }); -}); - /** * If the hat doesn't finish evaluating within one frame, it shouldn't be added again * on the next frame. (We skip execution by setting the step time to 0)