diff --git a/src/serialization/sb3.js b/src/serialization/sb3.js index d493656712..e309880371 100644 --- a/src/serialization/sb3.js +++ b/src/serialization/sb3.js @@ -801,6 +801,8 @@ const deserializeFields = function (fields) { * @return {object} input is modified and returned */ const deserializeBlocks = function (blocks) { + // oldToNew is used to fix comments attached to variable blocks: see #1893 + const oldToNew = {}; for (const blockId in blocks) { if (!Object.prototype.hasOwnProperty.call(blocks, blockId)) { continue; @@ -811,14 +813,14 @@ const deserializeBlocks = function (blocks) { // delete the old entry in object.blocks and replace it w/the // deserialized object delete blocks[blockId]; - deserializeInputDesc(block, null, false, blocks); + oldToNew[blockId] = deserializeInputDesc(block, null, false, blocks); continue; } block.id = blockId; // add id back to block since it wasn't serialized block.inputs = deserializeInputs(block.inputs, blockId, blocks); block.fields = deserializeFields(block.fields); } - return blocks; + return ({blocks, oldToNew}); }; @@ -930,12 +932,15 @@ const parseScratchObject = function (object, runtime, extensions, zip, assets) { // @todo: For now, load all Scratch objects (stage/sprites) as a Sprite. const sprite = new Sprite(blocks, runtime); + // Used later when parsing comments + let oldToNew = {}; + // Sprite/stage name from JSON. if (object.hasOwnProperty('name')) { sprite.name = object.name; } if (object.hasOwnProperty('blocks')) { - deserializeBlocks(object.blocks); + oldToNew = deserializeBlocks(object.blocks).oldToNew; // Take a second pass to create objects and add extensions for (const blockId in object.blocks) { if (!object.blocks.hasOwnProperty(blockId)) continue; @@ -1031,7 +1036,22 @@ const parseScratchObject = function (object, runtime, extensions, zip, assets) { comment.minimized ); if (comment.blockId) { - newComment.blockId = comment.blockId; + if (oldToNew.hasOwnProperty(comment.blockId)) { + // ID for the block is changed during deserialization + newComment.blockId = oldToNew[comment.blockId]; + } else { + newComment.blockId = comment.blockId; + } + // does the block really exist? if so, add reference to the comment + const block = target.blocks.getBlock(newComment.blockId); + if (block) { + block.comment = commentId; + } else { + // LLK/scratch-vm#2501 + log.warn(`${commentId} is attached to a block with ID ${newComment.blockId + }, but the block does not exist.`); + continue; + } } target.comments[newComment.id] = newComment; } diff --git a/test/unit/serialization_sb3.js b/test/unit/serialization_sb3.js index 5ef1ac80df..fb9bc94d8b 100644 --- a/test/unit/serialization_sb3.js +++ b/test/unit/serialization_sb3.js @@ -259,7 +259,7 @@ test('deserializeBlocks', t => { .then(() => { const blocks = vm.runtime.targets[1].blocks._blocks; const serialized = sb3.serializeBlocks(blocks)[0]; - const deserialized = sb3.deserializeBlocks(serialized); + const deserialized = sb3.deserializeBlocks(serialized).blocks; t.equal(Object.keys(deserialized).length, Object.keys(blocks).length, 'same number of blocks'); t.end(); }); @@ -271,8 +271,8 @@ test('deserializeBlocks on already deserialized input', t => { .then(() => { const blocks = vm.runtime.targets[1].blocks._blocks; const serialized = sb3.serializeBlocks(blocks)[0]; - const deserialized = sb3.deserializeBlocks(serialized); - const deserializedAgain = sb3.deserializeBlocks(deserialized); + const deserialized = sb3.deserializeBlocks(serialized).blocks; + const deserializedAgain = sb3.deserializeBlocks(deserialized).blocks; t.deepEqual(deserialized, deserializedAgain, 'no change from second pass of deserialize'); t.end(); });