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

Move stuff to new register function #295

Merged
merged 16 commits into from Dec 11, 2022
Merged

Move stuff to new register function #295

merged 16 commits into from Dec 11, 2022

Conversation

felixroos
Copy link
Collaborator

getting rid of all usages of Pattern.prototype.define,Pattern.prototype.bootstrap, Pattern.patternify, patternifyN

@felixroos felixroos changed the title Refactor bootstrap Move stuff to new register function Dec 10, 2022
@felixroos felixroos requested a review from yaxu December 10, 2022 22:18
@felixroos
Copy link
Collaborator Author

refactoring is mostly done, removed all all old define calls + stuff that is no longer needed. Had to rename csound to loadCsound to avoid overwriting the new toplevel csound function. Tests still fail.. some listening / testing to do

@felixroos
Copy link
Collaborator Author

not sure how to handle this: 306004e ref #245

@felixroos
Copy link
Collaborator Author

after looking at the failed snapshot tests, I think it's safe to overwrite the snaps. There are 3 reasons why things changed:

  1. haps are reordered (can be ignored)
  • example "someCycles" example index 0 1` mismatched
  • example "sometimes" example index 0 1` mismatched
  • tune: outroMusic 1` mismatched
  • tune: belldub 1` mismatched => haps are just sorted differently
  1. voicings in different octave after refactoring (sounds the same)
  • example "voicings" example index 0 1` => different range
  • example "addVoicings" example index 0 1` mismatched => voice leading did not work at snap time
  • tune: loungeSponge 1` mismatched => 1 octave down
  • tune: festivalOfFingers 1` mismatched => 1 octave down
  • tune: caverave 1` mismatched => 1 octave down
  1. mini notation random operators were stateful
  • example "degrade" example index 1 1` mismatched => now stateless "?"
  • example "degradeBy" example index 1 1` mismatched => now stateless "?"
  • tune: arpoon 1` mismatched => now stateless "?"
  • tune: sampleDemo 1` mismatched => now stateless "|"
  • tune: delay 1` mismatched => now stateless "|"

1 seems to be normal with the new register function. it already happened in #286

2 looks like the old data had wrong octaves the whole time. I listened to all the failing examples / tunes, they sound exactly the same as before

3 comes from the fact that the mini notation had a stateful handling of random values. I would remove that behavior, as it is undeterministic and leads to bugs, e.g. #245

@felixroos felixroos marked this pull request as ready for review December 11, 2022 12:52
@yaxu
Copy link
Member

yaxu commented Dec 11, 2022

Tidal has a 'defragment' function that combines adjacent parts with the same whole and value. Along with sortHapsByPart this can be 'good enough' for comparing equivalent but differently fragmented patterns in tests.

Just having a go at porting defragment..

@felixroos
Copy link
Collaborator Author

Just having a go at porting defragment..

do you plan to add it to this PR or should we merge this one first?

@yaxu
Copy link
Member

yaxu commented Dec 11, 2022

I can do it as a separate PR

@felixroos felixroos mentioned this pull request Dec 11, 2022
@felixroos felixroos merged commit 5f37a4a into main Dec 11, 2022
@felixroos felixroos deleted the refactor-bootstrap branch December 11, 2022 20:27
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.

None yet

2 participants