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

Release logic (multiple notes for release=trigger + rt_dead handling) #324

Merged
merged 8 commits into from
Aug 9, 2020

Conversation

paulfd
Copy link
Member

@paulfd paulfd commented Jul 19, 2020

Closes: #299

@paulfd paulfd requested a review from jpcima July 19, 2020 21:11
@paulfd
Copy link
Member Author

paulfd commented Jul 19, 2020

At this point maybe it would be more "logical" to let the Region object be purely a data storage, and handle all the changes of the "status" of each regions within the Synth (i.e. instead of having the registerNoteOn and co. change the internal private status of the regions, all status variable are public and handled by the synth). What do you think?

src/sfizz/Region.cpp Outdated Show resolved Hide resolved
@jpcima
Copy link
Collaborator

jpcima commented Jul 26, 2020

change the internal private status of the regions, all status variable are public and handled by the synth). What do you think?

Without a doubt yes. This helps to manage the complexity a bit as the software grows.
As source organization is concerned, there is an increasing number of sources at the root level which are purely utility, I would like these to be placed in a subdirectory preferably.

@paulfd
Copy link
Member Author

paulfd commented Aug 2, 2020

change the internal private status of the regions, all status variable are public and handled by the synth). What do you think?

Without a doubt yes. This helps to manage the complexity a bit as the software grows.
As source organization is concerned, there is an increasing number of sources at the root level which are purely utility, I would like these to be placed in a subdirectory preferably.

Sure and sure. I'll defer the passive region in another pull request, and feel free to reorganize the sources 🙂

@paulfd paulfd force-pushed the release-multiple-voices branch from 0ea2789 to cb367d3 Compare August 9, 2020 15:58
@paulfd paulfd changed the title If the region spans multiple keys they can all fire on pedal up Release logic (multiple notes for release=trigger + rt_dead handling) Aug 9, 2020
@paulfd
Copy link
Member Author

paulfd commented Aug 9, 2020

I added tests and support for the rt_dead opcode. The behavior now matches sforzando/dimension, and the trigger=release regions will only sound if there is a corresponding attack sample, corresponding meaning compatible in the key and velocity range.

The architecture is not entirely satisfactory but I will defer overhauling the activations/dispatching in another PR. The tests added in this one will help ensure nothing breaks, and I will add a more comprehensive set in the upcoming PR as this is relatively easy to test programmatically.

@paulfd paulfd requested a review from jpcima August 9, 2020 18:03
@paulfd paulfd merged commit 20bc1e9 into sfztools:develop Aug 9, 2020
@paulfd paulfd deleted the release-multiple-voices branch August 9, 2020 20:18
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.

Finer release behavior
2 participants