From 638062e982769553fead0db6cb838a5c1caf7b9c Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Mon, 8 Apr 2019 15:20:14 -0700 Subject: [PATCH 01/23] pass `blockInfo` to extension methods Sometimes a single extension method needs to service several different instances of the same opcode. This can happen with variables or custom procedures, for example. This change allows the extension method to inspect the `blockInfo` for instance data, including arbitrary extension-specific properties if necessary. --- src/extension-support/extension-manager.js | 27 +++++++++++----------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/extension-support/extension-manager.js b/src/extension-support/extension-manager.js index bf666a28..e70929b6 100644 --- a/src/extension-support/extension-manager.js +++ b/src/extension-support/extension-manager.js @@ -391,23 +391,22 @@ class ExtensionManager { } break; default: - if (!blockInfo.opcode) { - throw new Error('Missing opcode for block'); - } + if (blockInfo.opcode) { + const funcName = blockInfo.func ? this._sanitizeID(blockInfo.func) : blockInfo.opcode; - blockInfo.func = blockInfo.func ? this._sanitizeID(blockInfo.func) : blockInfo.opcode; - - // Avoid promise overhead if possible - 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); + // Avoid promise latency unless necessary + if (dispatch._isRemoteService(serviceName)) { + blockInfo.func = (args, util) => dispatch.call(serviceName, funcName, args, util, blockInfo); } else { - throw new Error(`Could not find extension block function called ${blockInfo.func}`); + const serviceObject = dispatch.services[serviceName]; + if (!serviceObject[funcName]) { + // The function might show up later as a dynamic property of the service object + log.warn(`Could not find extension block function called ${funcName}`); + } + blockInfo.func = (args, util) => serviceObject[funcName](args, util, blockInfo); } + } else { + throw new Error('Missing opcode for block'); } break; } From 3b395a10d191c648e5736eb19b8aebe9b6021075 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Mon, 8 Apr 2019 17:59:15 -0700 Subject: [PATCH 02/23] add test for extension methods receiving `blockInfo` --- test/integration/internal-extension.js | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/test/integration/internal-extension.js b/test/integration/internal-extension.js index 9ad39940..970db809 100644 --- a/test/integration/internal-extension.js +++ b/test/integration/internal-extension.js @@ -1,6 +1,8 @@ const test = require('tap').test; const Worker = require('tiny-worker'); +const BlockType = require('../../src/extension-support/block-type'); + const dispatch = require('../../src/dispatch/central-dispatch'); const VirtualMachine = require('../../src/virtual-machine'); @@ -33,8 +35,9 @@ class TestInternalExtension { }; } - go () { + go (args, util, blockInfo) { this.status.goCalled = true; + return blockInfo; } _buildAMenu () { @@ -62,9 +65,22 @@ test('internal extension', t => { t.type(func, 'function'); t.notOk(extension.status.goCalled); - func(); + const goBlockInfo = func(); t.ok(extension.status.goCalled); + // The 'go' block returns its own blockInfo. Make sure it matches the expected info. + // Note that the extension parser fills in missing fields so there are more fields here than in `getInfo`. + const expectedBlockInfo = { + arguments: {}, + blockAllThreads: false, + blockType: BlockType.COMMAND, + func: goBlockInfo.func, // Cheat since we don't have a good way to ensure we generate the same function + opcode: 'go', + terminal: false, + text: 'go' + }; + t.deepEqual(goBlockInfo, expectedBlockInfo); + // There should be 2 menus - one is an array, one is the function to call. t.equal(vm.runtime._blockInfo[0].menus.length, 2); // First menu has 3 items. From 833d33355c07897cd9b22e96a76e281b71be36f3 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Fri, 19 Apr 2019 12:28:19 -0700 Subject: [PATCH 03/23] retrieve blockInfo from args when isDynamic is set --- src/engine/mutation-adapter.js | 4 ++ src/extension-support/extension-manager.js | 46 ++++++++++++++-------- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/engine/mutation-adapter.js b/src/engine/mutation-adapter.js index d1984d30..8cfac763 100644 --- a/src/engine/mutation-adapter.js +++ b/src/engine/mutation-adapter.js @@ -13,6 +13,10 @@ const mutatorTagToObject = function (dom) { for (const prop in dom.attribs) { if (prop === 'xmlns') continue; obj[prop] = decodeHtml(dom.attribs[prop]); + if (prop === 'blockinfo') { + obj.blockInfo = JSON.parse(obj.blockinfo); + delete obj.blockinfo; + } } for (let i = 0; i < dom.children.length; i++) { obj.children.push( diff --git a/src/extension-support/extension-manager.js b/src/extension-support/extension-manager.js index e70929b6..a2f89aae 100644 --- a/src/extension-support/extension-manager.js +++ b/src/extension-support/extension-manager.js @@ -390,26 +390,40 @@ class ExtensionManager { log.warn(`Ignoring opcode "${blockInfo.opcode}" for button with text: ${blockInfo.text}`); } break; - default: - if (blockInfo.opcode) { - const funcName = blockInfo.func ? this._sanitizeID(blockInfo.func) : blockInfo.opcode; - - // Avoid promise latency unless necessary - if (dispatch._isRemoteService(serviceName)) { - blockInfo.func = (args, util) => dispatch.call(serviceName, funcName, args, util, blockInfo); - } else { - const serviceObject = dispatch.services[serviceName]; - if (!serviceObject[funcName]) { - // The function might show up later as a dynamic property of the service object - log.warn(`Could not find extension block function called ${funcName}`); - } - blockInfo.func = (args, util) => serviceObject[funcName](args, util, blockInfo); - } - } else { + default: { + if (!blockInfo.opcode) { throw new Error('Missing opcode for block'); } + + const funcName = blockInfo.func ? this._sanitizeID(blockInfo.func) : blockInfo.opcode; + + const getBlockInfo = blockInfo.isDynamic ? + args => args && args.mutation && args.mutation.blockInfo : + () => blockInfo; + const callBlockFunc = (() => { + if (dispatch._isRemoteService(serviceName)) { + return (args, util, realBlockInfo) => + dispatch.call(serviceName, funcName, args, util, realBlockInfo); + } + + // avoid promise latency if we can call direct + const serviceObject = dispatch.services[serviceName]; + if (!serviceObject[funcName]) { + // The function might show up later as a dynamic property of the service object + log.warn(`Could not find extension block function called ${funcName}`); + } + return (args, util, realBlockInfo) => + serviceObject[funcName](args, util, realBlockInfo); + })(); + + blockInfo.func = (args, util) => { + const realBlockInfo = getBlockInfo(args); + // TODO: filter args using the keys of realBlockInfo.arguments? maybe only if sandboxed? + return callBlockFunc(args, util, realBlockInfo); + }; break; } + } return blockInfo; } From bd1aaecdf372589530a6423646e23bd434de4014 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Thu, 18 Apr 2019 09:45:22 -0700 Subject: [PATCH 04/23] add category info to extension add & update events --- src/engine/runtime.js | 12 +++--------- src/virtual-machine.js | 8 ++++---- test/unit/extension_conversion.js | 3 ++- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index b5739579..310d9e77 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -787,11 +787,9 @@ class Runtime extends EventEmitter { this._fillExtensionCategory(categoryInfo, extensionInfo); - const fieldTypeDefinitionsForScratch = []; for (const fieldTypeName in categoryInfo.customFieldTypes) { if (extensionInfo.customFieldTypes.hasOwnProperty(fieldTypeName)) { const fieldTypeInfo = categoryInfo.customFieldTypes[fieldTypeName]; - fieldTypeDefinitionsForScratch.push(fieldTypeInfo.scratchBlocksDefinition); // Emit events for custom field types from extension this.emit(Runtime.EXTENSION_FIELD_ADDED, { @@ -801,9 +799,7 @@ class Runtime extends EventEmitter { } } - const allBlocks = fieldTypeDefinitionsForScratch.concat(categoryInfo.blocks).concat(categoryInfo.menus); - - this.emit(Runtime.EXTENSION_ADDED, allBlocks); + this.emit(Runtime.EXTENSION_ADDED, categoryInfo); } /** @@ -812,18 +808,16 @@ class Runtime extends EventEmitter { * @private */ _refreshExtensionPrimitives (extensionInfo) { - let extensionBlocks = []; for (const categoryInfo of this._blockInfo) { if (extensionInfo.id === categoryInfo.id) { categoryInfo.name = maybeFormatMessage(extensionInfo.name); categoryInfo.blocks = []; categoryInfo.menus = []; this._fillExtensionCategory(categoryInfo, extensionInfo); - extensionBlocks = extensionBlocks.concat(categoryInfo.blocks, categoryInfo.menus); + + this.emit(Runtime.BLOCKSINFO_UPDATE, categoryInfo); } } - - this.emit(Runtime.BLOCKSINFO_UPDATE, extensionBlocks); } /** diff --git a/src/virtual-machine.js b/src/virtual-machine.js index 3320238f..f383c0b0 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -109,14 +109,14 @@ class VirtualMachine extends EventEmitter { this.runtime.on(Runtime.BLOCK_DRAG_END, (blocks, topBlockId) => { this.emit(Runtime.BLOCK_DRAG_END, blocks, topBlockId); }); - this.runtime.on(Runtime.EXTENSION_ADDED, blocksInfo => { - this.emit(Runtime.EXTENSION_ADDED, blocksInfo); + this.runtime.on(Runtime.EXTENSION_ADDED, categoryInfo => { + this.emit(Runtime.EXTENSION_ADDED, categoryInfo); }); this.runtime.on(Runtime.EXTENSION_FIELD_ADDED, (fieldName, fieldImplementation) => { this.emit(Runtime.EXTENSION_FIELD_ADDED, fieldName, fieldImplementation); }); - this.runtime.on(Runtime.BLOCKSINFO_UPDATE, blocksInfo => { - this.emit(Runtime.BLOCKSINFO_UPDATE, blocksInfo); + this.runtime.on(Runtime.BLOCKSINFO_UPDATE, categoryInfo => { + this.emit(Runtime.BLOCKSINFO_UPDATE, categoryInfo); }); this.runtime.on(Runtime.BLOCKS_NEED_UPDATE, () => { this.emitWorkspaceUpdate(); diff --git a/test/unit/extension_conversion.js b/test/unit/extension_conversion.js index 3a59c0a0..001130a3 100644 --- a/test/unit/extension_conversion.js +++ b/test/unit/extension_conversion.js @@ -160,7 +160,8 @@ const testLoop = function (t, loop) { test('registerExtensionPrimitives', t => { const runtime = new Runtime(); - runtime.on(Runtime.EXTENSION_ADDED, blocksInfo => { + runtime.on(Runtime.EXTENSION_ADDED, categoryInfo => { + const blocksInfo = categoryInfo.blocks; t.equal(blocksInfo.length, testExtensionInfo.blocks.length); blocksInfo.forEach(blockInfo => { From 02474477921dba00f2d86cb4b01cef012ab1a5d7 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Mon, 22 Apr 2019 09:53:05 -0700 Subject: [PATCH 05/23] embed extension blockInfo into block XML --- src/engine/runtime.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 310d9e77..44b015fd 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -1127,7 +1127,9 @@ class Runtime extends EventEmitter { ++outLineNum; } - const blockXML = `${context.inputList.join('')}`; + const mutation = blockInfo.isDynamic ? `` : ''; + const inputs = context.inputList.join(''); + const blockXML = `${mutation}${inputs}`; return { info: context.blockInfo, From 107e49245fe2835b6a2e3dd4e92bdb2515413b38 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Mon, 22 Apr 2019 12:32:49 -0700 Subject: [PATCH 06/23] adjust getBlocksXML to return categories separately before: getBlocksXML returns one big XML string after: getBlocksXML returns an array of {id,xml}, one entry per category --- src/engine/runtime.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 44b015fd..a7f97d85 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -1243,11 +1243,12 @@ class Runtime extends EventEmitter { } /** - * @returns {string} scratch-blocks XML description for all dynamic blocks, wrapped in elements. + * @returns {Array.} scratch-blocks XML for each category of extension blocks, in category order. + * @property {string} id - the category / extension ID + * @property {string} xml - the XML text for this category, starting with `` and ending with `` */ getBlocksXML () { - const xmlParts = []; - for (const categoryInfo of this._blockInfo) { + return this._blockInfo.map(categoryInfo => { const {name, color1, color2} = categoryInfo; const paletteBlocks = categoryInfo.blocks.filter(block => !block.info.hideFromPalette); const colorXML = `colour="${color1}" secondaryColour="${color2}"`; @@ -1268,12 +1269,12 @@ class Runtime extends EventEmitter { statusButtonXML = 'showStatusButton="true"'; } - xmlParts.push(``); - xmlParts.push.apply(xmlParts, paletteBlocks.map(block => block.xml)); - xmlParts.push(''); - } - return xmlParts.join('\n'); + return { + id: categoryInfo.id, + xml: `${ + paletteBlocks.map(block => block.xml).join('')}` + }; + }); } /** From a27ea76d25bd1b09b92cfc261ed3bb6f83d477a7 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Mon, 22 Apr 2019 17:47:31 -0700 Subject: [PATCH 07/23] add "scratch_extension" only if a block has an icon --- src/engine/runtime.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index a7f97d85..8b066b0a 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -1003,8 +1003,7 @@ class Runtime extends EventEmitter { category: categoryInfo.name, colour: categoryInfo.color1, colourSecondary: categoryInfo.color2, - colourTertiary: categoryInfo.color3, - extensions: ['scratch_extension'] + colourTertiary: categoryInfo.color3 }; const context = { // TODO: store this somewhere so that we can map args appropriately after translation. @@ -1024,6 +1023,7 @@ class Runtime extends EventEmitter { const iconURI = blockInfo.blockIconURI || categoryInfo.blockIconURI; if (iconURI) { + blockJSON.extensions = ['scratch_extension']; blockJSON.message0 = '%1 %2'; const iconJSON = { type: 'field_image', From 297047a6b929f1c7ced03a7ea2a9464094758ccc Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Tue, 23 Apr 2019 11:44:45 -0400 Subject: [PATCH 08/23] Fix serialization of blockInfo mutation property to XML --- src/engine/blocks.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/engine/blocks.js b/src/engine/blocks.js index db0d5a5b..165d5096 100644 --- a/src/engine/blocks.js +++ b/src/engine/blocks.js @@ -1108,8 +1108,14 @@ class Blocks { let mutationString = `<${mutation.tagName}`; for (const prop in mutation) { if (prop === 'children' || prop === 'tagName') continue; - const mutationValue = (typeof mutation[prop] === 'string') ? + let mutationValue = (typeof mutation[prop] === 'string') ? xmlEscape(mutation[prop]) : mutation[prop]; + + // Handle dynamic extension blocks + if (prop === 'blockInfo') { + mutationValue = xmlEscape(JSON.stringify(mutation[prop])); + } + mutationString += ` ${prop}="${mutationValue}"`; } mutationString += '>'; From 91f0d59be078f8c120b5429352f284ce87bc8c6b Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Tue, 23 Apr 2019 12:17:48 -0700 Subject: [PATCH 09/23] fix extension block color application --- docs/extensions.md | 4 ++++ src/engine/runtime.js | 15 ++++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/docs/extensions.md b/docs/extensions.md index 6cb632e1..f2a8779a 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -152,6 +152,10 @@ class SomeBlocks { // Will be used as the extension's namespace. id: 'someBlocks', + // Core extensions only: override the default extension block colors. + color1: '#FF8C1A', + color2: '#DB6E00', + // Optional: the human-readable name of this extension as string. // This and any other string to be displayed in the Scratch UI may either be // a string or a call to `formatMessage`; a plain string will not be diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 8b066b0a..14ded044 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -40,6 +40,8 @@ const defaultBlockPackages = { scratch3_procedures: require('../blocks/scratch3_procedures') }; +const defaultExtensionColors = ['#0FBD8C', '#0DA57A', '#0B8E69']; + /** * Information used for converting Scratch argument types into scratch-blocks data. * @type {object.} @@ -775,14 +777,21 @@ class Runtime extends EventEmitter { showStatusButton: extensionInfo.showStatusButton, blockIconURI: extensionInfo.blockIconURI, menuIconURI: extensionInfo.menuIconURI, - color1: extensionInfo.colour || '#0FBD8C', - color2: extensionInfo.colourSecondary || '#0DA57A', - color3: extensionInfo.colourTertiary || '#0B8E69', customFieldTypes: {}, blocks: [], menus: [] }; + if (extensionInfo.color1) { + categoryInfo.color1 = extensionInfo.color1; + categoryInfo.color2 = extensionInfo.color2; + categoryInfo.color3 = extensionInfo.color3; + } else { + categoryInfo.color1 = defaultExtensionColors[0]; + categoryInfo.color2 = defaultExtensionColors[1]; + categoryInfo.color3 = defaultExtensionColors[2]; + } + this._blockInfo.push(categoryInfo); this._fillExtensionCategory(categoryInfo, extensionInfo); From 92a73fef5536b8e504804205a09571b839cc006a Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Wed, 24 Apr 2019 11:28:02 -0400 Subject: [PATCH 10/23] Add a runtime event to track when the toolbox extension blocks need updating. --- src/engine/runtime.js | 22 +++++++++++++++++++++- src/virtual-machine.js | 3 +++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 14ded044..26b7451c 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -509,6 +509,14 @@ class Runtime extends EventEmitter { return 'PROJECT_CHANGED'; } + /** + * Event name for report that a change was made that can be saved + * @const {string} + */ + static get TOOLBOX_EXTENSIONS_NEED_UPDATE () { + return 'TOOLBOX_EXTENSIONS_NEED_UPDATE'; + } + /** * Event name for targets update report. * @const {string} @@ -1963,10 +1971,15 @@ class Runtime extends EventEmitter { * @param {!Target} editingTarget New editing target. */ setEditingTarget (editingTarget) { + const oldEditingTarget = this._editingTarget; this._editingTarget = editingTarget; // Script glows must be cleared. this._scriptGlowsPreviousFrame = []; this._updateGlows(); + + if (oldEditingTarget !== this._editingTarget) { + this.requestToolboxExtensionsUpdate(); + } } /** @@ -2384,12 +2397,19 @@ class Runtime extends EventEmitter { } /** - * Emit an event that indicate that the blocks on the workspace need updating. + * Emit an event that indicates that the blocks on the workspace need updating. */ requestBlocksUpdate () { this.emit(Runtime.BLOCKS_NEED_UPDATE); } + /** + * Emit an event that indicates that the toolbox extension blocks need updating. + */ + requestToolboxExtensionsUpdate () { + this.emit(Runtime.TOOLBOX_EXTENSIONS_NEED_UPDATE); + } + /** * Set up timers to repeatedly step in a browser. */ diff --git a/src/virtual-machine.js b/src/virtual-machine.js index f383c0b0..b1f1d6b5 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -121,6 +121,9 @@ class VirtualMachine extends EventEmitter { this.runtime.on(Runtime.BLOCKS_NEED_UPDATE, () => { this.emitWorkspaceUpdate(); }); + this.runtime.on(Runtime.TOOLBOX_EXTENSIONS_NEED_UPDATE, () => { + this.extensionManager.refreshBlocks(); + }); this.runtime.on(Runtime.PERIPHERAL_LIST_UPDATE, info => { this.emit(Runtime.PERIPHERAL_LIST_UPDATE, info); }); From 39f15d3699f82283854c4aa62d22a8b652afeaa6 Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Thu, 25 Apr 2019 11:36:31 -0400 Subject: [PATCH 11/23] Update comment for new extension update event --- src/engine/runtime.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 26b7451c..1a1dd23f 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -510,7 +510,7 @@ class Runtime extends EventEmitter { } /** - * Event name for report that a change was made that can be saved + * Event name for report that a change was made to an extension in the toolbox. * @const {string} */ static get TOOLBOX_EXTENSIONS_NEED_UPDATE () { From f192c181bda473e55c0760eac3e2f4a3e86e125e Mon Sep 17 00:00:00 2001 From: Evelyn Eastmond Date: Mon, 3 Jun 2019 14:52:22 -0400 Subject: [PATCH 12/23] Rename scratchLinkSocket variables to 'socket' and 'ble' to avoid confusion with upcoming ScratchLinkWebSocket class. --- src/extensions/scratch3_gdx_for/index.js | 26 +++++++++---------- .../scratch-link-device-adapter.js | 8 +++--- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/extensions/scratch3_gdx_for/index.js b/src/extensions/scratch3_gdx_for/index.js index e201a790..173b1c93 100644 --- a/src/extensions/scratch3_gdx_for/index.js +++ b/src/extensions/scratch3_gdx_for/index.js @@ -134,7 +134,7 @@ class GdxFor { * @type {BLE} * @private */ - this._scratchLinkSocket = null; + this._ble = null; /** * An @vernier/godirect Device @@ -181,11 +181,11 @@ class GdxFor { * Called by the runtime when user wants to scan for a peripheral. */ scan () { - if (this._scratchLinkSocket) { - this._scratchLinkSocket.disconnect(); + if (this._ble) { + this._ble.disconnect(); } - this._scratchLinkSocket = new BLE(this._runtime, this._extensionId, { + this._ble = new BLE(this._runtime, this._extensionId, { filters: [ {namePrefix: 'GDX-FOR'} ], @@ -200,8 +200,8 @@ class GdxFor { * @param {number} id - the id of the peripheral to connect to. */ connect (id) { - if (this._scratchLinkSocket) { - this._scratchLinkSocket.connectPeripheral(id); + if (this._ble) { + this._ble.connectPeripheral(id); } } @@ -220,8 +220,8 @@ class GdxFor { spinSpeedY: 0, spinSpeedZ: 0 }; - if (this._scratchLinkSocket) { - this._scratchLinkSocket.disconnect(); + if (this._ble) { + this._ble.disconnect(); } } @@ -231,8 +231,8 @@ class GdxFor { */ isConnected () { let connected = false; - if (this._scratchLinkSocket) { - connected = this._scratchLinkSocket.isConnected(); + if (this._ble) { + connected = this._ble.isConnected(); } return connected; } @@ -242,7 +242,7 @@ class GdxFor { * @private */ _onConnect () { - const adapter = new ScratchLinkDeviceAdapter(this._scratchLinkSocket, BLEUUID); + const adapter = new ScratchLinkDeviceAdapter(this._ble, BLEUUID); godirect.createDevice(adapter, {open: true, startMeasurements: false}).then(device => { // Setup device this._device = device; @@ -262,7 +262,7 @@ class GdxFor { }); }); this._timeoutID = window.setInterval( - () => this._scratchLinkSocket.handleDisconnectError(BLEDataStoppedError), + () => this._ble.handleDisconnectError(BLEDataStoppedError), BLETimeout ); }); @@ -306,7 +306,7 @@ class GdxFor { // cancel disconnect timeout and start a new one window.clearInterval(this._timeoutID); this._timeoutID = window.setInterval( - () => this._scratchLinkSocket.handleDisconnectError(BLEDataStoppedError), + () => this._ble.handleDisconnectError(BLEDataStoppedError), BLETimeout ); } diff --git a/src/extensions/scratch3_gdx_for/scratch-link-device-adapter.js b/src/extensions/scratch3_gdx_for/scratch-link-device-adapter.js index 47ea3172..bad68d8b 100644 --- a/src/extensions/scratch3_gdx_for/scratch-link-device-adapter.js +++ b/src/extensions/scratch3_gdx_for/scratch-link-device-adapter.js @@ -4,8 +4,8 @@ const Base64Util = require('../../util/base64-util'); * Adapter class */ class ScratchLinkDeviceAdapter { - constructor (scratchLinkSocket, {service, commandChar, responseChar}) { - this.scratchLinkSocket = scratchLinkSocket; + constructor (socket, {service, commandChar, responseChar}) { + this.socket = socket; this._service = service; this._commandChar = commandChar; @@ -21,13 +21,13 @@ class ScratchLinkDeviceAdapter { writeCommand (commandBuffer) { const data = Base64Util.uint8ArrayToBase64(commandBuffer); - return this.scratchLinkSocket + return this.socket .write(this._service, this._commandChar, data, 'base64'); } setup ({onResponse}) { this._deviceOnResponse = onResponse; - return this.scratchLinkSocket + return this.socket .startNotifications(this._service, this._responseChar, this._onResponse); // TODO: From 418cd0a0f43116f13977874746252c9c3ae28829 Mon Sep 17 00:00:00 2001 From: Evelyn Eastmond Date: Wed, 12 Jun 2019 10:20:21 -0400 Subject: [PATCH 13/23] Removing comment after decision that this function name is accurate and distinct from coastAfter. --- src/extensions/scratch3_wedo2/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/extensions/scratch3_wedo2/index.js b/src/extensions/scratch3_wedo2/index.js index e08c987f..d7a69e48 100644 --- a/src/extensions/scratch3_wedo2/index.js +++ b/src/extensions/scratch3_wedo2/index.js @@ -297,7 +297,6 @@ class WeDo2Motor { /** * Start active braking on this motor. After a short time, the motor will turn off. - * // TODO: rename this to coastAfter? */ startBraking () { if (this._power === 0) return; From ab715901a6b84496d5c4c74bf12dbbadd7ac4a2d Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Tue, 11 Jun 2019 15:31:34 -0700 Subject: [PATCH 14/23] add tests for new e16n support features --- test/unit/engine_blocks.js | 21 ++++++++++++++- test/unit/engine_mutation-adapter.js | 26 +++++++++++++++++++ test/unit/extension_conversion.js | 39 +++++++++++++++++++++++++--- 3 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 test/unit/engine_mutation-adapter.js diff --git a/test/unit/engine_blocks.js b/test/unit/engine_blocks.js index 8ab483e2..7fdaafc7 100644 --- a/test/unit/engine_blocks.js +++ b/test/unit/engine_blocks.js @@ -25,7 +25,7 @@ test('spec', t => { t.type(b.getNextBlock, 'function'); t.type(b.getBranch, 'function'); t.type(b.getOpcode, 'function'); - + t.type(b.mutationToXML, 'function'); t.end(); }); @@ -239,6 +239,25 @@ test('getOpcode', t => { t.end(); }); +test('mutationToXML', t => { + const b = new Blocks(new Runtime()); + const testStringRaw = '"arbitrary" & \'complicated\' test string'; + const testStringEscaped = '\\"arbitrary\\" & 'complicated' test string'; + const mutation = { + tagName: 'mutation', + children: [], + blockInfo: { + text: testStringRaw + } + }; + const xml = b.mutationToXML(mutation); + t.equals( + xml, + `` + ); + t.end(); +}); + // Block events tests test('create', t => { const b = new Blocks(new Runtime()); diff --git a/test/unit/engine_mutation-adapter.js b/test/unit/engine_mutation-adapter.js new file mode 100644 index 00000000..30a62774 --- /dev/null +++ b/test/unit/engine_mutation-adapter.js @@ -0,0 +1,26 @@ +const test = require('tap').test; + +const mutationAdapter = require('../../src/engine/mutation-adapter'); + +test('spec', t => { + t.type(mutationAdapter, 'function'); + t.end(); +}); + +test('convert DOM to Scratch object', t => { + const testStringRaw = '"arbitrary" & \'complicated\' test string'; + const testStringEscaped = '\\"arbitrary\\" & 'complicated' test string'; + const xml = ``; + const expectedMutation = { + tagName: 'mutation', + children: [], + blockInfo: { + text: testStringRaw + } + }; + + // TODO: do we want to test passing a DOM node to `mutationAdapter`? Node.js doesn't have built-in DOM support... + const mutationFromString = mutationAdapter(xml); + t.deepEqual(mutationFromString, expectedMutation); + t.end(); +}); diff --git a/test/unit/extension_conversion.js b/test/unit/extension_conversion.js index 001130a3..58f4ad59 100644 --- a/test/unit/extension_conversion.js +++ b/test/unit/extension_conversion.js @@ -11,6 +11,9 @@ const ScratchBlocksConstants = require('../../src/engine/scratch-blocks-constant const testExtensionInfo = { id: 'test', name: 'fake test extension', + color1: '#111111', + color2: '#222222', + color3: '#333333', blocks: [ { func: 'MAKE_A_VARIABLE', @@ -20,7 +23,8 @@ const testExtensionInfo = { { opcode: 'reporter', blockType: BlockType.REPORTER, - text: 'simple text' + text: 'simple text', + blockIconURI: 'invalid icon URI' // trigger the 'scratch_extension' path }, '---', // separator between groups of blocks in an extension { @@ -63,6 +67,14 @@ const testExtensionInfo = { ] }; +const testCategoryInfo = function (t, block) { + t.equal(block.json.category, 'fake test extension'); + t.equal(block.json.colour, '#111111'); + t.equal(block.json.colourSecondary, '#222222'); + t.equal(block.json.colourTertiary, '#333333'); + t.equal(block.json.inputsInline, true); +}; + const testButton = function (t, button) { t.same(button.json, null); // should be null or undefined t.equal(button.xml, ''); @@ -70,13 +82,28 @@ const testButton = function (t, button) { const testReporter = function (t, reporter) { t.equal(reporter.json.type, 'test_reporter'); + testCategoryInfo(t, reporter); + t.equal(reporter.json.checkboxInFlyout, true); t.equal(reporter.json.outputShape, ScratchBlocksConstants.OUTPUT_SHAPE_ROUND); t.equal(reporter.json.output, 'String'); t.notOk(reporter.json.hasOwnProperty('previousStatement')); t.notOk(reporter.json.hasOwnProperty('nextStatement')); - t.equal(reporter.json.message0, 'simple text'); + t.same(reporter.json.extensions, ['scratch_extension']); + t.equal(reporter.json.message0, '%1 %2simple text'); // "%1 %2" from the block icon t.notOk(reporter.json.hasOwnProperty('message1')); - t.notOk(reporter.json.hasOwnProperty('args0')); + t.same(reporter.json.args0, [ + // %1 in message0: the block icon + { + type: 'field_image', + src: 'invalid icon URI', + width: 40, + height: 40 + }, + // %2 in message0: separator between icon and text (only added when there's also an icon) + { + type: 'field_vertical_separator' + } + ]); t.notOk(reporter.json.hasOwnProperty('args1')); t.equal(reporter.xml, ''); }; @@ -88,9 +115,11 @@ const testSeparator = function (t, separator) { const testCommand = function (t, command) { t.equal(command.json.type, 'test_command'); + testCategoryInfo(t, command); t.equal(command.json.outputShape, ScratchBlocksConstants.OUTPUT_SHAPE_SQUARE); t.assert(command.json.hasOwnProperty('previousStatement')); t.assert(command.json.hasOwnProperty('nextStatement')); + t.notOk(command.json.extensions && command.json.extensions.length); // OK if it's absent or empty t.equal(command.json.message0, 'text with %1'); t.notOk(command.json.hasOwnProperty('message1')); t.strictSame(command.json.args0[0], { @@ -105,9 +134,11 @@ const testCommand = function (t, command) { const testConditional = function (t, conditional) { t.equal(conditional.json.type, 'test_ifElse'); + testCategoryInfo(t, conditional); t.equal(conditional.json.outputShape, ScratchBlocksConstants.OUTPUT_SHAPE_SQUARE); t.ok(conditional.json.hasOwnProperty('previousStatement')); t.ok(conditional.json.hasOwnProperty('nextStatement')); + t.notOk(conditional.json.extensions && conditional.json.extensions.length); // OK if it's absent or empty t.equal(conditional.json.message0, 'test if %1 is spiffy and if so then'); t.equal(conditional.json.message1, '%1'); // placeholder for substack #1 t.equal(conditional.json.message2, 'or elsewise'); @@ -133,9 +164,11 @@ const testConditional = function (t, conditional) { const testLoop = function (t, loop) { t.equal(loop.json.type, 'test_loop'); + testCategoryInfo(t, loop); t.equal(loop.json.outputShape, ScratchBlocksConstants.OUTPUT_SHAPE_SQUARE); t.ok(loop.json.hasOwnProperty('previousStatement')); t.notOk(loop.json.hasOwnProperty('nextStatement')); // isTerminal is set on this block + t.notOk(loop.json.extensions && loop.json.extensions.length); // OK if it's absent or empty t.equal(loop.json.message0, 'loopty %1 loops'); t.equal(loop.json.message1, '%1'); // placeholder for substack t.equal(loop.json.message2, '%1'); // placeholder for loop arrow From e7bf49c8df0c05cc4bbbc5e3faef57453e12a90e Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Wed, 24 Apr 2019 11:15:58 -0700 Subject: [PATCH 15/23] support non-droppable menus in extensions --- docs/extensions.md | 11 ++- src/engine/runtime.js | 83 ++++++++++++++-------- src/extension-support/extension-manager.js | 23 +++--- 3 files changed, 80 insertions(+), 37 deletions(-) diff --git a/docs/extensions.md b/docs/extensions.md index f2a8779a..c4ba6070 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -311,7 +311,16 @@ class SomeBlocks { // Dynamic menu: returns an array as above. // Called each time the menu is opened. - menuB: 'getItemsForMenuB' + menuB: 'getItemsForMenuB', + + // The examples above are shorthand for setting only the `items` property in this full form: + menuC: { + // This flag makes a "non-droppable" menu: the menu will not accept reporters. + rejectReporters: true, + + // The `item` property may be an array or function name as in previous menu examples. + items: [/*...*/] || 'getItemsForMenuC' + } }, // Optional: translations (UNSTABLE - NOT YET SUPPORTED) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 1a1dd23f..7ed8ecad 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -784,10 +784,7 @@ class Runtime extends EventEmitter { name: maybeFormatMessage(extensionInfo.name), showStatusButton: extensionInfo.showStatusButton, blockIconURI: extensionInfo.blockIconURI, - menuIconURI: extensionInfo.menuIconURI, - customFieldTypes: {}, - blocks: [], - menus: [] + menuIconURI: extensionInfo.menuIconURI }; if (extensionInfo.color1) { @@ -828,8 +825,6 @@ class Runtime extends EventEmitter { for (const categoryInfo of this._blockInfo) { if (extensionInfo.id === categoryInfo.id) { categoryInfo.name = maybeFormatMessage(extensionInfo.name); - categoryInfo.blocks = []; - categoryInfo.menus = []; this._fillExtensionCategory(categoryInfo, extensionInfo); this.emit(Runtime.BLOCKSINFO_UPDATE, categoryInfo); @@ -838,18 +833,24 @@ class Runtime extends EventEmitter { } /** - * Read extension information, convert menus, blocks and custom field types + * Read extension information, convert menus, blocks and custom field types * 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) { + categoryInfo.blocks = []; + categoryInfo.customFieldTypes = {}; + categoryInfo.menus = []; + categoryInfo.menuInfo = {}; + for (const menuName in extensionInfo.menus) { if (extensionInfo.menus.hasOwnProperty(menuName)) { - const menuItems = extensionInfo.menus[menuName]; - const convertedMenu = this._buildMenuForScratchBlocks(menuName, menuItems, categoryInfo); + const menuValue = extensionInfo.menus[menuName]; + const convertedMenu = this._buildMenuForScratchBlocks(menuName, menuValue, categoryInfo); categoryInfo.menus.push(convertedMenu); + categoryInfo.menuInfo[menuName] = menuValue; } } for (const fieldTypeName in extensionInfo.customFieldTypes) { @@ -891,19 +892,21 @@ class Runtime extends EventEmitter { /** * 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 - * @param {array} menuItems - the list of items for this menu + * @param {object} menuInfo - a description of this menu and its items + * @property {*} items - an array of menu items or a function to retrieve such an array + * @property {boolean} [rejectReporters] - if true, prevent dropping reporters onto this menu * @param {CategoryInfo} categoryInfo - the category for this block * @returns {object} - a JSON-esque object ready for scratch-blocks' consumption * @private */ - _buildMenuForScratchBlocks (menuName, menuItems, categoryInfo) { + _buildMenuForScratchBlocks (menuName, menuInfo, categoryInfo) { const menuId = this._makeExtensionMenuId(menuName, categoryInfo.id); - let options = null; - if (typeof menuItems === 'function') { - options = menuItems; + let menuItems = null; + if (typeof menuInfo.items === 'function') { + menuItems = menuInfo.items; } else { const extensionMessageContext = this.makeMessageContextForTarget(); - options = menuItems.map(item => { + menuItems = menuInfo.items.map(item => { const formattedItem = maybeFormatMessage(item, extensionMessageContext); switch (typeof formattedItem) { case 'string': @@ -924,12 +927,13 @@ class Runtime extends EventEmitter { colour: categoryInfo.color1, colourSecondary: categoryInfo.color2, colourTertiary: categoryInfo.color3, - outputShape: ScratchBlocksConstants.OUTPUT_SHAPE_ROUND, + outputShape: menuInfo.rejectReporters ? + ScratchBlocksConstants.OUTPUT_SHAPE_SQUARE : ScratchBlocksConstants.OUTPUT_SHAPE_ROUND, args0: [ { type: 'field_dropdown', name: menuName, - options: options + options: menuItems } ] } @@ -1226,29 +1230,52 @@ class Runtime extends EventEmitter { argJSON.check = argTypeInfo.check; } - const shadowType = (argInfo.menu ? - this._makeExtensionMenuId(argInfo.menu, context.categoryInfo.id) : - argTypeInfo.shadowType); - const fieldType = argInfo.menu || argTypeInfo.fieldType; + let valueName; + let shadowType; + let fieldName; + if (argInfo.menu) { + const menuInfo = argInfo.menu && context.categoryInfo.menuInfo[argInfo.menu]; + if (menuInfo.rejectReporters) { + argJSON.type = 'field_dropdown'; + argJSON.options = menuInfo.items; + valueName = null; + shadowType = null; + fieldName = placeholder; + } else { + valueName = placeholder; + shadowType = this._makeExtensionMenuId(argInfo.menu, context.categoryInfo.id); + fieldName = argInfo.menu; + } + } else { + valueName = placeholder; + shadowType = argTypeInfo.shadowType; + fieldName = argTypeInfo.fieldType; + } // is the ScratchBlocks name for a block input. - context.inputList.push(``); + if (valueName) { + 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}`); - } + // 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 (fieldName) { + context.inputList.push(`${defaultValue}`); + } + if (shadowType) { context.inputList.push(''); } - context.inputList.push(''); + if (valueName) { + context.inputList.push(''); + } const argsName = `args${context.outLineNum}`; const blockArgs = (context.blockJSON[argsName] = context.blockJSON[argsName] || []); diff --git a/src/extension-support/extension-manager.js b/src/extension-support/extension-manager.js index a2f89aae..9114bec5 100644 --- a/src/extension-support/extension-manager.js +++ b/src/extension-support/extension-manager.js @@ -310,15 +310,22 @@ class ExtensionManager { _prepareMenuInfo (serviceName, menus) { const menuNames = Object.getOwnPropertyNames(menus); for (let i = 0; i < menuNames.length; i++) { - const item = menuNames[i]; + const menuName = menuNames[i]; + let menuInfo = menus[menuName]; + if (!menuInfo.items) { + menuInfo = { + items: menuInfo + }; + menus[menuName] = menuInfo; + } // If the value is a string, it should be the name of a function in the // extension object to call to populate the menu whenever it is opened. // Set up the binding for the function object here so // we can use it later when converting the menu for Scratch Blocks. - if (typeof menus[item] === 'string') { + if (typeof menuInfo.items === 'string') { + const menuItemFunctionName = menuInfo.items; const serviceObject = dispatch.services[serviceName]; - const menuName = menus[item]; - menus[item] = this._getExtensionMenuItems.bind(this, serviceObject, menuName); + menuInfo.items = this._getExtensionMenuItems.bind(this, serviceObject, menuItemFunctionName); } } return menus; @@ -327,11 +334,11 @@ class ExtensionManager { /** * Fetch the items for a particular extension menu, providing the target ID for context. * @param {object} extensionObject - the extension object providing the menu. - * @param {string} menuName - the name of the menu function to call. + * @param {string} menuItemFunctionName - the name of the menu function to call. * @returns {Array} menu items ready for scratch-blocks. * @private */ - _getExtensionMenuItems (extensionObject, menuName) { + _getExtensionMenuItems (extensionObject, menuItemFunctionName) { // Fetch the items appropriate for the target currently being edited. This assumes that menus only // collect items when opened by the user while editing a particular target. const editingTarget = this.runtime.getEditingTarget() || this.runtime.getTargetForStage(); @@ -339,7 +346,7 @@ class ExtensionManager { const extensionMessageContext = this.runtime.makeMessageContextForTarget(editingTarget); // TODO: Fix this to use dispatch.call when extensions are running in workers. - const menuFunc = extensionObject[menuName]; + const menuFunc = extensionObject[menuItemFunctionName]; const menuItems = menuFunc.call(extensionObject, editingTargetID).map( item => { item = maybeFormatMessage(item, extensionMessageContext); @@ -357,7 +364,7 @@ class ExtensionManager { }); if (!menuItems || menuItems.length < 1) { - throw new Error(`Extension menu returned no items: ${menuName}`); + throw new Error(`Extension menu returned no items: ${menuItemFunctionName}`); } return menuItems; } From 3fbed88a958eea8d8bdce2f77bed9cef1ef4e3cb Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Fri, 26 Apr 2019 13:49:13 -0700 Subject: [PATCH 16/23] switch from `rejectReporters` flag to `acceptReporters` Also add more extensive documentation around extension menu definitions, including some considerations to think about before making a menu accept reporters. --- docs/extensions.md | 121 +++++++++++++++++++++++++++++++++++++++++- src/engine/runtime.js | 16 +++--- 2 files changed, 127 insertions(+), 10 deletions(-) diff --git a/docs/extensions.md b/docs/extensions.md index c4ba6070..5bafb51b 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -120,6 +120,123 @@ class SomeBlocks { } ``` +### Defining a Menu + +To display a drop-down menu for a block argument, specify the `menu` property of that argument and a matching item in +the `menus` section of your extension's definition: + +```js +return { + // ... + blocks: [ + { + // ... + arguments: { + FOO: { + type: ArgumentType.NUMBER, + menu: 'fooMenu' + } + } + } + ], + menus: { + fooMenu: { + items: ['a', 'b', 'c'] + } + } +} +``` + +The items in a menu may be specified with an array or with the name of a function which returns an array. The two +simplest forms for menu definitions are: + +```js +getInfo () { + return { + menus: { + staticMenu: ['static 1', 'static 2', 'static 3'], + dynamicMenu: 'getDynamicMenuItems' + } + }; +} +// this member function will be called each time the menu opens +getDynamicMenuItems () { + return ['dynamic 1', 'dynamic 2', 'dynamic 3']; +} +``` + +The examples above are shorthand for these equivalent definitions: + +```js +getInfo () { + return { + menus: { + staticMenu: { + items: ['static 1', 'static 2', 'static 3'] + }, + dynamicMenu: { + items: 'getDynamicMenuItems' + } + } + }; +} +// this member function will be called each time the menu opens +getDynamicMenuItems () { + return ['dynamic 1', 'dynamic 2', 'dynamic 3']; +} +``` + +If a menu item needs a label that doesn't match its value -- for example, if the label needs to be displayed in the +user's language but the value needs to stay constant -- the menu item may be an object instead of a string. This works +for both static and dynamic menu items: + +```js +menus: { + staticMenu: [ + { + text: formatMessage(/* ... */), + value: 42 + } + ] +} +``` + +#### Accepting reporters ("droppable" menus) + +By default it is not possible to specify the value of a dropdown menu by inserting a reporter block. While we +encourage extension authors to make their menus accept reporters when possible, doing so requires careful +consideration to avoid confusion and frustration on the part of those using the extension. + +A few of these considerations include: + +* The valid values for the menu should not change when the user changes the Scratch language setting. + * In particular, changing languages should never break a working project. +* The average Scratch user should be able to figure out the valid values for this input without referring to extension + documentation. + * One way to ensure this is to make an item's text match or include the item's value. +* The block should accept any value as input, even "invalid" values. + * Scratch has no concept of a runtime error! + * For a command block, sometimes the best option is to do nothing. + * For a reporter, returning zero or the empty string might make sense. +* The block should be forgiving in its interpretation of inputs. + * For example, if the block expects a string and receives a number it may make sense to interpret the number as a + string instead of treating it as invalid input. + +The `acceptReporters` flag indicates that the user can drop a reporter onto the menu input: + +```js +menus: { + staticMenu: { + acceptReporters: true, + items: [/*...*/] + }, + dynamicMenu: { + acceptReporters: true, + items: 'getDynamicMenuItems' + } +} +``` + ## Annotated Example ```js @@ -315,8 +432,8 @@ class SomeBlocks { // The examples above are shorthand for setting only the `items` property in this full form: menuC: { - // This flag makes a "non-droppable" menu: the menu will not accept reporters. - rejectReporters: true, + // This flag makes a "droppable" menu: the menu will allow dropping a reporter in for the input. + acceptReporters: true, // The `item` property may be an array or function name as in previous menu examples. items: [/*...*/] || 'getItemsForMenuC' diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 7ed8ecad..ac6d005a 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -894,7 +894,7 @@ class Runtime extends EventEmitter { * @param {string} menuName - the name of the menu * @param {object} menuInfo - a description of this menu and its items * @property {*} items - an array of menu items or a function to retrieve such an array - * @property {boolean} [rejectReporters] - if true, prevent dropping reporters onto this menu + * @property {boolean} [acceptReporters] - if true, allow dropping reporters onto this menu * @param {CategoryInfo} categoryInfo - the category for this block * @returns {object} - a JSON-esque object ready for scratch-blocks' consumption * @private @@ -927,8 +927,8 @@ class Runtime extends EventEmitter { colour: categoryInfo.color1, colourSecondary: categoryInfo.color2, colourTertiary: categoryInfo.color3, - outputShape: menuInfo.rejectReporters ? - ScratchBlocksConstants.OUTPUT_SHAPE_SQUARE : ScratchBlocksConstants.OUTPUT_SHAPE_ROUND, + outputShape: menuInfo.acceptReporters ? + ScratchBlocksConstants.OUTPUT_SHAPE_ROUND : ScratchBlocksConstants.OUTPUT_SHAPE_SQUARE, args0: [ { type: 'field_dropdown', @@ -1235,16 +1235,16 @@ class Runtime extends EventEmitter { let fieldName; if (argInfo.menu) { const menuInfo = argInfo.menu && context.categoryInfo.menuInfo[argInfo.menu]; - if (menuInfo.rejectReporters) { + if (menuInfo.acceptReporters) { + valueName = placeholder; + shadowType = this._makeExtensionMenuId(argInfo.menu, context.categoryInfo.id); + fieldName = argInfo.menu; + } else { argJSON.type = 'field_dropdown'; argJSON.options = menuInfo.items; valueName = null; shadowType = null; fieldName = placeholder; - } else { - valueName = placeholder; - shadowType = this._makeExtensionMenuId(argInfo.menu, context.categoryInfo.id); - fieldName = argInfo.menu; } } else { valueName = placeholder; From cfee65630644d13927ccf0b2e6fc419eb58d4757 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Mon, 29 Apr 2019 12:33:56 -0700 Subject: [PATCH 17/23] fix tests: empty `menus` shouldn't be an array --- src/extension-support/extension-manager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extension-support/extension-manager.js b/src/extension-support/extension-manager.js index 9114bec5..757ec682 100644 --- a/src/extension-support/extension-manager.js +++ b/src/extension-support/extension-manager.js @@ -295,7 +295,7 @@ class ExtensionManager { } return results; }, []); - extensionInfo.menus = extensionInfo.menus || []; + extensionInfo.menus = extensionInfo.menus || {}; extensionInfo.menus = this._prepareMenuInfo(serviceName, extensionInfo.menus); return extensionInfo; } From 33a60714661ce6e1b31856b9ce2aba8ec44f706d Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Mon, 29 Apr 2019 13:07:08 -0700 Subject: [PATCH 18/23] for compatibility, make all existing extension menus accept reporters --- src/extensions/scratch3_boost/index.js | 503 +++++++++--------- src/extensions/scratch3_ev3/index.js | 10 +- src/extensions/scratch3_gdx_for/index.js | 25 +- src/extensions/scratch3_makeymakey/index.js | 104 ++-- src/extensions/scratch3_microbit/index.js | 30 +- src/extensions/scratch3_music/index.js | 10 +- src/extensions/scratch3_pen/index.js | 5 +- src/extensions/scratch3_text2speech/index.js | 10 +- src/extensions/scratch3_translate/index.js | 5 +- .../scratch3_video_sensing/index.js | 15 +- src/extensions/scratch3_wedo2/index.js | 284 +++++----- 11 files changed, 556 insertions(+), 445 deletions(-) diff --git a/src/extensions/scratch3_boost/index.js b/src/extensions/scratch3_boost/index.js index 89e103f5..1aeed758 100644 --- a/src/extensions/scratch3_boost/index.js +++ b/src/extensions/scratch3_boost/index.js @@ -541,7 +541,7 @@ class BoostMotor { BoostMotorProfile.DO_NOT_USE ] ); - + if (resetState) this.status = BoostMotorState.ON_FOR_ROTATION; this._pendingPositionDestination = this.position + (degrees * this.direction * direction); this._parent.send(BoostBLE.characteristic, cmd); @@ -1428,246 +1428,267 @@ class Scratch3BoostBlocks { } ], menus: { - MOTOR_ID: [ - { - text: formatMessage({ - id: 'boost.motorId.a', - default: BoostMotorLabel.A, - description: `label for motor A element in motor menu for LEGO Boost extension` - }), - value: BoostMotorLabel.A - }, - { - text: formatMessage({ - id: 'boost.motorId.b', - default: BoostMotorLabel.B, - description: `label for motor B element in motor menu for LEGO Boost extension` - }), - value: BoostMotorLabel.B - }, - { - text: formatMessage({ - id: 'boost.motorId.c', - default: BoostMotorLabel.C, - description: `label for motor C element in motor menu for LEGO Boost extension` - }), - value: BoostMotorLabel.C - }, - { - text: formatMessage({ - id: 'boost.motorId.d', - default: BoostMotorLabel.D, - description: `label for motor D element in motor menu for LEGO Boost extension` - }), - value: BoostMotorLabel.D - }, - { - text: formatMessage({ - id: 'boost.motorId.ab', - default: BoostMotorLabel.AB, - description: `label for motor A and B element in motor menu for LEGO Boost extension` - }), - value: BoostMotorLabel.AB - }, - { - text: formatMessage({ - id: 'boost.motorId.all', - default: BoostMotorLabel.ALL, - description: 'label for all motors element in motor menu for LEGO Boost extension' - }), - value: BoostMotorLabel.ALL - } - ], - MOTOR_REPORTER_ID: [ - { - text: formatMessage({ - id: 'boost.motorReporterId.a', - default: BoostMotorLabel.A, - description: 'label for motor A element in motor menu for LEGO Boost extension' - }), - value: BoostMotorLabel.A - }, - { - text: formatMessage({ - id: 'boost.motorReporterId.b', - default: BoostMotorLabel.B, - description: 'label for motor B element in motor menu for LEGO Boost extension' - }), - value: BoostMotorLabel.B - }, - { - text: formatMessage({ - id: 'boost.motorReporterId.c', - default: BoostMotorLabel.C, - description: 'label for motor C element in motor menu for LEGO Boost extension' - }), - value: BoostMotorLabel.C - }, - { - text: formatMessage({ - id: 'boost.motorReporterId.d', - default: BoostMotorLabel.D, - description: 'label for motor D element in motor menu for LEGO Boost extension' - }), - value: BoostMotorLabel.D - } - ], - MOTOR_DIRECTION: [ - { - text: formatMessage({ - id: 'boost.motorDirection.forward', - default: 'this way', - description: 'label for forward element in motor direction menu for LEGO Boost extension' - }), - value: BoostMotorDirection.FORWARD - }, - { - text: formatMessage({ - id: 'boost.motorDirection.backward', - default: 'that way', - description: 'label for backward element in motor direction menu for LEGO Boost extension' - }), - value: BoostMotorDirection.BACKWARD - }, - { - text: formatMessage({ - id: 'boost.motorDirection.reverse', - default: 'reverse', - description: 'label for reverse element in motor direction menu for LEGO Boost extension' - }), - value: BoostMotorDirection.REVERSE - } - ], - TILT_DIRECTION: [ - { - text: formatMessage({ - id: 'boost.tiltDirection.up', - default: 'up', - description: 'label for up element in tilt direction menu for LEGO Boost extension' - }), - value: BoostTiltDirection.UP - }, - { - text: formatMessage({ - id: 'boost.tiltDirection.down', - default: 'down', - description: 'label for down element in tilt direction menu for LEGO Boost extension' - }), - value: BoostTiltDirection.DOWN - }, - { - text: formatMessage({ - id: 'boost.tiltDirection.left', - default: 'left', - description: 'label for left element in tilt direction menu for LEGO Boost extension' - }), - value: BoostTiltDirection.LEFT - }, - { - text: formatMessage({ - id: 'boost.tiltDirection.right', - default: 'right', - description: 'label for right element in tilt direction menu for LEGO Boost extension' - }), - value: BoostTiltDirection.RIGHT - } - ], - TILT_DIRECTION_ANY: [ - { - text: formatMessage({ - id: 'boost.tiltDirection.up', - default: 'up' - }), - value: BoostTiltDirection.UP - }, - { - text: formatMessage({ - id: 'boost.tiltDirection.down', - default: 'down' - }), - value: BoostTiltDirection.DOWN - }, - { - text: formatMessage({ - id: 'boost.tiltDirection.left', - default: 'left' - }), - value: BoostTiltDirection.LEFT - }, - { - text: formatMessage({ - id: 'boost.tiltDirection.right', - default: 'right' - }), - value: BoostTiltDirection.RIGHT - }, - { - text: formatMessage({ - id: 'boost.tiltDirection.any', - default: 'any', - description: 'label for any element in tilt direction menu for LEGO Boost extension' - }), - value: BoostTiltDirection.ANY - } - ], - COLOR: [ - { - text: formatMessage({ - id: 'boost.color.red', - default: 'red', - description: 'the color red' - }), - value: BoostColor.RED - }, - { - text: formatMessage({ - id: 'boost.color.blue', - default: 'blue', - description: 'the color blue' - }), - value: BoostColor.BLUE - }, - { - text: formatMessage({ - id: 'boost.color.green', - default: 'green', - description: 'the color green' - }), - value: BoostColor.GREEN - }, - { - text: formatMessage({ - id: 'boost.color.yellow', - default: 'yellow', - description: 'the color yellow' - }), - value: BoostColor.YELLOW - }, - { - text: formatMessage({ - id: 'boost.color.white', - default: 'white', - desription: 'the color white' - }), - value: BoostColor.WHITE - }, - { - text: formatMessage({ - id: 'boost.color.black', - default: 'black', - description: 'the color black' - }), - value: BoostColor.BLACK - }, - { - text: formatMessage({ - id: 'boost.color.any', - default: 'any color', - description: 'any color' - }), - value: BoostColor.ANY - } - ] + MOTOR_ID: { + acceptReporters: true, + items: [ + { + text: formatMessage({ + id: 'boost.motorId.a', + default: BoostMotorLabel.A, + description: `label for motor A element in motor menu for LEGO Boost extension` + }), + value: BoostMotorLabel.A + }, + { + text: formatMessage({ + id: 'boost.motorId.b', + default: BoostMotorLabel.B, + description: `label for motor B element in motor menu for LEGO Boost extension` + }), + value: BoostMotorLabel.B + }, + { + text: formatMessage({ + id: 'boost.motorId.c', + default: BoostMotorLabel.C, + description: `label for motor C element in motor menu for LEGO Boost extension` + }), + value: BoostMotorLabel.C + }, + { + text: formatMessage({ + id: 'boost.motorId.d', + default: BoostMotorLabel.D, + description: `label for motor D element in motor menu for LEGO Boost extension` + }), + value: BoostMotorLabel.D + }, + { + text: formatMessage({ + id: 'boost.motorId.ab', + default: BoostMotorLabel.AB, + description: `label for motor A and B element in motor menu for LEGO Boost extension` + }), + value: BoostMotorLabel.AB + }, + { + text: formatMessage({ + id: 'boost.motorId.all', + default: BoostMotorLabel.ALL, + description: 'label for all motors element in motor menu for LEGO Boost extension' + }), + value: BoostMotorLabel.ALL + } + ] + }, + MOTOR_REPORTER_ID: { + acceptReporters: true, + items: [ + { + text: formatMessage({ + id: 'boost.motorReporterId.a', + default: BoostMotorLabel.A, + description: 'label for motor A element in motor menu for LEGO Boost extension' + }), + value: BoostMotorLabel.A + }, + { + text: formatMessage({ + id: 'boost.motorReporterId.b', + default: BoostMotorLabel.B, + description: 'label for motor B element in motor menu for LEGO Boost extension' + }), + value: BoostMotorLabel.B + }, + { + text: formatMessage({ + id: 'boost.motorReporterId.c', + default: BoostMotorLabel.C, + description: 'label for motor C element in motor menu for LEGO Boost extension' + }), + value: BoostMotorLabel.C + }, + { + text: formatMessage({ + id: 'boost.motorReporterId.d', + default: BoostMotorLabel.D, + description: 'label for motor D element in motor menu for LEGO Boost extension' + }), + value: BoostMotorLabel.D + } + ] + }, + MOTOR_DIRECTION: { + acceptReporters: true, + items: [ + { + text: formatMessage({ + id: 'boost.motorDirection.forward', + default: 'this way', + description: + 'label for forward element in motor direction menu for LEGO Boost extension' + }), + value: BoostMotorDirection.FORWARD + }, + { + text: formatMessage({ + id: 'boost.motorDirection.backward', + default: 'that way', + description: + 'label for backward element in motor direction menu for LEGO Boost extension' + }), + value: BoostMotorDirection.BACKWARD + }, + { + text: formatMessage({ + id: 'boost.motorDirection.reverse', + default: 'reverse', + description: + 'label for reverse element in motor direction menu for LEGO Boost extension' + }), + value: BoostMotorDirection.REVERSE + } + ] + }, + TILT_DIRECTION: { + acceptReporters: true, + items: [ + { + text: formatMessage({ + id: 'boost.tiltDirection.up', + default: 'up', + description: 'label for up element in tilt direction menu for LEGO Boost extension' + }), + value: BoostTiltDirection.UP + }, + { + text: formatMessage({ + id: 'boost.tiltDirection.down', + default: 'down', + description: 'label for down element in tilt direction menu for LEGO Boost extension' + }), + value: BoostTiltDirection.DOWN + }, + { + text: formatMessage({ + id: 'boost.tiltDirection.left', + default: 'left', + description: 'label for left element in tilt direction menu for LEGO Boost extension' + }), + value: BoostTiltDirection.LEFT + }, + { + text: formatMessage({ + id: 'boost.tiltDirection.right', + default: 'right', + description: 'label for right element in tilt direction menu for LEGO Boost extension' + }), + value: BoostTiltDirection.RIGHT + } + ] + }, + TILT_DIRECTION_ANY: { + acceptReporters: true, + items: [ + { + text: formatMessage({ + id: 'boost.tiltDirection.up', + default: 'up' + }), + value: BoostTiltDirection.UP + }, + { + text: formatMessage({ + id: 'boost.tiltDirection.down', + default: 'down' + }), + value: BoostTiltDirection.DOWN + }, + { + text: formatMessage({ + id: 'boost.tiltDirection.left', + default: 'left' + }), + value: BoostTiltDirection.LEFT + }, + { + text: formatMessage({ + id: 'boost.tiltDirection.right', + default: 'right' + }), + value: BoostTiltDirection.RIGHT + }, + { + text: formatMessage({ + id: 'boost.tiltDirection.any', + default: 'any', + description: 'label for any element in tilt direction menu for LEGO Boost extension' + }), + value: BoostTiltDirection.ANY + } + ] + }, + COLOR: { + acceptReporters: true, + items: [ + { + text: formatMessage({ + id: 'boost.color.red', + default: 'red', + description: 'the color red' + }), + value: BoostColor.RED + }, + { + text: formatMessage({ + id: 'boost.color.blue', + default: 'blue', + description: 'the color blue' + }), + value: BoostColor.BLUE + }, + { + text: formatMessage({ + id: 'boost.color.green', + default: 'green', + description: 'the color green' + }), + value: BoostColor.GREEN + }, + { + text: formatMessage({ + id: 'boost.color.yellow', + default: 'yellow', + description: 'the color yellow' + }), + value: BoostColor.YELLOW + }, + { + text: formatMessage({ + id: 'boost.color.white', + default: 'white', + desription: 'the color white' + }), + value: BoostColor.WHITE + }, + { + text: formatMessage({ + id: 'boost.color.black', + default: 'black', + description: 'the color black' + }), + value: BoostColor.BLACK + }, + { + text: formatMessage({ + id: 'boost.color.any', + default: 'any color', + description: 'any color' + }), + value: BoostColor.ANY + } + ] + } } }; } diff --git a/src/extensions/scratch3_ev3/index.js b/src/extensions/scratch3_ev3/index.js index ed47ffb5..8978f6b2 100644 --- a/src/extensions/scratch3_ev3/index.js +++ b/src/extensions/scratch3_ev3/index.js @@ -1112,8 +1112,14 @@ class Scratch3Ev3Blocks { } ], menus: { - motorPorts: this._formatMenu(Ev3MotorMenu), - sensorPorts: this._formatMenu(Ev3SensorMenu) + motorPorts: { + acceptReporters: true, + items: this._formatMenu(Ev3MotorMenu) + }, + sensorPorts: { + acceptReporters: true, + items: this._formatMenu(Ev3SensorMenu) + } } }; } diff --git a/src/extensions/scratch3_gdx_for/index.js b/src/extensions/scratch3_gdx_for/index.js index e201a790..27ab5146 100644 --- a/src/extensions/scratch3_gdx_for/index.js +++ b/src/extensions/scratch3_gdx_for/index.js @@ -757,11 +757,26 @@ class Scratch3GdxForBlocks { } ], menus: { - pushPullOptions: this.PUSH_PULL_MENU, - gestureOptions: this.GESTURE_MENU, - axisOptions: this.AXIS_MENU, - tiltOptions: this.TILT_MENU, - tiltAnyOptions: this.TILT_MENU_ANY + pushPullOptions: { + acceptReporters: true, + items: this.PUSH_PULL_MENU + }, + gestureOptions: { + acceptReporters: true, + items: this.GESTURE_MENU + }, + axisOptions: { + acceptReporters: true, + items: this.AXIS_MENU + }, + tiltOptions: { + acceptReporters: true, + items: this.TILT_MENU + }, + tiltAnyOptions: { + acceptReporters: true, + items: this.TILT_MENU_ANY + } } }; } diff --git a/src/extensions/scratch3_makeymakey/index.js b/src/extensions/scratch3_makeymakey/index.js index ed925ac9..a83ca661 100644 --- a/src/extensions/scratch3_makeymakey/index.js +++ b/src/extensions/scratch3_makeymakey/index.js @@ -207,55 +207,61 @@ class Scratch3MakeyMakeyBlocks { } ], menus: { - KEY: [ - { - text: formatMessage({ - id: 'makeymakey.spaceKey', - default: 'space', - description: 'The space key on a computer keyboard.' - }), - value: KEY_ID_SPACE - }, - { - text: formatMessage({ - id: 'makeymakey.upArrow', - default: 'up arrow', - description: 'The up arrow key on a computer keyboard.' - }), - value: KEY_ID_UP - }, - { - text: formatMessage({ - id: 'makeymakey.downArrow', - default: 'down arrow', - description: 'The down arrow key on a computer keyboard.' - }), - value: KEY_ID_DOWN - }, - { - text: formatMessage({ - id: 'makeymakey.rightArrow', - default: 'right arrow', - description: 'The right arrow key on a computer keyboard.' - }), - value: KEY_ID_RIGHT - }, - { - text: formatMessage({ - id: 'makeymakey.leftArrow', - default: 'left arrow', - description: 'The left arrow key on a computer keyboard.' - }), - value: KEY_ID_LEFT - }, - {text: 'w', value: 'w'}, - {text: 'a', value: 'a'}, - {text: 's', value: 's'}, - {text: 'd', value: 'd'}, - {text: 'f', value: 'f'}, - {text: 'g', value: 'g'} - ], - SEQUENCE: this.buildSequenceMenu(this.DEFAULT_SEQUENCES) + KEY: { + acceptReporters: true, + items: [ + { + text: formatMessage({ + id: 'makeymakey.spaceKey', + default: 'space', + description: 'The space key on a computer keyboard.' + }), + value: KEY_ID_SPACE + }, + { + text: formatMessage({ + id: 'makeymakey.upArrow', + default: 'up arrow', + description: 'The up arrow key on a computer keyboard.' + }), + value: KEY_ID_UP + }, + { + text: formatMessage({ + id: 'makeymakey.downArrow', + default: 'down arrow', + description: 'The down arrow key on a computer keyboard.' + }), + value: KEY_ID_DOWN + }, + { + text: formatMessage({ + id: 'makeymakey.rightArrow', + default: 'right arrow', + description: 'The right arrow key on a computer keyboard.' + }), + value: KEY_ID_RIGHT + }, + { + text: formatMessage({ + id: 'makeymakey.leftArrow', + default: 'left arrow', + description: 'The left arrow key on a computer keyboard.' + }), + value: KEY_ID_LEFT + }, + {text: 'w', value: 'w'}, + {text: 'a', value: 'a'}, + {text: 's', value: 's'}, + {text: 'd', value: 'd'}, + {text: 'f', value: 'f'}, + {text: 'g', value: 'g'} + ] + }, + SEQUENCE: { + acceptReporters: true, + items: this.buildSequenceMenu(this.DEFAULT_SEQUENCES) + } } }; } diff --git a/src/extensions/scratch3_microbit/index.js b/src/extensions/scratch3_microbit/index.js index 29b5d790..81476fa0 100644 --- a/src/extensions/scratch3_microbit/index.js +++ b/src/extensions/scratch3_microbit/index.js @@ -746,12 +746,30 @@ class Scratch3MicroBitBlocks { } ], menus: { - buttons: this.BUTTONS_MENU, - gestures: this.GESTURES_MENU, - pinState: this.PIN_STATE_MENU, - tiltDirection: this.TILT_DIRECTION_MENU, - tiltDirectionAny: this.TILT_DIRECTION_ANY_MENU, - touchPins: ['0', '1', '2'] + buttons: { + acceptReporters: true, + items: this.BUTTONS_MENU + }, + gestures: { + acceptReporters: true, + items: this.GESTURES_MENU + }, + pinState: { + acceptReporters: true, + items: this.PIN_STATE_MENU + }, + tiltDirection: { + acceptReporters: true, + items: this.TILT_DIRECTION_MENU + }, + tiltDirectionAny: { + acceptReporters: true, + items: this.TILT_DIRECTION_ANY_MENU + }, + touchPins: { + acceptReporters: true, + items: ['0', '1', '2'] + } } }; } diff --git a/src/extensions/scratch3_music/index.js b/src/extensions/scratch3_music/index.js index e1693e40..cd43b87b 100644 --- a/src/extensions/scratch3_music/index.js +++ b/src/extensions/scratch3_music/index.js @@ -911,8 +911,14 @@ class Scratch3MusicBlocks { } ], menus: { - DRUM: this._buildMenu(this.DRUM_INFO), - INSTRUMENT: this._buildMenu(this.INSTRUMENT_INFO) + DRUM: { + acceptReporters: true, + items: this._buildMenu(this.DRUM_INFO) + }, + INSTRUMENT: { + acceptReporters: true, + items: this._buildMenu(this.INSTRUMENT_INFO) + } } }; } diff --git a/src/extensions/scratch3_pen/index.js b/src/extensions/scratch3_pen/index.js index 2132d10a..78705a5f 100644 --- a/src/extensions/scratch3_pen/index.js +++ b/src/extensions/scratch3_pen/index.js @@ -477,7 +477,10 @@ class Scratch3PenBlocks { } ], menus: { - colorParam: this._initColorParam() + colorParam: { + acceptReporters: true, + items: this._initColorParam() + } } }; } diff --git a/src/extensions/scratch3_text2speech/index.js b/src/extensions/scratch3_text2speech/index.js index 6093a7c5..7d4ac1f8 100644 --- a/src/extensions/scratch3_text2speech/index.js +++ b/src/extensions/scratch3_text2speech/index.js @@ -462,8 +462,14 @@ class Scratch3Text2SpeechBlocks { } ], menus: { - voices: this.getVoiceMenu(), - languages: this.getLanguageMenu() + voices: { + acceptReporters: true, + items: this.getVoiceMenu() + }, + languages: { + acceptReporters: true, + items: this.getLanguageMenu() + } } }; } diff --git a/src/extensions/scratch3_translate/index.js b/src/extensions/scratch3_translate/index.js index a60c116f..cefb3d49 100644 --- a/src/extensions/scratch3_translate/index.js +++ b/src/extensions/scratch3_translate/index.js @@ -146,7 +146,10 @@ class Scratch3TranslateBlocks { } ], menus: { - languages: this._supportedLanguages + languages: { + acceptReporters: true, + items: this._supportedLanguages + } } }; } diff --git a/src/extensions/scratch3_video_sensing/index.js b/src/extensions/scratch3_video_sensing/index.js index 76d5a2c1..bbdf3c30 100644 --- a/src/extensions/scratch3_video_sensing/index.js +++ b/src/extensions/scratch3_video_sensing/index.js @@ -488,9 +488,18 @@ class Scratch3VideoSensingBlocks { } ], menus: { - ATTRIBUTE: this._buildMenu(this.ATTRIBUTE_INFO), - SUBJECT: this._buildMenu(this.SUBJECT_INFO), - VIDEO_STATE: this._buildMenu(this.VIDEO_STATE_INFO) + ATTRIBUTE: { + acceptReporters: true, + items: this._buildMenu(this.ATTRIBUTE_INFO) + }, + SUBJECT: { + acceptReporters: true, + items: this._buildMenu(this.SUBJECT_INFO) + }, + VIDEO_STATE: { + acceptReporters: true, + items: this._buildMenu(this.VIDEO_STATE_INFO) + } } }; } diff --git a/src/extensions/scratch3_wedo2/index.js b/src/extensions/scratch3_wedo2/index.js index e08c987f..ad854c44 100644 --- a/src/extensions/scratch3_wedo2/index.js +++ b/src/extensions/scratch3_wedo2/index.js @@ -1134,139 +1134,157 @@ class Scratch3WeDo2Blocks { } ], menus: { - MOTOR_ID: [ - { - text: formatMessage({ - id: 'wedo2.motorId.default', - default: 'motor', - description: 'label for motor element in motor menu for LEGO WeDo 2 extension' - }), - value: WeDo2MotorLabel.DEFAULT - }, - { - text: formatMessage({ - id: 'wedo2.motorId.a', - default: 'motor A', - description: 'label for motor A element in motor menu for LEGO WeDo 2 extension' - }), - value: WeDo2MotorLabel.A - }, - { - text: formatMessage({ - id: 'wedo2.motorId.b', - default: 'motor B', - description: 'label for motor B element in motor menu for LEGO WeDo 2 extension' - }), - value: WeDo2MotorLabel.B - }, - { - text: formatMessage({ - id: 'wedo2.motorId.all', - default: 'all motors', - description: 'label for all motors element in motor menu for LEGO WeDo 2 extension' - }), - value: WeDo2MotorLabel.ALL - } - ], - MOTOR_DIRECTION: [ - { - text: formatMessage({ - id: 'wedo2.motorDirection.forward', - default: 'this way', - description: 'label for forward element in motor direction menu for LEGO WeDo 2 extension' - }), - value: WeDo2MotorDirection.FORWARD - }, - { - text: formatMessage({ - id: 'wedo2.motorDirection.backward', - default: 'that way', - description: 'label for backward element in motor direction menu for LEGO WeDo 2 extension' - }), - value: WeDo2MotorDirection.BACKWARD - }, - { - text: formatMessage({ - id: 'wedo2.motorDirection.reverse', - default: 'reverse', - description: 'label for reverse element in motor direction menu for LEGO WeDo 2 extension' - }), - value: WeDo2MotorDirection.REVERSE - } - ], - TILT_DIRECTION: [ - { - text: formatMessage({ - id: 'wedo2.tiltDirection.up', - default: 'up', - description: 'label for up element in tilt direction menu for LEGO WeDo 2 extension' - }), - value: WeDo2TiltDirection.UP - }, - { - text: formatMessage({ - id: 'wedo2.tiltDirection.down', - default: 'down', - description: 'label for down element in tilt direction menu for LEGO WeDo 2 extension' - }), - value: WeDo2TiltDirection.DOWN - }, - { - text: formatMessage({ - id: 'wedo2.tiltDirection.left', - default: 'left', - description: 'label for left element in tilt direction menu for LEGO WeDo 2 extension' - }), - value: WeDo2TiltDirection.LEFT - }, - { - text: formatMessage({ - id: 'wedo2.tiltDirection.right', - default: 'right', - description: 'label for right element in tilt direction menu for LEGO WeDo 2 extension' - }), - value: WeDo2TiltDirection.RIGHT - } - ], - TILT_DIRECTION_ANY: [ - { - text: formatMessage({ - id: 'wedo2.tiltDirection.up', - default: 'up' - }), - value: WeDo2TiltDirection.UP - }, - { - text: formatMessage({ - id: 'wedo2.tiltDirection.down', - default: 'down' - }), - value: WeDo2TiltDirection.DOWN - }, - { - text: formatMessage({ - id: 'wedo2.tiltDirection.left', - default: 'left' - }), - value: WeDo2TiltDirection.LEFT - }, - { - text: formatMessage({ - id: 'wedo2.tiltDirection.right', - default: 'right' - }), - value: WeDo2TiltDirection.RIGHT - }, - { - text: formatMessage({ - id: 'wedo2.tiltDirection.any', - default: 'any', - description: 'label for any element in tilt direction menu for LEGO WeDo 2 extension' - }), - value: WeDo2TiltDirection.ANY - } - ], - OP: ['<', '>'] + MOTOR_ID: { + acceptReporters: true, + items: [ + { + text: formatMessage({ + id: 'wedo2.motorId.default', + default: 'motor', + description: 'label for motor element in motor menu for LEGO WeDo 2 extension' + }), + value: WeDo2MotorLabel.DEFAULT + }, + { + text: formatMessage({ + id: 'wedo2.motorId.a', + default: 'motor A', + description: 'label for motor A element in motor menu for LEGO WeDo 2 extension' + }), + value: WeDo2MotorLabel.A + }, + { + text: formatMessage({ + id: 'wedo2.motorId.b', + default: 'motor B', + description: 'label for motor B element in motor menu for LEGO WeDo 2 extension' + }), + value: WeDo2MotorLabel.B + }, + { + text: formatMessage({ + id: 'wedo2.motorId.all', + default: 'all motors', + description: 'label for all motors element in motor menu for LEGO WeDo 2 extension' + }), + value: WeDo2MotorLabel.ALL + } + ] + }, + MOTOR_DIRECTION: { + acceptReporters: true, + items: [ + { + text: formatMessage({ + id: 'wedo2.motorDirection.forward', + default: 'this way', + description: + 'label for forward element in motor direction menu for LEGO WeDo 2 extension' + }), + value: WeDo2MotorDirection.FORWARD + }, + { + text: formatMessage({ + id: 'wedo2.motorDirection.backward', + default: 'that way', + description: + 'label for backward element in motor direction menu for LEGO WeDo 2 extension' + }), + value: WeDo2MotorDirection.BACKWARD + }, + { + text: formatMessage({ + id: 'wedo2.motorDirection.reverse', + default: 'reverse', + description: + 'label for reverse element in motor direction menu for LEGO WeDo 2 extension' + }), + value: WeDo2MotorDirection.REVERSE + } + ] + }, + TILT_DIRECTION: { + acceptReporters: true, + items: [ + { + text: formatMessage({ + id: 'wedo2.tiltDirection.up', + default: 'up', + description: 'label for up element in tilt direction menu for LEGO WeDo 2 extension' + }), + value: WeDo2TiltDirection.UP + }, + { + text: formatMessage({ + id: 'wedo2.tiltDirection.down', + default: 'down', + description: 'label for down element in tilt direction menu for LEGO WeDo 2 extension' + }), + value: WeDo2TiltDirection.DOWN + }, + { + text: formatMessage({ + id: 'wedo2.tiltDirection.left', + default: 'left', + description: 'label for left element in tilt direction menu for LEGO WeDo 2 extension' + }), + value: WeDo2TiltDirection.LEFT + }, + { + text: formatMessage({ + id: 'wedo2.tiltDirection.right', + default: 'right', + description: 'label for right element in tilt direction menu for LEGO WeDo 2 extension' + }), + value: WeDo2TiltDirection.RIGHT + } + ] + }, + TILT_DIRECTION_ANY: { + acceptReporters: true, + items: [ + { + text: formatMessage({ + id: 'wedo2.tiltDirection.up', + default: 'up' + }), + value: WeDo2TiltDirection.UP + }, + { + text: formatMessage({ + id: 'wedo2.tiltDirection.down', + default: 'down' + }), + value: WeDo2TiltDirection.DOWN + }, + { + text: formatMessage({ + id: 'wedo2.tiltDirection.left', + default: 'left' + }), + value: WeDo2TiltDirection.LEFT + }, + { + text: formatMessage({ + id: 'wedo2.tiltDirection.right', + default: 'right' + }), + value: WeDo2TiltDirection.RIGHT + }, + { + text: formatMessage({ + id: 'wedo2.tiltDirection.any', + default: 'any', + description: 'label for any element in tilt direction menu for LEGO WeDo 2 extension' + }), + value: WeDo2TiltDirection.ANY + } + ] + }, + OP: { + acceptReporters: true, + items: ['<', '>'] + } } }; } From 18b1551cdb37740996bfc0c2891830cb712d8649 Mon Sep 17 00:00:00 2001 From: Chris Willis-Ford Date: Mon, 17 Jun 2019 13:22:52 -0700 Subject: [PATCH 19/23] remove redundant check Co-Authored-By: Karishma Chadha --- src/engine/runtime.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index ac6d005a..e76e12f3 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -1234,7 +1234,7 @@ class Runtime extends EventEmitter { let shadowType; let fieldName; if (argInfo.menu) { - const menuInfo = argInfo.menu && context.categoryInfo.menuInfo[argInfo.menu]; + const menuInfo = context.categoryInfo.menuInfo[argInfo.menu]; if (menuInfo.acceptReporters) { valueName = placeholder; shadowType = this._makeExtensionMenuId(argInfo.menu, context.categoryInfo.id); From 3c28f714bb981332d7f06e8bf88c645fe419d800 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Mon, 17 Jun 2019 21:00:35 -0700 Subject: [PATCH 20/23] respond to code review --- docs/extensions.md | 3 ++- src/engine/runtime.js | 9 ++++----- src/extension-support/extension-manager.js | 10 ++++++---- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/docs/extensions.md b/docs/extensions.md index 5bafb51b..add90188 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -213,7 +213,8 @@ A few of these considerations include: * In particular, changing languages should never break a working project. * The average Scratch user should be able to figure out the valid values for this input without referring to extension documentation. - * One way to ensure this is to make an item's text match or include the item's value. + * One way to ensure this is to make an item's text match or include the item's value. For example, the official Music + extension contains menu items with names like "(1) Piano" with value 1, "(8) Cello" with value 8, and so on. * The block should accept any value as input, even "invalid" values. * Scratch has no concept of a runtime error! * For a command block, sometimes the best option is to do nothing. diff --git a/src/engine/runtime.js b/src/engine/runtime.js index e76e12f3..903983db 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -847,10 +847,10 @@ class Runtime extends EventEmitter { for (const menuName in extensionInfo.menus) { if (extensionInfo.menus.hasOwnProperty(menuName)) { - const menuValue = extensionInfo.menus[menuName]; - const convertedMenu = this._buildMenuForScratchBlocks(menuName, menuValue, categoryInfo); + const menuInfo = extensionInfo.menus[menuName]; + const convertedMenu = this._buildMenuForScratchBlocks(menuName, menuInfo, categoryInfo); categoryInfo.menus.push(convertedMenu); - categoryInfo.menuInfo[menuName] = menuValue; + categoryInfo.menuInfo[menuName] = menuInfo; } } for (const fieldTypeName in extensionInfo.customFieldTypes) { @@ -1263,8 +1263,7 @@ class Runtime extends EventEmitter { 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. + // A displays a dynamic value: a user-editable text field, a drop-down menu, etc. if (fieldName) { context.inputList.push(`${defaultValue}`); } diff --git a/src/extension-support/extension-manager.js b/src/extension-support/extension-manager.js index 757ec682..7a5c1447 100644 --- a/src/extension-support/extension-manager.js +++ b/src/extension-support/extension-manager.js @@ -312,19 +312,21 @@ class ExtensionManager { for (let i = 0; i < menuNames.length; i++) { const menuName = menuNames[i]; let menuInfo = menus[menuName]; + + // If the menu description is in short form (items only) then normalize it to general form: an object with + // its items listed in an `items` property. if (!menuInfo.items) { menuInfo = { items: menuInfo }; menus[menuName] = menuInfo; } - // If the value is a string, it should be the name of a function in the - // extension object to call to populate the menu whenever it is opened. - // Set up the binding for the function object here so - // we can use it later when converting the menu for Scratch Blocks. + // If `items` is a string, it should be the name of a function in the extension object. Calling the + // function should return an array of items to populate the menu when it is opened. if (typeof menuInfo.items === 'string') { const menuItemFunctionName = menuInfo.items; const serviceObject = dispatch.services[serviceName]; + // Bind the function here so we can pass a simple item generation function to Scratch Blocks later. menuInfo.items = this._getExtensionMenuItems.bind(this, serviceObject, menuItemFunctionName); } } From edd6aafed01d49be35d7298ba2561376991f5b8e Mon Sep 17 00:00:00 2001 From: Chris Willis-Ford Date: Tue, 18 Jun 2019 14:32:00 -0700 Subject: [PATCH 21/23] call out capitalization change since it's important but easy to miss Co-Authored-By: Karishma Chadha --- src/engine/mutation-adapter.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/engine/mutation-adapter.js b/src/engine/mutation-adapter.js index 8cfac763..6c3bd84d 100644 --- a/src/engine/mutation-adapter.js +++ b/src/engine/mutation-adapter.js @@ -13,6 +13,8 @@ const mutatorTagToObject = function (dom) { for (const prop in dom.attribs) { if (prop === 'xmlns') continue; obj[prop] = decodeHtml(dom.attribs[prop]); + // Note: the capitalization of block info in the following lines is important. + // The lowercase is read in from xml which normalizes case. The VM uses camel case everywhere else. if (prop === 'blockinfo') { obj.blockInfo = JSON.parse(obj.blockinfo); delete obj.blockinfo; From fd776025e5fe2cfd2b2d5f7fa004ea94985b1447 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Tue, 18 Jun 2019 15:04:37 -0700 Subject: [PATCH 22/23] refactor _refreshExtensionPrimitives for clarity The previous form made it harder to see that at most one extension category is handled per call. --- src/engine/runtime.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 1a1dd23f..ac7c7bb2 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -825,15 +825,14 @@ class Runtime extends EventEmitter { * @private */ _refreshExtensionPrimitives (extensionInfo) { - for (const categoryInfo of this._blockInfo) { - if (extensionInfo.id === categoryInfo.id) { - categoryInfo.name = maybeFormatMessage(extensionInfo.name); - categoryInfo.blocks = []; - categoryInfo.menus = []; - this._fillExtensionCategory(categoryInfo, extensionInfo); + const categoryInfo = this._blockInfo.find(info => info.id === extensionInfo.id); + if (categoryInfo) { + categoryInfo.name = maybeFormatMessage(extensionInfo.name); + categoryInfo.blocks = []; + categoryInfo.menus = []; + this._fillExtensionCategory(categoryInfo, extensionInfo); - this.emit(Runtime.BLOCKSINFO_UPDATE, categoryInfo); - } + this.emit(Runtime.BLOCKSINFO_UPDATE, categoryInfo); } } From aae7607e10735a74b7aa9d519b4ef21c9878a976 Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Tue, 18 Jun 2019 21:03:17 -0700 Subject: [PATCH 23/23] fix extension dropdown menu item conversion --- src/engine/runtime.js | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 07f91219..a3976e0e 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -891,23 +891,16 @@ class Runtime extends EventEmitter { } /** - * 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 - * @param {object} menuInfo - a description of this menu and its items - * @property {*} items - an array of menu items or a function to retrieve such an array - * @property {boolean} [acceptReporters] - if true, allow dropping reporters onto this menu - * @param {CategoryInfo} categoryInfo - the category for this block - * @returns {object} - a JSON-esque object ready for scratch-blocks' consumption + * Convert the given extension menu items into the scratch-blocks style of list of pairs. + * If the menu is dynamic (e.g. the passed in argument is a function), return the input unmodified. + * @param {object} menuItems - an array of menu items or a function to retrieve such an array + * @returns {object} - an array of 2 element arrays or the original input function * @private */ - _buildMenuForScratchBlocks (menuName, menuInfo, categoryInfo) { - const menuId = this._makeExtensionMenuId(menuName, categoryInfo.id); - let menuItems = null; - if (typeof menuInfo.items === 'function') { - menuItems = menuInfo.items; - } else { + _convertMenuItems (menuItems) { + if (typeof menuItems !== 'function') { const extensionMessageContext = this.makeMessageContextForTarget(); - menuItems = menuInfo.items.map(item => { + return menuItems.map(item => { const formattedItem = maybeFormatMessage(item, extensionMessageContext); switch (typeof formattedItem) { case 'string': @@ -919,6 +912,22 @@ class Runtime extends EventEmitter { } }); } + return menuItems; + } + + /** + * 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 + * @param {object} menuInfo - a description of this menu and its items + * @property {*} items - an array of menu items or a function to retrieve such an array + * @property {boolean} [acceptReporters] - if true, allow dropping reporters onto this menu + * @param {CategoryInfo} categoryInfo - the category for this block + * @returns {object} - a JSON-esque object ready for scratch-blocks' consumption + * @private + */ + _buildMenuForScratchBlocks (menuName, menuInfo, categoryInfo) { + const menuId = this._makeExtensionMenuId(menuName, categoryInfo.id); + const menuItems = this._convertMenuItems(menuInfo.items); return { json: { message0: '%1', @@ -1242,7 +1251,7 @@ class Runtime extends EventEmitter { fieldName = argInfo.menu; } else { argJSON.type = 'field_dropdown'; - argJSON.options = menuInfo.items; + argJSON.options = this._convertMenuItems(menuInfo.items); valueName = null; shadowType = null; fieldName = placeholder;