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

The csound output does not trigger the onTrigger pattern #658

Closed
gogins opened this issue Aug 9, 2023 · 13 comments · Fixed by #660
Closed

The csound output does not trigger the onTrigger pattern #658

gogins opened this issue Aug 9, 2023 · 13 comments · Fixed by #660

Comments

@gogins
Copy link
Contributor

gogins commented Aug 9, 2023

The following Strudel patch can be pasted into the REPL and demonstrates the issue:

csound_trigger.txt

If the csound output is enabled by uncommenting line 51, then the Csound output plays, but the onTrigger pattern is not triggered, and the generated notes do not climb in pitch. If the csound output is disabled by commenting out line 51, then the piano output plays, the onTrigger pattern is triggered, and the generated notes do climb in pitch.

@gogins gogins mentioned this issue Aug 9, 2023
13 tasks
@felixroos
Copy link
Collaborator

yes, that is to be expected, because Pattern.csound will override the .onTrigger on the hap context. having multiple triggers is not implemented yet.. but it could be done easily, by just collecting triggers in an array.

@gogins
Copy link
Contributor Author

gogins commented Aug 9, 2023

What about using a non-dominant trigger for the Csound output, as in the stateful pattern example?

@felixroos
Copy link
Collaborator

felixroos commented Aug 10, 2023

What about using a non-dominant trigger for the Csound output, as in the stateful pattern example?

it still wouldn't work to call onTrigger 2 times, as the second call will always override the "onTrigger" property of Hap.context. it might be clearer by looking at the source

edit: I did not realize that multiple triggers are possible, but only when dominant is false, so it would technically work, but the problem would then be that the defaultOutput would also trigger, which is not what we want

@felixroos
Copy link
Collaborator

hm thinking about this again, it might also work by checking the dominantTrigger of the existing hap, as an existing onTrigger will already be called https://github.com/tidalcycles/strudel/blob/main/packages/core/pattern.mjs#L787

@felixroos
Copy link
Collaborator

felixroos commented Aug 10, 2023

this is the other relevant place: https://github.com/tidalcycles/strudel/blob/main/packages/core/repl.mjs#L82
the csound output (and similar ones) would need to override the defaultOutput (which is what dominantTrigger does) but still call pre-existing triggers.. so maybe it will already be fine if

https://github.com/tidalcycles/strudel/blob/main/packages/core/pattern.mjs#L786

is changed to

 if (hap.context.onTrigger) {

the question would be if there is a use case where you would want to ignore an existing onTrigger callback..

@gogins
Copy link
Contributor Author

gogins commented Aug 10, 2023

I have implemented your suggestion in my local Strudel and it seems to work fine without messing up existing pieces in the REPL "Shuffle."

@felixroos
Copy link
Collaborator

should now be fixed. there was another weird behavior when you'd add a non-dominant trigger after a dominant one, then the default output would be activated again.

@gogins
Copy link
Contributor Author

gogins commented Aug 14, 2023

This is no longer working as I expected. The attached patch can be run to demonstrate the issue.

  • For the default output, the notes appear to be correctly computed but the piano roll jumps up and down during performance.
  • For the csound output, the notes are not correctly computed; the value of state is computed, but is not added to the notes going to Csound.

csound_trigger.txt

@felixroos
Copy link
Collaborator

the changed merged in #660 is not yet deployed so nothing should've changed, which means .csound will still overwrite your trigger. without csound, the piano roll jumping around is to be expected because you're using global state that is not integrated with the used patterns

@felixroos
Copy link
Collaborator

now it's deployed, so now the onTrigger will always get called (with or without csound being used).

@gogins
Copy link
Contributor Author

gogins commented Aug 15, 2023

I made to sure to completely clear my browser cache and application data. The deployed REPL now does play notes with the added state, both for the default output and for the Csound output. Thanks for that!

However, the pianoroll display is still jumping up and down in both cases.

@felixroos
Copy link
Collaborator

However, the pianoroll display is still jumping up and down in both cases.

yes, that is because you're using an external state variable inside the withHap callback, turning it into a non-pure function, breaking the whole pure functional programming paradigm. As said earlier, it's not a solution, it's a workaround.
The pianoroll jumping around is only one symptom, you will also not be able to use pattern methods as expected.
I'd recommend trying to solve your problem without state (if possible).

in tidal, this problem is already solved like this: https://tidalcycles.org/docs/reference/state_values/

@gogins
Copy link
Contributor Author

gogins commented Aug 15, 2023 via email

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 a pull request may close this issue.

2 participants