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

Add lock checks for rest of methods and fix usage of Promise polyfill (#992) #995

Merged

Conversation

hansagames
Copy link

Fix me if I am wrong but I think that all methods should be added to the queue if promise is not resolved yet. Also added extra check to fix usage of non native Promises.

Please review @goldfire and @dmednis as you made initial lock fix

Copy link
Contributor

@dmednis dmednis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably rename the lock variable to something more generic since it now also locks the rest of the functions. But otherwise seems legit. 👍

@goldfire
Copy link
Owner

Huh, that is really odd. I'm using the same version of Chrome and macOS as you, but the play instanceof Promise check works for me (and thus the error doesn't happen). It seems like the promise change is the main thing needed, but I'm sure there are edge-cases that the lock would fix. Since the lock is still only waiting for the play call, I think that name still works fine.

@goldfire goldfire merged commit 0d8d947 into goldfire:master Jul 12, 2018
@hansagames hansagames deleted the fix-promise-based-sounds-#992 branch July 13, 2018 05:55
ringcrl pushed a commit to ringcrl/howler.js that referenced this pull request Apr 21, 2019
…ed-sounds-#992

Add lock checks for rest of methods and fix usage of Promise polyfill (goldfire#992)
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.

None yet

3 participants