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

added crow cv/just friends ii output #1

Merged
merged 1 commit into from Jan 9, 2021
Merged

Conversation

@swbain
Copy link
Contributor

@swbain swbain commented Jan 8, 2021

essentially just copied what @tehn did in this commit in awake: tehn/awake@e941cb9

@markwheeler
Copy link
Owner

@markwheeler markwheeler commented Jan 9, 2021

Hi, I'm unable to test this here but looks good. Much appreciated! :)

@markwheeler markwheeler merged commit 7b6d108 into markwheeler:master Jan 9, 2021
@markwheeler
Copy link
Owner

@markwheeler markwheeler commented Jan 9, 2021

@swbain actually one question on this: is it correct that the additional code within the output action is not part of the all_notes_kill() method? It shouldn't be executed when playback is stopped for example?

@swbain
Copy link
Contributor Author

@swbain swbain commented Jan 9, 2021

@markwheeler no problem! i've been really loving using loom with my new just friends module - many thanks for the work you've put into this. i haven't seen any issues so far but if i (or anyone else) come across bugs i'll patch and put up a PR. sometime in the next few days, i'll likely experiment with adding clocking via crow input.

i think moving that code into all_notes_kill() would result in some unnecessary method calls -- that code essentially just prepares crow/just friends to receive note events. crow and just friends don't really have the concept of turning a note off, so calling these setup methods outside of the output action when the output is changed doesn't seem needed, although it probably wouldn't introduce any noticeable issues.

@markwheeler
Copy link
Owner

@markwheeler markwheeler commented Jan 9, 2021

Cool makes sense, thanks for the explanation!

Re clocking, you'll notice that Loom is still using the beat_clock system – it'd probably make sense to move it over to the newer clock approach to better integrate/sync but it's not something I've looked into.

@swbain
Copy link
Contributor Author

@swbain swbain commented Jan 9, 2021

good to know, i'll keep that in mind as i start to dig in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants