[Draft] Kit-level arpeggiator #3349
Conversation
…synth and kit affect-entire
# Conflicts: # src/deluge/gui/views/audio_clip_view.cpp
# Conflicts: # src/deluge/processing/sound/sound.cpp
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
clang-tidy has made some suggestions. Please note that they are machine-generated, and you should review them carefully before applying to avoid introducing bugs.
There were too many comments to post at once. Showing the first 25 out of 88. Check the log or trigger a new build to see more.
| && !soundEditor.editingNonAudioDrumRow(); | ||
| } | ||
| void getColumnLabel(StringBuf& label) override { | ||
| label.append(deluge::l10n::getView(deluge::l10n::built_in::seven_segment, this->name).data()); |
There was a problem hiding this comment.
warning: result of a data() call may not be null terminated, provide size information to the callee to prevent potential issues [bugprone-suspicious-stringview-data-usage]
label.append(deluge::l10n::getView(deluge::l10n::built_in::seven_segment, this->name).data());
^| l10n::getView(STRING_FOR_AS_PLAYED), //< | ||
| l10n::getView(STRING_FOR_PATTERN), //< | ||
| }; | ||
| } |
There was a problem hiding this comment.
warning: do not use 'else' after 'return' [readability-else-after-return]
| } | |
| return { | |
| l10n::getView(STRING_FOR_UP), //< | |
| l10n::getView(STRING_FOR_DOWN), //< | |
| l10n::getView(STRING_FOR_UP_DOWN), //< | |
| l10n::getView(STRING_FOR_RANDOM), //< | |
| l10n::getView(STRING_FOR_WALK1), //< | |
| l10n::getView(STRING_FOR_WALK2), //< | |
| l10n::getView(STRING_FOR_WALK3), //< | |
| l10n::getView(STRING_FOR_AS_PLAYED), //< | |
| l10n::getView(STRING_FOR_PATTERN), //< | |
| }; | |
| l10n::getView(STRING_FOR_ALTERNATE), //< | ||
| l10n::getView(STRING_FOR_RANDOM), //< | ||
| }; | ||
| } |
There was a problem hiding this comment.
warning: do not use 'else' after 'return' [readability-else-after-return]
| } | |
| return { | |
| l10n::getView(STRING_FOR_UP), //< | |
| l10n::getView(STRING_FOR_DOWN), //< | |
| l10n::getView(STRING_FOR_UP_DOWN), //< | |
| l10n::getView(STRING_FOR_ALTERNATE), //< | |
| l10n::getView(STRING_FOR_RANDOM), //< | |
| }; | |
| return !soundEditor.editingGateDrumRow() && !soundEditor.editingKitAffectEntire(); | ||
| } | ||
| void getColumnLabel(StringBuf& label) override { | ||
| label.append(deluge::l10n::getView(deluge::l10n::built_in::seven_segment, this->name).data()); |
There was a problem hiding this comment.
warning: result of a data() call may not be null terminated, provide size information to the callee to prevent potential issues [bugprone-suspicious-stringview-data-usage]
label.append(deluge::l10n::getView(deluge::l10n::built_in::seven_segment, this->name).data());
^| @@ -40,6 +40,10 @@ class RandomizerLock final : public Selection { | |||
| }; | |||
| } | |||
|
|
|||
| void getColumnLabel(StringBuf& label) override { | |||
| label.append(deluge::l10n::getView(deluge::l10n::built_in::seven_segment, this->name).data()); | |||
There was a problem hiding this comment.
warning: result of a data() call may not be null terminated, provide size information to the callee to prevent potential issues [bugprone-suspicious-stringview-data-usage]
label.append(deluge::l10n::getView(deluge::l10n::built_in::seven_segment, this->name).data());
^| @@ -95,6 +94,7 @@ extern deluge::gui::menu_item::Submenu* parentsForSoundShortcuts[kDisplayWidth][ | |||
| extern deluge::gui::menu_item::Submenu* parentsForAudioShortcuts[kDisplayWidth][kDisplayHeight]; | |||
| extern deluge::gui::menu_item::Submenu* parentsForSongShortcuts[kDisplayWidth][kDisplayHeight]; | |||
| extern deluge::gui::menu_item::Submenu* parentsForKitGlobalFXShortcuts[kDisplayWidth][kDisplayHeight]; | |||
| extern deluge::gui::menu_item::Submenu* parentsForMidiOrCVParamShortcuts[kDisplayWidth][kDisplayHeight]; | |||
There was a problem hiding this comment.
warning: invalid case style for variable 'parentsForMidiOrCVParamShortcuts' [readability-identifier-naming]
| extern deluge::gui::menu_item::Submenu* parentsForMidiOrCVParamShortcuts[kDisplayWidth][kDisplayHeight]; | |
| extern deluge::gui::menu_item::Submenu* parents_for_midi_or_cv_param_shortcuts[kDisplayWidth][kDisplayHeight]; |
| @@ -95,6 +94,7 @@ extern deluge::gui::menu_item::Submenu* parentsForSoundShortcuts[kDisplayWidth][ | |||
| extern deluge::gui::menu_item::Submenu* parentsForAudioShortcuts[kDisplayWidth][kDisplayHeight]; | |||
| extern deluge::gui::menu_item::Submenu* parentsForSongShortcuts[kDisplayWidth][kDisplayHeight]; | |||
| extern deluge::gui::menu_item::Submenu* parentsForKitGlobalFXShortcuts[kDisplayWidth][kDisplayHeight]; | |||
| extern deluge::gui::menu_item::Submenu* parentsForMidiOrCVParamShortcuts[kDisplayWidth][kDisplayHeight]; | |||
There was a problem hiding this comment.
warning: static variable 'parentsForMidiOrCVParamShortcuts' may be dynamically initialized in this header file [bugprone-dynamic-static-initializers]
extern deluge::gui::menu_item::Submenu* parentsForMidiOrCVParamShortcuts[kDisplayWidth][kDisplayHeight];
^| @@ -527,8 +527,8 @@ void AudioClip::sampleZoneChanged(ModelStackWithTimelineCounter const* modelStac | |||
|
|
|||
| int32_t priorityRating = 1; // probably better fix this... | |||
|
|
|||
| voiceSample->sampleZoneChanged(&guide, ((Sample*)sampleHolder.audioFile), MarkerType::END, | |||
| getLoopingType(modelStack), priorityRating, true); | |||
| voiceSample->sampleZoneChanged(&guide, ((Sample*)sampleHolder.audioFile), sampleControls.isReversed(), | |||
There was a problem hiding this comment.
warning: do not use C-style cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-cstyle-cast]
| voiceSample->sampleZoneChanged(&guide, ((Sample*)sampleHolder.audioFile), sampleControls.isReversed(), | |
| dynamic_cast<Sample*>() |
| @@ -780,7 +780,7 @@ LoopType AudioClip::getLoopingType(ModelStackWithTimelineCounter const* modelSta | |||
| // end-point is set to beyond the waveform's length). | |||
|
|
|||
| bool shouldLoop = | |||
| (sampleControls.reversed || sampleHolder.endPos <= ((Sample*)sampleHolder.audioFile)->lengthInSamples) | |||
| (sampleControls.isReversed() || sampleHolder.endPos <= ((Sample*)sampleHolder.audioFile)->lengthInSamples) | |||
There was a problem hiding this comment.
warning: do not use C-style cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-cstyle-cast]
| (sampleControls.isReversed() || sampleHolder.endPos <= ((Sample*)sampleHolder.audioFile)->lengthInSamples) | |
| dynamic_cast<Sample*>() |
| @@ -2281,6 +2282,9 @@ Error InstrumentClip::setAudioInstrument(Instrument* newInstrument, Song* song, | |||
| if (newInstrument->type == OutputType::SYNTH) { | |||
There was a problem hiding this comment.
warning: repeated branch body in conditional chain [bugprone-branch-clone]
^Additional context
src/deluge/model/clip/instrument_clip.cpp:2284: end of the original
^src/deluge/model/clip/instrument_clip.cpp:2284: clone 1 starts here
^
The idea is to make some of the arpeggiator options available for kits when affect entire is on. That will make the note Ons and note Offs sent to the kit, or read from the sequencer to be passed first by an arpeggiator module that will decide when to send those note ons/offs to the noteRows themselves. What does that mean? That you could have the arpeggiator for the kit row also ON and have two levels of arpeggiation (you can go crazy..)
After checking which parameters can be used for kit affect-entire this is what I had to OMIT:
(everything related to changing pitch)
So it means that we have available for kit affect entire: