Fix tests

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 => {

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;

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++) {
+        for (let i = 0; i < this.sprite.costumes.length; i++) {
-            if (this.sprite.getCostumeByIndex(i).name === costumeName) {
+            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],

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<object>} 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);

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);
+        t.strictEqual(sprite.costumes.length, 2);
-        const [cat, squirrel] = sprite.getCostumes();
+        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();
 

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'},

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.costumes = [{id: 1}, {id: 2}, {id: 3}];
-    a.sprite.getCostumes() = costumes;
+    t.equals(a.getCostumes().length, 3);
-    t.equals(a.getCostumes(), costumes);
+    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();
 });