-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
feat(playback): add random variation on each keypress (#24) #44
Conversation
Codecov ReportAttention: ❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
📢 Thoughts on this report? Let us know!. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks pretty good! I tried it out and works as expected. Also, left a comment about changing the pitch in #24
Feel free to rebase main
and update docs when you have time.
cc @arda-guler for testing / finding the most sane defaults.
Yeah, well, regarding that "finding the most sane defaults" thing and the pitch implementation... The indirect implementation here by changing the speed is more than adequate enough IMO. In fact, I'm going to say that as far as my own opinion goes, a 0.1 volume variation and a 0.05 tempo variation sounds so much better - the current example values are a bit too pronounced. It should be a little more subtle. All else is very good! |
8be049f
to
80421d7
Compare
d9ec43e
to
53fae1d
Compare
53fae1d
to
3bc667f
Compare
Sorry for the git history confusion... Forgot to sign and messed up fixing it... 🤦 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff!
Description of change
Adds the possibility of random variation for each keypress
I added some values to the default preset. Maybe play around with those to find sensible defaults. ✔️
Closes #24
Configuration
I added configuration possibilities in 3 places:
KeyConfig
in Preset. These will override settings defined in the parent Preset.KeyConfig
s that do not provide variation.Currently the cli only accepts one value, that is used for both changes up and down, while the config file takes separate values. I am looking into a convenient way to allow both for the cli as well.The cli accepts (up to) two comma separated values. If only one is defined it is used for both.
How has this been tested? (if applicable)