widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1653254-flagaction-roads into lp:widelands.

Commit message:
Always close flag action window when building road is finished. Also close when configure economy window is opened. Renamed FieldActionWindow::okdialog() to FieldActionWindow::reset_mouse_and_die().

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1653254 in widelands: "Action window of Road stays open under some circumstance"
  https://bugs.launchpad.net/widelands/+bug/1653254

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1653254-flagaction-roads/+merge/317709
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1653254-flagaction-roads into lp:widelands.

=== modified file 'src/wui/fieldaction.cc'
--- src/wui/fieldaction.cc	2017-02-12 09:10:57 +0000
+++ src/wui/fieldaction.cc	2017-02-18 23:34:50 +0000
@@ -192,7 +192,7 @@
 	                       void (FieldActionWindow::*fn)(),
 	                       const std::string& tooltip_text,
 	                       bool repeating = false);
-	void okdialog();
+	void reset_mouse_and_die();
 
 	Widelands::Player* player_;
 	Widelands::Map* map_;
@@ -525,7 +525,7 @@
 It resets the mouse to its original position and closes the window
 ===============
 */
-void FieldActionWindow::okdialog() {
+void FieldActionWindow::reset_mouse_and_die() {
 	ibase().mouse_to_field(node_, MapView::Transition::Jump);
 	die();
 }
@@ -538,7 +538,7 @@
 void FieldActionWindow::act_watch() {
 	upcast(InteractiveGameBase, igbase, &ibase());
 	show_watch_window(*igbase, node_);
-	okdialog();
+	reset_mouse_and_die();
 }
 
 /*
@@ -549,13 +549,13 @@
 void FieldActionWindow::act_show_census() {
 	ibase().set_display_flag(
 	   InteractiveBase::dfShowCensus, !ibase().get_display_flag(InteractiveBase::dfShowCensus));
-	okdialog();
+	reset_mouse_and_die();
 }
 
 void FieldActionWindow::act_show_statistics() {
 	ibase().set_display_flag(InteractiveBase::dfShowStatistics,
 	                         !ibase().get_display_flag(InteractiveBase::dfShowStatistics));
-	okdialog();
+	reset_mouse_and_die();
 }
 
 /*
@@ -586,7 +586,7 @@
 		iaplayer->set_flag_to_connect(node_);
 	}
 
-	okdialog();
+	reset_mouse_and_die();
 }
 
 void FieldActionWindow::act_configure_economy() {
@@ -598,6 +598,7 @@
 			new EconomyOptionsWindow(dynamic_cast<UI::Panel*>(&ibase()), economy, can_act);
 		}
 	}
+	die();
 }
 
 /*
@@ -606,7 +607,7 @@
 ===============
 */
 void FieldActionWindow::act_ripflag() {
-	okdialog();
+	reset_mouse_and_die();
 	Widelands::EditorGameBase& egbase = ibase().egbase();
 	upcast(Game, game, &egbase);
 	upcast(InteractivePlayer, iaplayer, &ibase());
@@ -636,7 +637,7 @@
 	// If we area already building a road just ignore this
 	if (!ibase().is_building_road()) {
 		ibase().start_build_road(node_, player_->player_number());
-		okdialog();
+		reset_mouse_and_die();
 	}
 }
 
@@ -650,7 +651,7 @@
 		return;
 
 	ibase().abort_build_road();
-	okdialog();
+	reset_mouse_and_die();
 }
 
 /*
@@ -664,7 +665,7 @@
 		upcast(Game, game, &ibase().egbase());
 		game->send_player_bulldoze(*road, SDL_GetModState() & KMOD_CTRL);
 	}
-	okdialog();
+	reset_mouse_and_die();
 }
 
 /*
@@ -679,7 +680,7 @@
 	game->send_player_build(iaplayer->player_number(), node_, Widelands::DescriptionIndex(idx));
 	ibase().reference_player_tribe(player_->player_number(), &player_->tribe());
 	iaplayer->set_flag_to_connect(game->map().br_n(node_));
-	okdialog();
+	reset_mouse_and_die();
 }
 
 void FieldActionWindow::building_icon_mouse_out(Widelands::DescriptionIndex) {
@@ -707,7 +708,7 @@
 	if (upcast(Widelands::Flag, flag, game->map().get_immovable(node_))) {
 		game->send_player_flagaction(*flag);
 	}
-	okdialog();
+	reset_mouse_and_die();
 }
 
 /**
@@ -725,7 +726,7 @@
 			game->send_player_enemyflagaction(building->base_flag(), iaplayer->player_number(),
 			                                  attack_box_->soldiers() /*  number of soldiers */);
 		}
-	okdialog();
+	reset_mouse_and_die();
 }
 
 /*
@@ -781,14 +782,9 @@
 		}
 		if (finish) {
 			ibase->finish_build_road();
+			// We are done, so we close the window.
+			registry->destroy();
 			return;
 		}
 	}
-
-	// If we are here, no new window was opened yet - otherwise we would have
-	// returned already. A new window uses the same registry, so the old window
-	// will be closed. So this means the old window is still open, but but we
-	// still desire to close it. This can happen for example connecting the road
-	// we are building to an existing flag).
-	registry->destroy();
 }