-
Notifications
You must be signed in to change notification settings - Fork 227
Midimap+ #1284
base: main
Are you sure you want to change the base?
Midimap+ #1284
Conversation
great, yes that was still missing from the midimap PR. so i guess we discuss / merge #1244 first, before looking at this one? |
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)) { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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?
Its been a while. 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. |
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.
This PR includes updates from #1244