tidalcycles / strudel

Web-based environment for live coding algorithmic patterns, incorporating a faithful port of TidalCycles to JavaScript
https://strudel.cc/
GNU Affero General Public License v3.0
579 stars 105 forks source link

osc: couple of fixes #1093

Closed kasparsj closed 1 month ago

kasparsj commented 1 month ago
yaxu commented 1 month ago

Hi @kasparsj, thanks for all this!

I assume you mean createParam not registerParam which I couldn't find.

registerControl and createParam have different names but seem to do the same thing, besides registerControl accepting additional parameters for aliases. A bit confusing!

I propose renaming the current createParam to _createParam, the current registerControl to createParam, and only exporting the latter.

felixroos commented 1 month ago

The changes in this PR look fine to me!

Regarding registerControl, I would handle potential renamings in a different PR. Let me try to come up with how this came to be:

registerControl was thought to be an internal function similar to register (for functions), registerSound, registerWidget etc.. createParam and its sibling createParams existed before registerControl was added and was thought to be a function in "user land", also documented here. The difference between createParams and registerControl is that the former creates multiple params at once and the latter creates only one, but with aliases (if you pass more than one argument). Aliases allow multiple names to set a single property, e.g. registerControl('attack', 'att') creates attack and att functions / Pattern methods, but for both sets attack in the Hap.value. This mechanism is useful to avoid always needing to handle all aliases in the actual logic and instead just having a single name to handle.

I propose renaming the current createParam to _createParam, the current registerControl to createParam, and only exporting the latter.

I think it makes sense to keep registerControl to keep the registerX naming scheme + to avoid needing to refactor all internal registrations.

So maybe:

createParam -> _registerControl createParams -> registerControls registerControl -> keep

As a bonus, here's the verbose Java-style version:

createParam -> registerControl createParams -> registerControls registerControl -> registerControlWithAliases

Another bonus, the version where param wins over control but we still use the register verb:

createParam -> _registerParam createParams -> registerParams registerControl -> registerParam

The file is still called controls.mjs though.. I think variant 1 is the best

yaxu commented 1 month ago

Yes moving to registerX is a good plan, and settling on registerControl is fine by me.

While we're moving things round, I'm unsure whether it would be clearer to move from registerControl(names, ...aliases) to registerControl(names, aliases)?

Then registerControls could just be a basic mapping that accepts registerControls([names,aliases],[names,aliases])

yaxu commented 1 month ago

Thanks @kasparsj ! Lets continue discussion about renaming etc on issue #1098