widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

Review: Approve

1) makes sense
3) That should work too, I think.

About the playercolor() function: I am a little concerned that this now has to hit the disk everytime it is called (to see if _pc.png is there). I think on spindle disks this might get slow and could be noticable - ideally we would detect player color images already on game startup. I think this is unrelated to this branch. 

There is one NOCOM in animation.cc left - I was unsure what you wanted to fix about this. Otherwise this LGTM.

Diff comments:

> 
> === modified file 'src/graphic/animation.cc'
> --- src/graphic/animation.cc	2017-02-28 12:59:39 +0000
> +++ src/graphic/animation.cc	2017-04-26 07:33:43 +0000
> @@ -223,7 +223,7 @@
>  	assert(!image_files_.empty());
>  	const Image* image = g_gr->images().get(image_files_[0]);
>  	if (hasplrclrs_ && clr) {
> -		image = playercolor_image(clr, image, g_gr->images().get(pc_mask_image_files_[0]));
> +		image = playercolor_image(clr, image_files_[0]); // NOCOM

There is still a NOCOM here?

>  	}
>  	const int w = image->width();
>  	const int h = image->height();


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