From a39d9dc654907c33245b5c806fd4d2c967ba710c Mon Sep 17 00:00:00 2001 From: Paul Kaplan <pkaplan@media.mit.edu> Date: Tue, 23 Jan 2018 13:53:23 -0500 Subject: [PATCH] Fix trying to load sound index out of range. --- src/containers/sound-editor.jsx | 7 +++++-- test/integration/test.js | 24 ++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/containers/sound-editor.jsx b/src/containers/sound-editor.jsx index b3edc48fb..28fe3efb5 100644 --- a/src/containers/sound-editor.jsx +++ b/src/containers/sound-editor.jsx @@ -190,8 +190,11 @@ SoundEditor.propTypes = { }; const mapStateToProps = (state, {soundIndex}) => { - const sound = state.vm.editingTarget.sprite.sounds[soundIndex]; - const audioBuffer = state.vm.getSoundBuffer(soundIndex); + const sprite = state.vm.editingTarget.sprite; + // Make sure the sound index doesn't go out of range. + const index = soundIndex < sprite.sounds.length ? soundIndex : sprite.sounds.length - 1; + const sound = state.vm.editingTarget.sprite.sounds[index]; + const audioBuffer = state.vm.getSoundBuffer(index); return { soundId: sound.soundId, sampleRate: audioBuffer.sampleRate, diff --git a/test/integration/test.js b/test/integration/test.js index d28814cb4..7fa5cadb6 100644 --- a/test/integration/test.js +++ b/test/integration/test.js @@ -272,4 +272,28 @@ describe('costumes, sounds and variables', () => { const logs = await getLogs(errorWhitelist); await expect(logs).toEqual([]); }); + + // Regression test for gui issue #1320 + test('Switching sprites with different numbers of sounds', async () => { + await loadUri(uri); + await clickXpath('//button[@title="tryit"]'); + + // Add a sound so this sprite has 2 sounds. + await clickText('Sounds'); + await clickText('Add Sound'); + await clickText('A Bass'); // Closes the modal + + // Now add a sprite with only one sound. + await clickText('Add Sprite'); + await clickText('Abby'); // Doing this used to crash the editor. + + await new Promise(resolve => setTimeout(resolve, 1000)); // Wait for error + + // Make sure the 'Oops' screen is not visible + const content = await driver.getPageSource(); + expect(content.indexOf('Oops')).toEqual(-1); + + const logs = await getLogs(errorWhitelist); + await expect(logs).toEqual([]); + }); }); -- GitLab