Closed rocketinventor closed 7 years ago
@fulldecent If there's anything that needs to be changed in order for this to get merged, please let me know.
Everything looks good, thank you!
Has anyone else also reviewed this PR?
@fulldecent If you know how to contact whomever it was that tested out the original js code (with positive results), that would be great. I have not changed the code of the block/function that generates the tones (just moved it around) and everything else has been double-checked and tested, so in theory, there's no reason why it shouldn't work.
However since the nature of this project is that it exploits an arguably negative side effect of a certain runtime, rather than an official (or even unofficial) API, it is quite likely that the behavior will be temperamental and will require some finesse to get right.
As usual, thanks for timely responses and the opportunity to be a part of this project. Keep me updated!
@quanyang Do you think you could review this commit?
I put together my same setup as before and can confirm efficacy. I added a note for Chrome users -- Chrome blocks workers when using file://.
Thank you!
This pull request includes multiple improvements to the JS version, with the main improvement being performance. Before, the page would freeze/lock up (become unresponsive) when the script was working. This has been fixed. This also means that the page status will update in real-time. (Before, it only updated after the script finished running.) Because it is now possible for buttons to be clicked during operation, song playback can be stopped midway through. Song recursion should also be possible now because of this but, has not yet been implemented.
These performance improvements were possible due to the implementation of Web Workers (script running in the background). If anyone can test this build out, that would be great!
For some reason, Github is including already merged commits in this Pull request (why?).