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

change fanchor to 0 #1107

Merged
merged 5 commits into from
May 31, 2024
Merged

change fanchor to 0 #1107

merged 5 commits into from
May 31, 2024

Conversation

daslyfe
Copy link
Collaborator

@daslyfe daslyfe commented May 19, 2024

  • kinda breaking change: will change patterns without the fanchor set using fenv to be a bit brighter
  • It's time to make this change, it is a source of confusion when giving workshops why the filter env behaves in a bipolar fashion by default. Imagine strumming an instrument harder, and the harmonics fading faster. Bipolar envelopes can also cause clicks if the cutoff dips too low.

@daslyfe daslyfe requested a review from felixroos May 19, 2024 17:31
@felixroos
Copy link
Collaborator

Agreed that 0 is a better default!
It would be handy to have a quick way to repair old patterns. fanchor isn't even documented, so basically all patterns using a filter envelope will sound different. maybe there could be something like setDefault('fanchor', 0.5) .
In superdough, all default values can then be set like { fanchor = getDefault('fanchor') }

@daslyfe
Copy link
Collaborator Author

daslyfe commented May 20, 2024

Agreed that 0 is a better default! It would be handy to have a quick way to repair old patterns. fanchor isn't even documented, so basically all patterns using a filter envelope will sound different. maybe there could be something like setDefault('fanchor', 0.5) . In superdough, all default values can then be set like { fanchor = getDefault('fanchor') }

Cool sounds good :) I added the setDefault function. Im not sure how to go through the db and modify existing patterns though to add the old default

@felixroos
Copy link
Collaborator

looks good, thanks for adding! I think the last thing missing is something like resetDefaults, which is called in resetEditor, so defaults from other patterns won't interfere when switching patterns.
For the database, I can try to cook up an SQL query to add setDefaullt?.('fanchor', 0.5) to any pattern that includes then word lpenv

@daslyfe
Copy link
Collaborator Author

daslyfe commented May 21, 2024

What if we had a version tag at the top of the editor? like // @ver 2.15
That way we could have different defaults for different versions if needed and we would not need to update the DB

@felixroos
Copy link
Collaborator

What if we had a version tag at the top of the editor? like // @ver 2.15 That way we could have different defaults for different versions if needed and we would not need to update the DB

and no version tag would mean the old defaults are used? not sure if it's good to make the thing we want to standardize opt-in, rather than opt out.. kind of related: #998 (comment)

@felixroos
Copy link
Collaborator

continuing now here, because discord is too ephemeral: yep opt-out would also work with the version tag.. so if no version tag is present, use current defaults. if there is one, then use the defaults for that version.

}
export function setVersionDefaults(version) {
resetDefaultValues();
if (version === '1.0') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe like this? we might move this to core later as it could be useful for more general feature flagging

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, this makes sense to me 💯

@felixroos
Copy link
Collaborator

fixes #1055

@daslyfe
Copy link
Collaborator Author

daslyfe commented May 31, 2024

I'm guessing that you want to do the DB operation for adding the version tag before committing this?

@felixroos
Copy link
Collaborator

I'm guessing that you want to do the DB operation for adding the version tag before committing this?

ok, merging now then quickly running the db update when it's deployed

@felixroos felixroos merged commit 2cda78d into tidalcycles:main May 31, 2024
1 check passed
@felixroos
Copy link
Collaborator

db fix is out, i've added // @Version 1.0 to the bottom of each pattern in the db. personal patterns might still be off, so I guess we can mention that in the changelog for 1.1 that you can add the version tag to get the og version

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 this pull request may close these issues.

2 participants