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

Add offset to offbeat retrigger effect #329

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

zacharied
Copy link

The old behavior resulted in the retriggered sample to be that of the nearest division of updatePeriod. We now add samples to the start of that such that the retriggered sample will always be the sample on which the FX hold starts.

Old incorrect behavior: https://cdn.discordapp.com/attachments/384032012249464832/708104951318183975/fx_wrong.mp4

New correct behavior: https://cdn.discordapp.com/attachments/384032012249464832/708105005101744179/usc_fx_fixed.mp4

This time around the fix doesn't break other retriggers: https://cdn.discordapp.com/attachments/384032012249464832/708105235842727946/usc_fx_same.mp4

 The old behavior resulted in the retriggered sample to be that of the
 nearest division of `updatePeriod`. We now add samples to the start
 of that such that the retriggered sample will always be the sample
 on which the FX hold starts.
@zacharied
Copy link
Author

zacharied commented Apr 21, 2021

Just checking to see if there's any chance of merging this into develop/master. If you want more testing I would be happy to assist in that. Given how common retriggers are, this fix would bring many charts closer to the sound that the charter intended.

@Drewol
Copy link
Owner

Drewol commented Apr 21, 2021

Yeah with conflict fix and I'm gonna take it to develop first because that has some newer effect processing things.

@Drewol Drewol changed the base branch from master to develop April 21, 2021 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants