From 24b098257beea8b2d3d252ce57bc154e0329c1e0 Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Fri, 17 Aug 2018 14:01:24 -0400 Subject: [PATCH 1/3] Return restore sound function when deleting a sound. --- src/sprites/rendered-target.js | 11 ++++++++--- src/virtual-machine.js | 13 ++++++++++++- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/sprites/rendered-target.js b/src/sprites/rendered-target.js index 443b9cdc6b1..9a16bffda99 100644 --- a/src/sprites/rendered-target.js +++ b/src/sprites/rendered-target.js @@ -585,12 +585,17 @@ class RenderedTarget extends Target { /** * Delete a sound by index. * @param {number} index Sound index to be deleted + * @return {object} The deleted sound object, or null if no sound was deleted. */ deleteSound (index) { - this.sprite.sounds = this.sprite.sounds - .slice(0, index) - .concat(this.sprite.sounds.slice(index + 1)); + // Make sure the sound index is not out of bounds + if (index < 0 || index >= this.sounds.length) { + return null; + } + // Delete the sound at the given index + const deletedSound = this.sprite.sounds.splice(index, 1)[0]; this.runtime.requestTargetsUpdate(this); + return deletedSound; } /** diff --git a/src/virtual-machine.js b/src/virtual-machine.js index ed9c340dacb..d23c85773ed 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -675,9 +675,20 @@ class VirtualMachine extends EventEmitter { /** * Delete a sound from the current editing target. * @param {int} soundIndex - the index of the sound to be removed. + * @return {?Function} A function to restore the sound that was deleted, + * or null, if no sound was deleted. */ deleteSound (soundIndex) { - this.editingTarget.deleteSound(soundIndex); + const target = this.editingTarget; + const deletedSound = this.editingTarget.deleteSound(soundIndex); + if (deletedSound) { + const restoreFun = () => { + target.addSound(deletedSound); + this.emitTargetsUpdate(); + }; + return restoreFun; + } + return null; } /** From e0a1f464ce8054e068b2081299a77c952685d7f1 Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Mon, 20 Aug 2018 12:48:57 -0400 Subject: [PATCH 2/3] Add and update unit tests for sound deletion. --- src/sprites/rendered-target.js | 2 +- test/unit/sprites_rendered-target.js | 4 +++- test/unit/virtual-machine.js | 32 +++++++++++++++++++++++++--- 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/sprites/rendered-target.js b/src/sprites/rendered-target.js index 9a16bffda99..d2ed5393391 100644 --- a/src/sprites/rendered-target.js +++ b/src/sprites/rendered-target.js @@ -589,7 +589,7 @@ class RenderedTarget extends Target { */ deleteSound (index) { // Make sure the sound index is not out of bounds - if (index < 0 || index >= this.sounds.length) { + if (index < 0 || index >= this.sprite.sounds.length) { return null; } // Delete the sound at the given index diff --git a/test/unit/sprites_rendered-target.js b/test/unit/sprites_rendered-target.js index 65088730745..7d64d78417e 100644 --- a/test/unit/sprites_rendered-target.js +++ b/test/unit/sprites_rendered-target.js @@ -211,8 +211,10 @@ test('deleteSound', t => { const renderer = new FakeRenderer(); a.renderer = renderer; - a.deleteSound(0); + const firstDeleted = a.deleteSound(0); t.deepEqual(a.sprite.sounds, [o2, o3]); + t.type(firstDeleted, 'object'); + t.equals(firstDeleted.id, 1); // Allows deleting the only sound a.sprite.sounds = [o1]; diff --git a/test/unit/virtual-machine.js b/test/unit/virtual-machine.js index 0310da046bd..1bd56f50124 100644 --- a/test/unit/virtual-machine.js +++ b/test/unit/virtual-machine.js @@ -1,9 +1,35 @@ const test = require('tap').test; -const VirtualMachine = require('../../src/virtual-machine.js'); -const Sprite = require('../../src/sprites/sprite.js'); -const Variable = require('../../src/engine/variable.js'); +const VirtualMachine = require('../../src/virtual-machine'); +const Sprite = require('../../src/sprites/sprite'); +const Variable = require('../../src/engine/variable'); const adapter = require('../../src/engine/adapter'); const events = require('../fixtures/events.json'); +const Runtime = require('../../src/engine/runtime'); +const RenderedTarget = require('../../src/sprites/rendered-target'); + +test('deleteSound returns function after deleting or null if nothing was deleted', t => { + const vm = new VirtualMachine(); + const sprite = new Sprite(); + sprite.sounds = [{id: 1}, {id: 2}, {id: 3}]; + const rt = new Runtime(); + const target = new RenderedTarget(sprite, rt); + vm.editingTarget = target; + + const addFun = vm.deleteSound(1); + t.equal(sprite.sounds.length, 2); + t.equal(sprite.sounds[0].id, 1); + t.equal(sprite.sounds[1].id, 3); + t.type(addFun, 'function'); + + const noAddFun = vm.deleteSound(2); + t.equal(sprite.sounds.length, 2); + t.equal(sprite.sounds[0].id, 1); + t.equal(sprite.sounds[1].id, 3); + t.equal(noAddFun, null); + + t.end(); +}); + test('addSprite throws on invalid string', t => { const vm = new VirtualMachine(); From a8c56948cbcd20763421243f4ff1dcec42fd4468 Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Tue, 21 Aug 2018 15:23:14 -0400 Subject: [PATCH 3/3] Clean up test. --- test/unit/sprites_rendered-target.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/unit/sprites_rendered-target.js b/test/unit/sprites_rendered-target.js index 7d64d78417e..cbcc9e50b7f 100644 --- a/test/unit/sprites_rendered-target.js +++ b/test/unit/sprites_rendered-target.js @@ -213,8 +213,7 @@ test('deleteSound', t => { const firstDeleted = a.deleteSound(0); t.deepEqual(a.sprite.sounds, [o2, o3]); - t.type(firstDeleted, 'object'); - t.equals(firstDeleted.id, 1); + t.deepEqual(firstDeleted, o1); // Allows deleting the only sound a.sprite.sounds = [o1];