diff --git a/src/serialization/sb2.js b/src/serialization/sb2.js index ffde8220a..460921a5f 100644 --- a/src/serialization/sb2.js +++ b/src/serialization/sb2.js @@ -137,7 +137,7 @@ const parseBlockList = function (blockList, addBroadcastMsg, getVariableId, exte // Update commentIndex commentIndex = parsedBlockAndComments[1]; - if (typeof parsedBlock === 'undefined') continue; + if (!parsedBlock) continue; if (previousBlock) { parsedBlock.parent = previousBlock.id; previousBlock.next = parsedBlock.id; @@ -706,9 +706,20 @@ const specMapBlock = function (block) { * and second item is the updated comment index (after this block and its children are parsed) */ const parseBlock = function (sb2block, addBroadcastMsg, getVariableId, extensions, comments, commentIndex) { + const commentsForParsedBlock = (comments && typeof commentIndex === 'number' && !isNaN(commentIndex)) ? + comments[commentIndex] : null; const blockMetadata = specMapBlock(sb2block); if (!blockMetadata) { - return; + // No block opcode found, exclude this block, increment the commentIndex, + // make all block comments into workspace comments and send them to zero/zero + // to prevent serialization issues. + if (commentsForParsedBlock) { + commentsForParsedBlock.forEach(comment => { + comment.blockId = null; + comment.x = comment.y = 0; + }); + } + return [null, commentIndex + 1]; } const oldOpcode = sb2block[0]; @@ -731,15 +742,18 @@ const parseBlock = function (sb2block, addBroadcastMsg, getVariableId, extension }; // Attach any comments to this block.. - const commentsForParsedBlock = (comments && typeof commentIndex === 'number' && !isNaN(commentIndex)) ? - comments[commentIndex] : null; if (commentsForParsedBlock) { - // TODO currently only attaching the last comment to the block if there are multiple... - // not sure what to do here.. concatenate all the messages in all the comments and only - // keep one around? + // Attach only the last comment to the block, make all others workspace comments activeBlock.comment = commentsForParsedBlock[commentsForParsedBlock.length - 1].id; commentsForParsedBlock.forEach(comment => { - comment.blockId = activeBlock.id; + if (comment.id === activeBlock.comment) { + comment.blockId = activeBlock.id; + } else { + // All other comments don't get a block ID and are sent back to zero. + // This is important, because if they have `null` x/y, serialization breaks. + comment.blockId = null; + comment.x = comment.y = 0; + } }); } commentIndex++; diff --git a/test/fixtures/unknown-opcode.sb2 b/test/fixtures/unknown-opcode.sb2 new file mode 100644 index 000000000..cc3341aa5 Binary files /dev/null and b/test/fixtures/unknown-opcode.sb2 differ diff --git a/test/integration/unknown-opcode.js b/test/integration/unknown-opcode.js new file mode 100644 index 000000000..53b9b64f9 --- /dev/null +++ b/test/integration/unknown-opcode.js @@ -0,0 +1,55 @@ +const path = require('path'); +const test = require('tap').test; +const makeTestStorage = require('../fixtures/make-test-storage'); +const readFileToBuffer = require('../fixtures/readProjectFile').readFileToBuffer; +const VirtualMachine = require('../../src/index'); + +const uri = path.resolve(__dirname, '../fixtures/unknown-opcode.sb2'); +const project = readFileToBuffer(uri); + +test('unknown opcode', t => { + const vm = new VirtualMachine(); + vm.attachStorage(makeTestStorage()); + + vm.start(); + vm.clear(); + vm.setCompatibilityMode(false); + vm.setTurboMode(false); + vm.loadProject(project).then(() => { + vm.greenFlag(); + + // The project has 3 blocks in a single stack: + // play sound => "undefined" => play sound + // the "undefined" block has opcode "0" and was found in the wild. + // It should be parsed in without error and it should bridge together + // the two play sound blocks, so there should not be a third block. + const blocks = vm.runtime.targets[0].blocks; + const topBlockId = blocks.getScripts()[0]; + const secondBlockId = blocks.getNextBlock(topBlockId); + const thirdBlockId = blocks.getNextBlock(secondBlockId); + + t.equal(blocks.getBlock(topBlockId).opcode, 'sound_play'); + t.equal(blocks.getBlock(secondBlockId).opcode, 'sound_play'); + t.equal(thirdBlockId, null); + + const target = vm.runtime.targets[0]; + const topCommentId = blocks.getBlock(topBlockId).comment; + const secondCommentId = blocks.getBlock(secondBlockId).comment; + + t.equal(target.comments[topCommentId].text, 'pop1 comment'); + t.equal(target.comments[secondCommentId].text, 'pop2 comment'); + + // The comment previously attached to the undefined block should become + // a workspace comment, at 0/0, with the same text as it had. + const undefinedCommentId = Object.keys(target.comments).filter(id => + id !== topCommentId && id !== secondCommentId)[0]; + const undefinedComment = target.comments[undefinedCommentId]; + t.equal(undefinedComment.blockId, null); + t.equal(undefinedComment.text, 'undefined comment'); + t.equal(undefinedComment.x, 0); + t.equal(undefinedComment.y, 0); + + t.end(); + process.nextTick(process.exit); + }); +});