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

Auto play songs as keys are mashed #28

Closed
pserwylo opened this issue May 24, 2021 · 5 comments · Fixed by #54
Closed

Auto play songs as keys are mashed #28

pserwylo opened this issue May 24, 2021 · 5 comments · Fixed by #54

Comments

@pserwylo
Copy link
Collaborator

Closely related to #4, but opening a new ticket as this is a little more narrow in focus.

We have a battery powered toy at home which has 5 keys on it. It can be used in one of two modes. Normal mode, where pressing a key corresponds to a specific note as one would expect, and lets call it "Song mode", where pressing any key will always play the next note in a particular song.

So if the song was Twinkle Twinkle Little Star, you could mash any key you like, and no matter which order you play the keys in, it will always play the song. The only thing you need to worry about is the tempo at which you play the keys. Once a particular song is finished, it queues up the next song.

Little players still get the benefit of pressing keys, seeing them light up, and hearing music in response. They don't get to use it in order to learn how to play songs on a real piano in this mode.

I feel this may be a bit simpler than #4 to implement, because there is no further UI required (other than another option on the settings screen). The main screen works exactly as is.

Thoughts anyone?

@nicolasbrailo
Copy link
Owner

This feels a lot more doable than #4! I think it should be possible to subclass the Piano class and override the play_sound method to ignore the key index, and do instead play(sound_path[current_note++]). Some refactoring around PianoCanvas should be needed, so Piano isn't hardcoded anymore (which sounds like a good idea anyway). Ideally a melody would be stored as an asset too, but I guess we can live with an ugly hardcoded uint-array as a minimum viable prototype as well.

My only concern here would be the limited number of notes we have (23?) - is that enough for a simple melody, or do we need to extend the sound set a lot more to play something meaningful?

@nicolasbrailo
Copy link
Owner

Pushed a branch with an example of how this may work: https://github.com/nicolasbrailo/PianOli/tree/prototype_playamelody

@pserwylo
Copy link
Collaborator Author

Looking great. I've taken the liberty of building on this a little, and added a few songs:

https://github.com/pserwylo/PianOli/tree/prototype_playmelody_songs

Also added a few songs and a mapping from musical notation to the keys we have available.

Still need to add preferences to enable this feature (thinking of replacing the current settings with a more typical Android settings screen. Thoughts? It does lose a little personality whenever you do this, but adds some functionality. Happy to put the toggle in the existing settings if you prefer.

@nicolasbrailo
Copy link
Owner

Awesome diff, looking really good! One thought: if you just mash the keys it doesn't really form a melody, it just creates noise. We'd need to have some kind of minimum wait time between notes to make it sound like an actual melody. Is that preferable, or is the goal teaching babies to maintain some sort of tempo? (I don't have an opinion either way)

thinking of replacing the current settings with a more typical Android settings screen

Can we keep the banner that directs people to github? That's probably the only source of bug reports we get (also... TBH, I kind of like it, it's sort of 'my' signature in the app :))

@pserwylo
Copy link
Collaborator Author

Is that preferable, or is the goal teaching babies to maintain some sort of tempo? (I don't have an opinion either way)

Also have no preference. Obviously easier to implement without tempo, maybe push that out, and then try adding the tempo afterwards if it seems like a win.

To be honest, I sometimes feel like the melodies are for us, not the kids ;)

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 a pull request may close this issue.

2 participants