diff --git a/src/engine/blocks.js b/src/engine/blocks.js
index 5a0bbd965..b70c564c0 100644
--- a/src/engine/blocks.js
+++ b/src/engine/blocks.js
@@ -964,6 +964,14 @@ class Blocks {
}
}
+ // Delete comments attached to the block.
+ if (block.comment) {
+ const editingTarget = this.runtime.getEditingTarget();
+ if (editingTarget && editingTarget.comments.hasOwnProperty(block.comment)) {
+ delete editingTarget.comments[block.comment];
+ }
+ }
+
// Delete any script starting with this block.
this._deleteScript(blockId);
diff --git a/src/engine/target.js b/src/engine/target.js
index 166e44369..01cce1d47 100644
--- a/src/engine/target.js
+++ b/src/engine/target.js
@@ -20,9 +20,10 @@ class Target extends EventEmitter {
/**
* @param {Runtime} runtime Reference to the runtime.
* @param {?Blocks} blocks Blocks instance for the blocks owned by this target.
+ * @param {?Object.<string, Comment>} comments Array of comments owned by this target.
* @constructor
*/
- constructor (runtime, blocks) {
+ constructor (runtime, blocks, comments) {
super();
if (!blocks) {
@@ -55,7 +56,7 @@ class Target extends EventEmitter {
* Key is the comment id.
* @type {Object.<string,*>}
*/
- this.comments = {};
+ this.comments = comments || {};
/**
* Dictionary of custom state for this target.
* This can be used to store target-specific custom state for blocks which need it.
diff --git a/src/sprites/rendered-target.js b/src/sprites/rendered-target.js
index e4a12e244..d3230cce1 100644
--- a/src/sprites/rendered-target.js
+++ b/src/sprites/rendered-target.js
@@ -15,7 +15,7 @@ class RenderedTarget extends Target {
* @constructor
*/
constructor (sprite, runtime) {
- super(runtime, sprite.blocks);
+ super(runtime, sprite.blocks, sprite.comments);
/**
* Reference to the sprite that this is a render of.
diff --git a/src/sprites/sprite.js b/src/sprites/sprite.js
index a38fa129c..bb6d920f3 100644
--- a/src/sprites/sprite.js
+++ b/src/sprites/sprite.js
@@ -1,10 +1,12 @@
const RenderedTarget = require('./rendered-target');
const Blocks = require('../engine/blocks');
+const Comment = require('../engine/comment');
const {loadSoundFromAsset} = require('../import/load-sound');
const {loadCostumeFromAsset} = require('../import/load-costume');
const newBlockIds = require('../util/new-block-ids');
const StringUtil = require('../util/string-util');
const StageLayering = require('../engine/stage-layering');
+const log = require('../util/log');
class Sprite {
/**
@@ -54,6 +56,13 @@ class Sprite {
if (this.runtime && this.runtime.audioEngine) {
this.soundBank = this.runtime.audioEngine.createBank();
}
+
+ /**
+ * Dictionary of comments for this target.
+ * Key is the comment id.
+ * @type {Object.<string, Comment>}
+ */
+ this.comments = {};
}
/**
@@ -140,11 +149,48 @@ class Sprite {
const blocksContainer = this.blocks._blocks;
const originalBlocks = Object.keys(blocksContainer).map(key => blocksContainer[key]);
const copiedBlocks = JSON.parse(JSON.stringify(originalBlocks));
- newBlockIds(copiedBlocks);
+ const oldToNew = newBlockIds(copiedBlocks);
copiedBlocks.forEach(block => {
newSprite.blocks.createBlock(block);
});
+ Object.keys(this.comments).forEach(commentId => {
+ const comment = this.comments[commentId];
+ const newComment = new Comment(
+ null, // generate new comment id
+ comment.text,
+ comment.x,
+ comment.y,
+ comment.width,
+ comment.height,
+ comment.minimized
+ );
+ // If the comment is attached to a block, it has a blockId property for the block it's attached to.
+ // Because we generate new block IDs for all the duplicated blocks, change all the comments' attached-block
+ // IDs from the old block IDs to the new ones.
+ if (comment.blockId) {
+ const newBlockId = oldToNew[comment.blockId];
+ if (newBlockId) {
+ newComment.blockId = newBlockId;
+ const newBlock = newSprite.blocks.getBlock(newBlockId);
+ if (newBlock) {
+ newBlock.comment = newComment.id;
+ } else {
+ log.warn(`Could not find block with id ${newBlockId} associated with comment ${newComment.id}`);
+ }
+ } else {
+ // Comments did not get deleted when the block got deleted
+ // Such comments have blockId, but because the block is deleted
+ // oldToNew mapping does not include that blockId
+ // Handle it as workspace comment
+ // TODO do not load such comments when deserializing
+ log.warn(`Could not find block with id ${comment.blockId
+ } associated with comment ${comment.id} in the block ID mapping`);
+ }
+ }
+ newSprite.comments[newComment.id] = newComment;
+ });
+
const allNames = this.runtime.targets.map(t => t.sprite.name);
newSprite.name = StringUtil.unusedName(this.name, allNames);
diff --git a/src/util/new-block-ids.js b/src/util/new-block-ids.js
index 1fc5f6967..a167e7f48 100644
--- a/src/util/new-block-ids.js
+++ b/src/util/new-block-ids.js
@@ -4,6 +4,7 @@ const uid = require('./uid');
* Mutate the given blocks to have new IDs and update all internal ID references.
* Does not return anything to make it clear that the blocks are updated in-place.
* @param {array} blocks - blocks to be mutated.
+ * @returns {object.<string, string>} - mapping of old block ID to new block ID
*/
module.exports = blocks => {
const oldToNew = {};
@@ -30,4 +31,7 @@ module.exports = blocks => {
blocks[i].next = oldToNew[blocks[i].next];
}
}
+
+ // There are other things that need this mapping e.g. comments
+ return oldToNew;
};
diff --git a/test/unit/engine_blocks.js b/test/unit/engine_blocks.js
index f4451aa39..80b99b06d 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 a9823424a..ff5e1fa69 100644
--- a/test/unit/sprites_rendered-target.js
+++ b/test/unit/sprites_rendered-target.js
@@ -45,6 +45,21 @@ 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 => {
+ // Not ok because comment id should be re-generated
+ 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);