Main menu: Fix mod detection within nested modpacks

* Re-use the C++ implementation of mod detection
* Correctly show up nested modpacks in the main menu

builtin/mainmenu/content/pkgmgr.lua

@@ -94,77 +94,25 @@ pkgmgr = {}
 -- @param path         Absolute directory path to scan recursively
 -- @param virtual_path Prettified unique path (e.g. "mods", "mods/mt_modpack")
 -- @param listing      Input. Flat array to insert located mods and modpacks
--- @param modpack      Currently processing modpack or nil/"" if none (recursion)
+function pkgmgr.get_mods(path, virtual_path, listing)
-function pkgmgr.get_mods(path, virtual_path, listing, modpack)
+	local mods = core.get_mod_list(path, virtual_path)
-	local mods = core.get_dir_list(path, true)
+	local parent = {}
-	local added = {}
+	for i, toadd in ipairs(mods) do
-	for _, name in ipairs(mods) do
-		if name:sub(1, 1) ~= "." then
-			local mod_path = path .. DIR_DELIM .. name
-			local mod_virtual_path = virtual_path .. "/" .. name
-			local toadd = {
-				dir_name = name,
-				parent_dir = path,
-			}
 		listing[#listing + 1] = toadd
-			added[#added + 1] = toadd
 
-			-- Get config file
+		if toadd.is_modpack then
-			local mod_conf
+			parent[toadd.modpack_depth + 1] = toadd
-			local modpack_conf = io.open(mod_path .. DIR_DELIM .. "modpack.conf")
+		elseif parent[toadd.modpack_depth] then
-			if modpack_conf then
+			toadd.modpack = parent[toadd.modpack_depth].name
-				toadd.is_modpack = true
-				modpack_conf:close()
-
-				mod_conf = Settings(mod_path .. DIR_DELIM .. "modpack.conf"):to_table()
-				if mod_conf.name then
-					name = mod_conf.name
-					toadd.is_name_explicit = true
-				end
-			else
-				mod_conf = Settings(mod_path .. DIR_DELIM .. "mod.conf"):to_table()
-				if mod_conf.name then
-					name = mod_conf.name
-					toadd.is_name_explicit = true
-				end
 		end
 
-			-- Read from config
+		local parent_dir, dir_name = toadd.path:match("^(.+)[/\\]([^/\\]+)$")
-			toadd.name = name
+		toadd.dir_name = dir_name
-			toadd.title = mod_conf.title
+		toadd.parent_dir = parent_dir
-			toadd.author = mod_conf.author
+		toadd.type = toadd.is_modpack and "modpack" or "mod"
-			toadd.release = tonumber(mod_conf.release) or 0
-			toadd.path = mod_path
-			toadd.virtual_path = mod_virtual_path
-			toadd.type = "mod"
-
-			-- Check modpack.txt
-			-- Note: modpack.conf is already checked above
-			local modpackfile = io.open(mod_path .. DIR_DELIM .. "modpack.txt")
-			if modpackfile then
-				modpackfile:close()
-				toadd.is_modpack = true
 	end
 
-			-- Deal with modpack contents
+	pkgmgr.update_translations(mods)
-			if modpack and modpack ~= "" then
-				toadd.modpack = modpack
-			elseif toadd.is_modpack then
-				toadd.type = "modpack"
-				toadd.is_modpack = true
-				pkgmgr.get_mods(mod_path, mod_virtual_path, listing, name)
-			end
-		end
-	end
-
-	pkgmgr.update_translations(added)
-
-	if not modpack then
-		-- Sort all when the recursion is done
-		table.sort(listing, function(a, b)
-			return a.virtual_path:lower() < b.virtual_path:lower()
-		end)
-	end
 end
 
 --------------------------------------------------------------------------------
@@ -346,11 +294,8 @@ function pkgmgr.render_packagelist(render_list, use_technical_names, with_icon)
 		end
 
 		retval[#retval + 1] = color
-		if v.modpack ~= nil or v.loc == "game" then
+		-- `v.modpack_depth` is `nil` for the selected game (treated as level 0)
-			retval[#retval + 1] = "1"
+		retval[#retval + 1] = (v.modpack_depth or 0) + (v.loc == "game" and 1 or 0)
-		else
-			retval[#retval + 1] = "0"
-		end
 
 		if with_icon then
 			retval[#retval + 1] = icon

doc/menu_lua_api.md

@@ -342,6 +342,10 @@ Package - content which is downloadable from the content db, may or may not be i
           optional_depends = {"mod", "names"}, -- mods only
       }
       ```
+* `core.get_mod_list(path, virtual_path)`
+    * Returns a flat list of mod and modpack information found within the specified path.
+    * Each entry consists of the fields `name`, `author`, `release`, `description`,
+      `path`, `virtual_path`, `is_name_explicit`, `is_modpack`, `modpack_depth`.
 * `core.check_mod_configuration(world_path, mod_paths)`
     * Checks whether configuration is valid.
     * `world_path`: path to the world

src/content/mod_configuration.cpp

@@ -60,7 +60,7 @@ void ModConfiguration::addMods(const std::vector<ModSpec> &new_mods)
 		std::set<std::string> seen_this_iteration;
 
 		for (const ModSpec &mod : new_mods) {
-			if (mod.part_of_modpack != want_from_modpack)
+			if ((mod.modpack_depth > 0) != want_from_modpack)
 				continue;
 
 			// unrelated to this code, but we want to assert it somewhere

src/content/mods.cpp

@@ -61,24 +61,39 @@ bool parseModContents(ModSpec &spec)
 	spec.is_modpack = false;
 	spec.modpack_content.clear();
 
+	std::string conf_filename;
+
 	// Handle modpacks (defined by containing modpack.txt)
-	if (fs::IsFile(spec.path + DIR_DELIM + "modpack.txt") ||
+	if (fs::IsFile(spec.path + DIR_DELIM + "modpack.conf")) {
-			fs::IsFile(spec.path + DIR_DELIM + "modpack.conf")) {
+		spec.is_modpack = true;
+		conf_filename = "modpack.conf";
+	} else if (fs::IsFile(spec.path + DIR_DELIM + "modpack.txt")) {
 		spec.is_modpack = true;
-		spec.modpack_content = getModsInPath(spec.path, spec.virtual_path, true);
-		return true;
 	} else if (!fs::IsFile(spec.path + DIR_DELIM + "init.lua")) {
 		return false;
+	} else {
+		// Is a mod
+		conf_filename = "mod.conf";
 	}
 
+	if (spec.is_modpack)
+		spec.modpack_content = getModsInPath(spec.path, spec.virtual_path, spec.modpack_depth + 1);
 
 	Settings info;
-	info.readConfigFile((spec.path + DIR_DELIM + "mod.conf").c_str());
+	if (!conf_filename.empty())
+		info.readConfigFile((spec.path + DIR_DELIM + conf_filename).c_str());
 
-	if (info.exists("name"))
+	if (info.exists("name")) {
 		spec.name = info.get("name");
-	else
+		spec.is_name_explicit = true;
+	} else if (!spec.is_modpack) {
 		spec.deprecation_msgs.push_back("Mods not having a mod.conf file with the name is deprecated.");
+	}
+
+	if (info.exists("description"))
+		spec.desc = info.get("description");
+	else if (fs::ReadFile(spec.path + DIR_DELIM + "description.txt", spec.desc))
+		spec.deprecation_msgs.push_back("description.txt is deprecated, please use mod[pack].conf instead.");
 
 	if (info.exists("author"))
 		spec.author = info.get("author");
@@ -86,6 +101,12 @@ bool parseModContents(ModSpec &spec)
 	if (info.exists("release"))
 		spec.release = info.getS32("release");
 
+
+	// The subsequent fields are not available for modpacks
+	if (spec.is_modpack)
+		return true;
+
+
 	// Attempt to load dependencies from mod.conf
 	bool mod_conf_has_depends = false;
 	if (info.exists("depends")) {
@@ -135,16 +156,11 @@ bool parseModContents(ModSpec &spec)
 		}
 	}
 
-	if (info.exists("description"))
-		spec.desc = info.get("description");
-	else if (fs::ReadFile(spec.path + DIR_DELIM + "description.txt", spec.desc))
-		spec.deprecation_msgs.push_back("description.txt is deprecated, please use mod.conf instead.");
-
 	return true;
 }
 
 std::map<std::string, ModSpec> getModsInPath(
-		const std::string &path, const std::string &virtual_path, bool part_of_modpack)
+		const std::string &path, const std::string &virtual_path, int modpack_depth)
 {
 	// NOTE: this function works in mutual recursion with parseModContents
 
@@ -171,7 +187,7 @@ std::map<std::string, ModSpec> getModsInPath(
 		// Intentionally uses / to keep paths same on different platforms
 		mod_virtual_path.append(virtual_path).append("/").append(modname);
 
-		ModSpec spec(modname, mod_path, part_of_modpack, mod_virtual_path);
+		ModSpec spec(modname, mod_path, modpack_depth, mod_virtual_path);
 		if (parseModContents(spec)) {
 			result[modname] = std::move(spec);
 		}
@@ -179,19 +195,19 @@ std::map<std::string, ModSpec> getModsInPath(
 	return result;
 }
 
-std::vector<ModSpec> flattenMods(const std::map<std::string, ModSpec> &mods)
+std::vector<ModSpec> flattenMods(const std::map<std::string, ModSpec> &mods,
+		bool discard_modpacks)
 {
 	std::vector<ModSpec> result;
 	for (const auto &it : mods) {
 		const ModSpec &mod = it.second;
+		if (!mod.is_modpack || !discard_modpacks) {
+			result.push_back(mod);
+		}
 		if (mod.is_modpack) {
-			std::vector<ModSpec> content = flattenMods(mod.modpack_content);
+			std::vector<ModSpec> content = flattenMods(mod.modpack_content, discard_modpacks);
 			result.reserve(result.size() + content.size());
 			result.insert(result.end(), content.begin(), content.end());
-
-		} else // not a modpack
-		{
-			result.push_back(mod);
 		}
 	}
 	return result;

src/content/mods.h

@@ -16,6 +16,7 @@ class ModStorageDatabase;
 
 struct ModSpec
 {
+	bool is_name_explicit = false; //< 'Specified in a .conf file?'
 	std::string name;
 	std::string author;
 	std::string path; // absolute path on disk
@@ -27,7 +28,7 @@ struct ModSpec
 	std::unordered_set<std::string> optdepends;
 	std::unordered_set<std::string> unsatisfied_depends;
 
-	bool part_of_modpack = false;
+	int modpack_depth = 0; //< Modpack depth, 0 = no parent modpack
 	bool is_modpack = false;
 
 	/**
@@ -58,8 +59,8 @@ struct ModSpec
 	{
 	}
 
-	ModSpec(const std::string &name, const std::string &path, bool part_of_modpack, const std::string &virtual_path) :
+	ModSpec(const std::string &name, const std::string &path, int modpack_depth, const std::string &virtual_path) :
-			name(name), path(path), part_of_modpack(part_of_modpack), virtual_path(virtual_path)
+			name(name), path(path), modpack_depth(modpack_depth), virtual_path(virtual_path)
 	{
 	}
 
@@ -77,15 +78,16 @@ struct ModSpec
  * Gets a list of all mods and modpacks in path
  *
  * @param Path to search, should be absolute
- * @param part_of_modpack Is this searching within a modpack?
+ * @param modpack_depth If > 0: Is this searching within a modpack
  * @param virtual_path Virtual path for this directory, see comment in ModSpec
  * @returns map of mods
  */
 std::map<std::string, ModSpec> getModsInPath(const std::string &path,
-		const std::string &virtual_path, bool part_of_modpack = false);
+		const std::string &virtual_path, int modpack_depth = 0);
 
 // replaces modpack Modspecs with their content
-std::vector<ModSpec> flattenMods(const std::map<std::string, ModSpec> &mods);
+std::vector<ModSpec> flattenMods(const std::map<std::string, ModSpec> &mods,
+		bool discard_modpacks = true);
 
 
 class ModStorage : public IMetadata

src/gui/guiEngine.cpp

@@ -211,7 +211,7 @@ std::string findLocaleFileWithExtension(const std::string &path)
 /******************************************************************************/
 std::string findLocaleFileInMods(const std::string &path, const std::string &filename_no_ext)
 {
-	std::vector<ModSpec> mods = flattenMods(getModsInPath(path, "root", true));
+	std::vector<ModSpec> mods = flattenMods(getModsInPath(path, "root", 0));
 
 	for (const auto &mod : mods) {
 		std::string ret = findLocaleFileWithExtension(

src/script/common/c_content.cpp

@@ -2654,6 +2654,7 @@ void push_mod_spec(lua_State *L, const ModSpec &spec, bool include_unsatisfied)
 	lua_pushstring(L, spec.virtual_path.c_str());
 	lua_setfield(L, -2, "virtual_path");
 
+	if (include_unsatisfied) {
 		lua_newtable(L);
 		int i = 1;
 		for (const auto &dep : spec.unsatisfied_depends) {
@@ -2662,3 +2663,4 @@ void push_mod_spec(lua_State *L, const ModSpec &spec, bool include_unsatisfied)
 		}
 		lua_setfield(L, -2, "unsatisfied_depends");
 	}
+}

src/script/lua_api/l_mainmenu.cpp

@@ -421,6 +421,34 @@ int ModApiMainMenu::l_get_content_info(lua_State *L)
 	return 1;
 }
 
+/******************************************************************************/
+int ModApiMainMenu::l_get_mod_list(lua_State *L)
+{
+	std::string path = luaL_checkstring(L, 1);
+	std::string virtual_path = luaL_checkstring(L, 2);
+
+	CHECK_SECURE_PATH(L, path.c_str(), false)
+
+	std::vector<ModSpec> mods_flat = flattenMods(getModsInPath(path, virtual_path), false);
+	int i = 0;
+	lua_createtable(L, mods_flat.size(), 0);
+	for (const ModSpec &spec : mods_flat) {
+		push_mod_spec(L, spec, false);
+
+		lua_pushboolean(L, spec.is_name_explicit);
+		lua_setfield(L, -2, "is_name_explicit");
+
+		lua_pushboolean(L, spec.is_modpack);
+		lua_setfield(L, -2, "is_modpack");
+
+		lua_pushinteger(L, spec.modpack_depth);
+		lua_setfield(L, -2, "modpack_depth");
+
+		lua_rawseti(L, -2, ++i); // assign to return table
+	}
+	return 1;
+}
+
 /******************************************************************************/
 int ModApiMainMenu::l_check_mod_configuration(lua_State *L)
 {
@@ -1045,6 +1073,7 @@ void ModApiMainMenu::Initialize(lua_State *L, int top)
 	API_FCT(get_worlds);
 	API_FCT(get_games);
 	API_FCT(get_content_info);
+	API_FCT(get_mod_list);
 	API_FCT(check_mod_configuration);
 	API_FCT(get_content_translation);
 	API_FCT(start);

src/script/lua_api/l_mainmenu.h

@@ -58,6 +58,8 @@ private:
 
 	static int l_get_content_info(lua_State *L);
 
+	static int l_get_mod_list(lua_State *L);
+
 	static int l_check_mod_configuration(lua_State *L);
 
 	static int l_get_content_translation(lua_State *L);