← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/sound_improvements into lp:widelands

 

Review: Approve

One tiny nit - it's up to you if you want to change it. Not compiled or tested.

Diff comments:

> === modified file 'src/sound/sound_handler.cc'
> --- src/sound/sound_handler.cc	2016-10-22 18:19:22 +0000
> +++ src/sound/sound_handler.cc	2017-01-03 20:52:54 +0000
> @@ -129,6 +130,10 @@
>  		return;
>  	}
>  
> +	if (Mix_AllocateChannels(kNumMixingChannels) != kNumMixingChannels) {

I'm not a big fan of side effects in if statements - grabbing the return value in a const variable first might me more readable.

> +		initialization_error(Mix_GetError());
> +	}
> +
>  	Mix_HookMusicFinished(SoundHandler::music_finished_callback);
>  	Mix_ChannelFinished(SoundHandler::fx_finished_callback);
>  	load_system_sounds();


-- 
https://code.launchpad.net/~widelands-dev/widelands/sound_improvements/+merge/314035
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/sound_improvements.


References