← Back to team overview

widelands-dev team mailing list archive

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

 

Review: Approve

> OverlayLevel is part of the public API of this class and kLevelForBuildHelp should never be used when 'register_overlay' is called. I therefore opted to not make it part of the enum class to make misuse harder.

This has convinced me, plus that we really don't want to use more memory. Let's keep it as it is then.

Regarding the overlay on/off thing, how about switching it on when the tool gets activated? IMO it might be annoying with the increase/decrease tool if the user had to remember to switch it on in order to see what they are doing.

I agree that deactivating the tool when the overlay is switched off would be confusing for the user and cause some hair-pulling - reactions would probably be "Where on Earth has my bl** tool gone?"

Code LGTM, I'll do a bit of testing.
-- 
https://code.launchpad.net/~widelands-dev/widelands/toggle_resources/+merge/328958
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/buildhelp_overlay.


References