← Back to team overview

widelands-dev team mailing list archive

[Merge] lp:~widelands-dev/widelands/obsolete_format_macro into lp:widelands

 

GunChleoc has proposed merging lp:~widelands-dev/widelands/obsolete_format_macro into lp:widelands.

Commit message:
Cleaned up switch statement in Building::info_string: Replaced FORMAT macro with enum class and got rid of unused cases.

Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/obsolete_format_macro/+merge/284590

Another cleanup - got rid of ugly macro and unused cases. This also removes some translations that were never seen.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/obsolete_format_macro into lp:widelands.
=== modified file 'src/logic/map_objects/tribes/building.cc'
--- src/logic/map_objects/tribes/building.cc	2016-01-29 08:37:22 +0000
+++ src/logic/map_objects/tribes/building.cc	2016-02-01 11:51:52 +0000
@@ -481,66 +481,28 @@
 		egbase.create_immovable(pos, "destroyed_building", MapObjectDescr::OwnerType::kTribe);
 }
 
-
-#define FORMAT(key, value) case key: result << value; break
-std::string Building::info_string(const std::string & format) {
-	std::ostringstream result;
-
-	for (std::string::const_iterator format_iter = format.begin();
-		  format_iter != format.end();
-		  ++format_iter) {
-
-		if (*format_iter == '%') {
-			if (++format_iter == format.end()) { // unterminated format sequence
-				result << '%';
-				break;
-			}
-
-			switch (*format_iter) {
-			FORMAT('%', '%');
-			FORMAT('i', serial());
-			FORMAT('t', update_and_get_statistics_string());
-			FORMAT
-				('s',
-				 (descr().get_ismine()                  ? _("mine")   :
-				  get_size  () == BaseImmovable::SMALL  ? _("small")  :
-				  get_size  () == BaseImmovable::MEDIUM ? _("medium") : _("big")));
-			FORMAT
-				('S',
-				 (descr().get_ismine()                  ? _("Mine")   :
-				  get_size  () == BaseImmovable::SMALL  ? _("Small")  :
-				  get_size  () == BaseImmovable::MEDIUM ? _("Medium") : _("Big")));
-			FORMAT('x', get_position().x);
-			FORMAT('y', get_position().y);
-			FORMAT
-				('c', '(' << get_position().x << ", " << get_position().y << ')');
-			FORMAT('A', descr().descname());
-			FORMAT('a', descr().name());
-			case 'N':
-				if (upcast(ConstructionSite const, constructionsite, this))
-					result << constructionsite->building().descname();
-				else
-					result << descr().descname();
-				break;
-			case 'n':
-				if (upcast(ConstructionSite const, constructionsite, this))
-					result << constructionsite->building().name();
-				else
-					result << descr().name();
-				break;
-			case 'r':
-				if (upcast(ProductionSite const, productionsite, this))
-					result << productionsite->production_result();
-				break;
-			default: //  invalid format sequence
-				result << '%' << *format_iter;
-				break;
-			}
-		} else
-			result << *format_iter;
+std::string Building::info_string(InfoStringFormat format) {
+	std::string result = "";
+	switch (format) {
+	case InfoStringFormat::kCensus:
+		if (upcast(ConstructionSite const, constructionsite, this)) {
+			result = constructionsite->building().descname();
+		} else {
+			result = descr().descname();
+		}
+		break;
+	case InfoStringFormat::kStatistics:
+		result = update_and_get_statistics_string();
+		break;
+	case InfoStringFormat::kTooltip:
+		if (upcast(ProductionSite const, productionsite, this)) {
+			result = productionsite->production_result();
+		}
+		break;
+	default:
+		NEVER_HERE();
 	}
-	const std::string result_str = result.str();
-	return result_str.empty() ? result_str : as_uifont(result_str);
+	return result.empty() ? result : as_uifont(result);
 }
 
 
@@ -710,7 +672,7 @@
 	uint32_t const dpyflags = igbase.get_display_flags();
 
 	if (dpyflags & InteractiveBase::dfShowCensus) {
-		const std::string info = info_string(igbase.building_census_format());
+		const std::string info = info_string(InfoStringFormat::kCensus);
 		if (!info.empty()) {
 			dst.blit(pos - Point(0, 48), UI::g_fh1->render(info), BlendMode::UseAlpha, UI::Align::kCenter);
 		}
@@ -722,7 +684,7 @@
 				(!iplayer->player().see_all() &&
 				 iplayer->player().is_hostile(*get_owner()))
 				return;
-		const std::string info = info_string(igbase.building_statistics_format());
+		const std::string info = info_string(InfoStringFormat::kStatistics);
 		if (!info.empty()) {
 			dst.blit(pos - Point(0, 35), UI::g_fh1->render(info), BlendMode::UseAlpha, UI::Align::kCenter);
 		}

=== modified file 'src/logic/map_objects/tribes/building.h'
--- src/logic/map_objects/tribes/building.h	2016-01-06 19:11:20 +0000
+++ src/logic/map_objects/tribes/building.h	2016-02-01 11:51:52 +0000
@@ -173,6 +173,12 @@
 	using FormerBuildings = std::vector<DescriptionIndex>;
 
 public:
+	enum class InfoStringFormat {
+		kCensus,
+		kStatistics,
+		kTooltip
+	};
+
 	Building(const BuildingDescr&);
 	virtual ~Building();
 
@@ -187,7 +193,7 @@
 	virtual Coords get_position() const {return m_position;}
 	PositionList get_positions (const EditorGameBase &) const override;
 
-	std::string info_string(const std::string & format);
+	std::string info_string(InfoStringFormat format);
 
 	// Return the overlay string that is displayed on the map view when enabled
 	// by the player.

=== modified file 'src/wui/interactive_base.cc'
--- src/wui/interactive_base.cc	2016-01-31 21:03:15 +0000
+++ src/wui/interactive_base.cc	2016-02-01 11:51:52 +0000
@@ -206,7 +206,7 @@
 						   player.is_hostile(*productionsite->get_owner())))
 						return set_tooltip("");
 				}
-				set_tooltip(productionsite->info_string(igbase->building_tooltip_format()));
+				set_tooltip(productionsite->info_string(Widelands::Building::InfoStringFormat::kTooltip));
 				return;
 			}
 	}

=== modified file 'src/wui/interactive_gamebase.cc'
--- src/wui/interactive_gamebase.cc	2016-01-29 08:37:22 +0000
+++ src/wui/interactive_gamebase.cc	2016-02-01 11:51:52 +0000
@@ -52,12 +52,6 @@
 	:
 	InteractiveBase(_game, global_s),
 	chat_provider_(nullptr),
-	building_census_format_
-		(global_s.get_string("building_census_format",       "%N")),
-	building_statistics_format_
-		(global_s.get_string("building_statistics_format",   "%t")),
-	building_tooltip_format_
-		(global_s.get_string("building_tooltip_format",      "%r")),
 	chatenabled_(chatenabled),
 	multiplayer_(multiplayer),
 	playertype_(pt),

=== modified file 'src/wui/interactive_gamebase.h'
--- src/wui/interactive_gamebase.h	2016-01-24 20:11:53 +0000
+++ src/wui/interactive_gamebase.h	2016-02-01 11:51:52 +0000
@@ -60,17 +60,6 @@
 	void set_chat_provider(ChatProvider &);
 	ChatProvider * get_chat_provider();
 
-	// TODO(sirver): Remove the use of these methods as the strings are no longer configurable.
-	const std::string & building_census_format      () const {
-		return building_census_format_;
-	}
-	const std::string & building_statistics_format  () const {
-		return building_statistics_format_;
-	}
-	const std::string & building_tooltip_format     () const {
-		return building_tooltip_format_;
-	}
-
 	virtual bool can_see(Widelands::PlayerNumber) const = 0;
 	virtual bool can_act(Widelands::PlayerNumber) const = 0;
 	virtual Widelands::PlayerNumber player_number() const = 0;
@@ -92,9 +81,6 @@
 
 	GameMainMenuWindows      main_windows_;
 	ChatProvider           * chat_provider_;
-	std::string              building_census_format_;
-	std::string              building_statistics_format_;
-	std::string              building_tooltip_format_;
 	bool                     chatenabled_;
 	bool                     multiplayer_;
 	PlayerType playertype_;


Follow ups