Merge lp:~widelands-dev/widelands/bug-1538549-dismantle-enemy-buildings into lp:widelands

[GunChleoc]

For the dismantle function, buildings now check which wares the tribe can use and adjust the dismantle button / returned wares accordingly.

See commit message.

[Miroslav Remák (miroslavr256)]

LGTM.

review: Approve (code review + testing)

[TiborB (tiborb95)]

Just lately I modified AI to sent away wares before upgrading, but I believe no conflict here...

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 961. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/120671195.
Appveyor build 794. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1538549_dismantle_enemy_buildings-794.

[GunChleoc (gunchleoc)]

No, no there shouldn't be a conflict.

@bunnybot merge

=== modified file 'src/logic/map_objects/tribes/dismantlesite.cc'
--- src/logic/map_objects/tribes/dismantlesite.cc	2016-03-19 09:58:41 +0000
+++ src/logic/map_objects/tribes/dismantlesite.cc	2016-04-04 16:27:33 +0000
@@ -107,45 +107,37 @@
 {
 	PartiallyFinishedBuilding::init(egbase);
 
-	std::map<DescriptionIndex, uint8_t> wares;
-	count_returned_wares(this, wares);
-
-	std::map<DescriptionIndex, uint8_t>::const_iterator it = wares.begin();
-	wares_.resize(wares.size());
-
-	for (size_t i = 0; i < wares.size(); ++i, ++it) {
-		WaresQueue & wq =
-			*(wares_[i] = new WaresQueue(*this, it->first, it->second));
-
-		wq.set_filled(it->second);
-		work_steps_ += it->second;
+	for (const auto& ware: count_returned_wares(this)) {
+		WaresQueue* wq = new WaresQueue(*this, ware.first, ware.second);
+		wq->set_filled(ware.second);
+		wares_.push_back(wq);
+		work_steps_ += ware.second;
 	}
 }
 
 /*
 ===============
-Count wich wares you get back if you dismantle the given building
+Count which wares you get back if you dismantle the given building
 ===============
 */
-void DismantleSite::count_returned_wares
-	(Building* building,
-	 std::map<DescriptionIndex, uint8_t>   & res)
+const Buildcost DismantleSite::count_returned_wares(Building* building)
 {
+	Buildcost result;
 	for (DescriptionIndex former_idx : building->get_former_buildings()) {
-		const std::map<DescriptionIndex, uint8_t> * return_wares;
 		const BuildingDescr* former_descr = building->owner().tribe().get_building_descr(former_idx);
-		if (former_idx != building->get_former_buildings().front()) {
-			return_wares = & former_descr->returned_wares_enhanced();
-		} else {
-			return_wares = & former_descr->returned_wares();
-		}
-		assert(return_wares != nullptr);
+		const Buildcost& return_wares =
+				former_idx != building->get_former_buildings().front() ?
+									  former_descr->returned_wares_enhanced() :
+									  former_descr->returned_wares();
 
-		std::map<DescriptionIndex, uint8_t>::const_iterator i;
-		for (i = return_wares->begin(); i != return_wares->end(); ++i) {
-			res[i->first] += i->second;
+		for (const auto& ware : return_wares) {
+			// TODO(GunChleoc): Once we have trading, we might want to return all wares again.
+			if (building->owner().tribe().has_ware(ware.first)) {
+				result[ware.first] += ware.second;
+			}
 		}
 	}
+	return result;
 }
 
 

=== modified file 'src/logic/map_objects/tribes/dismantlesite.h'
--- src/logic/map_objects/tribes/dismantlesite.h	2016-01-31 15:31:00 +0000
+++ src/logic/map_objects/tribes/dismantlesite.h	2016-04-04 16:27:33 +0000
@@ -72,7 +72,7 @@
 
 	bool get_building_work(Game &, Worker &, bool success) override;
 
-	static void count_returned_wares(Building* building, std::map<DescriptionIndex, uint8_t> & res);
+	static const Buildcost count_returned_wares(Building* building);
 
 protected:
 	void update_statistics_string(std::string*) override;

=== modified file 'src/wui/buildingwindow.cc'
--- src/wui/buildingwindow.cc	2016-03-29 10:04:48 +0000
+++ src/wui/buildingwindow.cc	2016-04-04 16:27:33 +0000
@@ -272,21 +272,19 @@
 		}
 
 		if (capscache_ & Widelands::Building::PCap_Dismantle) {
-			std::map<Widelands::DescriptionIndex, uint8_t> wares;
-			Widelands::DismantleSite::count_returned_wares(&building_, wares);
-			UI::Button * dismantlebtn =
-				new UI::Button
-					(capsbuttons, "dismantle", 0, 0, 34, 34,
-					 g_gr->images().get("images/ui_basic/but4.png"),
-					 g_gr->images().get(pic_dismantle),
-					 std::string(_("Dismantle")) + "<br><font size=11>" + _("Returns:") + "</font><br>" +
-						 waremap_to_richtext(owner.tribe(), wares));
-			dismantlebtn->sigclicked.connect(boost::bind(&BuildingWindow::act_dismantle, boost::ref(*this)));
-			capsbuttons->add
-				(dismantlebtn,
-				 UI::Align::kHCenter);
-
-			requires_destruction_separator = true;
+			const Widelands::Buildcost wares = Widelands::DismantleSite::count_returned_wares(&building_);
+			if (!wares.empty()) {
+				UI::Button * dismantlebtn =
+					new UI::Button
+						(capsbuttons, "dismantle", 0, 0, 34, 34,
+						 g_gr->images().get("images/ui_basic/but4.png"),
+						 g_gr->images().get(pic_dismantle),
+						 std::string(_("Dismantle")) + "<br><font size=11>" + _("Returns:") + "</font><br>" +
+							 waremap_to_richtext(owner.tribe(), wares));
+				dismantlebtn->sigclicked.connect(boost::bind(&BuildingWindow::act_dismantle, boost::ref(*this)));
+				capsbuttons->add(dismantlebtn, UI::Align::kHCenter);
+				requires_destruction_separator = true;
+			}
 		}
 
 		if (requires_destruction_separator && can_see) {