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

Removing SoundManager2 dependency #87

Merged
merged 1 commit into from
Aug 17, 2016
Merged

Removing SoundManager2 dependency #87

merged 1 commit into from
Aug 17, 2016

Conversation

gregor
Copy link
Contributor

@gregor gregor commented Aug 11, 2016

No description provided.

@bennycode
Copy link
Contributor

@mythsunwind and @Yserz are thinking about writing tests which can read from the JS dev console if a sound started to play. So there needs to be a test which checks for this line:
@logger.log @logger.levels.INFO, "Playing sound '#{audio_id}'", audio

@gregor
Copy link
Contributor Author

gregor commented Aug 17, 2016

Please take another look. Unit tests have been added. Positive cases are x'ed out due to karma-runner/karma#2320.

"sinon": "1.17.4"
"load-grunt-tasks": "3.5.2",
"request": "2.74.0",
"sinon": "1.17.5"
Copy link
Contributor

Choose a reason for hiding this comment

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

scary :D

amplify.subscribe z.event.WebApp.AUDIO.PLAY_IN_LOOP, @, @_play_in_loop
amplify.subscribe z.event.WebApp.AUDIO.STOP, @, @_stop
init: (skip_preloading = false) =>
@_init_sounds()
Copy link
Contributor

Choose a reason for hiding this comment

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

I would vote for preload with default value false

@gregor
Copy link
Contributor Author

gregor commented Aug 17, 2016

@mythsunwind To simplify the logging the final message to check for is Playing sound '{audio_id}' (in loop: '#{true|false}')

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.

4 participants