From fe3b5f45765fef9a93e94dbeacea3114411c7933 Mon Sep 17 00:00:00 2001 From: Chris Beck Date: Sat, 28 Jun 2014 22:08:22 -0400 Subject: [PATCH 1/7] add adv. preference for number of menu items displayed at once See earlier commit: f24f6adee142c088595647d5506737c48892bf38 Forum discussion: http://forums.wesnoth.org/viewtopic.php?f=6&t=40668 --- data/advanced_preferences.cfg | 11 +++ data/test/scenarios/test_max_menu_items.cfg | 98 +++++++++++++++++++++ src/game_preferences.cpp | 10 +++ src/game_preferences.hpp | 3 + src/wmi_pager.cpp | 32 ++++--- src/wmi_pager.hpp | 3 +- 6 files changed, 143 insertions(+), 14 deletions(-) create mode 100644 data/test/scenarios/test_max_menu_items.cfg diff --git a/data/advanced_preferences.cfg b/data/advanced_preferences.cfg index 88a520ca1ff7..a1462a823f5e 100644 --- a/data/advanced_preferences.cfg +++ b/data/advanced_preferences.cfg @@ -158,6 +158,17 @@ step=1 [/advanced_preference] +[advanced_preference] + field=max_wml_menu_items + name=_ "Max WML menu items" + description= _ "Maximum number of WML menu items displayed at once" + type=int + default=7 + min=3 + max=32 + step=1 +[/advanced_preference] + [advanced_preference] field=use_twelve_hour_clock_format name= _ "Use 12-hour clock format" diff --git a/data/test/scenarios/test_max_menu_items.cfg b/data/test/scenarios/test_max_menu_items.cfg new file mode 100644 index 000000000000..c74b6f918e52 --- /dev/null +++ b/data/test/scenarios/test_max_menu_items.cfg @@ -0,0 +1,98 @@ +{GENERIC_UNIT_TEST "test_max_menu_items" ( + [event] + name=start + [set_menu_item] + id=bar1 + description=foo1 + [/set_menu_item] + [set_menu_item] + id=bar2 + description=foo2 + [/set_menu_item] + [set_menu_item] + id=bar3 + description=foo3 + [command] + [chat] + message="ASDFSAASDF" + [/chat] + [/command] + [/set_menu_item] + [set_menu_item] + id=bar4 + description=foo4 + [/set_menu_item] + [set_menu_item] + id=bar5 + description=foo5 + [/set_menu_item] + [set_menu_item] + id=bar6 + description=foo6 + [/set_menu_item] + [set_menu_item] + id=bar7 + description=foo7 + [/set_menu_item] + [set_menu_item] + id=bar8 + description=foo8 + [/set_menu_item] + [clear_menu_item] + id=bar5 + [/clear_menu_item] + [set_menu_item] + id=bar9 + description=foo9 + [/set_menu_item] + [set_menu_item] + id=bar10 + description=foo10 + [/set_menu_item] + [set_menu_item] + id=bar12 + description=foo12 + [/set_menu_item] + [set_menu_item] + id=bar13 + description=foo13 + [command] + [chat] + message="ASDFSAASDF" + [/chat] + [/command] + [/set_menu_item] + [set_menu_item] + id=bar14 + description=foo14 + [/set_menu_item] + [set_menu_item] + id=bar15 + description=foo15 + [/set_menu_item] + [set_menu_item] + id=bar16 + description=foo16 + [/set_menu_item] + [set_menu_item] + id=bar17 + description=foo17 + [/set_menu_item] + [set_menu_item] + id=bar18 + description=foo18 + [/set_menu_item] + [set_menu_item] + id=bar19 + description=foo19 + [/set_menu_item] + [set_menu_item] + id=bar20 + description=foo20 + [/set_menu_item] + [set_menu_item] + id=bar21 + description=foo21 + [/set_menu_item] + [/event] +)} diff --git a/src/game_preferences.cpp b/src/game_preferences.cpp index 44c2b759a4ba..761bf64cfcf7 100644 --- a/src/game_preferences.cpp +++ b/src/game_preferences.cpp @@ -987,6 +987,16 @@ int chat_message_aging() return lexical_cast_default(preferences::get("chat_message_aging"), 20); } +void set_max_wml_menu_items(int max) +{ + preferences::set("max_wml_menu_items", max); +} + +int max_wml_menu_items() +{ + return lexical_cast_default(preferences::get("max_wml_menu_items"), 7); +} + bool show_all_units_in_help() { return preferences::get("show_all_units_in_help", false); } diff --git a/src/game_preferences.hpp b/src/game_preferences.hpp index 7c5985d8bb0b..6ae34c532f08 100644 --- a/src/game_preferences.hpp +++ b/src/game_preferences.hpp @@ -233,6 +233,9 @@ class acquaintance; int chat_message_aging(); void set_chat_message_aging(const int aging); + int max_wml_menu_items(); + void set_max_wml_menu_items(int max); + bool show_all_units_in_help(); void set_show_all_units_in_help(bool value); diff --git a/src/wmi_pager.cpp b/src/wmi_pager.cpp index dceb0987a1a0..75ed754128ef 100644 --- a/src/wmi_pager.cpp +++ b/src/wmi_pager.cpp @@ -19,10 +19,12 @@ #include "config.hpp" #include "game_events/menu_item.hpp" #include "game_events/wmi_container.hpp" +#include "game_preferences.hpp" #include "gettext.hpp" #include #include +#include #include //std::advance #include #include @@ -74,9 +76,15 @@ void wmi_pager::get_items(const map_location& hex, return; } - assert(page_size_ > 2u); //if we dont have at least 3 items, we can't display anything... + int page_size_int = preferences::max_wml_menu_items(); - if (foo_->size() <= page_size_) { //In this case the first page is sufficient and we don't have to do anything. + assert(page_size_int >= 0 && "max wml menu items cannot be negative, this indicates preferences corruption"); + + size_t page_size = page_size_int; + + assert(page_size > 2u && "if we dont have at least 3 items, we can't display anything on a middle page..."); + + if (foo_->size() <= page_size) { //In this case the first page is sufficient and we don't have to do anything. foo_->get_items(hex, items, descriptions); page_num_ = 0; //reset page num in case there are more items later. return; @@ -87,9 +95,9 @@ void wmi_pager::get_items(const map_location& hex, page_num_ = 0; } - if (page_num_ == 0) { //we are on the first page, so show page_size_-1 items and a next button + if (page_num_ == 0) { //we are on the first page, so show page_size-1 items and a next button wmi_it end_first_page = foo_->begin(); - std::advance(end_first_page, page_size_ - 1); + std::advance(end_first_page, page_size - 1); foo_->get_items(hex, items, descriptions, foo_->begin(), end_first_page); add_next_page_item(items, descriptions); @@ -98,31 +106,31 @@ void wmi_pager::get_items(const map_location& hex, add_prev_page_item(items, descriptions); //this will be necessary since we aren't on the first page - // first page has page_size_ - 1. - // last page has page_size_ - 1. - // all other pages have page_size_ - 2; + // first page has page_size - 1. + // last page has page_size - 1. + // all other pages have page_size - 2; - size_t first_displayed_index = (page_size_ - 2) * page_num_ + 1; //this is the 0-based index of the first item displayed on this page. + size_t first_displayed_index = (page_size - 2) * page_num_ + 1; //this is the 0-based index of the first item displayed on this page. //alternatively, the number of items displayed on earlier pages while (first_displayed_index >= foo_->size()) { page_num_--; //The list must have gotten shorter and our page counter is now off the end, so decrement - first_displayed_index = (page_size_ - 2) * page_num_ + 1; //recalculate + first_displayed_index = (page_size - 2) * page_num_ + 1; //recalculate } - // ^ This loop terminates with first_displayed_index > 0, because foo_->size() > page_size_ or else we exited earlier, and we only decrease by (page_size_-2) each time. + // ^ This loop terminates with first_displayed_index > 0, because foo_->size() > page_size or else we exited earlier, and we only decrease by (page_size-2) each time. wmi_it start_range = foo_->begin(); std::advance(start_range, first_displayed_index); // <-- get an iterator to the start of our range. begin() + n doesn't work because map is not random access //^ = foo_->begin() + first_displayed_index - if (first_displayed_index + page_size_-1 >= foo_->size()) //if this can be the last page, then we won't put next page at the bottom. + if (first_displayed_index + page_size-1 >= foo_->size()) //if this can be the last page, then we won't put next page at the bottom. { foo_->get_items(hex, items, descriptions, start_range, foo_->end()); // display all of the remaining items return; } else { //we are in a middle page wmi_it end_range = start_range; - std::advance(end_range, page_size_-2); + std::advance(end_range, page_size-2); foo_->get_items(hex, items, descriptions, start_range, end_range); add_next_page_item(items, descriptions); diff --git a/src/wmi_pager.hpp b/src/wmi_pager.hpp index 86276f8e083e..a608935a105b 100644 --- a/src/wmi_pager.hpp +++ b/src/wmi_pager.hpp @@ -32,11 +32,10 @@ namespace game_events { class wmi_container; } class wmi_pager { private: int page_num_; //!< Current page number - size_t page_size_; //!< Current size of a page const game_events::wmi_container * foo_; //!< Internal pointer to the collection of wml menu items public: - wmi_pager() : page_num_(0), page_size_(7), foo_(NULL) {} + wmi_pager() : page_num_(0), foo_(NULL) {} void update_ref(game_events::wmi_container * ptr) { foo_ = ptr; } //!< Updates the internal wmi_container pointer From 75f5076d6c78ed38c1ca1c3107b71e2b20068671 Mon Sep 17 00:00:00 2001 From: Chris Beck Date: Sat, 28 Jun 2014 22:55:45 -0400 Subject: [PATCH 2/7] change rendering of last page of wmi's, to prevent menu resize This commit makes the pager just show enough entries from the previous page to ensure that we have a full page. When the number of displayed entries is large, if we don't do this then the menu can resize dramatically which is a bit jarring. --- src/wmi_pager.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/wmi_pager.cpp b/src/wmi_pager.cpp index 75ed754128ef..b9630d0fd0be 100644 --- a/src/wmi_pager.cpp +++ b/src/wmi_pager.cpp @@ -120,15 +120,19 @@ void wmi_pager::get_items(const map_location& hex, } // ^ This loop terminates with first_displayed_index > 0, because foo_->size() > page_size or else we exited earlier, and we only decrease by (page_size-2) each time. - wmi_it start_range = foo_->begin(); - std::advance(start_range, first_displayed_index); // <-- get an iterator to the start of our range. begin() + n doesn't work because map is not random access - //^ = foo_->begin() + first_displayed_index - if (first_displayed_index + page_size-1 >= foo_->size()) //if this can be the last page, then we won't put next page at the bottom. { - foo_->get_items(hex, items, descriptions, start_range, foo_->end()); // display all of the remaining items + //The last page we treat differently -- we always want to display (page_size) entries, to prevent resizing the context menu, so count back from end. + wmi_it end_range = foo_->end(); // It doesn't really matter if we display some entries that appeared on the previous page by doing this. + wmi_it start_range = end_range; + std::advance(start_range, -(page_size-1)); + + foo_->get_items(hex, items, descriptions, start_range, end_range); // display all of the remaining items return; } else { //we are in a middle page + wmi_it start_range = foo_->begin(); + std::advance(start_range, first_displayed_index); // <-- get an iterator to the start of our range. begin() + n doesn't work because map is not random access + wmi_it end_range = start_range; std::advance(end_range, page_size-2); From a968bc6254162ccd112860b4789a3ced3c485af6 Mon Sep 17 00:00:00 2001 From: Chris Beck Date: Sat, 28 Jun 2014 22:57:03 -0400 Subject: [PATCH 3/7] const-correctness fixup (just for clarity) --- src/wmi_pager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wmi_pager.cpp b/src/wmi_pager.cpp index b9630d0fd0be..cb31393634ec 100644 --- a/src/wmi_pager.cpp +++ b/src/wmi_pager.cpp @@ -76,11 +76,11 @@ void wmi_pager::get_items(const map_location& hex, return; } - int page_size_int = preferences::max_wml_menu_items(); + const int page_size_int = preferences::max_wml_menu_items(); assert(page_size_int >= 0 && "max wml menu items cannot be negative, this indicates preferences corruption"); - size_t page_size = page_size_int; + const size_t page_size = page_size_int; assert(page_size > 2u && "if we dont have at least 3 items, we can't display anything on a middle page..."); From df44636990dc21f4349642e30ba3246bff5a9ec2 Mon Sep 17 00:00:00 2001 From: Chris Beck Date: Tue, 1 Jul 2014 14:22:00 -0400 Subject: [PATCH 4/7] add [show_if]'s to the test scenario for this feature --- data/test/scenarios/test_max_menu_items.cfg | 22 +++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/data/test/scenarios/test_max_menu_items.cfg b/data/test/scenarios/test_max_menu_items.cfg index c74b6f918e52..7993e7ee485f 100644 --- a/data/test/scenarios/test_max_menu_items.cfg +++ b/data/test/scenarios/test_max_menu_items.cfg @@ -1,4 +1,14 @@ {GENERIC_UNIT_TEST "test_max_menu_items" ( + [event] + name=side 1 turn + first_time_only=no + {VARIABLE current_side 1} + [/event] + [event] + name=side 2 turn + first_time_only=no + {VARIABLE current_side 2} + [/event] [event] name=start [set_menu_item] @@ -8,6 +18,9 @@ [set_menu_item] id=bar2 description=foo2 + [show_if] + {VARIABLE_CONDITIONAL current_side equals 1} + [/show_if] [/set_menu_item] [set_menu_item] id=bar3 @@ -25,6 +38,9 @@ [set_menu_item] id=bar5 description=foo5 + [show_if] + {VARIABLE_CONDITIONAL current_side equals 1} + [/show_if] [/set_menu_item] [set_menu_item] id=bar6 @@ -44,10 +60,16 @@ [set_menu_item] id=bar9 description=foo9 + [show_if] + {VARIABLE_CONDITIONAL current_side equals 1} + [/show_if] [/set_menu_item] [set_menu_item] id=bar10 description=foo10 + [show_if] + {VARIABLE_CONDITIONAL current_side equals 1} + [/show_if] [/set_menu_item] [set_menu_item] id=bar12 From 5a8b024003dac3c292ff6b13a5d2f5fead1f7d6a Mon Sep 17 00:00:00 2001 From: Chris Beck Date: Tue, 1 Jul 2014 15:25:11 -0400 Subject: [PATCH 5/7] fixup wmi_pager so that it takes account of [show_if] when paging I also change the interface to wmi_container to be a bit more natural. --- src/game_events/wmi_container.cpp | 12 ++++---- src/game_events/wmi_container.hpp | 10 ++----- src/wmi_pager.cpp | 48 ++++++++++++++++++++++--------- 3 files changed, 44 insertions(+), 26 deletions(-) diff --git a/src/game_events/wmi_container.cpp b/src/game_events/wmi_container.cpp index d8188188d82e..ee5fe8fee765 100644 --- a/src/game_events/wmi_container.cpp +++ b/src/game_events/wmi_container.cpp @@ -105,13 +105,13 @@ bool wmi_container::fire_item(const std::string & id, const map_location & hex) * @param[out] items Pointers to applicable menu items will be pushed onto @a items. * @param[out] descriptions Menu item text will be pushed onto @descriptions (in the same order as @a items). */ -void wmi_container::get_items(const map_location& hex, - std::vector > & items, - std::vector & descriptions, const_iterator start, const_iterator finish) const +std::vector, std::string> > wmi_container::get_items(const map_location& hex, + const_iterator start, const_iterator finish) const { + std::vector, std::string> > ret; if ( empty() ) // Nothing to do (skip setting game variables). - return; + return ret; // Prepare for can show(). resources::gamedata->get_variable("x1") = hex.x + 1; @@ -125,10 +125,10 @@ void wmi_container::get_items(const map_location& hex, if ( item->use_wml_menu() && item->can_show(hex) ) { // Include this item. - items.push_back(item); - descriptions.push_back(item->menu_text()); + ret.push_back(std::make_pair(item, item->menu_text())); } } + return ret; } /** diff --git a/src/game_events/wmi_container.hpp b/src/game_events/wmi_container.hpp index c8a6167927be..d2d9762c0c2d 100644 --- a/src/game_events/wmi_container.hpp +++ b/src/game_events/wmi_container.hpp @@ -75,15 +75,11 @@ class wmi_container{ /// Fires the menu item with the given @a id. bool fire_item(const std::string & id, const map_location & hex) const; /// Returns the menu items that can be shown for the given location. - void get_items(const map_location& hex, - std::vector > & items, - std::vector & descriptions, + std::vector, std::string> > get_items(const map_location& hex, const_iterator start, const_iterator finish) const; /// Range over all items by default - void get_items(const map_location& hex, - std::vector > & items, - std::vector & descriptions) const { - get_items(hex, items, descriptions, begin(), end()); + std::vector, std::string> > get_items(const map_location& hex) const { + return get_items(hex, begin(), end()); } /// Initializes the implicit event handlers for inlined [command]s. void init_handlers() const; diff --git a/src/wmi_pager.cpp b/src/wmi_pager.cpp index cb31393634ec..97f15cc09852 100644 --- a/src/wmi_pager.cpp +++ b/src/wmi_pager.cpp @@ -22,6 +22,7 @@ #include "game_preferences.hpp" #include "gettext.hpp" +#include //std::transform #include #include #include @@ -66,10 +67,22 @@ bool wmi_pager::capture ( const game_events::wml_menu_item & item ) return false; } -typedef game_events::wmi_container::const_iterator wmi_it; +typedef boost::shared_ptr wmi_ptr; +typedef std::pair wmi_pair; +typedef std::vector::iterator wmi_it; + +static wmi_ptr select_first(const wmi_pair & p) +{ + return p.first; +} + +static std::string select_second(const wmi_pair & p) +{ + return p.second; +} void wmi_pager::get_items(const map_location& hex, - std::vector > & items, + std::vector & items, std::vector & descriptions) { if (!foo_) { @@ -84,8 +97,12 @@ void wmi_pager::get_items(const map_location& hex, assert(page_size > 2u && "if we dont have at least 3 items, we can't display anything on a middle page..."); - if (foo_->size() <= page_size) { //In this case the first page is sufficient and we don't have to do anything. - foo_->get_items(hex, items, descriptions); + std::vector bar = foo_->get_items(hex); + + if (bar.size() <= page_size) { //In this case the first page is sufficient and we don't have to do anything. + std::transform(bar.begin(), bar.end(), back_inserter(items), select_first); + std::transform(bar.begin(), bar.end(), back_inserter(descriptions), select_second); + page_num_ = 0; //reset page num in case there are more items later. return; } @@ -96,10 +113,12 @@ void wmi_pager::get_items(const map_location& hex, } if (page_num_ == 0) { //we are on the first page, so show page_size-1 items and a next button - wmi_it end_first_page = foo_->begin(); + wmi_it end_first_page = bar.begin(); std::advance(end_first_page, page_size - 1); - foo_->get_items(hex, items, descriptions, foo_->begin(), end_first_page); + std::transform(bar.begin(), end_first_page, back_inserter(items), select_first); + std::transform(bar.begin(), end_first_page, back_inserter(descriptions), select_second); + add_next_page_item(items, descriptions); return; } @@ -113,30 +132,33 @@ void wmi_pager::get_items(const map_location& hex, size_t first_displayed_index = (page_size - 2) * page_num_ + 1; //this is the 0-based index of the first item displayed on this page. //alternatively, the number of items displayed on earlier pages - while (first_displayed_index >= foo_->size()) + while (first_displayed_index >= bar.size()) { page_num_--; //The list must have gotten shorter and our page counter is now off the end, so decrement first_displayed_index = (page_size - 2) * page_num_ + 1; //recalculate } - // ^ This loop terminates with first_displayed_index > 0, because foo_->size() > page_size or else we exited earlier, and we only decrease by (page_size-2) each time. + // ^ This loop terminates with first_displayed_index > 0, because bar.size() > page_size or else we exited earlier, and we only decrease by (page_size-2) each time. - if (first_displayed_index + page_size-1 >= foo_->size()) //if this can be the last page, then we won't put next page at the bottom. + if (first_displayed_index + page_size-1 >= bar.size()) //if this can be the last page, then we won't put next page at the bottom. { //The last page we treat differently -- we always want to display (page_size) entries, to prevent resizing the context menu, so count back from end. - wmi_it end_range = foo_->end(); // It doesn't really matter if we display some entries that appeared on the previous page by doing this. + wmi_it end_range = bar.end(); // It doesn't really matter if we display some entries that appeared on the previous page by doing this. wmi_it start_range = end_range; std::advance(start_range, -(page_size-1)); - foo_->get_items(hex, items, descriptions, start_range, end_range); // display all of the remaining items + std::transform(start_range, end_range, back_inserter(items), select_first); + std::transform(start_range, end_range, back_inserter(descriptions), select_second); return; } else { //we are in a middle page - wmi_it start_range = foo_->begin(); + wmi_it start_range = bar.begin(); std::advance(start_range, first_displayed_index); // <-- get an iterator to the start of our range. begin() + n doesn't work because map is not random access wmi_it end_range = start_range; std::advance(end_range, page_size-2); - foo_->get_items(hex, items, descriptions, start_range, end_range); + std::transform(start_range, end_range, back_inserter(items), select_first); + std::transform(start_range, end_range, back_inserter(descriptions), select_second); + add_next_page_item(items, descriptions); return; } From b1b493bb57e50eba523edb8bd5f5425a33accd53 Mon Sep 17 00:00:00 2001 From: Chris Beck Date: Tue, 1 Jul 2014 17:11:37 -0400 Subject: [PATCH 6/7] add TODO notes about improving the wmi pager --- data/advanced_preferences.cfg | 1 + src/wmi_pager.hpp | 3 +++ 2 files changed, 4 insertions(+) diff --git a/data/advanced_preferences.cfg b/data/advanced_preferences.cfg index a1462a823f5e..91c597f1556a 100644 --- a/data/advanced_preferences.cfg +++ b/data/advanced_preferences.cfg @@ -160,6 +160,7 @@ [advanced_preference] field=max_wml_menu_items + # TODO: It would be better to eliminate this preference and have it instead determined by the gui layout algorithm. name=_ "Max WML menu items" description= _ "Maximum number of WML menu items displayed at once" type=int diff --git a/src/wmi_pager.hpp b/src/wmi_pager.hpp index a608935a105b..c03464e9f890 100644 --- a/src/wmi_pager.hpp +++ b/src/wmi_pager.hpp @@ -17,6 +17,9 @@ * container. It is an adapter, managing the production of items lists * from the container, and screening the "fire" signals coming back * in to intercept the paging signals. + * + * TODO: Implement this as a helper class for menu perhaps, so that it + * can interact with the gui layout algorithm. */ struct map_location; From 188b3be18988ff99f98f20efd6b104d181fb15d2 Mon Sep 17 00:00:00 2001 From: Chris Beck Date: Fri, 4 Jul 2014 14:49:41 -0400 Subject: [PATCH 7/7] change a label "Earlier Items" -> "Previous Items" --- src/wmi_pager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wmi_pager.cpp b/src/wmi_pager.cpp index 97f15cc09852..d5e8858ded15 100644 --- a/src/wmi_pager.cpp +++ b/src/wmi_pager.cpp @@ -48,7 +48,7 @@ static void add_next_page_item( std::vector > & items, std::vector & descriptions) { - std::string desc = _("Earlier Items"); + std::string desc = _("Previous Items"); config temp; temp["description"] = desc; items.push_back(boost::make_shared(prev_id, temp));