Merge pull request #1069 from thisandagain/bugfix/underscore

Replace extension delimiter with '_'
scratchfoundation · Apr 20, 2018 · 968643e · 968643e
2 parents 0f16e2f + 192084e
commit 968643e
Show file tree

Hide file tree

Showing 7 changed files with 85 additions and 49 deletions.
diff --git a/src/engine/runtime.js b/src/engine/runtime.js
@@ -471,7 +471,7 @@ class Runtime extends EventEmitter {
      * @private
      */
     _makeExtensionMenuId (menuName, extensionId) {
-        return `${extensionId}.menu.${escapeHtml(menuName)}`;
+        return `${extensionId}_menu_${escapeHtml(menuName)}`;
     }
 
     /**
@@ -625,7 +625,7 @@ class Runtime extends EventEmitter {
      * @private
      */
     _convertForScratchBlocks (blockInfo, categoryInfo) {
-        const extendedOpcode = `${categoryInfo.id}.${blockInfo.opcode}`;
+        const extendedOpcode = `${categoryInfo.id}_${blockInfo.opcode}`;
 
         const blockJSON = {
             type: extendedOpcode,

diff --git a/src/extension-support/extension-manager.js b/src/extension-support/extension-manager.js
@@ -188,7 +188,7 @@ class ExtensionManager {
     _registerInternalExtension (extensionObject) {
         const extensionInfo = extensionObject.getInfo();
         const fakeWorkerId = this.nextExtensionWorker++;
-        const serviceName = `extension.${fakeWorkerId}.${extensionInfo.id}`;
+        const serviceName = `extension_${fakeWorkerId}_${extensionInfo.id}`;
         return dispatch.setService(serviceName, extensionObject)
             .then(() => {
                 dispatch.call('extensions', 'registerExtensionService', serviceName);
@@ -229,7 +229,9 @@ class ExtensionManager {
      */
     _prepareExtensionInfo (serviceName, extensionInfo) {
         extensionInfo = Object.assign({}, extensionInfo);
-        extensionInfo.id = this._sanitizeID(extensionInfo.id);
+        if (!/^[a-z0-9]+$/i.test(extensionInfo.id)) {
+            throw new Error('Invalid extension id');
+        }
         extensionInfo.name = extensionInfo.name || extensionInfo.id;
         extensionInfo.blocks = extensionInfo.blocks || [];
         extensionInfo.targetTypes = extensionInfo.targetTypes || [];

diff --git a/src/serialization/sb2.js b/src/serialization/sb2.js
@@ -19,6 +19,21 @@ const {loadCostume} = require('../import/load-costume.js');
 const {loadSound} = require('../import/load-sound.js');
 const {deserializeCostume, deserializeSound} = require('./deserialize-assets.js');
 
+// Constants used during deserialization of an SB2 file
+const CORE_EXTENSIONS = [
+    'argument',
+    'control',
+    'data',
+    'event',
+    'looks',
+    'math',
+    'motion',
+    'operator',
+    'procedures',
+    'sensing',
+    'sound'
+];
+
 /**
  * Convert a Scratch 2.0 procedure string (e.g., "my_procedure %s %b %n")
  * into an argument map. This allows us to provide the expected inputs
@@ -501,12 +516,14 @@ const parseBlock = function (sb2block, addBroadcastMsg, getVariableId, extension
         return;
     }
     const oldOpcode = sb2block[0];
+
     // If the block is from an extension, record it.
-    const dotIndex = blockMetadata.opcode.indexOf('.');
-    if (dotIndex >= 0) {
-        const extension = blockMetadata.opcode.substring(0, dotIndex);
-        extensions.extensionIDs.add(extension);
+    const index = blockMetadata.opcode.indexOf('_');
+    const prefix = blockMetadata.opcode.substring(0, index);
+    if (CORE_EXTENSIONS.indexOf(prefix) === -1) {
+        if (prefix !== '') extensions.extensionIDs.add(prefix);
     }
+
     // Block skeleton.
     const activeBlock = {
         id: uid(), // Generate a new block unique ID.

diff --git a/src/serialization/sb2_specmap.js b/src/serialization/sb2_specmap.js
@@ -420,11 +420,11 @@ const specMap = {
         ]
     },
     'playDrum': {
-        opcode: 'music.playDrumForBeats',
+        opcode: 'music_playDrumForBeats',
         argMap: [
             {
                 type: 'input',
-                inputOp: 'music.menu.DRUM',
+                inputOp: 'music_menu_DRUM',
                 inputName: 'DRUM'
             },
             {
@@ -435,7 +435,7 @@ const specMap = {
         ]
     },
     'rest:elapsed:from:': {
-        opcode: 'music.restForBeats',
+        opcode: 'music_restForBeats',
         argMap: [
             {
                 type: 'input',
@@ -445,7 +445,7 @@ const specMap = {
         ]
     },
     'noteOn:duration:elapsed:from:': {
-        opcode: 'music.playNoteForBeats',
+        opcode: 'music_playNoteForBeats',
         argMap: [
             {
                 type: 'input',
@@ -460,11 +460,11 @@ const specMap = {
         ]
     },
     'instrument:': {
-        opcode: 'music.setInstrument',
+        opcode: 'music_setInstrument',
         argMap: [
             {
                 type: 'input',
-                inputOp: 'music.menu.INSTRUMENT',
+                inputOp: 'music_menu_INSTRUMENT',
                 inputName: 'INSTRUMENT'
             }
         ]
@@ -495,7 +495,7 @@ const specMap = {
         ]
     },
     'changeTempoBy:': {
-        opcode: 'music.changeTempo',
+        opcode: 'music_changeTempo',
         argMap: [
             {
                 type: 'input',
@@ -505,7 +505,7 @@ const specMap = {
         ]
     },
     'setTempoTo:': {
-        opcode: 'music.setTempo',
+        opcode: 'music_setTempo',
         argMap: [
             {
                 type: 'input',
@@ -515,32 +515,32 @@ const specMap = {
         ]
     },
     'tempo': {
-        opcode: 'music.getTempo',
+        opcode: 'music_getTempo',
         argMap: [
         ]
     },
     'clearPenTrails': {
-        opcode: 'pen.clear',
+        opcode: 'pen_clear',
         argMap: [
         ]
     },
     'stampCostume': {
-        opcode: 'pen.stamp',
+        opcode: 'pen_stamp',
         argMap: [
         ]
     },
     'putPenDown': {
-        opcode: 'pen.penDown',
+        opcode: 'pen_penDown',
         argMap: [
         ]
     },
     'putPenUp': {
-        opcode: 'pen.penUp',
+        opcode: 'pen_penUp',
         argMap: [
         ]
     },
     'penColor:': {
-        opcode: 'pen.setPenColorToColor',
+        opcode: 'pen_setPenColorToColor',
         argMap: [
             {
                 type: 'input',
@@ -550,7 +550,7 @@ const specMap = {
         ]
     },
     'changePenHueBy:': {
-        opcode: 'pen.changePenHueBy',
+        opcode: 'pen_changePenHueBy',
         argMap: [
             {
                 type: 'input',
@@ -560,7 +560,7 @@ const specMap = {
         ]
     },
     'setPenHueTo:': {
-        opcode: 'pen.setPenHueToNumber',
+        opcode: 'pen_setPenHueToNumber',
         argMap: [
             {
                 type: 'input',
@@ -570,7 +570,7 @@ const specMap = {
         ]
     },
     'changePenShadeBy:': {
-        opcode: 'pen.changePenShadeBy',
+        opcode: 'pen_changePenShadeBy',
         argMap: [
             {
                 type: 'input',
@@ -580,7 +580,7 @@ const specMap = {
         ]
     },
     'setPenShadeTo:': {
-        opcode: 'pen.setPenShadeToNumber',
+        opcode: 'pen_setPenShadeToNumber',
         argMap: [
             {
                 type: 'input',
@@ -590,7 +590,7 @@ const specMap = {
         ]
     },
     'changePenSizeBy:': {
-        opcode: 'pen.changePenSizeBy',
+        opcode: 'pen_changePenSizeBy',
         argMap: [
             {
                 type: 'input',
@@ -600,7 +600,7 @@ const specMap = {
         ]
     },
     'penSize:': {
-        opcode: 'pen.setPenSizeTo',
+        opcode: 'pen_setPenSizeTo',
         argMap: [
             {
                 type: 'input',
@@ -610,16 +610,16 @@ const specMap = {
         ]
     },
     'senseVideoMotion': {
-        opcode: 'videoSensing.videoOn',
+        opcode: 'videoSensing_videoOn',
         argMap: [
             {
                 type: 'input',
-                inputOp: 'videoSensing.menu.ATTRIBUTE',
+                inputOp: 'videoSensing_menu_ATTRIBUTE',
                 inputName: 'ATTRIBUTE'
             },
             {
                 type: 'input',
-                inputOp: 'videoSensing.menu.SUBJECT',
+                inputOp: 'videoSensing_menu_SUBJECT',
                 inputName: 'SUBJECT'
             }
         ]
@@ -655,7 +655,7 @@ const specMap = {
     'whenSensorGreaterThan': ([, sensor]) => {
         if (sensor === 'video motion') {
             return {
-                opcode: 'videoSensing.whenMotionGreaterThan',
+                opcode: 'videoSensing_whenMotionGreaterThan',
                 argMap: [
                     // skip the first arg, since we converted to a video specific sensing block
                     {},
@@ -980,17 +980,17 @@ const specMap = {
     //     ]
     // },
     'setVideoState': {
-        opcode: 'videoSensing.videoToggle',
+        opcode: 'videoSensing_videoToggle',
         argMap: [
             {
                 type: 'input',
-                inputOp: 'videoSensing.menu.VIDEO_STATE',
+                inputOp: 'videoSensing_menu_VIDEO_STATE',
                 inputName: 'VIDEO_STATE'
             }
         ]
     },
     'setVideoTransparency': {
-        opcode: 'videoSensing.setVideoTransparency',
+        opcode: 'videoSensing_setVideoTransparency',
         argMap: [
             {
                 type: 'input',

diff --git a/src/serialization/sb3.js b/src/serialization/sb3.js
@@ -33,6 +33,22 @@ const INPUT_BLOCK_NO_SHADOW = 2; // no shadow
 const INPUT_DIFF_BLOCK_SHADOW = 3; // obscured shadow
 // There shouldn't be a case where block is null, but shadow is present...
 
+// Constants used during deserialization of an SB3 file
+const CORE_EXTENSIONS = [
+    'argument',
+    'colour',
+    'control',
+    'data',
+    'event',
+    'looks',
+    'math',
+    'motion',
+    'operator',
+    'procedures',
+    'sensing',
+    'sound'
+];
+
 // Constants referring to 'primitive' blocks that are usually shadows,
 // or in the case of variables and lists, appear quite often in projects
 // math_number
@@ -700,10 +716,11 @@ const parseScratchObject = function (object, runtime, extensions, zip) {
             const blockJSON = object.blocks[blockId];
             blocks.createBlock(blockJSON);
 
-            const dotIndex = blockJSON.opcode.indexOf('.');
-            if (dotIndex >= 0) {
-                const extensionId = blockJSON.opcode.substring(0, dotIndex);
-                extensions.extensionIDs.add(extensionId);
+            // If the block is from an extension, record it.
+            const index = blockJSON.opcode.indexOf('_');
+            const prefix = blockJSON.opcode.substring(0, index);
+            if (CORE_EXTENSIONS.indexOf(prefix) === -1) {
+                if (prefix !== '') extensions.extensionIDs.add(prefix);
             }
         }
     }

diff --git a/test/integration/internal-extension.js b/test/integration/internal-extension.js
@@ -16,7 +16,7 @@ class TestInternalExtension {
     getInfo () {
         this.status.getInfoCalled = true;
         return {
-            id: 'test-internal-extension',
+            id: 'testInternalExtension',
             name: 'Test Internal Extension',
             blocks: [
                 {
@@ -55,7 +55,7 @@ test('internal extension', t => {
     return vm.extensionManager._registerInternalExtension(extension).then(() => {
         t.ok(extension.status.getInfoCalled);
 
-        const func = vm.runtime.getOpcodeFunction('test-internal-extension.go');
+        const func = vm.runtime.getOpcodeFunction('testInternalExtension_go');
         t.type(func, 'function');
 
         t.notOk(extension.status.goCalled);