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

Pedal should not be considered up at the exact moment when it goes down #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

broadwell
Copy link

We're using this in an app that plays MIDI files in sync with an interactive piano roll visualization. Overall it's working incredibly well, but we've encountered one scenario in which the app and the Piano can get into an inconsistent state: if a Piano.pedalDown() is followed immediately by a Piano.pedalUp() on the same timestamp, the false isDown() check causes the Piano.pedalUp() to terminate without changing the pedal state. The app thus assumes that the pedal is up when in fact the Piano still has it in the "down" state.

We could sidestep this scenario by rewriting our app to avoid overlapping pedal events, or by artificially incrementing the time parameter between them. But the proposed change below is the simplest, and moreover, in our view it makes the Piano's behavior a bit more logically consistent. At least, we can't see any case in which it's beneficial to consider the pedal not to be down at the very moment at which it has moved to the "down" state.

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.

1 participant