Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Arpeggiator extended #2130

Merged
merged 1 commit into from
Sep 10, 2016
Merged

Conversation

zonkmachine
Copy link
Member

@zonkmachine zonkmachine commented Jun 23, 2015

Se issue: #2080
Here are some of the new arpeggiator functions.


New functions:

  • Skip - Skips notes. Full on 1% of the notes are played.
  • Miss - Miss notes. Full on this is the same as having 'Random' setting in direction.

@zonkmachine
Copy link
Member Author

I couldn't make the original 'Skip' function work as I intended it and I'm not sure it's my coding that's the problem this time. It could be some other bug that's being exposed here.
Anyway. I let the Skip function mute notes last in the arpeggiator function instead and that's a bit wasteful compared to the earlier method of actually skipping over notes but now it works.

@zonkmachine
Copy link
Member Author

Toggle switch removed + Squashed.

@tresf
Copy link
Member

tresf commented Sep 17, 2015

👍

@zonkmachine
Copy link
Member Author

zonkmachine commented Feb 24, 2016

@Fastigium
With the fix in #2603 this hack got one step closer to merge.
I rebased it to the latest master and reverted/commented out, a workaround to counter for the base notes hanging which got especially annoying at higher randomness. With #2603 this is fixed but not for the single streamed instruments. The workaround unfortunately leaves lb302 totally untouched by the effect. There is a muted lb302 track in the BBTrack of the demo.
arpextendeddemo.mmp.zip

@Fastigium
Copy link
Contributor

@zonkmachine Woah there, I'm not sure I get everything you're saying/asking. Let me see...
You're talking about your new Skip function, right? I'm actually not sure why #2603 fixed the base notes hanging for certain instruments. Normally, the NotePlayHandle that was originally sent to an instrument with stacking/arpeggio enabled gets muted because it gets child NotePlayHandles. You can see this here. NotePlayHandle::isMasterNote() returns true if the NotePlayHandle has subnotes or has had them in the past.
So that means that if you skip the first arpeggio note, the original note (base note?) does not get muted that way. Perhaps it gets muted some other way for non-single-streamed instruments now? Weird stuff.

As for the muted lb302 track in the BBTrack, I can't seem to find it. The B&B Editor only shows a kick and a snare, and the lb302 track under the BBTrack is not muted. That one does leave the original NotePlayHandle hanging though, probably for the reason I outlined above. To prevent it, you could add a method setMasterNote() to NotePlayHandle that sets m_hadChildren to true. That'd allow you to mute the base note without actually instantiating a child note.

@zonkmachine zonkmachine changed the title [WIP] Arpeggiator extended Arpeggiator extended Sep 7, 2016
@zonkmachine
Copy link
Member Author

zonkmachine commented Sep 7, 2016

@Fastigium Thank you so very much! I'm sorry for not replying earlier but I fell out of coding a bit. My post wasn't very clear and the demo project was ill labelled, but you managed to get what I was after.
Anyway, this was precisely what I needed to stomp out the hanging notes bug and by doing so finishing a coding project that's been following me for over two years.

To prevent it, you could add a method setMasterNote() to NotePlayHandle that sets m_hadChildren to true. That'd allow you to mute the base note without actually instantiating a child note.

Worked like a charm... together with setUsesBuffer( false );

@LMMS/developers The new arpeggiator functions for randomisation skip and miss are now ready for testing. Also, a new range limiting function is under development.

@zonkmachine zonkmachine force-pushed the arpeggiator-extended-squashed branch 2 times, most recently from abbb028 to 175dd42 Compare September 8, 2016 13:33
@zonkmachine zonkmachine modified the milestones: 1.2.0, 1.3.0 Sep 8, 2016
@zonkmachine
Copy link
Member Author

Since the 'hanging note bug' is fixed I'm now switching this back to 1.2

if( m_arpSkipModel.value() )
{

if( 101 * ( (float) rand() / (float) RAND_MAX ) < m_arpSkipModel.value() )
Copy link
Member

Choose a reason for hiding this comment

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

Why 101 * r / RM < skip instead of 100 * r / (RM + 1) < skip? 100% skip rate should skip always.

Copy link
Member Author

@zonkmachine zonkmachine Sep 9, 2016

Choose a reason for hiding this comment

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

Yeah. I was trying to find a more interesting response from the knobs. I've reverted to the same algorithm for both functions. Keep it simple, etc...

100% skip rate should skip always.

You're right, and i promised full amnesia.

tr( "The skip function will make the arpeggiator pause one step "
"randomly. From it's start in full counter clockwise "
"position and no effect it will gradually progress to "
"more or less full amnesia at maximum setting.") );
Copy link
Member

Choose a reason for hiding this comment

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

more or less should not be necessary.

@zonkmachine zonkmachine force-pushed the arpeggiator-extended-squashed branch 2 times, most recently from 6277410 to e7a2dd2 Compare September 9, 2016 02:20
@zonkmachine
Copy link
Member Author

@jasp00 I've complied with all of your suggestions. 🐶

@@ -137,6 +139,22 @@ InstrumentFunctionArpeggioView::InstrumentFunctionArpeggioView( InstrumentFuncti
"number of octaves." ) );


m_arpSkipKnob->setLabel( tr( "SKIP" ) );
m_arpSkipKnob->setHintText( tr( "Skip rate:" ) + " ", " " + tr( "%" ) );
Copy link
Member

Choose a reason for hiding this comment

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

According to the general style, there is no space before %.

Copy link
Member

Choose a reason for hiding this comment

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

The description is trimmed, so there is no need for a space after tr( "Skip rate:" ).

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed! I have not clue as to what precisely I was thinking when laying out the text. The Gate knob already has the layout I needed?

@jasp00 jasp00 merged commit a4f5834 into LMMS:master Sep 10, 2016
@zonkmachine
Copy link
Member Author

🎉

Thanks for meticulous scrutiny and merge!

@zonkmachine zonkmachine deleted the arpeggiator-extended-squashed branch September 10, 2016 03:43
sambler added a commit to sambler/lmms that referenced this pull request Sep 19, 2016
* master: (213 commits)
  Update Pattern and AutomationPattern length (LMMS#3037)
  Refresh i18n strings
  Hint text update
  Drop notes with length zero (LMMS#3031)
  Background tweak
  Background
  Update Flanger
  Exclude .ts files from the Github linguist
  Redesign Multitap echo (LMMS#3008)
  Update i18n source strings
  Extended arpeggiator functions (LMMS#2130)
  Fix sample track playback in BB tracks (LMMS#3023)
  Sort plug-in embedded resources (LMMS#3014)
  Implement version major.minor.release-stage.build (LMMS#3011)
  Fix regressions on loading broken projects (LMMS#3013)
  Improved file input validation. (LMMS#2523)
  Fix sample track view in BB editor (LMMS#3002)
  Request change in model when dropping a track (LMMS#3000)
  Add LocklessAllocator and use it in LocklessList (LMMS#2998)
  Drop forceStep in AutomatableModel (LMMS#3010)
  ...
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
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.

4 participants