Merge pull request #1241 from kchadha/comment-serialization-dedupe

Clean up comment serialization

src/serialization/sb3.js

@@ -395,7 +395,6 @@ const serializeComments = function (comments) {
         const comment = comments[commentId];
 
         const serializedComment = Object.create(null);
-        serializedComment.id = comment.id;
         serializedComment.blockId = comment.blockId;
         serializedComment.x = comment.x;
         serializedComment.y = comment.y;
@@ -864,7 +863,7 @@ const parseScratchObject = function (object, runtime, extensions, zip) {
         for (const commentId in object.comments) {
             const comment = object.comments[commentId];
             const newComment = new Comment(
-                comment.id,
+                commentId,
                 comment.text,
                 comment.x,
                 comment.y,

test/unit/serialization_sb3.js

@@ -5,6 +5,8 @@ const sb3 = require('../../src/serialization/sb3');
 const readFileToBuffer = require('../fixtures/readProjectFile').readFileToBuffer;
 const exampleProjectPath = path.resolve(__dirname, '../fixtures/clone-cleanup.sb2');
 const commentsSB2ProjectPath = path.resolve(__dirname, '../fixtures/comments.sb2');
+const commentsSB3ProjectPath = path.resolve(__dirname, '../fixtures/comments.sb3');
+const commentsSB3NoDupeIds = path.resolve(__dirname, '../fixtures/comments_no_duplicate_id_serialization.sb3');
 
 test('serialize', t => {
     const vm = new VirtualMachine();
@@ -66,3 +68,77 @@ test('serialize sb2 project with comments as sb3', t => {
             t.end();
         });
 });
+
+test('deserialize sb3 project with comments', t => {
+    const vm = new VirtualMachine();
+    vm.loadProject(readFileToBuffer(commentsSB3ProjectPath))
+        .then(() => {
+            const runtime = vm.runtime;
+
+            t.type(runtime.targets, 'object');
+            t.equal(Array.isArray(runtime.targets), true);
+            t.equal(runtime.targets.length, 2);
+
+            const stage = runtime.targets[0];
+            t.equal(stage.isStage, true);
+            // The stage has 0 blocks, and 1 workspace comment
+            t.type(stage.blocks, 'object');
+            t.equal(Object.keys(stage.blocks._blocks).length, 0);
+            t.type(stage.comments, 'object');
+            t.equal(Object.keys(stage.comments).length, 1);
+            const stageBlockComments = Object.values(stage.comments).filter(comment => !!comment.blockId);
+            const stageWorkspaceComments = Object.values(stage.comments).filter(comment => comment.blockId === null);
+            t.equal(stageBlockComments.length, 0);
+            t.equal(stageWorkspaceComments.length, 1);
+
+            const sprite = runtime.targets[1];
+            t.equal(sprite.isStage, false);
+            t.type(sprite.blocks, 'object');
+            // Sprite 1 has 6 blocks, 5 block comments, and 1 workspace comment
+            t.equal(Object.values(sprite.blocks._blocks).filter(block => !block.shadow).length, 6);
+            t.type(sprite.comments, 'object');
+            t.equal(Object.keys(sprite.comments).length, 6);
+
+            const spriteBlockComments = Object.values(sprite.comments).filter(comment => !!comment.blockId);
+            const spriteWorkspaceComments = Object.values(sprite.comments).filter(comment => comment.blockId === null);
+            t.equal(spriteBlockComments.length, 5);
+            t.equal(spriteWorkspaceComments.length, 1);
+
+            t.end();
+        });
+});
+
+test('deserialize sb3 project with comments - no duplicate id serialization', t => {
+    const vm = new VirtualMachine();
+    vm.loadProject(readFileToBuffer(commentsSB3NoDupeIds))
+        .then(() => {
+            const runtime = vm.runtime;
+
+            t.type(runtime.targets, 'object');
+            t.equal(Array.isArray(runtime.targets), true);
+            t.equal(runtime.targets.length, 2);
+
+            const stage = runtime.targets[0];
+            t.equal(stage.isStage, true);
+            // The stage has 0 blocks, and 0 workspace comment
+            t.type(stage.blocks, 'object');
+            t.equal(Object.keys(stage.blocks._blocks).length, 0);
+            t.type(stage.comments, 'object');
+            t.equal(Object.keys(stage.comments).length, 0);
+
+            const sprite = runtime.targets[1];
+            t.equal(sprite.isStage, false);
+            t.type(sprite.blocks, 'object');
+            // Sprite1 has 1 blocks, 1 block comment, and 1 workspace comment
+            t.equal(Object.values(sprite.blocks._blocks).filter(block => !block.shadow).length, 1);
+            t.type(sprite.comments, 'object');
+            t.equal(Object.keys(sprite.comments).length, 2);
+
+            const spriteBlockComments = Object.values(sprite.comments).filter(comment => !!comment.blockId);
+            const spriteWorkspaceComments = Object.values(sprite.comments).filter(comment => comment.blockId === null);
+            t.equal(spriteBlockComments.length, 1);
+            t.equal(spriteWorkspaceComments.length, 1);
+
+            t.end();
+        });
+});