Review: Approve
two nits - always prefer lambdas over bind if you can.
Diff comments:
>
> === modified file 'src/wui/game_options_menu.cc'
> --- src/wui/game_options_menu.cc 2017-02-21 07:56:18 +0000
> +++ src/wui/game_options_menu.cc 2017-11-29 05:07:40 +0000
> @@ -114,14 +114,17 @@
> exit_game_.sigclicked.connect(
> boost::bind(&GameOptionsMenu::clicked_exit_game, boost::ref(*this)));
>
> - windows_.sound_options.assign_toggle_button(&sound_);
> + if (windows_.sound_options.window) {
> + sound_.set_style(UI::Button::Style::kPermpressed);
> + }
> +
> windows_.sound_options.opened.connect(boost::bind(&UI::Button::set_style,
> &sound_, UI::Button::Style::kPermpressed));
Please use a lambda function instead of bind. std::bind has unexpected
behaviour around references and should be avoided. In c++14 there is no reason
to ever use it, in c++11 there are very, very few.
> +
> windows_.sound_options.closed.connect(boost::bind(&UI::Button::set_style,
> &sound_, UI::Button::Style::kRaised));
>
> if (get_usedefaultpos())
> center_to_parent();
> }
>
> GameOptionsMenu::~GameOptionsMenu() {
> - windows_.sound_options.unassign_toggle_button();
> }
>
> void GameOptionsMenu::clicked_save_game() {
>
> === modified file 'src/wui/game_statistics_menu.cc'
> --- src/wui/game_statistics_menu.cc 2017-02-21 07:56:18 +0000
> +++ src/wui/game_statistics_menu.cc 2017-11-29 05:07:40 +0000
> @@ -72,10 +72,11 @@
> g_gr->images().get("images/" + image_basename +
> ".png"), tooltip_text);
> box_.add(button);
> if (window) {
> - if (!window->on_create) {
> - window->assign_toggle_button(button);
> - registries_.push_back(*window);
> + if (window->window) {
> + button->set_style(UI::Button::Style::kPermpressed);
> }
> + window->opened.connect(boost::bind(&UI::Button::set_style,
> button, UI::Button::Style::kPermpressed));
also lambdas.
> + window->closed.connect(boost::bind(&UI::Button::set_style,
> button, UI::Button::Style::kRaised));
> button->sigclicked.connect(
> boost::bind(&UI::UniqueWindow::Registry::toggle,
> boost::ref(*window)));
> }
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1734046-statistics_menu/+merge/334398
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1734046-statistics_menu.
_______________________________________________
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
