07-22-2018, 05:59 PM
As the person who suggested the Invert tool, I am disappointed with its current state. Here are my suggested deoptimizations!
Seriously though, I have an optimized version as well.
Comments are mine.
Old (menus[2].items.invert.action):
Suggestion (unoptimized):
Suggestion (optimized):
I can see why .map was eschewed because of its poor performance, but there is no reason I have seen not to only use selected notes and predefine inversePiano. Of course, not predefining inversePiano may for sustainability, which would make sense if available notes were to fluctuate in the future. Also, here is a version that inverts all notes if someone doesn't select anything first:
I hope these suggestions will be considered, even if this feature is really not that important.
Seriously though, I have an optimized version as well.
Comments are mine.
Old (menus[2].items.invert.action):
Code:
//All of this is done on click.
var inversePiano = {};
for (i = 0; i < piano.length; i++) {
inversePiano[piano[i]] = piano[piano.length - 1 - i]
}
for (i = 0; i < song.notes.length; i++) { //ALL NOTES IN THE SONG?! IT WAS ORIGINALLY THE SELECTED NOTES ONLY?! *TRIGGERED*
var note = song.notes[i];
song.moveNote(note, note.instrument, note.instrument, note.time, note.time, note.type, inversePiano[note.type]);
song.update(note);
}
keySelect.selectedIndex = 0;
keySelect.onchange();Suggestion (unoptimized):
Code:
//Predefined
inversePiano = piano.map(x => piano[settings.numNotes - 1 - pianoToIndex[x]]) //One liner that should be executed once immediately.
//On click
selectedNotes.map(function(note) { //.map is 10x slower, but it looks cool! //Also, only selected notes!
song.moveNote(note, note.instrument, note.instrument, note.time, note.time, note.type, inversePiano[pianoToIndex[note.type]]);
song.update(note);
})
keySelect.selectedIndex = 0;
keySelect.onchange();Suggestion (optimized):
Code:
//Predefined
var inversePiano = {};
for (i = 0; i < piano.length; i++) {
inversePiano[piano[i]] = piano[piano.length - 1 - i]
}
//On click
for (i = 0; i < selectedNotes.length; i++) {
var note = selectedNotes[i];
song.moveNote(note, note.instrument, note.instrument, note.time, note.time, note.type, inversePiano[note.type]);
song.update(note);
}
keySelect.selectedIndex = 0;
keySelect.onchange();I can see why .map was eschewed because of its poor performance, but there is no reason I have seen not to only use selected notes and predefine inversePiano. Of course, not predefining inversePiano may for sustainability, which would make sense if available notes were to fluctuate in the future. Also, here is a version that inverts all notes if someone doesn't select anything first:
Code:
//Predefined
var inversePiano = {};
for (i = 0; i < piano.length; i++) {
inversePiano[piano[i]] = piano[piano.length - 1 - i]
}
//On click
if (selectedNotes.length != 0) {
for (i = 0; i < selectedNotes.length; i++) {
var note = selectedNotes[i];
song.moveNote(note, note.instrument, note.instrument, note.time, note.time, note.type, inversePiano[note.type]);
song.update(note);
}
} else {
for (i = 0; i < song.notes.length; i++) {
var note = song.notes[i];
song.moveNote(note, note.instrument, note.instrument, note.time, note.time, note.type, inversePiano[note.type]);
song.update(note);
}
}
keySelect.selectedIndex = 0;
keySelect.onchange();I hope these suggestions will be considered, even if this feature is really not that important.
