From cf0eb00518ab94cd8127b5375b316b4643fce6d1 Mon Sep 17 00:00:00 2001
From: Subhraman Sarkar <suvrax@gmail.com>
Date: Mon, 3 Mar 2025 09:57:05 +0530
Subject: [PATCH] help topic generator: cleanup, move special notes to separate
 function

---
 src/help/help_topic_generators.cpp | 101 +++++++++++++++--------------
 1 file changed, 53 insertions(+), 48 deletions(-)

diff --git a/src/help/help_topic_generators.cpp b/src/help/help_topic_generators.cpp
index 5a3011e713e..0e10ef31f80 100644
--- a/src/help/help_topic_generators.cpp
+++ b/src/help/help_topic_generators.cpp
@@ -59,14 +59,16 @@ struct terrain_movement_info
 	}
 };
 
-static std::string best_str(bool best) {
+namespace {
+
+std::string best_str(bool best) {
 	std::string lang_policy = (best ? _("Best of") : _("Worst of"));
 	std::string color_policy = (best ? "green": "red");
 
 	return markup::span_color(color_policy, lang_policy);
 }
 
-static std::string format_mp_entry(const int cost, const int max_cost) {
+std::string format_mp_entry(const int cost, const int max_cost) {
 	std::stringstream str_unformatted;
 	const bool cannot = cost < max_cost;
 
@@ -96,7 +98,7 @@ static std::string format_mp_entry(const int cost, const int max_cost) {
 
 typedef t_translation::ter_list::const_iterator ter_iter;
 // Gets an english description of a terrain ter_list alias behavior: "Best of cave, hills", "Worst of Swamp, Forest" etc.
-static std::string print_behavior_description(
+std::string print_behavior_description(
 	const ter_iter& start,
 	const ter_iter& end,
 	const std::shared_ptr<terrain_type_data>& tdata,
@@ -140,12 +142,7 @@ static std::string print_behavior_description(
 
 		ss << best_str(best) << " ";
 		if (!first_level) ss << "( ";
-		ss << names.at(0);
-
-		for (std::size_t i = 1; i < names.size(); i++) {
-			ss << ", " << names.at(i);
-		}
-
+		ss << utils::join(names, ", ");
 		if (!first_level) ss << " )";
 	} else {
 		std::vector<std::string> names;
@@ -163,7 +160,7 @@ static std::string print_behavior_description(
 		if (!first_level) ss << "( ";
 		ss << print_behavior_description(start, *last_change_pos-1, tdata, false, begin_best);
 		// Printed the (parenthesized) leading part from before the change, now print the remaining names in this group.
-		for (const std::string & s : names) {
+		for (const std::string& s : names) {
 			ss << ", " << s;
 		}
 		if (!first_level) ss << " )";
@@ -171,6 +168,39 @@ static std::string print_behavior_description(
 	return ss.str();
 }
 
+std::vector<std::string> get_special_notes(const terrain_type& type) {
+	// Special notes are generated from the terrain's properties - at the moment there's no way for WML authors
+	// to add their own via a [special_note] tag.
+	std::vector<std::string> special_notes;
+
+	if(type.is_village()) {
+		special_notes.push_back(_("Villages allow any unit stationed therein to heal, or to be cured of poison."));
+	} else if(type.gives_healing() > 0) {
+		auto symbols = utils::string_map{{"amount", std::to_string(type.gives_healing())}};
+		// TRANSLATORS: special note for terrains such as the oasis; the only terrain in core with this property heals 8 hp just like a village.
+		// For the single-hitpoint variant, the wording is different because I assume the player will be more interested in the curing-poison part than the minimal healing.
+		auto message = VNGETTEXT("This terrain allows units to be cured of poison, or to heal a single hitpoint.",
+			"This terrain allows units to heal $amount hitpoints, or to be cured of poison, as if stationed in a village.",
+			type.gives_healing(), symbols);
+		special_notes.push_back(std::move(message));
+	}
+
+	if(type.is_castle()) {
+		special_notes.push_back(_("This terrain is a castle — units can be recruited onto it from a connected keep."));
+	}
+	if(type.is_keep() && type.is_castle()) {
+		// TRANSLATORS: The "this terrain is a castle" note will also be shown directly above this one.
+		special_notes.push_back(_("This terrain is a keep — a leader can recruit from this hex onto connected castle hexes."));
+	} else if(type.is_keep() && !type.is_castle()) {
+		// TRANSLATORS: Special note for a terrain, but none of the terrains in mainline do this.
+		special_notes.push_back(_("This unusual keep allows a leader to recruit while standing on it, but does not allow a leader on a connected keep to recruit onto this hex."));
+	}
+
+	return special_notes;
+}
+
+}
+
 std::string terrain_topic_generator::operator()() const {
 	std::stringstream ss;
 
@@ -200,45 +230,21 @@ std::string terrain_topic_generator::operator()() const {
 		ss << "Base terrain: ";
 		const auto base_t = tdata->get_terrain_info(
 			t_translation::terrain_code(type_.number().base, t_translation::NO_LAYER));
-		ss << markup::make_link(base_t.editor_name(), terrain_prefix + base_t.id());
+		ss << markup::make_link(base_t.editor_name(),
+			(base_t.hide_help() ? "." : "") + terrain_prefix + base_t.id());
 		ss << ", ";
 		ss << "Overlay terrain: ";
 		const auto overlay_t = tdata->get_terrain_info(
 			t_translation::terrain_code(t_translation::NO_LAYER, type_.number().overlay));
-		ss << markup::make_link(overlay_t.editor_name(), (overlay_t.hide_help() ? "." : "") + terrain_prefix + overlay_t.id());
+		ss << markup::make_link(overlay_t.editor_name(),
+			(overlay_t.hide_help() ? "." : "") + terrain_prefix + overlay_t.id());
 		ss << "\n";
 	}
 
-	// Special notes are generated from the terrain's properties - at the moment there's no way for WML authors
-	// to add their own via a [special_note] tag.
-	std::vector<std::string> special_notes;
-
-	if(type_.is_village()) {
-		special_notes.push_back(_("Villages allow any unit stationed therein to heal, or to be cured of poison."));
-	} else if(type_.gives_healing() > 0) {
-		auto symbols = utils::string_map{{"amount", std::to_string(type_.gives_healing())}};
-		// TRANSLATORS: special note for terrains such as the oasis; the only terrain in core with this property heals 8 hp just like a village.
-		// For the single-hitpoint variant, the wording is different because I assume the player will be more interested in the curing-poison part than the minimal healing.
-		auto message = VNGETTEXT("This terrain allows units to be cured of poison, or to heal a single hitpoint.",
-			"This terrain allows units to heal $amount hitpoints, or to be cured of poison, as if stationed in a village.",
-			type_.gives_healing(), symbols);
-		special_notes.push_back(std::move(message));
-	}
-
-	if(type_.is_castle()) {
-		special_notes.push_back(_("This terrain is a castle — units can be recruited onto it from a connected keep."));
-	}
-	if(type_.is_keep() && type_.is_castle()) {
-		// TRANSLATORS: The "this terrain is a castle" note will also be shown directly above this one.
-		special_notes.push_back(_("This terrain is a keep — a leader can recruit from this hex onto connected castle hexes."));
-	} else if(type_.is_keep() && !type_.is_castle()) {
-		// TRANSLATORS: Special note for a terrain, but none of the terrains in mainline do this.
-		special_notes.push_back(_("This unusual keep allows a leader to recruit while standing on it, but does not allow a leader on a connected keep to recruit onto this hex."));
-	}
-
-	if(!special_notes.empty()) {
+	const auto& notes = get_special_notes(type_);
+	if(!notes.empty()) {
 		ss << "\n\n" << markup::tag("header", _("Special Notes")) << "\n\n";
-		for(const auto& note : special_notes) {
+		for(const auto& note : notes) {
 			ss << font::unicode_bullet << " " << note << '\n';
 		}
 	}
@@ -306,21 +312,20 @@ std::string terrain_topic_generator::operator()() const {
 
 		ss << type_.income_description();
 
-		if (type_.editor_image().empty()) { // Note: this is purely temporary to help make a different help entry
-			ss << "\nEditor Image: Empty\n";
-		} else {
-			ss << "\nEditor Image: " << type_.editor_image() << "\n";
-		}
+		ss << "\nEditor Image: ";
+		// Note: this is purely temporary to help make a different help entry
+		ss << (type_.editor_image().empty() ? "Empty" : type_.editor_image());
+		ss << "\n";
 
 		const t_translation::ter_list& underlying_mvt_terrains = tdata->underlying_mvt_terrain(type_.number());
 		ss << "\nDebug Mvt Description String:";
-		for (const t_translation::terrain_code & t : underlying_mvt_terrains) {
+		for (const t_translation::terrain_code& t : underlying_mvt_terrains) {
 			ss << " " << t;
 		}
 
 		const t_translation::ter_list& underlying_def_terrains = tdata->underlying_def_terrain(type_.number());
 		ss << "\nDebug Def Description String:";
-		for (const t_translation::terrain_code & t : underlying_def_terrains) {
+		for (const t_translation::terrain_code& t : underlying_def_terrains) {
 			ss << " " << t;
 		}