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

Fix Initialize sounds (dispose of null SoundSource) #13535

Merged
merged 1 commit into from Jun 24, 2017

Conversation

Projects
None yet
3 participants
@rob-v
Contributor

rob-v commented Jun 22, 2017

Fixes #13534.

In case of missing sound, it crashed on soundSource.Dispose.
This fixes PR #13461, so this PR should be also included in next playtest.

@pchote pchote added this to the Playtest featuring updated HitShapes milestone Jun 22, 2017

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Jun 22, 2017

Member

I was only expecting this to need the null check. What do the other changes achieve?

Member

pchote commented Jun 22, 2017

I was only expecting this to need the null check. What do the other changes achieve?

@rob-v

This comment has been minimized.

Show comment
Hide comment
@rob-v

rob-v Jun 22, 2017

Contributor

I added null check also for music track, just to be sure. Other changes are cosmetic - removed = null for music variables which were already set to null in StopMusic so little clean up.

Contributor

rob-v commented Jun 22, 2017

I added null check also for music track, just to be sure. Other changes are cosmetic - removed = null for music variables which were already set to null in StopMusic so little clean up.

@reaperrr

Haven't run into #13534, but the code makes sense.

@rob-v

This comment has been minimized.

Show comment
Hide comment
@rob-v

rob-v Jun 23, 2017

Contributor

In previous Sound PR I wanted to move 4 lines from OpenAlSound ctor (called on each Play2D call) to sourcePool initialization as they overwrite the same values so it can be done only once on source generation.
https://github.com/OpenRA/OpenRA/blob/bleed/OpenRA.Platforms.Default/OpenAlSoundEngine.cs#L422

Can I add new commit to this PR?

Contributor

rob-v commented Jun 23, 2017

In previous Sound PR I wanted to move 4 lines from OpenAlSound ctor (called on each Play2D call) to sourcePool initialization as they overwrite the same values so it can be done only once on source generation.
https://github.com/OpenRA/OpenRA/blob/bleed/OpenRA.Platforms.Default/OpenAlSoundEngine.cs#L422

Can I add new commit to this PR?

@pchote

pchote approved these changes Jun 24, 2017

@pchote pchote merged commit 6df7f18 into OpenRA:bleed Jun 24, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Jun 24, 2017

Member

Can I add new commit to this PR?

New PR, please.

Member

pchote commented Jun 24, 2017

Can I add new commit to this PR?

New PR, please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment