From b4d6db0ad9c6c187d426309ad837a2524af94588 Mon Sep 17 00:00:00 2001 From: DD Date: Thu, 22 Feb 2018 15:42:35 -0500 Subject: [PATCH] Fix tests --- src/serialization/sb2.js | 2 +- src/serialization/sb3.js | 2 +- src/sprites/rendered-target.js | 26 +++++----- src/sprites/sprite.js | 47 +++++++---------- test/integration/sb3-roundtrip.js | 8 +-- test/unit/blocks_looks.js | 3 ++ test/unit/sprites_rendered-target.js | 78 ++++++++++++++++++---------- 7 files changed, 92 insertions(+), 74 deletions(-) diff --git a/src/serialization/sb2.js b/src/serialization/sb2.js index 0cb3e6ae3..1102c005e 100644 --- a/src/serialization/sb2.js +++ b/src/serialization/sb2.js @@ -324,7 +324,7 @@ const parseScratchObject = function (object, runtime, extensions, topLevel) { target.isStage = topLevel; Promise.all(costumePromises).then(costumes => { - sprite.addCostumes(costumes); + sprite.costumes = costumes; }); Promise.all(soundPromises).then(sounds => { diff --git a/src/serialization/sb3.js b/src/serialization/sb3.js index a9ba94f77..9e8bcbf4e 100644 --- a/src/serialization/sb3.js +++ b/src/serialization/sb3.js @@ -156,7 +156,7 @@ const parseScratchObject = function (object, runtime, extensions) { target.isStage = object.isStage; } Promise.all(costumePromises).then(costumes => { - sprite.addCostumes(costumes); + sprite.costumes = costumes; }); Promise.all(soundPromises).then(sounds => { sprite.sounds = sounds; diff --git a/src/sprites/rendered-target.js b/src/sprites/rendered-target.js index 5742e15dc..c2802117d 100644 --- a/src/sprites/rendered-target.js +++ b/src/sprites/rendered-target.js @@ -372,10 +372,10 @@ class RenderedTarget extends Target { // Keep the costume index within possible values. index = Math.round(index); this.currentCostume = MathUtil.wrapClamp( - index, 0, this.getCostumes().length - 1 + index, 0, this.sprite.costumes.length - 1 ); if (this.renderer) { - const costume = this.sprite.getCostumeByIndex(this.currentCostume); + const costume = this.getCostumes()[this.currentCostume]; const drawableProperties = { skinId: costume.skinId, costumeResolution: costume.bitmapResolution @@ -403,7 +403,7 @@ class RenderedTarget extends Target { * @param {!object} costumeObject Object representing the costume. */ addCostume (costumeObject) { - this.sprite.addCostumeAt(costumeObject, this.getCostumes().length); + this.sprite.addCostumeAt(costumeObject, this.sprite.costumes.length); } /** @@ -421,12 +421,12 @@ class RenderedTarget extends Target { * @param {string} newName - the desired new name of the costume (will be modified if already in use). */ renameCostume (costumeIndex, newName) { - const usedNames = this.getCostumes() + const usedNames = this.sprite.costumes .filter((costume, index) => costumeIndex !== index) .map(costume => costume.name); - const oldName = this.sprite.getCostumeByIndex(costumeIndex).name; + const oldName = this.getCostumes()[costumeIndex].name; const newUnusedName = StringUtil.unusedName(newName, usedNames); - this.sprite.getCostumeByIndex(costumeIndex).name = newUnusedName; + this.getCostumes()[costumeIndex].name = newUnusedName; if (this.isStage) { // Since this is a backdrop, go through all targets and @@ -447,10 +447,10 @@ class RenderedTarget extends Target { * @param {number} index Costume index to be deleted */ deleteCostume (index) { - const originalCostumeCount = this.getCostumes().length; + const originalCostumeCount = this.sprite.costumes.length; if (originalCostumeCount === 1) return; - this.sprite.deleteCostumeByIndex(index); + this.sprite.deleteCostumeAt(index); if (index === this.currentCostume && index === originalCostumeCount - 1) { this.setCostume(index - 1); @@ -530,8 +530,8 @@ class RenderedTarget extends Target { * @return {number} Index of the named costume, or -1 if not present. */ getCostumeIndexByName (costumeName) { - for (let i = 0; i < this.getCostumes().length; i++) { - if (this.sprite.getCostumeByIndex(i).name === costumeName) { + for (let i = 0; i < this.sprite.costumes.length; i++) { + if (this.getCostumes()[i].name === costumeName) { return i; } } @@ -543,7 +543,7 @@ class RenderedTarget extends Target { * @return {object} current costume */ getCurrentCostume () { - return this.sprite.getCostumeByIndex(this.currentCostume); + return this.getCostumes()[this.currentCostume]; } /** @@ -551,7 +551,7 @@ class RenderedTarget extends Target { * @return {object[]} list of costumes */ getCostumes () { - return this.sprite.getCostumes(); + return this.sprite.costumes; } /** @@ -569,7 +569,7 @@ class RenderedTarget extends Target { updateAllDrawableProperties () { if (this.renderer) { const renderedDirectionScale = this._getRenderedDirectionAndScale(); - const costume = this.sprite.getCostumeByIndex(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 3c30a08da..d3e53b8db 100644 --- a/src/sprites/sprite.js +++ b/src/sprites/sprite.js @@ -52,48 +52,37 @@ class Sprite { */ this.nextCostumeId = 0; } - - /** - * Get full costume list - * @return {object[]} list of costumes - */ - getCostumes () { - return this.costumes_; - } - - /** - * Get costume at index - * @param {int} index Index - * @return {object} costumes - */ - getCostumeByIndex (index) { - return this.costumes_[index]; - } - /** - * Add a costume, taking care to avoid duplicate names. - * @param {!object} costumeObject Object representing the costume. - */ - addCostume (costumeObject) { - this.addCostumeAt(costumeObject, this.costumes_.length); - } - /** * Add an array of costumes, taking care to avoid duplicate names. * @param {!Array} costumes Array of objects representing costumes. */ - addCostumes (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); costumeObject.costumeId = this.nextCostumeId; @@ -105,7 +94,7 @@ class Sprite { * Delete a costume by index. * @param {number} index Costume index to be deleted */ - deleteCostumeByIndex (index) { + deleteCostumeAt (index) { this.costumes_ = this.costumes_ .slice(0, index) .concat(this.costumes_.slice(index + 1)); @@ -152,12 +141,12 @@ class Sprite { const assetPromises = []; - newSprite.addCostumes(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)); return newCostume; - })); + }); newSprite.sounds = this.sounds.map(sound => { const newSound = Object.assign({}, sound); diff --git a/test/integration/sb3-roundtrip.js b/test/integration/sb3-roundtrip.js index 6233502fa..5704e33b7 100644 --- a/test/integration/sb3-roundtrip.js +++ b/test/integration/sb3-roundtrip.js @@ -52,8 +52,8 @@ test('sb3-roundtrip', t => { t.strictEqual(sprite.clones.length, 1); t.strictEqual(sprite.clones[0], spriteClone); - t.strictEqual(sprite.getCostumes().length, 2); - const [cat, squirrel] = sprite.getCostumes(); + t.strictEqual(sprite.costumes.length, 2); + const [cat, squirrel] = sprite.costumes; t.strictEqual(cat.assetId, 'f88bf1935daea28f8ca098462a31dbb0'); t.strictEqual(cat.dataFormat, 'svg'); t.strictEqual(squirrel.assetId, '7e24c99c1b853e52f8e7f9004416fa34'); @@ -77,7 +77,7 @@ test('sb3-roundtrip', t => { const stageBlocks = new Blocks(); const stage = new Sprite(stageBlocks, runtime1); stage.name = 'Stage'; - stage.addCostume(building); + stage.costumes = [building]; stage.sounds = []; const stageClone = stage.createClone(); stageClone.isStage = true; @@ -85,7 +85,7 @@ test('sb3-roundtrip', t => { const spriteBlocks = new Blocks(); const sprite = new Sprite(spriteBlocks, runtime1); sprite.name = 'Sprite'; - sprite.addCostumes([cat, squirrel]); + sprite.costumes = [cat, squirrel]; sprite.sounds = [meow]; const spriteClone = sprite.createClone(); 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 262a834a5..51c670dc9 100644 --- a/test/unit/sprites_rendered-target.js +++ b/test/unit/sprites_rendered-target.js @@ -90,7 +90,7 @@ test('setCostume', t => { const o = new Object(); const s = new Sprite(); const r = new Runtime(); - s.addCostume(o); + s.costumes = [o]; const a = new RenderedTarget(s, r); const renderer = new FakeRenderer(); a.renderer = renderer; @@ -107,7 +107,7 @@ test('deleteCostume', t => { const s = new Sprite(); const r = new Runtime(); - s.addCostumes([o1, o2, o3]); + s.costumes = [o1, o2, o3]; const a = new RenderedTarget(s, r); const renderer = new FakeRenderer(); a.renderer = renderer; @@ -117,32 +117,39 @@ test('deleteCostume', t => { // Costume 3 a.setCostume(0); a.deleteCostume(0); - t.deepEqual(a.sprite.getCostumes(), [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 // x* Costume 2 => * Costume 3 // Costume 3 - a.sprite.addCostumes([o1, o2, o3]); + a.sprite.costumes = [o1, o2, o3]; a.setCostume(1); a.deleteCostume(1); - t.deepEqual(a.sprite.getCostumes(), [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 // Costume 2 => * Costume 2 // x* Costume 3 - a.sprite.addCostumes([o1, o2, o3]); + a.sprite.costumes = [o1, o2, o3]; a.setCostume(2); a.deleteCostume(2); - t.deepEqual(a.sprite.getCostumes(), [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.addCostume(o1); + a.sprite.costumes = [o1]; a.setCostume(0); a.deleteCostume(0); - t.deepEqual(a.sprite.getCostumes(), [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 @@ -150,10 +157,14 @@ test('deleteCostume', t => { // Costume 3 => * Costume 4 // * Costume 4 Costume 5 // Costume 5 - a.sprite.addCostumes([o1, o2, o3, o4, o5]); + a.sprite.costumes = [o1, o2, o3, o4, o5]; a.setCostume(3); a.deleteCostume(1); - t.deepEqual(a.sprite.getCostumes(), [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 @@ -161,10 +172,14 @@ test('deleteCostume', t => { // Costume 3 => Costume 3 // x Costume 4 Costume 5 // Costume 5 - a.sprite.addCostumes([o1, o2, o3, o4, o5]); + a.sprite.costumes = [o1, o2, o3, o4, o5]; a.setCostume(1); a.deleteCostume(3); - t.deepEqual(a.sprite.getCostumes(), [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 @@ -172,10 +187,14 @@ test('deleteCostume', t => { // Costume 3 => Costume 3 // Costume 4 Costume 4 // x Costume 5 - a.sprite.addCostumes([o1, o2, o3, o4, o5]); + a.sprite.costumes = [o1, o2, o3, o4, o5]; a.setCostume(1); a.deleteCostume(4); - t.deepEqual(a.sprite.getCostumes(), [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.getCostumes() = 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.getCostumes() = costumes; + a.sprite.costumes = [{id: 1}, {id: 2}, {id: 3}]; t.same(a.toJSON().sounds, sounds); - t.same(a.toJSON().getCostumes(), 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.addCostumes([{name: 'first'}]); a.addCostume({name: 'first'}); - t.deepEqual(a.sprite.getCostumes(), [{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(); }); @@ -392,10 +414,14 @@ test('#renameSound does not duplicate names', t => { test('#renameCostume does not duplicate names', t => { const spr = new Sprite(); const a = new RenderedTarget(spr, null); - a.sprite.addCostumes([{name: 'first'}, {name: 'second'}]); + a.sprite.costumes = [{name: 'first'}, {name: 'second'}]; a.renameCostume(0, 'first'); // Shouldn't increment the name, noop - t.deepEqual(a.sprite.getCostumes(), [{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.getCostumes(), [{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(); });