Thanks for the review! I have hopefully fixed the compiler warnings now - we'll 
just have to wait what clang says.

I also addressed your points from a few posts up and added some more 
documentation to the RendererText header file - I hope it will make the code a 
bit more clear. Yes, it's a bit complicated due to the cropping involved.

2 notes:

1. CropMode::kVertical doesn't exist because CropMode::kHorizontal was a stupid 
name. I went with kSelf now. Btter ideas are welcome :)

2. Re. better replace true / false by kBackGroundColorSet and 
!kBackGroundColorSet or such: While I generally prefer enum classes over bools 
myself, I think it would actually make the code harder to read in this 
instance, because there is no "else" branch when it is checked. This bool is 
only ever assigned in the private constructors, so it shouldn't cause any 
interface readability issues.

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

_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to     : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to