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

Music Keyboard has a lot of regressions. #2839

Closed
ricknjacky opened this issue Feb 12, 2021 · 5 comments
Closed

Music Keyboard has a lot of regressions. #2839

ricknjacky opened this issue Feb 12, 2021 · 5 comments

Comments

@ricknjacky
Copy link
Member

In the thread at #2826 @walterbender pointed out one of the regressions that has been there in MusicKeyBoard all this while:-

1) Adding pitch/hertz erratic behavior described in detail here.

However there are many other regressions that have gone un-noticed.

2) If we add a pitch/hertz block, and ignore the regression described in 1), close the widget and reopen it. There are new unwanted keys that get added before actual keys present in clamp blocks.

3) The new keys added produce a lot of errors. They also don't generate any sound.

Notice the regression in action below:-

Regression_MKB.mp4

4) As we saw in the video above, there was no sound in unwanted keys. But, master branch of MusicBlocks in addition to reproducing 2) and 3) does produce sound and that is erratic in itself. One of the key on being played goes on to play forever. Even after moving the widget to trash.

Major_Regression.mp4

All in all, widget has some major regressions that need to be addressed.

@ricknjacky ricknjacky changed the title Music Keyboard has ton a lot of regressions. Music Keyboard has a lot of regressions. Feb 12, 2021
@ricknjacky
Copy link
Member Author

new block is getting connected to blocks in the clamp but isn't joining them in the clamp blocks.. something is not right.

MBRegression.mp4

@ricknjacky
Copy link
Member Author

@walterbender there seems to be one more regression that was in commits that have solved most of the above mentioned regressions.

Adding a new pitch back to back generates erratic behavior. The clamp just freezes upon doing so.

mbregression.mp4

Any heads up on what might be causing this?

@ksraj123
Copy link
Member

ksraj123 commented Feb 17, 2021

Hey @ricknjacky, Thanks for pointing this out. Apparently there was another regression with adding pitch blocks consecutively i.e. the same pitch in the next octave got added easy time we added a pitch.

The video below shows behaviour before the changes. Please notice sol 5 gets added and then sol 6 which is not the correct behaviour. I think both of these were present prior to the recent changes but could'nt be noticed as the blocks were not aligning properly.

Screen.Recording.2021-02-17.at.10.22.49.PM.mov

Turns out, the fix for both these issues was simple, I have opened a PR #2849 fixing these. Thanks

@ricknjacky
Copy link
Member Author

A minor fix, @ksraj123 has proposed the same in 60737e , issuing a PR

@walterbender
Copy link
Member

I think these are all addressed now by #2849

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

No branches or pull requests

3 participants