From e8b750f8a7223a17362c105a2ceb9b3709705e14 Mon Sep 17 00:00:00 2001
From: Charles Dang <exodia339@gmail.com>
Date: Tue, 3 Apr 2018 20:19:02 +1100
Subject: [PATCH] Help: fixed unit section such as the Walking Corpse's not
 generating

I didn't realize that is_valid_id was only meant to check IDs provided in the help config and
not IDs generated dynamically; the latter are always correct. Granted, the old code did actually
call its analogous codepath for race and era generation, but it did not for unit sections. My
refactor made it so this check happened *any* time a section was created, regardless of its source.

To rectify that, I moved ID validation for sections into the static section generation loop like
for topics. I also moved the topic id validation prior to the ToD generation, tweaked the invalid
id message, and renamed a variable for clarity and consistency.
---
 src/help/section.cpp | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/help/section.cpp b/src/help/section.cpp
index 72486496001e..81ac37bcf685 100644
--- a/src/help/section.cpp
+++ b/src/help/section.cpp
@@ -63,10 +63,6 @@ void section::initialize(const config& section_cfg)
 		throw max_recursion_reached("Maximum section depth has been reached. Possible circular dependency?");
 	}
 
-	if(recursion_level_ > 0 && !is_valid_id(id)) {
-		throw parse_error(formatter() << "Invalid ID, used for internal purpose: '" << id << "'");
-	}
-
 	if(section_cfg.empty()) {
 		return; // TODO: throw something?
 	}
@@ -74,12 +70,16 @@ void section::initialize(const config& section_cfg)
 	//
 	// Parse static sub-sections.
 	//
-	for(const std::string& child_section : utils::quoted_split(section_cfg["sections"])) {
-		if(const config& child_cfg = manager_->get_section_config(child_section)) {
+	for(const std::string& section_id : utils::quoted_split(section_cfg["sections"])) {
+		if(const config& child_cfg = manager_->get_section_config(section_id)) {
+			if(!is_valid_id(section_id)) {
+				throw parse_error(formatter() << "Invalid ID, used for internal purposes: '" << id << "'");
+			}
+
 			add_section(child_cfg);
 		} else {
 			throw parse_error(formatter()
-				<< "Help section '" << child_section << "' referenced from '" << id << "' but could not be found.");
+				<< "Help section '" << section_id << "' referenced from '" << id << "' but could not be found.");
 		}
 	}
 
@@ -117,14 +117,14 @@ void section::initialize(const config& section_cfg)
 	//
 	for(const std::string& topic_id : utils::quoted_split(section_cfg["topics"])) {
 		if(const config& topic_cfg = manager_->get_topic_config(topic_id)) {
+			if(!is_valid_id(topic_id)) {
+				throw parse_error(formatter() << "Invalid ID, used for internal purposes: '" << id << "'");
+			}
+
 			std::ostringstream text;
 			text << topic_cfg["text"];
 			text << generate_table_of_contents(topic_cfg["generator"]);
 
-			if(!is_valid_id(topic_id)) {
-				throw parse_error(formatter() << "Invalid ID, used for internal purpose: '" << id << "'");
-			}
-
 			// We don't need to use add_topic here since we don't need a special text generator.
 			topics_.emplace_back(topic_id, topic_cfg["title"], text.str());
 		} else {