Skip to content

Feature: FILL iteration mode for notes - #211

Merged
jamiefaye merged 39 commits into
SynthstromAudible:communityfrom
weavermedia:fill-iteration
Aug 20, 2023
Merged

Feature: FILL iteration mode for notes#211
jamiefaye merged 39 commits into
SynthstromAudible:communityfrom
weavermedia:fill-iteration

Conversation

@weavermedia

@weavermedia weavermedia commented Jul 19, 2023

Copy link
Copy Markdown
Contributor

This is a draft PR for a new note iteration mode called FILL that only plays the note if the FILL function is currently enabled by some key press.

Todo

  • Choose proper key combo to activate the fill mode (preferably MIDI-assignable) = assignable in community settings
  • Decide where to put the FILL setting in the probability/iteration list = zero position, before 5%
  • Properly light the SYNC_SCALING LED while held (if set in community settings)
  • Add a NOT FILL mode where notes won't play when FILL is active 👉 Future enhancement
  • Add community setting that allows Fill Mode to be chosen as a primary action for the Sync Scaling button
  • Implement FILL. (dot) value as a NOT FILL note mode 👉 Future enhancement
  • Add FILL to Global MIDI Commands, note on activates and note off deactivates
  • Change community setting from simple on/off to hold/toggle/once 👉 Future enhancement

I realize there's an existing and much more complex 'Fill clips' PR here and this may cause some confusion so I'm open to whatever needs to be done to fix that. UPDATE: communicated with the PR author and hopefully we can treat these as two separate features.

For this simple proof of concept I hijacked the 95% note probability setting as FILL and the SHIFT+TRIPLETS key combo to activate the fill notes.

The FILL setting has been added to the start of the probability/iteration list:
FILL, 5%, 10%, ...

This seems a logical place because [1] apart from putting it next to 100% this is the least scrolling to get to it (less than after 8of8) and [2] in a way you are actually setting it to 0% chance of playing during normal playback.

And, in no small part, putting it at position zero in the list was by far the easiest and least intrusive way to code it! 🙌

Video attached because it's easier to see it in action. In the video the majority of notes in the drum pattern are set to FILL therefore do not sound during normal playback. I activate fill mode by holding SHIFT+TRIPLETS (since changed to SYNC_SCALING). While those keys are pressed the FILL notes will sound.

This should work in all modes - Clip View, Song View and Arrange View - both as a performance tool and for re-recording/resampling new variations in the studio.

Deluge-FillMode-NoteIteration.mp4

@weavermedia
weavermedia marked this pull request as draft July 19, 2023 02:10
@weavermedia weavermedia changed the title [WIP] FILL iteration mode for notes [DRAFT] FILL iteration mode for notes Jul 19, 2023
@weavermedia weavermedia changed the title [DRAFT] FILL iteration mode for notes [DRAFT] FILL iteration mode for notes [HELP NEEDED] Jul 19, 2023
@weavermedia

weavermedia commented Jul 19, 2023

Copy link
Copy Markdown
Contributor Author

DONE Need help figuring out how to integrate a FILL value into the current list of probability/iteration values.

@weavermedia weavermedia changed the title [DRAFT] FILL iteration mode for notes [HELP NEEDED] [DRAFT] FILL iteration mode for notes Jul 19, 2023
@weavermedia
weavermedia marked this pull request as ready for review July 21, 2023 00:49
@weavermedia

Copy link
Copy Markdown
Contributor Author

I believe this is ready for review. If anyone has any feedback or spots and bugs please let me know.

@weavermedia weavermedia changed the title [DRAFT] FILL iteration mode for notes FILL iteration mode for notes Jul 21, 2023
@chrisbc chrisbc mentioned this pull request Jul 21, 2023
5 tasks
@weavermedia

weavermedia commented Jul 21, 2023

Copy link
Copy Markdown
Contributor Author

Actually looks like there's competition for the sync scaling button so let's put it in a preference. So far I think the idea is something like this:

Button Functions > Sync Scaling > Sync Scaling / Fill Mode / etc (where other functions can be added here)

Does this look like what we talked about @chrisbc ?

I do think that given the performance nature of FILL mode the user should be able to assign a primary, non-shift button to this feature.

Will we also eventually need another menu entry for shift+ actions too?

@weavermedia
weavermedia marked this pull request as draft July 21, 2023 16:54
Comment thread src/deluge/model/settings/runtime_feature_settings.cpp
@weavermedia
weavermedia marked this pull request as ready for review August 13, 2023 03:43
@weavermedia

weavermedia commented Aug 13, 2023

Copy link
Copy Markdown
Contributor Author

@m-m-adams This is ready for review, when you have a moment. Thanks for the help debugging the weird build errors.

@weavermedia weavermedia changed the title FILL iteration mode for notes Feature: FILL iteration mode for notes Aug 13, 2023

@m-m-adams m-m-adams left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good - I've just got a couple small questions where I think code clarity could be improved

Comment thread src/definitions_cxx.hpp Outdated
Comment thread src/deluge/gui/views/instrument_clip_view.cpp
Comment thread src/deluge/gui/views/instrument_clip_view.cpp
Comment thread src/deluge/model/note/note_row.cpp
@sichtbeton

Copy link
Copy Markdown
Contributor

will this make the feature cutoff? would be super cool.

@m-m-adams m-m-adams left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved - would be nice to add a LAST member to global midi commands but it's not critical. With your explanations I'm fine with the rest of the code

@weavermedia

Copy link
Copy Markdown
Contributor Author

... would be nice to add a LAST member to global midi commands but it's not critical.

I added a LAST member to global MIDI commands as suggested.

@sichtbeton

Copy link
Copy Markdown
Contributor

Could this get merged in before the cutoff?

@weavermedia

Copy link
Copy Markdown
Contributor Author

Could this get merged in before the cutoff?

Hope so! 😀

@soymonitus

Copy link
Copy Markdown
Collaborator

Could this get merged in before the cutoff?

Hope so! 😀

Please fix conflicts. Hope this is merged soon

@jamiefaye
jamiefaye added this pull request to the merge queue Aug 20, 2023
Merged via the queue into SynthstromAudible:community with commit 6bcc90b Aug 20, 2023
@trappar trappar mentioned this pull request Aug 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants