Merge pull request #2083 from cwillisf/extension-buttons

Allow extensions to make buttons

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,

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: '<sep gap="36"/>'
-};
-
 /**
  * Numeric ID for Runtime._step in Profiler instances.
  * @type {number}
@@ -866,14 +856,11 @@ 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 (convertedBlock.json) {
+                    const opcode = convertedBlock.json.type;
                     if (blockInfo.blockType !== BlockType.EVENT) {
                         this._primitives[opcode] = convertedBlock.info.func;
                     }
@@ -883,6 +870,7 @@ class Runtime extends EventEmitter {
                             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: '<sep gap="36"/>'
+        };
+    }
+
+    /**
+     * 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: `<button text="${buttonText}" callbackKey="${buttonInfo.func}"></button>`
+        };
+    }
+
     /**
      * 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.

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
      */

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;
 
-            /**
+            // Avoid promise overhead if possible
-             * 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 {
@@ -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) {
+            break;
-            log.warn(`Ignoring function "${blockInfo.func}" for event block ${blockInfo.opcode}`);
         }
 
         return blockInfo;

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.type(vm.runtime._blockInfo[0].blocks[0].info.func, 'MAKE_A_VARIABLE');
-    t.equal(vm.runtime._blockInfo[0].blocks[0].info.blockType, 'reporter');
+    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();
 });

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, '<button text="this is a button" callbackKey="MAKE_A_VARIABLE"></button>');
+};
+
 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, '<sep gap="36"/>');
 };
 
@@ -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
+        blocksInfo.forEach(blockInfo => {
-        const [reporter, separator, command, conditional, loop] = blocksInfo;
+            // `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);