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

Add useful pattern selection behavior for performing. #897

Merged
merged 35 commits into from Jan 21, 2024

Conversation

daslyfe
Copy link
Collaborator

@daslyfe daslyfe commented Jan 8, 2024

Selected pattern will show in the window, and the selected pattern button will be highlighted.
Selected pattern will not play automatically.
on Evaluated, selected pattern will become the active pattern.
Active pattern button will have an outline around it when playing.

Additional change:
Added setCps(1) to all of the example files to support the change in behavior, so they will be set to the proper cps when shuffled. I think this will actually help new users as how to change the speed is not immediately apparent. I can find another way around this if this is not an acceptable change.

Screenshot 2024-01-08 at 12 24 19 AM

@daslyfe
Copy link
Collaborator Author

daslyfe commented Jan 8, 2024

It looks like the tests did not like the setCps() function added to the examples, is there a way around this, or should I just remove it?

@felixroos
Copy link
Collaborator

good one! it should be noted that there are a lot of edge cases around the pattern tab that have to be tested manually (mainly to make sure the previously opened pattern is not accidentally overwritten when loading another one)

It looks like the tests did not like the setCps() function added to the examples, is there a way around this, or should I just remove it?

all functions used in tests have to be present in test/runtime.mjs (many are just stubs, as not relevant for testing hap output only). But I don't think the setCps is needed, as shuffling will reset the cps to 1 anyway:

const handleShuffle = async () => {
// window.postMessage('strudel-shuffle');
const { code, name } = getRandomTune();
logger(`[repl] ✨ loading random tune "${name}"`);
setActivePattern(name);
clearCanvas();
resetLoadedSounds();
editorRef.current.repl.setCps(1);

same for handleUpdate, which is used by the patterns tab:

const handleUpdate = async (newCode, reset = false) => {
if (reset) {
clearCanvas();
resetLoadedSounds();
editorRef.current.repl.setCps(1);

@daslyfe
Copy link
Collaborator Author

daslyfe commented Jan 8, 2024

all functions used in tests have to be present in test/runtime.mjs (many are just stubs, as not relevant for testing hap output only). But I don't think the setCps is needed, as shuffling will reset the cps to 1 anyway:

Okay I made it so that clicking on an example pattern will run reset, and reversed changes on the example patterns

Regarding the edge cases causing overwriting patterns, yes I discovered that as I was working on this feature :o I believe I solved for them all after a lot of manual testing

@felixroos
Copy link
Collaborator

fixed some merge conflicts after #891

@felixroos
Copy link
Collaborator

just tested it and it's super handy! I've noticed some minor things:

  • when selecting the playing pattern again (after selecting another one), the highlighting does not work anymore until the next eval
  • the selection color of some themes creates a low contrast with the black/white font of the pattern tab

I don't think these are very critical and they might be fixed in a future PR.

There is now also #910 which probably conflicts with this one.

@daslyfe
Copy link
Collaborator Author

daslyfe commented Jan 19, 2024

Screenshot 2024-01-19 at 12 10 25 AM
@felixroos made some updates to incorporate your latest changes to the patterns tab. Also made all pattern UI consistent between different types of patterns, and organized/simplified the logic. This is ready for review

@felixroos
Copy link
Collaborator

haven't fully tested everything but some things i've noticed:

  • getting alot of things labeled "Invalid Date", e.g. "toplap-fromscratch: Invalid Date by Anonymous". Maybe happens when there is no metadata?
  • my pattern list is now very long, maybe it makes sense to not scroll the selected pattern title out of view? + selecting a pattern from Featured / Last Creations / Stock Examples could also be shown at the top, just without a delete button. The user patterns could then also be labeled like "My Patterns" or whatever
  • maaybe the user pattern list could also be limited to 20 with a "show more..." link
image

@felixroos
Copy link
Collaborator

another idea: maybe the types of patterns could be organized into a secondary tab bar, like sounds:

image

so there could be examples | featured | latest | user

@daslyfe
Copy link
Collaborator Author

daslyfe commented Jan 20, 2024

another idea: maybe the types of patterns could be organized into a secondary tab bar, like sounds:

image so there could be examples | featured | latest | user

I like that idea, I’ll give it a shot

@daslyfe
Copy link
Collaborator Author

daslyfe commented Jan 20, 2024

Maybe we do not need the examples displayed now that we have community patterns?

we could also split it into community and user

@felixroos
Copy link
Collaborator

Maybe we do not need the examples displayed now that we have community patterns?

we could also split it into community and user

yap examples should go.. the question is what shuffle does then

@daslyfe
Copy link
Collaborator Author

daslyfe commented Jan 20, 2024

@felixroos I updated it so the UI is consistent with the sounds tab. Shuffle still loads an example pattern, I think thats probably fine for now.

@daslyfe
Copy link
Collaborator Author

daslyfe commented Jan 20, 2024

maybe someday "shuffle" could autogenerate something interesting using math and music theory :)

@daslyfe daslyfe merged commit 2cc428e into tidalcycles:main Jan 21, 2024
1 check passed
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