diff --git a/src/engine/blocks.js b/src/engine/blocks.js index f8d8da0c138..8dd4df26333 100644 --- a/src/engine/blocks.js +++ b/src/engine/blocks.js @@ -824,7 +824,7 @@ class Blocks { // Delete comments attached to the block. if (block.comment) { const editingTarget = this.runtime.getEditingTarget(); - if (editingTarget.comments.hasOwnProperty(block.comment)) { + if (editingTarget && editingTarget.comments.hasOwnProperty(block.comment)) { delete editingTarget.comments[block.comment]; } } diff --git a/src/serialization/sb3.js b/src/serialization/sb3.js index e309880371f..820e987fe1a 100644 --- a/src/serialization/sb3.js +++ b/src/serialization/sb3.js @@ -798,7 +798,7 @@ const deserializeFields = function (fields) { * work with pre-parsed deserialized blocks. * * @param {object} blocks Serialized SB3 "blocks" property of a target. Will be mutated. - * @return {object} input is modified and returned + * @return {object} object that has "blocks" and "oldToNew" attributes */ const deserializeBlocks = function (blocks) { // oldToNew is used to fix comments attached to variable blocks: see #1893 diff --git a/test/fixtures/variable-comment.sb3 b/test/fixtures/variable-comment.sb3 new file mode 100644 index 00000000000..18ef05f21b1 Binary files /dev/null and b/test/fixtures/variable-comment.sb3 differ diff --git a/test/integration/variable-comment.js b/test/integration/variable-comment.js new file mode 100644 index 00000000000..0c352056547 --- /dev/null +++ b/test/integration/variable-comment.js @@ -0,0 +1,53 @@ +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 projectUri = path.resolve(__dirname, '../fixtures/variable-comment.sb3'); +const project = readFileToBuffer(projectUri); + +test('load an sb3 project with comments attached to variable blocks (#1893)', t => { + const vm = new VirtualMachine(); + vm.attachStorage(makeTestStorage()); + + // Evaluate playground data and exit + vm.on('playgroundData', e => { + const threads = JSON.parse(e.threads); + t.equal(threads.length, 0); + + const stage = vm.runtime.targets[0]; + const stageComments = Object.values(stage.comments); + + // Stage has 2 comments + t.equal(stageComments.length, 2); + + // comment1 is atached to variable block + t.equal(stageComments[0].text, 'comment1'); + const variableBlock = stage.blocks.getBlock(stageComments[0].blockId); + t.equal(variableBlock.opcode, 'data_variable'); + + // comment2 is atached to variable block + t.equal(stageComments[1].text, 'comment2'); + const listBlock = stage.blocks.getBlock(stageComments[1].blockId); + t.equal(listBlock.opcode, 'data_listcontents'); + + t.end(); + process.nextTick(process.exit); + }); + + // Start VM, load project, and run + t.doesNotThrow(() => { + vm.start(); + vm.clear(); + vm.setCompatibilityMode(false); + vm.setTurboMode(false); + vm.loadProject(project).then(() => { + vm.greenFlag(); + setTimeout(() => { + vm.getPlaygroundData(); + vm.stopAll(); + }, 100); + }); + }); +}); diff --git a/test/unit/engine_blocks.js b/test/unit/engine_blocks.js index f4451aa39fb..80b99b06dea 100644 --- a/test/unit/engine_blocks.js +++ b/test/unit/engine_blocks.js @@ -600,6 +600,25 @@ test('delete inputs', t => { t.end(); }); +test('delete block with comment', t => { + const b = new Blocks(new Runtime()); + const fakeTarget = { + comments: { + bar: { + blockId: 'foo' + } + } + }; + b.runtime.getEditingTarget = () => fakeTarget; + b.createBlock({ + id: 'foo', + comment: 'bar' + }); + b.deleteBlock('foo'); + t.notOk(fakeTarget.comments.hasOwnProperty('bar')); + t.end(); +}); + test('updateAssetName function updates name in sound field', t => { const b = new Blocks(new Runtime()); b.createBlock({ diff --git a/test/unit/sprites_rendered-target.js b/test/unit/sprites_rendered-target.js index 681a10d86db..da4eb7dc6ef 100644 --- a/test/unit/sprites_rendered-target.js +++ b/test/unit/sprites_rendered-target.js @@ -45,6 +45,19 @@ test('blocks get new id on duplicate', t => { }); }); +test('comments are duplicated when duplicating target', t => { + const r = new Runtime(); + const s = new Sprite(null, r); + const rt = new RenderedTarget(s, r); + rt.createComment('commentid', null, 'testcomment', 0, 0, 100, 100, false); + t.ok(s.comments.hasOwnProperty('commentid')); + return rt.duplicate().then(duplicate => { + t.notOk(duplicate.comments.hasOwnProperty('commentid')); + t.ok(Object.keys(duplicate.comments).length === 1); + t.end(); + }); +}); + test('direction', t => { const r = new Runtime(); const s = new Sprite(null, r);