Feature: Added new scales - #991
Conversation
Make scale detector give preference to any previously set scale if the notes contained in the sequencer clips properly match the scale (usefull for when you disable scale and then enable it again, you don’t want your scale to change because Deluge detected the majorness or minorness of it), useful to protect 5-notes scales to be upgraded to a 7-note scale
|
Ready for review! |
Hmm, I think having invisible notes (which, I presume would still get played) in the sequencer could be quite confusing... Just as a thought - How about coloring out-of-scale notes differently? Conceptually like notes that are not on the current zoom level's timing grid, but of course not using the same color as for time-shifted notes - the obvious choice would be the middle color between the adjacent notes, but the color difference might be too subtle to see. |
|
I need to fix a bug when creating a new song initialized with a less-than-7-notes scale. |
I checked with other members and they found that possibility to have notes not be transposed when changing scale to be a plus. If I keep the old transposing behavior, it would be totally impossible to change from a scale to another scale with fewer notes because then it is not clear which rows in the octave are transposed to which rows in the new octave set. Imagine you have notes in all the notes within an octave of a 7-note scale, and then you change to a blues scale (6-notes), what do I do with the extra row? I cannot delete it, and I cannot quantize and merge with another row, that would be worse. Hence the option to not transpose at all. When you create a song you would normally choose your scale at the beginning. If you ever want to change your scale, it would be up to you to edit your melody to the new notes. It happens that if you go out of scale mode, then move some notes up or down, when you press the scale button again, the Deluge will guess the new scale based on the notes present in the sequencer so it is not a big deal. |
|
Sorry, my post was unclear, I not referrin to
For which I understand the motivation when there are scales with different number of notes. But I was referring to:
Which I understand as: The notes are still there, but as they are not in the current scale, they are not visible in the sequencer, but are audible. This, I would find very confusing. I hear notes playing, but I can not see them - unless I change out of scale mode, or into a scale that contains them, and then they suddenly pop up out of nowhere? Thus, the idea to still display the out-of-scale notes, in an adjacent note row of the scale, but mark them by a different color (just as out-of-timing notes are not hidden, but displayed on the nearest grid pad in a special color). This way, they could still be seen, and edited, even when not belonging to the scale. I think this would be a valuable addition, even with the existing scales, as it would allow staying in scale mode, while occasionally "borrowing" a note from out-of scale (which is how a lot of music is composed). |
|
To be even more annoying (Sorry!):
What if there were two options available to the user (mabye discerned by holding shift): I think both have their uses, but a) is one of those happy-accident machines that I would strongly prefer not to lose! Writing a melody or harmony and then changing scales is just so much fun! Important point: This is definitely easier to implement than my idea with the colored off-scale notes! |
…d scale cannot fit the number of notes from the clip, then you cycle to the beginning of the list
|
@PaulFreund @m-m-adams @m0r172 I have revamped the implementation to allow for intelligent/conditional transposition. No more need for community feature setting. |
sapphire-arches
left a comment
sapphire-arches
left a comment
There was a problem hiding this comment.
1 minor style nit and maybe something we can improve on the flash storage, otherwise this looks OK architecturally.
| } | ||
| FlashStorage::defaultScale = v; | ||
| } | ||
| } |
There was a problem hiding this comment.
style nit: newline here
| defaultKeyMenu.upper = buffer[58]; | ||
|
|
||
| defaultScale = buffer[59]; | ||
| defaultScale = buffer[148]; |
There was a problem hiding this comment.
we should read the old location and leave a tombstone value (e.g. 0xff), using the new location if the old location contains the tombstone.
There was a problem hiding this comment.
Also, I'm assuming you're moving locations to have a larger datatype? But that isn't necessary for now so maybe finding a way to re-use the existing location would be ideal
There was a problem hiding this comment.
What about checking for firmware version? The thing is that RANDOM and NONE are adjacent to the existing scales so that doesn't allow us to insert more scales in the middle. Now that I moved Random and None to the end (254 and 255) to have free space for new scales, I need to use a different FlashStorage slot as if we use the existing one, and a user loads the official firmware on top of the community one, he will get rubbish from the presetScaleNotes array, pointing to bad memory places. I added different locations for 7, 6 and 5 note scales as then we can add more scales in the future, have them in order, and have the saved flashstorage be still valid when you upgrade firmware. I will add extra code to check for out of bounds indexes to reset it back to 0, in case the user downgrades to a previous community version with less scales.
There was a problem hiding this comment.
To avoid crashing the official firmware we could consider the selection "NONE" as a tombstone value for the old slot. So if the user goes back to official firmware they will have None as selected default scale, if they change it to something else (like Minor or Dorian or whatever), and then later install the community firmware again, we recover that value again to the new slot, and write None again in the old slot
- Small formatting nit - Import old scale value when reading settings. Write a tombstone value to it so it is not read anymore (will be “NONE”)
|
@sapphire-arches I have added code to import the old value into the new slot. I decided to use the scale NONE as the detected tombstone value, to avoid crashing official firmware if you downgrade. |
# Conflicts: # src/deluge/model/song/song.cpp
# Conflicts: # src/deluge/gui/menu_item/defaults/scale.h
* Added new scales Make scale detector give preference to any previously set scale if the notes contained in the sequencer clips properly match the scale (usefull for when you disable scale and then enable it again, you don’t want your scale to change because Deluge detected the majorness or minorness of it), useful to protect 5-notes scales to be upgraded to a 7-note scale * Fix typo * Clean names * Remove similar scales to make them be 16 * dbt formt * Fix to avoid preferring Other Scale when coming back to scale mode * Don’t transpose notes when cycling through scales * chore change comments * Allow Lumi code to search for scales that might be equivalent but are transposed * Lumi: Search all transpositions in an octave * Update preset name * And comment * and scale notes * dbt format * Refactoring * Fix typo * Fix bug when creating new song and the scale was less than 7 notes * dbt format * Lots of fixes Added runtime flag for enabling uneven length scales. Bring back transposing scales notes when cycling through scales, if the runtime flag is off (default setting and default official behavior) If the flag is On, we include the 6-note and 5-note scales in the list, and transposing scales notes is disabled when cycling through scales * Use a new byte from flash storage for the Default Scale as we now save RANDOM as 254 and NONE as 255, to leave space for future scales, and loading the default scale set from this build, from a official build can totally screw the note rows * Remove feature flag * Allow for backwards compatibility * Implemented adaptative transposition between scales. If the new cycled scale cannot fit the number of notes from the clip, then you cycle to the beginning of the list * nit * Feedback from sapphire-arches - Small formatting nit - Import old scale value when reading settings. Write a tombstone value to it so it is not read anymore (will be “NONE”) * Also import the old RANDOM scale index properly, and simplify logic * Fix merge compile issues * Dbt format * Remove magic number
3 participants
TODO: