diff --git a/src/blocks/scratch3_core_example.js b/src/blocks/scratch3_core_example.js index 7781bf25a..501e569c4 100644 --- a/src/blocks/scratch3_core_example.js +++ b/src/blocks/scratch3_core_example.js @@ -22,6 +22,11 @@ class Scratch3CoreExample { id: 'coreExample', name: 'CoreEx', // This string does not need to be translated as this extension is only used as an example. blocks: [ + { + func: 'MAKE_A_VARIABLE', + blockType: BlockType.BUTTON, + text: 'make a variable (CoreEx)' + }, { opcode: 'exampleOpcode', blockType: BlockType.REPORTER, diff --git a/src/engine/runtime.js b/src/engine/runtime.js index c2e37f3c7..204cd4633 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -123,16 +123,6 @@ const cloudDataManager = () => { }; }; -/** - * Predefined "Converted block info" for a separator between blocks in a block category - * @type {ConvertedBlockInfo} - */ -const ConvertedSeparator = { - info: {}, - json: null, - xml: '' -}; - /** * Numeric ID for Runtime._step in Profiler instances. * @type {number} @@ -866,22 +856,20 @@ class Runtime extends EventEmitter { } 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); - if (blockInfo.blockType !== BlockType.EVENT) { - this._primitives[opcode] = convertedBlock.info.func; - } - if (blockInfo.blockType === BlockType.EVENT || blockInfo.blockType === BlockType.HAT) { - this._hats[opcode] = { - edgeActivated: blockInfo.isEdgeActivated, - restartExistingThreads: blockInfo.shouldRestartExistingThreads - }; + if (convertedBlock.json) { + const opcode = convertedBlock.json.type; + if (blockInfo.blockType !== BlockType.EVENT) { + this._primitives[opcode] = convertedBlock.info.func; + } + if (blockInfo.blockType === BlockType.EVENT || blockInfo.blockType === BlockType.HAT) { + this._hats[opcode] = { + edgeActivated: blockInfo.isEdgeActivated, + restartExistingThreads: blockInfo.shouldRestartExistingThreads + }; + } } } catch (e) { log.error('Error parsing block: ', {block: blockInfo, error: e}); @@ -986,6 +974,25 @@ class Runtime extends EventEmitter { }; } + /** + * Convert ExtensionBlockMetadata into data ready for scratch-blocks. + * @param {ExtensionBlockMetadata} blockInfo - the block info to convert + * @param {CategoryInfo} categoryInfo - the category for this block + * @returns {ConvertedBlockInfo} - the converted & original block information + * @private + */ + _convertForScratchBlocks (blockInfo, categoryInfo) { + if (blockInfo === '---') { + return this._convertSeparatorForScratchBlocks(blockInfo); + } + + if (blockInfo.blockType === BlockType.BUTTON) { + return this._convertButtonForScratchBlocks(blockInfo); + } + + return this._convertBlockForScratchBlocks(blockInfo, categoryInfo); + } + /** * Convert ExtensionBlockMetadata into scratch-blocks JSON & XML, and generate a proxy function. * @param {ExtensionBlockMetadata} blockInfo - the block to convert @@ -993,7 +1000,7 @@ class Runtime extends EventEmitter { * @returns {ConvertedBlockInfo} - the converted & original block information * @private */ - _convertForScratchBlocks (blockInfo, categoryInfo) { + _convertBlockForScratchBlocks (blockInfo, categoryInfo) { const extendedOpcode = `${categoryInfo.id}_${blockInfo.opcode}`; const blockJSON = { @@ -1135,6 +1142,43 @@ class Runtime extends EventEmitter { }; } + /** + * Generate a separator between blocks categories or sub-categories. + * @param {ExtensionBlockMetadata} blockInfo - the block to convert + * @param {CategoryInfo} categoryInfo - the category for this block + * @returns {ConvertedBlockInfo} - the converted & original block information + * @private + */ + _convertSeparatorForScratchBlocks (blockInfo) { + return { + info: blockInfo, + xml: '' + }; + } + + /** + * Convert a button for scratch-blocks. A button has no opcode but specifies a callback name in the `func` field. + * @param {ExtensionBlockMetadata} buttonInfo - the button to convert + * @property {string} func - the callback name + * @param {CategoryInfo} categoryInfo - the category for this button + * @returns {ConvertedBlockInfo} - the converted & original button information + * @private + */ + _convertButtonForScratchBlocks (buttonInfo) { + // for now we only support these pre-defined callbacks handled in scratch-blocks + const supportedCallbackKeys = ['MAKE_A_LIST', 'MAKE_A_PROCEDURE', 'MAKE_A_VARIABLE']; + if (supportedCallbackKeys.indexOf(buttonInfo.func) < 0) { + log.error(`Custom button callbacks not supported yet: ${buttonInfo.func}`); + } + + const extensionMessageContext = this.makeMessageContextForTarget(); + const buttonText = maybeFormatMessage(buttonInfo.text, extensionMessageContext); + return { + info: buttonInfo, + xml: `` + }; + } + /** * 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. diff --git a/src/extension-support/block-type.js b/src/extension-support/block-type.js index 3b38c0511..e7f0b18d1 100644 --- a/src/extension-support/block-type.js +++ b/src/extension-support/block-type.js @@ -8,6 +8,11 @@ const BlockType = { */ BOOLEAN: 'Boolean', + /** + * A button (not an actual block) for some special action, like making a variable + */ + BUTTON: 'button', + /** * Command block */ diff --git a/src/extension-support/extension-manager.js b/src/extension-support/extension-manager.js index 99fa83bf1..d3ced5df1 100644 --- a/src/extension-support/extension-manager.js +++ b/src/extension-support/extension-manager.js @@ -375,16 +375,28 @@ class ExtensionManager { blockAllThreads: false, arguments: {} }, blockInfo); - blockInfo.opcode = this._sanitizeID(blockInfo.opcode); + blockInfo.opcode = blockInfo.opcode && this._sanitizeID(blockInfo.opcode); blockInfo.text = blockInfo.text || blockInfo.opcode; - if (blockInfo.blockType !== BlockType.EVENT) { + switch (blockInfo.blockType) { + case BlockType.EVENT: + if (blockInfo.func) { + log.warn(`Ignoring function "${blockInfo.func}" for event block ${blockInfo.opcode}`); + } + break; + case BlockType.BUTTON: + if (blockInfo.opcode) { + log.warn(`Ignoring opcode "${blockInfo.opcode}" for button with text: ${blockInfo.text}`); + } + break; + default: + if (!blockInfo.opcode) { + throw new Error('Missing opcode for block'); + } + 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" - */ + // Avoid promise overhead if possible if (dispatch._isRemoteService(serviceName)) { blockInfo.func = dispatch.call.bind(dispatch, serviceName, blockInfo.func); } else { @@ -392,12 +404,11 @@ class ExtensionManager { const func = serviceObject[blockInfo.func]; if (func) { blockInfo.func = func.bind(serviceObject); - } else if (blockInfo.blockType !== BlockType.EVENT) { + } else { 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}`); + break; } return blockInfo; diff --git a/test/integration/internal-extension.js b/test/integration/internal-extension.js index 476981c05..9ad399402 100644 --- a/test/integration/internal-extension.js +++ b/test/integration/internal-extension.js @@ -83,13 +83,18 @@ test('load sync', t => { t.ok(vm.extensionManager.isExtensionLoaded('coreExample')); t.equal(vm.runtime._blockInfo.length, 1); - t.equal(vm.runtime._blockInfo[0].blocks.length, 1); + + // blocks should be an array of two items: a button pseudo-block and a reporter block. + t.equal(vm.runtime._blockInfo[0].blocks.length, 2); t.type(vm.runtime._blockInfo[0].blocks[0].info, 'object'); - t.equal(vm.runtime._blockInfo[0].blocks[0].info.opcode, 'exampleOpcode'); - t.equal(vm.runtime._blockInfo[0].blocks[0].info.blockType, 'reporter'); + t.type(vm.runtime._blockInfo[0].blocks[0].info.func, 'MAKE_A_VARIABLE'); + t.equal(vm.runtime._blockInfo[0].blocks[0].info.blockType, 'button'); + t.type(vm.runtime._blockInfo[0].blocks[1].info, 'object'); + t.equal(vm.runtime._blockInfo[0].blocks[1].info.opcode, 'exampleOpcode'); + t.equal(vm.runtime._blockInfo[0].blocks[1].info.blockType, 'reporter'); // Test the opcode function - t.equal(vm.runtime._blockInfo[0].blocks[0].info.func(), 'no stage yet'); + t.equal(vm.runtime._blockInfo[0].blocks[1].info.func(), 'no stage yet'); const sprite = new Sprite(null, vm.runtime); sprite.name = 'Stage'; @@ -97,7 +102,7 @@ test('load sync', t => { stage.isStage = true; vm.runtime.targets = [stage]; - t.equal(vm.runtime._blockInfo[0].blocks[0].info.func(), 'Stage'); + t.equal(vm.runtime._blockInfo[0].blocks[1].info.func(), 'Stage'); t.end(); }); diff --git a/test/unit/extension_conversion.js b/test/unit/extension_conversion.js index 07f599230..3a59c0a01 100644 --- a/test/unit/extension_conversion.js +++ b/test/unit/extension_conversion.js @@ -12,6 +12,11 @@ const testExtensionInfo = { id: 'test', name: 'fake test extension', blocks: [ + { + func: 'MAKE_A_VARIABLE', + blockType: BlockType.BUTTON, + text: 'this is a button' + }, { opcode: 'reporter', blockType: BlockType.REPORTER, @@ -58,6 +63,11 @@ const testExtensionInfo = { ] }; +const testButton = function (t, button) { + t.same(button.json, null); // should be null or undefined + t.equal(button.xml, ''); +}; + const testReporter = function (t, reporter) { t.equal(reporter.json.type, 'test_reporter'); t.equal(reporter.json.outputShape, ScratchBlocksConstants.OUTPUT_SHAPE_ROUND); @@ -72,7 +82,7 @@ const testReporter = function (t, reporter) { }; const testSeparator = function (t, separator) { - t.equal(separator.json, null); + t.same(separator.json, null); // should be null or undefined t.equal(separator.xml, ''); }; @@ -153,9 +163,15 @@ test('registerExtensionPrimitives', t => { runtime.on(Runtime.EXTENSION_ADDED, blocksInfo => { t.equal(blocksInfo.length, testExtensionInfo.blocks.length); - // Note that this also implicitly tests that block order is preserved - const [reporter, separator, command, conditional, loop] = blocksInfo; + blocksInfo.forEach(blockInfo => { + // `true` here means "either an object or a non-empty string but definitely not null or undefined" + t.true(blockInfo.info, 'Every block and pseudo-block must have a non-empty "info" field'); + }); + // Note that this also implicitly tests that block order is preserved + const [button, reporter, separator, command, conditional, loop] = blocksInfo; + + testButton(t, button); testReporter(t, reporter); testSeparator(t, separator); testCommand(t, command);