From 7d253cfb3cd5d77bc1af159846f848a30aa210ed Mon Sep 17 00:00:00 2001 From: gfgtdf Date: Tue, 14 Nov 2017 22:44:32 +0100 Subject: [PATCH] fix recrision in gui2 mp create depckeck selectable::set_value now takes an optional fire_vent parmeter also we fix a case where display_games_of_type always invoked a on_game_select for the first element in the list before the correct level was selected. --- .../dialogs/multiplayer/mp_create_game.cpp | 27 +++++++++++-------- .../dialogs/multiplayer/mp_create_game.hpp | 4 +-- src/gui/widgets/menu_button.cpp | 9 ++++--- src/gui/widgets/menu_button.hpp | 4 +-- src/gui/widgets/selectable_item.hpp | 6 ++--- src/gui/widgets/toggle_button.cpp | 12 +++++---- src/gui/widgets/toggle_button.hpp | 2 +- src/gui/widgets/toggle_panel.cpp | 20 +++++--------- src/gui/widgets/toggle_panel.hpp | 2 +- 9 files changed, 44 insertions(+), 42 deletions(-) diff --git a/src/gui/dialogs/multiplayer/mp_create_game.cpp b/src/gui/dialogs/multiplayer/mp_create_game.cpp index ef415b004a75d..ca25f26668df5 100644 --- a/src/gui/dialogs/multiplayer/mp_create_game.cpp +++ b/src/gui/dialogs/multiplayer/mp_create_game.cpp @@ -153,12 +153,12 @@ void mp_create_game::pre_show(window& win) // Set up filtering // connect_signal_notify_modified(find_widget(&win, "num_players", false), - std::bind(&mp_create_game::on_filter_change, this, std::ref(win), "num_players")); + std::bind(&mp_create_game::on_filter_change, this, std::ref(win), "num_players", true)); text_box& filter = find_widget(&win, "game_filter", false); filter.set_text_changed_callback( - std::bind(&mp_create_game::on_filter_change, this, std::ref(win), "game_filter")); + std::bind(&mp_create_game::on_filter_change, this, std::ref(win), "game_filter", true)); // Note this cannot be in the keyboard chain or it will capture focus from other text boxes win.keyboard_capture(&filter); @@ -218,8 +218,8 @@ void mp_create_game::pre_show(window& win) create_engine_.active_mods().push_back(mod->id); mog_toggle.set_value_bool(true); } - - connect_signal_notify_modified(mog_toggle, std::bind(&mp_create_game::on_mod_toggle, this, std::ref(win), i)); + auto pmog_toggle = &mog_toggle; + connect_signal_notify_modified(mog_toggle, std::bind(&mp_create_game::on_mod_toggle, this, std::ref(win), i, pmog_toggle)); } // No mods, hide the header @@ -366,7 +366,7 @@ void mp_create_game::pre_show(window& win) create_engine_.set_current_era_index(cfg["index"].to_int()); }, true); plugins_context_->set_callback("select_mod", [this, &win](const config& cfg) { - on_mod_toggle(win, cfg["index"].to_int()); + on_mod_toggle(win, cfg["index"].to_int(), nullptr); }, true); plugins_context_->set_accessor("game_config", [this](const config&) {return cfg_; }); @@ -437,7 +437,7 @@ void mp_create_game::sync_with_depcheck(window& window) } template -void mp_create_game::on_filter_change(window& window, const std::string& id) +void mp_create_game::on_filter_change(window& window, const std::string& id, bool do_select) { create_engine_.apply_level_filter(find_widget(&window, id, false).get_value()); @@ -449,8 +449,9 @@ void mp_create_game::on_filter_change(window& window, const std::string& id) } game_list.set_row_shown(filtered); - - on_game_select(window); + if(do_select) { + on_game_select(window); + } } void mp_create_game::on_game_select(window& window) @@ -513,8 +514,12 @@ void mp_create_game::on_tab_select(window& window) } } -void mp_create_game::on_mod_toggle(window& window, const int index) +void mp_create_game::on_mod_toggle(window& window, const int index, toggle_button* sender) { + (void)sender; + if(sender && (sender->get_value_bool() == create_engine_.dependency_manager().is_modification_active(index))) { + assert(false); + } create_engine_.toggle_mod(index); sync_with_depcheck(window); @@ -582,8 +587,8 @@ void mp_create_game::display_games_of_type(window& window, ng::level::TYPE type, if(!level.empty() && !list.get_rows_shown().empty()) { // Recalculate which rows should be visible - on_filter_change(window, "num_players"); - on_filter_change(window, "game_filter"); + on_filter_change(window, "num_players", false); + on_filter_change(window, "game_filter", false); int level_index = create_engine_.find_level_by_id(level).second; if(level_index >= 0 && size_t(level_index) < list.get_item_count()) { diff --git a/src/gui/dialogs/multiplayer/mp_create_game.hpp b/src/gui/dialogs/multiplayer/mp_create_game.hpp index 080e579fd1397..4a9103350f9bf 100644 --- a/src/gui/dialogs/multiplayer/mp_create_game.hpp +++ b/src/gui/dialogs/multiplayer/mp_create_game.hpp @@ -109,12 +109,12 @@ class mp_create_game : public modal_dialog, private plugin_executor field_integer* action_bonus_; template - void on_filter_change(window& window, const std::string& id); + void on_filter_change(window& window, const std::string& id, bool do_select); void on_game_select(window& window); void on_tab_select(window& window); void on_era_select(window& window); - void on_mod_toggle(window& window, const int index); + void on_mod_toggle(window& window, const int index, toggle_button* sender); void on_random_faction_mode_select(window& window); std::vector get_active_mods(window& window); diff --git a/src/gui/widgets/menu_button.cpp b/src/gui/widgets/menu_button.cpp index 657dabbd804f9..aaa2a801850d8 100644 --- a/src/gui/widgets/menu_button.cpp +++ b/src/gui/widgets/menu_button.cpp @@ -143,7 +143,7 @@ void menu_button::signal_handler_left_button_click(const event::ui_event event, return; } - set_selected(selected); + set_selected(selected, true); if(retval_ != 0) { if(window* window = get_window()) { @@ -171,7 +171,7 @@ void menu_button::set_values(const std::vector<::config>& values, int selected) set_label(values_[selected_]["label"]); } -void menu_button::set_selected(int selected) +void menu_button::set_selected(int selected, bool fire_event) { assert(static_cast(selected) < values_.size()); assert(static_cast(selected_) < values_.size()); @@ -183,8 +183,9 @@ void menu_button::set_selected(int selected) selected_ = selected; set_label(values_[selected_]["label"]); - - fire(event::NOTIFY_MODIFIED, *this, nullptr); + if (fire_event) { + fire(event::NOTIFY_MODIFIED, *this, nullptr); + } } // }---------- DEFINITION ---------{ diff --git a/src/gui/widgets/menu_button.hpp b/src/gui/widgets/menu_button.hpp index dad7d40eb49e1..da2dd23c9987e 100644 --- a/src/gui/widgets/menu_button.hpp +++ b/src/gui/widgets/menu_button.hpp @@ -75,13 +75,13 @@ class menu_button : public styled_widget, public selectable_item void set_values(const std::vector<::config>& values, int selected = 0); - void set_selected(int selected); + void set_selected(int selected, bool fire_event = true); /** Inherited from selectable_item */ virtual unsigned get_value() const override { return selected_; } /** Inherited from selectable_item */ - virtual void set_value(const unsigned value ) override { set_selected(value); } + virtual void set_value(unsigned value, bool fire_event = false) override { set_selected(value, fire_event); } /** Inherited from selectable_item */ virtual unsigned num_states() const override { return values_.size(); } diff --git a/src/gui/widgets/selectable_item.hpp b/src/gui/widgets/selectable_item.hpp index bea7c2b393d95..90d0253074d96 100644 --- a/src/gui/widgets/selectable_item.hpp +++ b/src/gui/widgets/selectable_item.hpp @@ -39,7 +39,7 @@ class selectable_item virtual unsigned get_value() const = 0; /** Select the styled_widget. */ - virtual void set_value(const unsigned) = 0; + virtual void set_value(unsigned value, bool fire_event = false) = 0; /** The number of states, that is 2 for normal buttons, 3 for tristate buttons. */ virtual unsigned num_states() const = 0; @@ -50,10 +50,10 @@ class selectable_item return get_value() != 0; } - void set_value_bool(const bool value) + void set_value_bool(bool value, bool fire_event = false) { assert(num_states() == 2); - return set_value(value); + return set_value(value, fire_event); } }; diff --git a/src/gui/widgets/toggle_button.cpp b/src/gui/widgets/toggle_button.cpp index bd8ca35ac7692..121c772532e99 100644 --- a/src/gui/widgets/toggle_button.cpp +++ b/src/gui/widgets/toggle_button.cpp @@ -110,12 +110,13 @@ void toggle_button::update_canvas() set_is_dirty(true); } -void toggle_button::set_value(const unsigned selected) +void toggle_button::set_value(unsigned selected, bool fire_event) { + selected = selected % num_states(); if(selected == get_value()) { return; } - state_num_ = selected % num_states(); + state_num_ = selected; set_is_dirty(true); // Check for get_window() is here to prevent the callback from @@ -123,8 +124,9 @@ void toggle_button::set_value(const unsigned selected) if(!get_window()) { return; } - - fire(event::NOTIFY_MODIFIED, *this, nullptr); + if (fire_event) { + fire(event::NOTIFY_MODIFIED, *this, nullptr); + } } void toggle_button::set_retval(const int retval) @@ -168,7 +170,7 @@ void toggle_button::signal_handler_left_button_click(const event::ui_event event sound::play_UI_sound(settings::sound_toggle_button_click); - set_value(get_value() + 1); + set_value(get_value() + 1, true); handled = true; } diff --git a/src/gui/widgets/toggle_button.hpp b/src/gui/widgets/toggle_button.hpp index b882e1ad14e49..376bad8ae449e 100644 --- a/src/gui/widgets/toggle_button.hpp +++ b/src/gui/widgets/toggle_button.hpp @@ -62,7 +62,7 @@ class toggle_button : public styled_widget, public selectable_item /** Inherited from selectable_item */ unsigned num_states() const override; /** Inherited from selectable_item */ - void set_value(const unsigned selected) override; + void set_value(unsigned selected, bool fire_event = false) override; /***** ***** ***** setters / getters for members ***** ****** *****/ diff --git a/src/gui/widgets/toggle_panel.cpp b/src/gui/widgets/toggle_panel.cpp index 33d7ffae26de8..7c821572a8f8f 100644 --- a/src/gui/widgets/toggle_panel.cpp +++ b/src/gui/widgets/toggle_panel.cpp @@ -159,15 +159,15 @@ point toggle_panel::border_space() const return point(conf->left_border + conf->right_border, conf->top_border + conf->bottom_border); } -void toggle_panel::set_value(const unsigned selected) +void toggle_panel::set_value(unsigned selected, bool fire_event) { + selected = selected % num_states(); if(selected == get_value()) { return; } - state_num_ = selected % num_states(); + state_num_ = selected; set_is_dirty(true); -#if 0 /* * Disabled since this causes problems all over the place. * This was added in acea15c312f178b2b6fe4556ca6b190b00866557 but clashes with the @@ -185,12 +185,9 @@ void toggle_panel::set_value(const unsigned selected) // Check for get_window() is here to prevent the callback from // being called when the initial value is set. - if(!get_window()) { - return; + if(get_window() && fire_event) { + fire(event::NOTIFY_MODIFIED, *this, nullptr); } - - fire(event::NOTIFY_MODIFIED, *this, nullptr); -#endif } void toggle_panel::set_retval(const int retval) @@ -252,7 +249,7 @@ toggle_panel::signal_handler_pre_left_button_click(const event::ui_event event) { DBG_GUI_E << get_control_type() << "[" << id() << "]: " << event << ".\n"; - set_value(1); + set_value(1, true); #if 0 /* @@ -281,10 +278,7 @@ void toggle_panel::signal_handler_left_button_click(const event::ui_event event, sound::play_UI_sound(settings::sound_toggle_panel_click); - set_value(get_value() + 1); - - /** @todo remove. See comment in @ref set_value. */ - fire(event::NOTIFY_MODIFIED, *this, nullptr); + set_value(get_value() + 1, true); handled = true; } diff --git a/src/gui/widgets/toggle_panel.hpp b/src/gui/widgets/toggle_panel.hpp index bada469714aba..2217f1d71a3ae 100644 --- a/src/gui/widgets/toggle_panel.hpp +++ b/src/gui/widgets/toggle_panel.hpp @@ -96,7 +96,7 @@ class toggle_panel : public panel, public selectable_item } /** Inherited from selectable_item */ - void set_value(const unsigned selected) override; + void set_value(const unsigned selected, bool fire_event = false) override; /** Inherited from selectable_item */ unsigned num_states() const override;