8000 Midimap+ by nkymut · Pull Request #1284 · tidalcycles/strudel · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Jun 19, 2025. It is now read-only.

Midimap+ #1284

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Midimap+ #1284

wants to merge 5 commits into from

Conversation

nkymut
Copy link
Contributor
@nkymut nkymut commented Feb 9, 2025

This update enables midimaps to register control names that are not limited to Strudel's controlNames.
This provides greater flexibility in customization while maintaining compatibility with existing mappings.
However, further consideration is needed regarding potential namespace pollution.

midimaps({ nsx: { gain: 7, 
                  pan:10, 
                  modulate:1, 
                  express:11, 
                  sustain:64, 
                  reverb:91,
                  chorus:93,
                  variation:94  } })
           

This PR includes updates from #1244

@felixroos
Copy link
Collaborator

great, yes that was still missing from the midimap PR. so i guess we discuss / merge #1244 first, before looking at this one?

@nkymut
Copy link
Contributor Author
nkymut commented Feb 9, 2025

great, yes that was still missing from the midimap PR. so i guess we discuss / merge #1244 first, before looking at this one?

Yup, #1244 is good to go. And we can also look at if we map non-cc messages.

@felixroos
Copy link
Collaborator

as #1244 is now (finally) merged, we could go with this next.. @nkymut could you maybe merge main into this one? it's a bit hard to see changes otherwise

@nkymut
Copy link
Contributor Author
nkymut commented Mar 22, 2025

Hi @felixroos thank you! I have merged the main.

const controlName = getControlName(key);

// Register the control in the midicontrolMap if it doesn't exist in
if (!midicontrolMap.has(controlName)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

what if the control name exists already? then we shouldn't register it i think. when doing so, we might override it in a way that breaks it. for example, some controls are allowed to set multiple controls, like cutoff:resonance:lpenv, which would probably break when calling registerControl('cutoff'), as things like note("g1").cutoff("200:8:4") wouldn't set resonance and lpenv anymore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's what I meant by namespace pollution.
I think an easier solution would be to add a prefix like mm_ to distinguish the names.
Alternatively, it could just throw an error if controlAlias.has(controlName) returns true, and prompt the user to choose a different control name.

Copy link
Collaborator
@felixroos felixroos Mar 26, 2025

Choose a reason for hiding this comment

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

why not reuse existing names? one idea behind these midimaps was that you could easily play a non-midi pattern with your midi device without changing control names (e.g. lpf maps to your synths cutoff cc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, in that case how about registering a selective control function that checks whether hap.value.midi exists, and then decides whether to use the original lpf or the midi-mapped lpf? Or is there a better way to tell if the current trigger is to MIDI, SuperDough, or else?

@nkymut
Copy link
Contributor Author
nkymut commented May 18, 2025

Its been a while.
As an interim fix, a warning is now shown when midimap overwrites a Strudel API method.

I've tried Throwing an error, but I find it too restrictive. For long-term flexibility—especially to allow smooth switching between MIDI and non-MIDI in the same code—it may be better to permit explicit destructive changes.

It would be more ideal if we could switch controlAlias based on output modes (midi, OSC, superDough) and active midimap, since the root cause is that all Pattern.prototypes are shared.

And currently It is hard to restore overwritten Strudel API methods to their default state beside refreshing the page.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0