widelands-dev team mailing list archive

[Klaus Halfmann <klaus.halfmann@xxxxxxxx>]

Review, found so far:
* restructered the sound directory wihht more subdirectories
* Help now shows --nosound            Starts the game with sound disabled.
* Using FXset one could have different soundscapes for testing or scenarions, interesting
* I assuem the random must be a _local_ random, as sounds are per player.
* More coommetns please, see inline comments

For the wishlist: a helpscreen explainng the sounds :-)

Checekd upto "TODO(GunChleoc): Compatibility code to avoid getting bug reports about unread sections. Remove after Build 21." sorry can nott do more tonight



Diff comments:

> 
> === modified file 'src/economy/road.h'
> --- src/economy/road.h	2019-02-27 17:19:00 +0000
> +++ src/economy/road.h	2019-04-09 04:52:18 +0000
> @@ -131,8 +131,7 @@
>  	void cleanup(EditorGameBase&) override;
>  
>  	void draw(uint32_t gametime,
> -	          TextToDraw draw_text,
> -	          const Vector2f& point_on_dst,
> +	          TextToDraw draw_text, const Vector2f&, const Coords&,

mmh, i like those named varibales better, Now I must guess what "const Coords&" may be used for?

>  	          float scale,
>  	          RenderTarget* dst) override;
>  
> 
> === modified file 'src/editor/editorinteractive.cc'
> --- src/editor/editorinteractive.cc	2019-02-27 17:19:00 +0000
> +++ src/editor/editorinteractive.cc	2019-04-09 04:52:18 +0000
> @@ -405,6 +405,10 @@
>  	is_painting_ = false;
>  }
>  
> +bool EditorInteractive::player_hears_field(const Widelands::Coords&) const {

Did I tell you, that I like comments?

> +	return true;
> +}
> +
>  void EditorInteractive::on_buildhelp_changed(const bool value) {
>  	toggle_buildhelp_->set_perm_pressed(value);
>  }


-- 
https://code.launchpad.net/~widelands-dev/widelands/cleanup-soundhandler/+merge/365001
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/cleanup-soundhandler into lp:widelands.