Merge pull request #1727 from paulkaplan/add-ext-on-share

Add unloaded extensions on block sharing

src/serialization/sb3.js

@@ -271,6 +271,19 @@ const compressInputTree = function (block, blocks) {
     return block;
 };
 
+/**
+ * Get non-core extension ID for a given sb3 opcode.
+ * @param {!string} opcode The opcode to examine for extension.
+ * @return {?string} The extension ID, if it exists and is not a core extension.
+ */
+const getExtensionIdForOpcode = function (opcode) {
+    const index = opcode.indexOf('_');
+    const prefix = opcode.substring(0, index);
+    if (CORE_EXTENSIONS.indexOf(prefix) === -1) {
+        if (prefix !== '') return prefix;
+    }
+};
+
 /**
  * Serialize the given blocks object (representing all the blocks for the target
  * currently being serialized.)
@@ -285,10 +298,9 @@ const serializeBlocks = function (blocks) {
     for (const blockID in blocks) {
         if (!blocks.hasOwnProperty(blockID)) continue;
         obj[blockID] = serializeBlock(blocks[blockID], blocks);
-        const index = blocks[blockID].opcode.indexOf('_');
+        const extensionID = getExtensionIdForOpcode(blocks[blockID].opcode);
-        const prefix = blocks[blockID].opcode.substring(0, index);
+        if (extensionID) {
-        if (CORE_EXTENSIONS.indexOf(prefix) === -1) {
+            extensionIDs.add(extensionID);
-            if (prefix !== '') extensionIDs.add(prefix);
         }
     }
     // once we have completed a first pass, do a second pass on block inputs
@@ -812,10 +824,9 @@ const parseScratchObject = function (object, runtime, extensions, zip) {
             blocks.createBlock(blockJSON);
 
             // If the block is from an extension, record it.
-            const index = blockJSON.opcode.indexOf('_');
+            const extensionID = getExtensionIdForOpcode(blockJSON.opcode);
-            const prefix = blockJSON.opcode.substring(0, index);
+            if (extensionID) {
-            if (CORE_EXTENSIONS.indexOf(prefix) === -1) {
+                extensions.extensionIDs.add(extensionID);
-                if (prefix !== '') extensions.extensionIDs.add(prefix);
             }
         }
     }
@@ -1049,5 +1060,6 @@ module.exports = {
     serialize: serialize,
     deserialize: deserialize,
     deserializeBlocks: deserializeBlocks,
-    serializeBlocks: serializeBlocks
+    serializeBlocks: serializeBlocks,
+    getExtensionIdForOpcode: getExtensionIdForOpcode
 };

src/virtual-machine.js

@@ -1107,6 +1107,7 @@ class VirtualMachine extends EventEmitter {
      * @param {!string} targetId Id of target to add blocks to.
      * @param {?string} optFromTargetId Optional target id indicating that blocks are being
      * shared from that target. This is needed for resolving any potential variable conflicts.
+     * @return {!Promise} Promise that resolves when the extensions and blocks have been added.
      */
     shareBlocksToTarget (blocks, targetId, optFromTargetId) {
         const copiedBlocks = JSON.parse(JSON.stringify(blocks));
@@ -1119,10 +1120,24 @@ class VirtualMachine extends EventEmitter {
             fromTarget.resolveVariableSharingConflictsWithTarget(copiedBlocks, target);
         }
 
-        for (let i = 0; i < copiedBlocks.length; i++) {
+        // Create a unique set of extensionIds that are not yet loaded
-            target.blocks.createBlock(copiedBlocks[i]);
+        const extensionIDs = new Set(copiedBlocks
-        }
+            .map(b => sb3.getExtensionIdForOpcode(b.opcode))
+            .filter(id => !!id) // Remove ids that do not exist
+            .filter(id => !this.extensionManager.isExtensionLoaded(id)) // and remove loaded extensions
+        );
+
+        // Create an array promises for extensions to load
+        const extensionPromises = Array.from(extensionIDs,
+            id => this.extensionManager.loadExtensionURL(id)
+        );
+
+        return Promise.all(extensionPromises).then(() => {
+            copiedBlocks.forEach(block => {
+                target.blocks.createBlock(block);
+            });
             target.blocks.updateTargetSpecificBlocks(target.isStage);
+        });
     }
 
     /**

test/unit/serialization_sb3.js

@@ -221,3 +221,18 @@ test('deserializeBlocks on already deserialized input', t => {
             t.end();
         });
 });
+
+test('getExtensionIdForOpcode', t => {
+    t.equal(sb3.getExtensionIdForOpcode('wedo_loopy'), 'wedo');
+
+    // does not consider CORE to be extensions
+    t.false(sb3.getExtensionIdForOpcode('control_loopy'));
+
+    // only considers things before the first underscore
+    t.equal(sb3.getExtensionIdForOpcode('hello_there_loopy'), 'hello');
+
+    // does not return anything for opcodes with no extension
+    t.false(sb3.getExtensionIdForOpcode('hello'));
+
+    t.end();
+});

test/unit/virtual-machine.js

@@ -744,7 +744,7 @@ test('shareBlocksToTarget shares global variables without any name changes', t =
     t.type(stage.blocks.getBlock('a block'), 'undefined');
 
     // Share the block to the stage
-    vm.shareBlocksToTarget([target.blocks.getBlock('a block')], stage.id, target.id);
+    vm.shareBlocksToTarget([target.blocks.getBlock('a block')], stage.id, target.id).then(() => {
 
         // Verify that the block now exists on the target as well as the stage
         t.type(target.blocks.getBlock('a block'), 'object');
@@ -767,6 +767,7 @@ test('shareBlocksToTarget shares global variables without any name changes', t =
 
         t.end();
     });
+});
 
 test('shareBlocksToTarget shares a local variable to the stage, creating a global variable with a new name', t => {
     const vm = new VirtualMachine();
@@ -803,8 +804,7 @@ test('shareBlocksToTarget shares a local variable to the stage, creating a globa
     t.type(stage.blocks.getBlock('a block'), 'undefined');
 
     // Share the block to the stage
-    vm.shareBlocksToTarget([target.blocks.getBlock('a block')], stage.id, target.id);
+    vm.shareBlocksToTarget([target.blocks.getBlock('a block')], stage.id, target.id).then(() => {
-
         // Verify that the block still exists on the target and remains unchanged
         t.type(target.blocks.getBlock('a block'), 'object');
         t.type(target.blocks.getBlock('a block').fields, 'object');
@@ -834,6 +834,7 @@ test('shareBlocksToTarget shares a local variable to the stage, creating a globa
 
         t.end();
     });
+});
 
 test('shareBlocksToTarget chooses a fresh name for a new global variable checking for conflicts on all sprites', t => {
     const vm = new VirtualMachine();
@@ -877,8 +878,7 @@ test('shareBlocksToTarget chooses a fresh name for a new global variable checkin
     otherTarget.createVariable('a different var', 'Stage: a mock variable', Variable.SCALAR_TYPE);
 
     // Share the block to the stage
-    vm.shareBlocksToTarget([target.blocks.getBlock('a block')], stage.id, target.id);
+    vm.shareBlocksToTarget([target.blocks.getBlock('a block')], stage.id, target.id).then(() => {
-
         // Verify that the block still exists on the target and remains unchanged
         t.type(target.blocks.getBlock('a block'), 'object');
         t.type(target.blocks.getBlock('a block').fields, 'object');
@@ -908,6 +908,36 @@ test('shareBlocksToTarget chooses a fresh name for a new global variable checkin
 
         t.end();
     });
+});
+
+test('shareBlocksToTarget loads extensions that have not yet been loaded', t => {
+    const vm = new VirtualMachine();
+    const runtime = vm.runtime;
+    const spr1 = new Sprite(null, runtime);
+    const stage = spr1.createClone();
+    runtime.targets = [stage];
+
+    const fakeBlocks = [
+        {opcode: 'loaded_fakeblock'},
+        {opcode: 'notloaded_fakeblock'}
+    ];
+
+    // Stub the extension manager
+    const loadedIds = [];
+    vm.extensionManager = {
+        isExtensionLoaded: id => id === 'loaded',
+        loadExtensionURL: id => new Promise(resolve => {
+            loadedIds.push(id);
+            resolve();
+        })
+    };
+
+    vm.shareBlocksToTarget(fakeBlocks, stage.id).then(() => {
+        // Verify that only the not-loaded extension gets loaded
+        t.deepEqual(loadedIds, ['notloaded']);
+        t.end();
+    });
+});
 
 test('Setting turbo mode emits events', t => {
     let turboMode = null;