From af058b8146651b70fb1a224587711735c3682bc6 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Tue, 27 Mar 2018 14:35:04 -0700 Subject: [PATCH] Support LOOP and CONDITIONAL blocks in extensions A LOOP block is like a conditional, but the LOOP block will be re-evaluated after any child branch runs. Also: - Support using '---' as a block separator - Refactor common code from `_registerExtensionPrimitives` and `_refreshExtensionPrimitives` into new `_fillExtensionCategory` - Improve error reporting during block conversion --- src/engine/runtime.js | 290 ++++++++++++-------- src/extension-support/block-type.js | 7 + src/extension-support/extension-manager.js | 61 ++-- src/extension-support/extension-metadata.js | 4 +- 4 files changed, 229 insertions(+), 133 deletions(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index aa73c9690..86881d166 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -8,6 +8,7 @@ const BlockType = require('../extension-support/block-type'); const Sequencer = require('./sequencer'); const Thread = require('./thread'); const Profiler = require('./profiler'); +const log = require('../util/log'); // Virtual I/O devices. const Clock = require('../io/clock'); @@ -55,6 +56,16 @@ const ArgumentTypeMap = (() => { return map; })(); +/** + * Predefined "Converted block info" for a separator between blocks in a block category + * @type {ConvertedBlockInfo} + */ +const ConvertedSeparator = { + info: {}, + json: null, + xml: '' +}; + /** * These constants are copied from scratch-blocks/core/constants.js * @TODO find a way to require() these... maybe make a scratch-blocks/dist/constants.js or something like that? @@ -486,7 +497,7 @@ class Runtime extends EventEmitter { /** * Register the primitives provided by an extension. - * @param {ExtensionInfo} extensionInfo - information about the extension (id, blocks, etc.) + * @param {ExtensionMetadata} extensionInfo - information about the extension (id, blocks, etc.) * @private */ _registerExtensionPrimitives (extensionInfo) { @@ -504,30 +515,14 @@ class Runtime extends EventEmitter { this._blockInfo.push(categoryInfo); - for (const menuName in extensionInfo.menus) { - if (extensionInfo.menus.hasOwnProperty(menuName)) { - const menuItems = extensionInfo.menus[menuName]; - const convertedMenu = this._buildMenuForScratchBlocks(menuName, menuItems, categoryInfo); - categoryInfo.menus.push(convertedMenu); - } - } - for (const blockInfo of extensionInfo.blocks) { - const convertedBlock = this._convertForScratchBlocks(blockInfo, categoryInfo); - const opcode = convertedBlock.json.type; - categoryInfo.blocks.push(convertedBlock); - this._primitives[opcode] = convertedBlock.info.func; - if (blockInfo.blockType === BlockType.HAT) { - this._hats[opcode] = {edgeActivated: true}; /** @TODO let extension specify this */ - } - } + this._fillExtensionCategory(categoryInfo, extensionInfo); this.emit(Runtime.EXTENSION_ADDED, categoryInfo.blocks.concat(categoryInfo.menus)); } /** * Reregister the primitives for an extension - * @param {ExtensionInfo} extensionInfo - new info (results of running getInfo) - * for an extension + * @param {ExtensionMetadata} extensionInfo - new info (results of running getInfo) for an extension * @private */ _refreshExtensionPrimitives (extensionInfo) { @@ -536,22 +531,7 @@ class Runtime extends EventEmitter { if (extensionInfo.id === categoryInfo.id) { categoryInfo.blocks = []; categoryInfo.menus = []; - for (const menuName in extensionInfo.menus) { - if (extensionInfo.menus.hasOwnProperty(menuName)) { - const menuItems = extensionInfo.menus[menuName]; - const convertedMenu = this._buildMenuForScratchBlocks(menuName, menuItems, categoryInfo); - categoryInfo.menus.push(convertedMenu); - } - } - for (const blockInfo of extensionInfo.blocks) { - const convertedBlock = this._convertForScratchBlocks(blockInfo, categoryInfo); - const opcode = convertedBlock.json.type; - categoryInfo.blocks.push(convertedBlock); - this._primitives[opcode] = convertedBlock.info.func; - if (blockInfo.blockType === BlockType.HAT) { - this._hats[opcode] = {edgeActivated: true}; /** @TODO let extension specify this */ - } - } + this._fillExtensionCategory(categoryInfo, extensionInfo); extensionBlocks = extensionBlocks.concat(categoryInfo.blocks, categoryInfo.menus); } } @@ -559,6 +539,40 @@ class Runtime extends EventEmitter { this.emit(Runtime.BLOCKSINFO_UPDATE, extensionBlocks); } + /** + * Read extension information, convert menus and blocks, and store the results in the provided category object. + * @param {CategoryInfo} categoryInfo - the category to be filled + * @param {ExtensionMetadata} extensionInfo - the extension metadata to read + * @private + */ + _fillExtensionCategory (categoryInfo, extensionInfo) { + for (const menuName in extensionInfo.menus) { + if (extensionInfo.menus.hasOwnProperty(menuName)) { + const menuItems = extensionInfo.menus[menuName]; + const convertedMenu = this._buildMenuForScratchBlocks(menuName, menuItems, categoryInfo); + categoryInfo.menus.push(convertedMenu); + } + } + for (const blockInfo of extensionInfo.blocks) { + if (blockInfo === '---') { + categoryInfo.blocks.push(ConvertedSeparator); + continue; + } + try { + const convertedBlock = this._convertForScratchBlocks(blockInfo, categoryInfo); + const opcode = convertedBlock.json.type; + categoryInfo.blocks.push(convertedBlock); + this._primitives[opcode] = convertedBlock.info.func; + if (blockInfo.blockType === BlockType.HAT) { + this._hats[opcode] = {edgeActivated: true}; + /** @TODO let extension specify this */ + } + } catch (e) { + log.error('Error parsing block: ', {block: blockInfo, error: e}); + } + } + } + /** * Build the scratch-blocks JSON for a menu. Note that scratch-blocks treats menus as a special kind of block. * @param {string} menuName - the name of the menu @@ -609,11 +623,12 @@ class Runtime extends EventEmitter { * Convert BlockInfo into scratch-blocks JSON & XML, and generate a proxy function. * @param {BlockInfo} blockInfo - the block to convert * @param {CategoryInfo} categoryInfo - the category for this block - * @returns {{info: BlockInfo, json: object, xml: string}} - the converted & original block information + * @returns {ConvertedBlockInfo} - the converted & original block information * @private */ _convertForScratchBlocks (blockInfo, categoryInfo) { const extendedOpcode = `${categoryInfo.id}.${blockInfo.opcode}`; + const blockJSON = { type: extendedOpcode, inputsInline: true, @@ -621,19 +636,19 @@ class Runtime extends EventEmitter { colour: categoryInfo.color1, colourSecondary: categoryInfo.color2, colourTertiary: categoryInfo.color3, - args0: [], extensions: ['scratch_extension'] }; - - const inputList = []; - - // TODO: store this somewhere so that we can map args appropriately after translation. - // This maps an arg name to its relative position in the original (usually English) block text. - // When displaying a block in another language we'll need to run a `replace` action similar to the one below, - // but each `[ARG]` will need to be replaced with the number in this map instead of `args0.length`. - const argsMap = {}; - - blockJSON.message0 = ''; + const context = { + // TODO: store this somewhere so that we can map args appropriately after translation. + // This maps an arg name to its relative position in the original (usually English) block text. + // When displaying a block in another language we'll need to run a `replace` action similar to the one + // below, but each `[ARG]` will need to be replaced with the number in this map. + argsMap: {}, + blockJSON, + categoryInfo, + blockInfo, + inputList: [] + }; // If an icon for the extension exists, prepend it to each block, with a vertical separator. if (categoryInfo.blockIconURI) { @@ -647,60 +662,12 @@ class Runtime extends EventEmitter { const separatorJSON = { type: 'field_vertical_separator' }; - blockJSON.args0.push(iconJSON); - blockJSON.args0.push(separatorJSON); + blockJSON.args0 = [ + iconJSON, + separatorJSON + ]; } - blockJSON.message0 += blockInfo.text.replace(/\[(.+?)]/g, (match, placeholder) => { - // Sanitize the placeholder to ensure valid XML - placeholder = placeholder.replace(/[<"&]/, '_'); - - const argJSON = { - type: 'input_value', - name: placeholder - }; - - const argInfo = blockInfo.arguments[placeholder] || {}; - const argTypeInfo = ArgumentTypeMap[argInfo.type] || {}; - const defaultValue = (typeof argInfo.defaultValue === 'undefined' ? - '' : - escapeHtml(argInfo.defaultValue.toString())); - - if (argTypeInfo.check) { - argJSON.check = argTypeInfo.check; - } - - const shadowType = (argInfo.menu ? - this._makeExtensionMenuId(argInfo.menu, categoryInfo.id) : - argTypeInfo.shadowType); - const fieldType = argInfo.menu || argTypeInfo.fieldType; - - // is the ScratchBlocks name for a block input. - inputList.push(``); - - // The is a placeholder for a reporter and is visible when there's no reporter in this input. - // Boolean inputs don't need to specify a shadow in the XML. - if (shadowType) { - inputList.push(``); - - // is a text field that the user can type into. Some shadows, like the color picker, don't allow - // text input and therefore don't need a field element. - if (fieldType) { - inputList.push(`${defaultValue}`); - } - - inputList.push(''); - } - - inputList.push(''); - - // scratch-blocks uses 1-based argument indexing - blockJSON.args0.push(argJSON); - const argNum = blockJSON.args0.length; - argsMap[placeholder] = argNum; - return `%${argNum}`; - }); - switch (blockInfo.blockType) { case BlockType.COMMAND: blockJSON.outputShape = ScratchBlocksConstants.OUTPUT_SHAPE_SQUARE; @@ -722,33 +689,130 @@ class Runtime extends EventEmitter { blockJSON.nextStatement = null; // null = available connection; undefined = terminal break; case BlockType.CONDITIONAL: - // Statement inputs get names like 'SUBSTACK', 'SUBSTACK2', 'SUBSTACK3', ... - for (let branchNum = 1; branchNum <= blockInfo.branchCount; ++branchNum) { - blockJSON[`message${branchNum}`] = '%1'; - blockJSON[`args${branchNum}`] = [{ - type: 'input_statement', - name: `SUBSTACK${branchNum > 1 ? branchNum : ''}` - }]; - } + case BlockType.LOOP: + blockInfo.branchCount = blockInfo.branchCount || 1; blockJSON.outputShape = ScratchBlocksConstants.OUTPUT_SHAPE_SQUARE; blockJSON.previousStatement = null; // null = available connection; undefined = hat - blockJSON.nextStatement = null; // null = available connection; undefined = terminal + if (!blockInfo.isTerminal) { + blockJSON.nextStatement = null; // null = available connection; undefined = terminal + } break; } - if (blockInfo.isTerminal) { - delete blockJSON.nextStatement; + const blockText = Array.isArray(blockInfo.text) ? blockInfo.text : [blockInfo.text]; + let inTextNum = 0; // text for the next block "arm" is blockText[inTextNum] + let inBranchNum = 0; // how many branches have we placed into the JSON so far? + let outLineNum = 0; // used for scratch-blocks `message${outLineNum}` and `args${outLineNum}` + const convertPlaceholders = this._convertPlaceholders.bind(this, context); + + // alternate between a block "arm" with text on it and an open slot for a substack + while (inTextNum < blockText.length || inBranchNum < blockInfo.branchCount) { + if (inTextNum < blockText.length) { + context.outLineNum = outLineNum; + const convertedText = blockText[inTextNum].replace(/\[(.+?)]/g, convertPlaceholders); + if (blockJSON[`message${outLineNum}`]) { + blockJSON[`message${outLineNum}`] += convertedText; + } else { + blockJSON[`message${outLineNum}`] = convertedText; + } + ++inTextNum; + ++outLineNum; + } + if (inBranchNum < blockInfo.branchCount) { + blockJSON[`message${outLineNum}`] = '%1'; + blockJSON[`args${outLineNum}`] = [{ + type: 'input_statement', + name: `SUBSTACK${inBranchNum > 0 ? inBranchNum + 1 : ''}` + }]; + ++inBranchNum; + ++outLineNum; + } } - const blockXML = `${inputList.join('')}`; + // Add icon to the bottom right of a loop block + if (blockInfo.blockType === BlockType.LOOP) { + blockJSON[`lastDummyAlign${outLineNum}`] = 'RIGHT'; + blockJSON[`message${outLineNum}`] = '%1'; + blockJSON[`args${outLineNum}`] = [{ + type: 'field_image', + src: './static/blocks-media/repeat.svg', // TODO: use a constant or make this configurable? + width: 24, + height: 24, + alt: '*', + flip_rtl: true + }]; + ++outLineNum; + } + + const blockXML = `${context.inputList.join('')}`; return { - info: blockInfo, - json: blockJSON, + info: context.blockInfo, + json: context.blockJSON, xml: blockXML }; } + /** + * Helper for _convertForScratchBlocks which handles linearization of argument placeholders. Called as a callback + * from string#replace. In addition to the return value the JSON and XML items in the context will be filled. + * @param {object} context - information shared with _convertForScratchBlocks about the block, etc. + * @param {string} match - the overall string matched by the placeholder regex, including brackets: '[FOO]'. + * @param {string} placeholder - the name of the placeholder being matched: 'FOO'. + * @return {string} scratch-blocks placeholder for the argument: '%1'. + * @private + */ + _convertPlaceholders (context, match, placeholder) { + // Sanitize the placeholder to ensure valid XML + placeholder = placeholder.replace(/[<"&]/, '_'); + + const argJSON = { + type: 'input_value', + name: placeholder + }; + + const argInfo = context.blockInfo.arguments[placeholder] || {}; + const argTypeInfo = ArgumentTypeMap[argInfo.type] || {}; + const defaultValue = (typeof argInfo.defaultValue === 'undefined' ? + '' : + escapeHtml(argInfo.defaultValue.toString())); + + if (argTypeInfo.check) { + argJSON.check = argTypeInfo.check; + } + + const shadowType = (argInfo.menu ? + this._makeExtensionMenuId(argInfo.menu, context.categoryInfo.id) : + argTypeInfo.shadowType); + const fieldType = argInfo.menu || argTypeInfo.fieldType; + + // is the ScratchBlocks name for a block input. + context.inputList.push(``); + + // The is a placeholder for a reporter and is visible when there's no reporter in this input. + // Boolean inputs don't need to specify a shadow in the XML. + if (shadowType) { + context.inputList.push(``); + + // is a text field that the user can type into. Some shadows, like the color picker, don't allow + // text input and therefore don't need a field element. + if (fieldType) { + context.inputList.push(`${defaultValue}`); + } + + context.inputList.push(''); + } + + context.inputList.push(''); + + const argsName = `args${context.outLineNum}`; + const blockArgs = (context.blockJSON[argsName] = context.blockJSON[argsName] || []); + blockArgs.push(argJSON); + const argNum = blockArgs.length; + context.argsMap[placeholder] = argNum; + return `%${argNum}`; + } + /** * @returns {string} scratch-blocks XML description for all dynamic blocks, wrapped in elements. */ diff --git a/src/extension-support/block-type.js b/src/extension-support/block-type.js index 9a87b745f..4db5e5d52 100644 --- a/src/extension-support/block-type.js +++ b/src/extension-support/block-type.js @@ -15,6 +15,7 @@ const BlockType = { /** * Specialized command block which may or may not run a child branch + * The thread continues with the next block whether or not a child branch ran. */ CONDITIONAL: 'conditional', @@ -23,6 +24,12 @@ const BlockType = { */ HAT: 'hat', + /** + * Specialized command block which may or may not run a child branch + * If a child branch runs, the thread evaluates the loop block again. + */ + LOOP: 'loop', + /** * General reporter with numeric or string value */ diff --git a/src/extension-support/extension-manager.js b/src/extension-support/extension-manager.js index bf988adb9..ba3ac6a9b 100644 --- a/src/extension-support/extension-manager.js +++ b/src/extension-support/extension-manager.js @@ -35,13 +35,23 @@ const builtinExtensions = { * @property {Boolean|undefined} hideFromPalette - true if should not be appear in the palette. (default false) */ +/** + * @typedef {object} ConvertedBlockInfo - Raw extension block data paired with processed data ready for scratch-blocks + * @property {BlockInfo} info - the raw block info + * @property {object} json - the scratch-blocks JSON definition for this block + * @property {string} xml - the scratch-blocks XML definition for this block + */ + /** * @typedef {object} CategoryInfo - Information about a block category * @property {string} id - the unique ID of this category + * @property {string} name - the human-readable name of this category + * @property {string|undefined} blockIconURI - optional URI for the block icon image * @property {string} color1 - the primary color for this category, in '#rrggbb' format * @property {string} color2 - the secondary color for this category, in '#rrggbb' format * @property {string} color3 - the tertiary color for this category, in '#rrggbb' format - * @property {Array.} block - the blocks in this category + * @property {Array.} blocks - the blocks, separators, etc. in this category + * @property {Array.} menus - the menus provided by this category */ /** @@ -205,7 +215,7 @@ class ExtensionManager { _registerExtensionInfo (serviceName, extensionInfo) { extensionInfo = this._prepareExtensionInfo(serviceName, extensionInfo); dispatch.call('runtime', '_registerExtensionPrimitives', extensionInfo).catch(e => { - log.error(`Failed to register primitives for extension on service ${serviceName}: ${JSON.stringify(e)}`); + log.error(`Failed to register primitives for extension on service ${serviceName}:`, e); }); } @@ -233,14 +243,23 @@ class ExtensionManager { extensionInfo.name = extensionInfo.name || extensionInfo.id; extensionInfo.blocks = extensionInfo.blocks || []; extensionInfo.targetTypes = extensionInfo.targetTypes || []; - extensionInfo.blocks = extensionInfo.blocks.reduce((result, blockInfo) => { + extensionInfo.blocks = extensionInfo.blocks.reduce((results, blockInfo) => { try { - result.push(this._prepareBlockInfo(serviceName, blockInfo)); + let result; + switch (blockInfo) { + case '---': // separator + result = '---'; + break; + default: // a BlockInfo object + result = this._prepareBlockInfo(serviceName, blockInfo); + break; + } + results.push(result); } catch (e) { // TODO: more meaningful error reporting log.error(`Error processing block: ${e.message}, Block:\n${JSON.stringify(blockInfo)}`); } - return result; + return results; }, []); extensionInfo.menus = extensionInfo.menus || []; extensionInfo.menus = this._prepareMenuInfo(serviceName, extensionInfo.menus); @@ -309,24 +328,30 @@ class ExtensionManager { arguments: {} }, blockInfo); blockInfo.opcode = this._sanitizeID(blockInfo.opcode); - blockInfo.func = blockInfo.func ? this._sanitizeID(blockInfo.func) : blockInfo.opcode; blockInfo.text = blockInfo.text || blockInfo.opcode; - /** - * This is only here because the VM performs poorly when blocks return promises. - * @TODO make it possible for the VM to resolve a promise and continue during the same frame. - */ - if (dispatch._isRemoteService(serviceName)) { - blockInfo.func = dispatch.call.bind(dispatch, serviceName, blockInfo.func); - } else { - const serviceObject = dispatch.services[serviceName]; - const func = serviceObject[blockInfo.func]; - if (func) { - blockInfo.func = func.bind(serviceObject); + if (blockInfo.blockType !== BlockType.EVENT) { + blockInfo.func = blockInfo.func ? this._sanitizeID(blockInfo.func) : blockInfo.opcode; + + /** + * This is only here because the VM performs poorly when blocks return promises. + * @TODO make it possible for the VM to resolve a promise and continue during the same Scratch "tick" + */ + if (dispatch._isRemoteService(serviceName)) { + blockInfo.func = dispatch.call.bind(dispatch, serviceName, blockInfo.func); } else { - throw new Error(`Could not find extension block function called ${blockInfo.func}`); + const serviceObject = dispatch.services[serviceName]; + const func = serviceObject[blockInfo.func]; + if (func) { + blockInfo.func = func.bind(serviceObject); + } else if (blockInfo.blockType !== BlockType.EVENT) { + throw new Error(`Could not find extension block function called ${blockInfo.func}`); + } } + } else if (blockInfo.func) { + log.warn(`Ignoring function "${blockInfo.func}" for event block ${blockInfo.opcode}`); } + return blockInfo; } } diff --git a/src/extension-support/extension-metadata.js b/src/extension-support/extension-metadata.js index 81dbe5bf6..515c853ba 100644 --- a/src/extension-support/extension-metadata.js +++ b/src/extension-support/extension-metadata.js @@ -6,8 +6,8 @@ * @property {string} blockIconURI - URI for an image to be placed on each block in this extension. Data URI ok. * @property {string} menuIconURI - URI for an image to be placed on this extension's category menu entry. Data URI ok. * @property {string} docsURI - link to documentation content for this extension. - * @property {Array.} - the blocks provided by this extension, with optional separators. - * @property {Object.} - map of menu name to metadata about each of this extension's menus. + * @property {Array.} blocks - the blocks provided by this extension, plus separators. + * @property {Object.} menus - map of menu name to metadata about each of this extension's menus. */ /**