Merge pull request #229 from cbeck88/wmi_items_preference

add adv. preference for number of menu items displayed at once,

and remaining fixups there

data/advanced_preferences.cfg

@@ -158,6 +158,18 @@
     step=1
 [/advanced_preference]
 
+[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
+    default=7
+    min=3
+    max=32
+    step=1
+[/advanced_preference]
+
 [advanced_preference]
     field=use_twelve_hour_clock_format
     name= _ "Use 12-hour clock format"

data/test/scenarios/test_max_menu_items.cfg

@@ -0,0 +1,120 @@
+{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]
+            id=bar1
+            description=foo1
+        [/set_menu_item]
+        [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
+            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
+            [show_if]
+                {VARIABLE_CONDITIONAL current_side equals 1}
+            [/show_if]
+        [/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
+            [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
+            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]
+)}

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<boost::shared_ptr<const wml_menu_item> > & items,
-                              std::vector<std::string> & descriptions, const_iterator start, const_iterator finish) const
+std::vector<std::pair<boost::shared_ptr<const wml_menu_item>, std::string> > wmi_container::get_items(const map_location& hex,
+                              const_iterator start, const_iterator finish) const
 {
+	std::vector<std::pair<boost::shared_ptr<const wml_menu_item>, 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;
 }
 
 /**

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<boost::shared_ptr<const wml_menu_item> > & items,
-			std::vector<std::string> & descriptions,
+	std::vector<std::pair<boost::shared_ptr<const wml_menu_item>, 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<boost::shared_ptr<const wml_menu_item> > & items,
-			std::vector<std::string> & descriptions) const {
-		get_items(hex, items, descriptions, begin(), end());
+	std::vector<std::pair<boost::shared_ptr<const wml_menu_item>, 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;

src/game_preferences.cpp

@@ -987,6 +987,16 @@ int chat_message_aging()
 	return lexical_cast_default<int>(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<int>(preferences::get("max_wml_menu_items"), 7);
+}
+
 bool show_all_units_in_help() {
 	return preferences::get("show_all_units_in_help", false);
 }

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);
 

src/wmi_pager.cpp

@@ -19,10 +19,13 @@
 #include "config.hpp"
 #include "game_events/menu_item.hpp"
 #include "game_events/wmi_container.hpp"
+#include "game_preferences.hpp"
 #include "gettext.hpp"
 
+#include <algorithm> //std::transform
 #include <boost/make_shared.hpp>
 #include <boost/shared_ptr.hpp>
+#include <cassert>
 #include <iterator> //std::advance
 #include <string>
 #include <vector>
@@ -45,7 +48,7 @@ static void add_next_page_item( std::vector<boost::shared_ptr<const game_events:
 static void add_prev_page_item( std::vector<boost::shared_ptr<const game_events::wml_menu_item> > & items,
                std::vector<std::string> & descriptions)
 {
-	std::string desc = _("Earlier Items");
+	std::string desc = _("Previous Items");
 	config temp;
 	temp["description"] = desc;
 	items.push_back(boost::make_shared<const game_events::wml_menu_item>(prev_id, temp));
@@ -64,20 +67,42 @@ 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<const game_events::wml_menu_item> wmi_ptr;
+typedef std::pair<wmi_ptr, std::string> wmi_pair;
+typedef std::vector<wmi_pair>::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<boost::shared_ptr<const game_events::wml_menu_item> > & items,
+               std::vector<wmi_ptr > & items,
                std::vector<std::string> & descriptions)
 {
 	if (!foo_) {
 		return;
 	}
 
-	assert(page_size_ > 2u); //if we dont have at least 3 items, we can't display anything...
+	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");
+
+	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...");
+
+	std::vector<wmi_pair > 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);
 
-	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,44 +112,53 @@ 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
-		wmi_it end_first_page = foo_->begin();
-		std::advance(end_first_page, page_size_ - 1);
+	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 = 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;
 	}
 
 	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())
+	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
+		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.
 
-	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 >= bar.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 = 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));
+
+		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 = 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);
+		std::advance(end_range, page_size-2);
+
+		std::transform(start_range, end_range, back_inserter(items), select_first);
+		std::transform(start_range, end_range, back_inserter(descriptions), select_second);
 
-		foo_->get_items(hex, items, descriptions, start_range, end_range);
 		add_next_page_item(items, descriptions);
 		return;
 	}

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;
@@ -32,11 +35,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