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

Feat/jazzkeys keys config #143

Merged
merged 6 commits into from
Apr 29, 2024
Merged

Conversation

jmolero
Copy link
Contributor

@jmolero jmolero commented Apr 7, 2024

These changes should allow to set the jazzkeys keys in the config. It also has a fix for an issue I had after doing the changes, where the velocity/octaves were changing if holding the key, which made it difficult to use because it was skipping the values too fast.

@laamaa Can you take a look and see if it works ok for you? I tried to keep the previous numpad keys as the default in the config.

I think the readme could be updated if merging this feature.

Thanks!

jmolero added 4 commits March 22, 2024 21:50
Add the four jazzkeys to the config file and uses them to make changes
in the velocity and octave when in jazzkeys mode.
It works better if the changes only occur once per key press.
@laamaa
Copy link
Owner

laamaa commented Apr 10, 2024

Thanks! The config stuff is a good addition. If repeating keys are a problem. would it be simpler to just check for the "repeat" variable in the triggering input event or use SDL_Keyup (that might feel a little weird though)? https://wiki.libsdl.org/SDL2/SDL_KeyboardEvent

jmolero added 2 commits April 23, 2024 18:00
This is much simpler than the previous implementation with custom
variables used as semaphors.
@jmolero
Copy link
Contributor Author

jmolero commented Apr 23, 2024

Yes, much simpler! I don't know much about SDL2 and didn't know about the repeat in the event, thanks for pointing it out. I have added a refactor commit that skips the repeat and key up to avoid the issue when holding the key.

@laamaa laamaa merged commit abda0e3 into laamaa:main Apr 29, 2024
4 checks passed
@laamaa
Copy link
Owner

laamaa commented Apr 29, 2024

Looks good, thank you!

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