widelands-dev team mailing list archive

[Benedikt Straub <benedikt-straub@xxxxxx>]

Benedikt Straub has proposed merging lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands.

Commit message:
Move the economy options window to top when clicking Configure Economy

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1827696 in widelands: "Allow users to define their own economy default settings"
  https://bugs.launchpad.net/widelands/+bug/1827696
  Bug #1831196 in widelands: "Economy profile selection doesn't work"
  https://bugs.launchpad.net/widelands/+bug/1831196
  Bug #1839948 in widelands: "Missing can_act check in EconomyOptionsWindow"
  https://bugs.launchpad.net/widelands/+bug/1839948
  Bug #1842335 in widelands: "Economy settings don't move to foreground on fieldaction"
  https://bugs.launchpad.net/widelands/+bug/1842335

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/economy-target-profiles/+merge/372447
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/economy-target-profiles into lp:widelands.

=== modified file 'src/economy/economy.cc'
--- src/economy/economy.cc	2019-06-23 11:41:17 +0000
+++ src/economy/economy.cc	2019-09-07 16:28:50 +0000
@@ -52,7 +52,7 @@
 }
 
 Economy::Economy(Player& player, Serial init_serial)
-   : serial_(init_serial), owner_(player), request_timerid_(0), has_window_(false) {
+   : serial_(init_serial), owner_(player), request_timerid_(0), options_window_(nullptr) {
 	last_economy_serial_ = std::max(last_economy_serial_, serial_ + 1);
 	const TribeDescr& tribe = player.tribe();
 	DescriptionIndex const nr_wares = player.egbase().tribes().nrwares();
@@ -535,7 +535,7 @@
 
 	//  If the options window for e is open, but not the one for this, the user
 	//  should still have an options window after the merge.
-	if (e.has_window() && !has_window()) {
+	if (e.get_options_window() && !get_options_window()) {
 		Notifications::publish(NoteEconomy{e.serial(), serial_, NoteEconomy::Action::kMerged});
 	}
 

=== modified file 'src/economy/economy.h'
--- src/economy/economy.h	2019-02-23 11:00:49 +0000
+++ src/economy/economy.h	2019-09-07 16:28:50 +0000
@@ -196,11 +196,11 @@
 		return worker_target_quantities_[i];
 	}
 
-	bool has_window() const {
-		return has_window_;
+	void* get_options_window() const {
+		return options_window_;
 	}
-	void set_has_window(bool yes) {
-		has_window_ = yes;
+	void set_options_window(void* window) {
+		options_window_ = window;
 	}
 
 	const WareList& get_wares() const {
@@ -289,7 +289,11 @@
 	uint32_t request_timerid_;
 
 	static std::unique_ptr<Soldier> soldier_prototype_;
-	bool has_window_;
+
+	// This is always an EconomyOptionsWindow* (or nullptr) but I don't want a wui dependency here.
+	// We cannot use UniqueWindow to make sure an economy never has two windows because the serial
+	// may change when merging while the window is open, so we have to keep track of it here.
+	void* options_window_;
 
 	// 'list' of unique providers
 	std::map<UniqueDistance, Supply*> available_supplies_;

=== modified file 'src/wui/economy_options_window.cc'
--- src/wui/economy_options_window.cc	2019-08-25 14:50:16 +0000
+++ src/wui/economy_options_window.cc	2019-09-07 16:28:50 +0000
@@ -124,7 +124,7 @@
 	main_box_.add_space(8);
 	main_box_.add(&dropdown_box_, UI::Box::Resizing::kAlign, UI::Align::kCenter);
 
-	economy->set_has_window(true);
+	economy->set_options_window(static_cast<void*>(this));
 	economynotes_subscriber_ = Notifications::subscribe<Widelands::NoteEconomy>(
 	   [this](const Widelands::NoteEconomy& note) { on_economy_note(note); });
 	profilenotes_subscriber_ =
@@ -145,7 +145,7 @@
 EconomyOptionsWindow::~EconomyOptionsWindow() {
 	Widelands::Economy* economy = player_->get_economy(serial_);
 	if (economy != nullptr) {
-		economy->set_has_window(false);
+		economy->set_options_window(nullptr);
 	}
 	if (save_profile_dialog_) {
 		save_profile_dialog_->unset_parent();
@@ -162,7 +162,7 @@
 				die();
 				return;
 			}
-			economy->set_has_window(true);
+			economy->set_options_window(static_cast<void*>(this));
 			ware_panel_->set_economy(note.new_economy);
 			worker_panel_->set_economy(note.new_economy);
 			move_to_top();

=== modified file 'src/wui/fieldaction.cc'
--- src/wui/fieldaction.cc	2019-09-05 19:57:55 +0000
+++ src/wui/fieldaction.cc	2019-09-07 16:28:50 +0000
@@ -632,7 +632,9 @@
 void FieldActionWindow::act_configure_economy() {
 	if (upcast(const Widelands::Flag, flag, node_.field->get_immovable())) {
 		Widelands::Economy* economy = flag->get_economy();
-		if (!economy->has_window()) {
+		if (economy->get_options_window()) {
+			static_cast<EconomyOptionsWindow*>(economy->get_options_window())->move_to_top();
+		} else {
 			bool can_act =
 			   dynamic_cast<InteractiveGameBase&>(ibase()).can_act(economy->owner().player_number());
 			new EconomyOptionsWindow(dynamic_cast<UI::Panel*>(&ibase()), economy, can_act);