diff --git a/src/blocks/scratch3_looks.js b/src/blocks/scratch3_looks.js index 8c17bb1b0..6b9059c09 100644 --- a/src/blocks/scratch3_looks.js +++ b/src/blocks/scratch3_looks.js @@ -332,7 +332,7 @@ class Scratch3LooksBlocks { } if (target === this.runtime.getTargetForStage()) { // Target is the stage - start hats. - const newName = target.sprite.costumes[target.currentCostume].name; + const newName = target.getCostumes()[target.currentCostume].name; return this.runtime.startHats('event_whenbackdropswitchesto', { BACKDROP: newName }); @@ -442,7 +442,7 @@ class Scratch3LooksBlocks { return stage.currentCostume + 1; } // Else return name - return stage.sprite.costumes[stage.currentCostume].name; + return stage.getCostumes()[stage.currentCostume].name; } getCostumeNumberName (args, util) { @@ -450,7 +450,7 @@ class Scratch3LooksBlocks { return util.target.currentCostume + 1; } // Else return name - return util.target.sprite.costumes[util.target.currentCostume].name; + return util.target.getCostumes()[util.target.currentCostume].name; } } diff --git a/src/blocks/scratch3_sensing.js b/src/blocks/scratch3_sensing.js index f7a39b6b9..602452669 100644 --- a/src/blocks/scratch3_sensing.js +++ b/src/blocks/scratch3_sensing.js @@ -237,7 +237,7 @@ class Scratch3SensingBlocks { case 'backdrop #': return attrTarget.currentCostume + 1; case 'backdrop name': - return attrTarget.sprite.costumes[attrTarget.currentCostume].name; + return attrTarget.getCostumes()[attrTarget.currentCostume].name; case 'volume': return; // @todo: Keep this in mind for sound blocks! } } else { @@ -247,7 +247,7 @@ class Scratch3SensingBlocks { case 'direction': return attrTarget.direction; case 'costume #': return attrTarget.currentCostume + 1; case 'costume name': - return attrTarget.sprite.costumes[attrTarget.currentCostume].name; + return attrTarget.getCostumes()[attrTarget.currentCostume].name; case 'size': return attrTarget.size; case 'volume': return; // @todo: above, keep in mind for sound blocks.. } diff --git a/src/sprites/rendered-target.js b/src/sprites/rendered-target.js index 538cc576f..8da2d2b5a 100644 --- a/src/sprites/rendered-target.js +++ b/src/sprites/rendered-target.js @@ -381,7 +381,7 @@ class RenderedTarget extends Target { index, 0, this.sprite.costumes.length - 1 ); if (this.renderer) { - const costume = this.sprite.costumes[this.currentCostume]; + const costume = this.getCostumes()[this.currentCostume]; const drawableProperties = { skinId: costume.skinId, costumeResolution: costume.bitmapResolution @@ -410,12 +410,10 @@ class RenderedTarget extends Target { * @param {?int} index Index at which to add costume */ addCostume (costumeObject, index) { - const usedNames = this.sprite.costumes.map(costume => costume.name); - costumeObject.name = StringUtil.unusedName(costumeObject.name, usedNames); if (index) { - this.sprite.costumes.splice(index, 0, costumeObject); + this.sprite.addCostumeAt(costumeObject, index); } else { - this.sprite.costumes.push(costumeObject); + this.sprite.addCostumeAt(costumeObject, this.sprite.costumes.length); } } @@ -428,9 +426,9 @@ class RenderedTarget extends Target { const usedNames = this.sprite.costumes .filter((costume, index) => costumeIndex !== index) .map(costume => costume.name); - const oldName = this.sprite.costumes[costumeIndex].name; + const oldName = this.getCostumes()[costumeIndex].name; const newUnusedName = StringUtil.unusedName(newName, usedNames); - this.sprite.costumes[costumeIndex].name = newUnusedName; + this.getCostumes()[costumeIndex].name = newUnusedName; if (this.isStage) { // Since this is a backdrop, go through all targets and @@ -454,9 +452,7 @@ class RenderedTarget extends Target { const originalCostumeCount = this.sprite.costumes.length; if (originalCostumeCount === 1) return; - this.sprite.costumes = this.sprite.costumes - .slice(0, index) - .concat(this.sprite.costumes.slice(index + 1)); + this.sprite.deleteCostumeAt(index); if (index === this.currentCostume && index === originalCostumeCount - 1) { this.setCostume(index - 1); @@ -542,7 +538,7 @@ class RenderedTarget extends Target { */ getCostumeIndexByName (costumeName) { for (let i = 0; i < this.sprite.costumes.length; i++) { - if (this.sprite.costumes[i].name === costumeName) { + if (this.getCostumes()[i].name === costumeName) { return i; } } @@ -554,7 +550,7 @@ class RenderedTarget extends Target { * @return {object} current costume */ getCurrentCostume () { - return this.sprite.costumes[this.currentCostume]; + return this.getCostumes()[this.currentCostume]; } /** @@ -580,7 +576,7 @@ class RenderedTarget extends Target { updateAllDrawableProperties () { if (this.renderer) { const renderedDirectionScale = this._getRenderedDirectionAndScale(); - const costume = this.sprite.costumes[this.currentCostume]; + const costume = this.getCostumes()[this.currentCostume]; const bitmapResolution = costume.bitmapResolution || 1; const props = { position: [this.x, this.y], diff --git a/src/sprites/sprite.js b/src/sprites/sprite.js index 941b20403..3e3aca60e 100644 --- a/src/sprites/sprite.js +++ b/src/sprites/sprite.js @@ -36,7 +36,7 @@ class Sprite { * } * @type {Array.} */ - this.costumes = []; + this.costumes_ = []; /** * List of sounds for this sprite. */ @@ -47,6 +47,51 @@ class Sprite { */ this.clones = []; } + + /** + * Add an array of costumes, taking care to avoid duplicate names. + * @param {!Array} costumes Array of objects representing costumes. + */ + set costumes (costumes) { + this.costumes_ = []; + for (const costume of costumes) { + this.addCostumeAt(costume, this.costumes_.length); + } + } + + /** + * Get full costume list + * @return {object[]} list of costumes. Note that mutating the returned list will not + * mutate the list on the sprite. The sprite list should be mutated by calling + * addCostumeAt, deleteCostumeAt, or setting costumes. + */ + get costumes () { + return this.costumes_.slice(0); + } + + /** + * Add a costume at the given index, taking care to avoid duplicate names. + * @param {!object} costumeObject Object representing the costume. + * @param {!int} index Index at which to add costume + */ + addCostumeAt (costumeObject, index) { + if (!costumeObject.name) { + costumeObject.name = ''; + } + const usedNames = this.costumes_.map(costume => costume.name); + costumeObject.name = StringUtil.unusedName(costumeObject.name, usedNames); + this.costumes_.splice(index, 0, costumeObject); + } + + /** + * Delete a costume by index. + * @param {number} index Costume index to be deleted + */ + deleteCostumeAt (index) { + this.costumes_ = this.costumes_ + .slice(0, index) + .concat(this.costumes_.slice(index + 1)); + } /** * Create a clone of this sprite. @@ -89,7 +134,7 @@ class Sprite { const assetPromises = []; - newSprite.costumes = this.costumes.map(costume => { + newSprite.costumes = this.costumes_.map(costume => { const newCostume = Object.assign({}, costume); const costumeAsset = this.runtime.storage.get(costume.assetId); assetPromises.push(loadCostumeFromAsset(newCostume, costumeAsset, this.runtime)); diff --git a/src/virtual-machine.js b/src/virtual-machine.js index 3d3c16791..a0063d5a5 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -465,7 +465,7 @@ class VirtualMachine extends EventEmitter { * @return {string} the costume's SVG string, or null if it's not an SVG costume. */ getCostumeSvg (costumeIndex) { - const id = this.editingTarget.sprite.costumes[costumeIndex].assetId; + const id = this.editingTarget.getCostumes()[costumeIndex].assetId; if (id && this.runtime && this.runtime.storage && this.runtime.storage.get(id).dataFormat === 'svg') { return this.runtime.storage.get(id).decodeText(); @@ -481,7 +481,7 @@ class VirtualMachine extends EventEmitter { * @param {number} rotationCenterY y of point about which the costume rotates, relative to its upper left corner */ updateSvg (costumeIndex, svg, rotationCenterX, rotationCenterY) { - const costume = this.editingTarget.sprite.costumes[costumeIndex]; + const costume = this.editingTarget.getCostumes()[costumeIndex]; if (costume && this.runtime && this.runtime.renderer) { costume.rotationCenterX = rotationCenterX; costume.rotationCenterY = rotationCenterY; @@ -510,7 +510,7 @@ class VirtualMachine extends EventEmitter { return loadCostume(md5ext, backdropObject, this.runtime).then(() => { const stage = this.runtime.getTargetForStage(); stage.addCostume(backdropObject); - stage.setCostume(stage.sprite.costumes.length - 1); + stage.setCostume(stage.getCostumes().length - 1); }); } diff --git a/test/unit/blocks_looks.js b/test/unit/blocks_looks.js index 6c1877c8a..c7b4c3d7b 100644 --- a/test/unit/blocks_looks.js +++ b/test/unit/blocks_looks.js @@ -4,6 +4,9 @@ const Runtime = require('../../src/engine/runtime'); const util = { target: { currentCostume: 0, // Internally, current costume is 0 indexed + getCostumes: function () { + return this.sprite.costumes; + }, sprite: { costumes: [ {name: 'first name'}, diff --git a/test/unit/sprites_rendered-target.js b/test/unit/sprites_rendered-target.js index 332a78111..51c670dc9 100644 --- a/test/unit/sprites_rendered-target.js +++ b/test/unit/sprites_rendered-target.js @@ -117,7 +117,9 @@ test('deleteCostume', t => { // Costume 3 a.setCostume(0); a.deleteCostume(0); - t.deepEqual(a.sprite.costumes, [o2, o3]); + t.equals(a.sprite.costumes.length, 2); + t.equals(a.sprite.costumes[0].id, 2); + t.equals(a.sprite.costumes[1].id, 3); t.equals(a.currentCostume, 0); // Costume 1 Costume 1 @@ -126,7 +128,9 @@ test('deleteCostume', t => { a.sprite.costumes = [o1, o2, o3]; a.setCostume(1); a.deleteCostume(1); - t.deepEqual(a.sprite.costumes, [o1, o3]); + t.equals(a.sprite.costumes.length, 2); + t.equals(a.sprite.costumes[0].id, 1); + t.equals(a.sprite.costumes[1].id, 3); t.equals(a.currentCostume, 1); // Costume 1 Costume 1 @@ -135,14 +139,17 @@ test('deleteCostume', t => { a.sprite.costumes = [o1, o2, o3]; a.setCostume(2); a.deleteCostume(2); - t.deepEqual(a.sprite.costumes, [o1, o2]); + t.equals(a.sprite.costumes.length, 2); + t.equals(a.sprite.costumes[0].id, 1); + t.equals(a.sprite.costumes[1].id, 2); t.equals(a.currentCostume, 1); // Refuses to delete only costume a.sprite.costumes = [o1]; a.setCostume(0); a.deleteCostume(0); - t.deepEqual(a.sprite.costumes, [o1]); + t.equals(a.sprite.costumes.length, 1); + t.equals(a.sprite.costumes[0].id, 1); t.equals(a.currentCostume, 0); // Costume 1 Costume 1 @@ -153,7 +160,11 @@ test('deleteCostume', t => { a.sprite.costumes = [o1, o2, o3, o4, o5]; a.setCostume(3); a.deleteCostume(1); - t.deepEqual(a.sprite.costumes, [o1, o3, o4, o5]); + t.equals(a.sprite.costumes.length, 4); + t.equals(a.sprite.costumes[0].id, 1); + t.equals(a.sprite.costumes[1].id, 3); + t.equals(a.sprite.costumes[2].id, 4); + t.equals(a.sprite.costumes[3].id, 5); t.equals(a.currentCostume, 2); // Costume 1 Costume 1 @@ -164,7 +175,11 @@ test('deleteCostume', t => { a.sprite.costumes = [o1, o2, o3, o4, o5]; a.setCostume(1); a.deleteCostume(3); - t.deepEqual(a.sprite.costumes, [o1, o2, o3, o5]); + t.equals(a.sprite.costumes.length, 4); + t.equals(a.sprite.costumes[0].id, 1); + t.equals(a.sprite.costumes[1].id, 2); + t.equals(a.sprite.costumes[2].id, 3); + t.equals(a.sprite.costumes[3].id, 5); t.equals(a.currentCostume, 1); // Costume 1 Costume 1 @@ -175,7 +190,11 @@ test('deleteCostume', t => { a.sprite.costumes = [o1, o2, o3, o4, o5]; a.setCostume(1); a.deleteCostume(4); - t.deepEqual(a.sprite.costumes, [o1, o2, o3, o4]); + t.equals(a.sprite.costumes.length, 4); + t.equals(a.sprite.costumes[0].id, 1); + t.equals(a.sprite.costumes[1].id, 2); + t.equals(a.sprite.costumes[2].id, 3); + t.equals(a.sprite.costumes[3].id, 4); t.equals(a.currentCostume, 1); t.end(); }); @@ -333,9 +352,11 @@ test('#stopAll clears graphics effects', t => { test('#getCostumes returns the costumes', t => { const spr = new Sprite(); const a = new RenderedTarget(spr, null); - const costumes = [1, 2, 3]; - a.sprite.costumes = costumes; - t.equals(a.getCostumes(), costumes); + a.sprite.costumes = [{id: 1}, {id: 2}, {id: 3}]; + t.equals(a.getCostumes().length, 3); + t.equals(a.getCostumes()[0].id, 1); + t.equals(a.getCostumes()[1].id, 2); + t.equals(a.getCostumes()[2].id, 3); t.end(); }); @@ -352,11 +373,10 @@ test('#toJSON returns the sounds and costumes', t => { const spr = new Sprite(); const a = new RenderedTarget(spr, null); const sounds = [1, 2, 3]; - const costumes = ['a', 'b', 'c']; a.sprite.sounds = sounds; - a.sprite.costumes = costumes; + a.sprite.costumes = [{id: 1}, {id: 2}, {id: 3}]; t.same(a.toJSON().sounds, sounds); - t.same(a.toJSON().costumes, costumes); + t.same(a.toJSON().costumes, a.sprite.costumes); t.end(); }); @@ -372,9 +392,11 @@ test('#addSound does not duplicate names', t => { test('#addCostume does not duplicate names', t => { const spr = new Sprite(); const a = new RenderedTarget(spr, null); - a.sprite.costumes = [{name: 'first'}]; a.addCostume({name: 'first'}); - t.deepEqual(a.sprite.costumes, [{name: 'first'}, {name: 'first2'}]); + a.addCostume({name: 'first'}); + t.equal(a.sprite.costumes.length, 2); + t.equal(a.sprite.costumes[0].name, 'first'); + t.equal(a.sprite.costumes[1].name, 'first2'); t.end(); }); @@ -394,8 +416,12 @@ test('#renameCostume does not duplicate names', t => { const a = new RenderedTarget(spr, null); a.sprite.costumes = [{name: 'first'}, {name: 'second'}]; a.renameCostume(0, 'first'); // Shouldn't increment the name, noop - t.deepEqual(a.sprite.costumes, [{name: 'first'}, {name: 'second'}]); + t.equal(a.sprite.costumes.length, 2); + t.equal(a.sprite.costumes[0].name, 'first'); + t.equal(a.sprite.costumes[1].name, 'second'); a.renameCostume(1, 'first'); - t.deepEqual(a.sprite.costumes, [{name: 'first'}, {name: 'first2'}]); + t.equal(a.sprite.costumes.length, 2); + t.equal(a.sprite.costumes[0].name, 'first'); + t.equal(a.sprite.costumes[1].name, 'first2'); t.end(); });