widelands-dev team mailing list archive

[Miroslav Remák <miroslavr256@xxxxxxxxx>]

Miroslav Remák has proposed merging lp:~widelands-dev/widelands/watchwindow-fixes into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/watchwindow-fixes/+merge/289573

- Fixes some oddities such as view duplication
- Possibly fixes bug #1553699 (probable cause: std::erase used on invalid position, which results in undefined behavior)
- Highlights the current view button, as it's difficult to tell which view is selected
- Cleans up the code a bit

-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/watchwindow-fixes into lp:widelands.

=== modified file 'src/wui/watchwindow.cc'
--- src/wui/watchwindow.cc	2016-02-18 18:27:52 +0000
+++ src/wui/watchwindow.cc	2016-03-19 23:00:17 +0000
@@ -62,8 +62,8 @@
 	boost::signals2::signal<void (Point)> warp_mainview;
 
 	void add_view(Widelands::Coords);
-	void next_view(bool first = false);
-	void show_view(bool first = false);
+	void next_view();
+	void show_view();
 	Point calc_coords(Widelands::Coords);
 	void save_coords();
 	void close_cur_view();
@@ -76,7 +76,8 @@
 private:
 	void do_follow();
 	void do_goto();
-	void setview(uint8_t index);
+	void view_button_clicked(uint8_t index);
+	void set_current_view(uint8_t idx, bool save_previous = true);
 
 	MapView mapview;
 	uint32_t last_visit;
@@ -98,7 +99,8 @@
 	UI::Window(&parent, "watch", x, y, w, h, _("Watch")),
 	mapview   (this, 0, 0, 200, 166, parent),
 	last_visit(game().get_gametime()),
-	single_window(init_single_window)
+	single_window(init_single_window),
+	cur_index(0)
 {
 	UI::Button * followbtn =
 		new UI::Button
@@ -128,7 +130,7 @@
 					 "-", std::string(),
 					 false);
 			view_btns[i]->sigclicked.connect
-				(boost::bind(&WatchWindow::setview, this, i));
+				(boost::bind(&WatchWindow::view_button_clicked, this, i));
 		}
 
 		UI::Button * closebtn =
@@ -146,7 +148,7 @@
 	warp_mainview.connect(boost::bind(&InteractiveBase::move_view_to_point, &parent, _1));
 
 	add_view(coords);
-	next_view(true);
+	set_current_view(0, false);
 }
 
 /**
@@ -180,16 +182,8 @@
 }
 
 // Switch to next view
-void WatchWindow::next_view(bool const first) {
-	if (!first && views.size() == 1)
-		return;
-	if (!first)
-		save_coords();
-	if (first || (cur_index == views.size() - 1 && cur_index != 0))
-		cur_index = 0;
-	else if (cur_index < views.size() - 1)
-		++cur_index;
-	show_view(first);
+void WatchWindow::next_view() {
+	set_current_view((cur_index + 1) % views.size());
 }
 
 
@@ -212,8 +206,23 @@
 	}
 }
 
+void WatchWindow::set_current_view(uint8_t idx, bool save_previous)
+{
+	assert(idx < views.size());
+
+	if (save_previous)
+		save_coords();
+
+	if (single_window) {
+		view_btns[cur_index]->set_perm_pressed(false);
+		view_btns[idx]->set_perm_pressed(true);
+	}
+	cur_index = idx;
+	show_view();
+}
+
 // Draws the current view
-void WatchWindow::show_view(bool) {
+void WatchWindow::show_view() {
 	mapview.set_viewpoint(views[cur_index].view_point, true);
 }
 
@@ -345,12 +354,10 @@
 /**
  * Sets the current view to @p index and resets timeout.
  */
-void WatchWindow::setview(uint8_t index)
+void WatchWindow::view_button_clicked(uint8_t index)
 {
-	save_coords();
-	cur_index = index;
+	set_current_view(index);
 	last_visit = game().get_gametime();
-	show_view();
 }
 
 /**
@@ -366,13 +373,8 @@
 		return;
 	}
 
-	uint8_t const old_index = cur_index;
-	next_view();
-	std::vector<WatchWindowView>::iterator view_it = views.begin();
-	for (uint8_t i = 0; i < old_index; ++i)
-		++view_it;
-	view_btns[cur_index]->set_enabled(false);
-	views.erase(view_it);
+	views.erase(views.begin() + cur_index);
+	set_current_view(cur_index % views.size(), false);
 	toggle_buttons();
 }