builtin/mainmenu/dlg_config_world.lua

@@ -132,20 +132,33 @@ local function handle_buttons(this, fields)
 
 		local rawlist = this.data.list:get_raw_list()
 
+		-- mod_decisions helps keeping track of what value to assign to the
+		-- load_mod_modname setting, as only one mod with a particular name
+		-- can be set.
+		-- Values: nil = not yet decided, otherwise the value of mod.enabled
+		-- for the mod that was chosen.
+		local mod_decisions = {}
+
 		for i = 1, #rawlist do
 			local mod = rawlist[i]
+			local load_mod_setting = "load_mod_" .. mod.name
 			if not mod.is_modpack and
 					not mod.is_game_content then
 				if modname_valid(mod.name) then
-					worldfile:set("load_mod_" .. mod.name,
-							tostring(mod.enabled))
+					local decision = mod_decisions[mod.name]
+					-- Is it either enabled or the first one seen?
+					if decision == nil or mod.enabled then
+						mod_decisions[mod.name] = mod.enabled
+						worldfile:set(load_mod_setting, mod.enabled and
+							pkgmgr.get_modpack_path(mod) or "false")
+					end
 				elseif mod.enabled then
 					gamedata.errormessage = fgettext_ne("Failed to enable mo" ..
 							"d \"$1\" as it contains disallowed characters. " ..
 							"Only chararacters [a-z0-9_] are allowed.",
 							mod.name)
 				end
-				mods["load_mod_" .. mod.name] = nil
+				mods[load_mod_setting] = nil
 			end
 		end
 

builtin/mainmenu/pkgmgr.lua

@@ -160,6 +160,11 @@ function pkgmgr.get_base_folder(temppath)
 end
 
 --------------------------------------------------------------------------------
+function pkgmgr.get_modpack_path(mod)
+	return mod.modpack and "mods." .. mod.modpack or "mods"
+end
+
+--------------------------------------------------------------------------------
 function pkgmgr.isValidModname(modpath)
 	if modpath:find("-") ~= nil then
 		return false
@@ -275,7 +280,7 @@ function pkgmgr.render_packagelist(render_list)
 
 			for j = 1, #rawlist, 1 do
 				if rawlist[j].modpack == list[i].name and
-						rawlist[j].enabled ~= true then
+						not rawlist[j].enabled then
 					-- Modpack not entirely enabled so showing as grey
 					color = mt_color_grey
 					break
@@ -332,21 +337,34 @@ function pkgmgr.enable_mod(this, toset)
 	-- toggle or en/disable the mod
 	if not mod.is_modpack then
 		if toset == nil then
-			mod.enabled = not mod.enabled
-		else
-			mod.enabled = toset
+			toset = not mod.enabled
+		end
+		-- Disable all other mods with the same name in other paths
+		-- and enable this one.
+		for i, mod_to_set in ipairs(pkgmgr.mods_by_name[mod.name]) do
+			if not mod_to_set.is_game_content then
+				mod_to_set.enabled = mod_to_set.modpack == mod.modpack and toset
+			end
 		end
 		return
 	end
 
 	-- toggle or en/disable every mod in the modpack, interleaved unsupported
 	local list = this.data.list:get_raw_list()
-	for i = 1, #list do
-		if list[i].modpack == mod.name then
+	for i, this_mod in ipairs(list) do
+		if not this_mod.is_game_content and this_mod.modpack == mod.name then
+
 			if toset == nil then
-				toset = not list[i].enabled
+				toset = not this_mod.enabled
+			end
+			-- For each mod in the modpack, disable all mods with the same
+			-- name in other paths and enable this one.
+			for i, mod_to_set in ipairs(pkgmgr.mods_by_name[this_mod.name]) do
+				if not mod_to_set.is_game_content then
+					local same_mp = mod_to_set.modpack == this_mod.modpack
+					mod_to_set.enabled = same_mp and toset
+				end
 			end
-			list[i].enabled = toset
 		end
 	end
 end
@@ -366,7 +384,10 @@ function pkgmgr.get_worldconfig(worldpath)
 		if key == "gameid" then
 			worldconfig.id = value
 		elseif key:sub(0, 9) == "load_mod_" then
-			worldconfig.global_mods[key] = core.is_yes(value)
+			-- Compatibility: Check against "nil" which was erroneously used
+			-- as value for fresh configured worlds
+			worldconfig.global_mods[key] = value ~= "false" and value ~= "nil"
+				and value
 		else
 			worldconfig[key] = value
 		end
@@ -554,25 +575,67 @@ function pkgmgr.preparemodlist(data)
 
 	local worldfile = Settings(filename)
 
-	for key,value in pairs(worldfile:to_table()) do
-		if key:sub(1, 9) == "load_mod_" then
-			key = key:sub(10)
-			local element = nil
-			for i=1,#retval,1 do
-				if retval[i].name == key and
-					not retval[i].is_modpack then
-					element = retval[i]
-					break
+	pkgmgr.mods_by_name = {}
+	-- Note mods_active_by_name tracks only mods with a setting value of "true".
+	local mods_active_by_name = {}
+	-- missing_configured_mods tracks the mods that have a load_mod_XXX but
+	-- aren't found in any folder
+	local missing_configured_mods = worldfile:to_table()
+
+	for i, mod in ipairs(retval) do
+		if not mod.is_modpack then
+			pkgmgr.mods_by_name[mod.name] = pkgmgr.mods_by_name[mod.name] or {}
+			table.insert(pkgmgr.mods_by_name[mod.name], mod)
+			local key = "load_mod_" .. mod.name
+			local value = worldfile:get(key)
+			if value then
+				local is_true = core.is_yes(value)
+				if is_true and mod.typ == "global_mod" then
+					mods_active_by_name[mod.name] = mods_active_by_name[mod.name] or {}
+					table.insert(mods_active_by_name[mod.name], mod)
+				end
+				mod.enabled = is_true or value == pkgmgr.get_modpack_path(mod)
+				if mod.enabled then
+					-- Configured another one.
+					missing_configured_mods[key] = nil
 				end
 			end
-			if element ~= nil then
-				element.enabled = core.is_yes(value)
-			else
-				core.log("info", "Mod: " .. key .. " " .. dump(value) .. " but not found")
-			end
 		end
 	end
 
+	for key, value in pairs(missing_configured_mods) do
+		if key:sub(1, 9) == "load_mod_" and value ~= "false" and
+				value ~= "nil" then
+			core.log("info", "Config says that mod \"" .. key:sub(10)
+				.. "\" should be loaded from " .. dump(value)
+				.. " but it was not found there")
+		end
+	end
+
+	for name, conflicting in pairs(mods_active_by_name) do
+		-- No conflict if there's just one.
+		local nconflicting = #conflicting
+		if nconflicting > 1 then
+			-- Resolve conflict following the core's algorithm:
+			-- 1. If it is in a modpack, disable it.
+			for j = nconflicting, 1, -1 do
+				if conflicting[j].modpack then
+					conflicting[j].enabled = false
+					-- Remove element from list in O(1) operations.
+					conflicting[j] = conflicting[nconflicting]
+					conflicting[nconflicting] = nil
+					nconflicting = nconflicting - 1
+				end
+			end
+			-- 2. Disable all in root if more than one.
+			if nconflicting > 1 then
+				for j = 1, nconflicting do
+					conflicting[j].enabled = false
+				end
+ 			end
+ 		end
+ 	end
+
 	return retval
 end
 

doc/world_format.txt

@@ -138,6 +138,13 @@ Example content (added indentation and - explanations):
   load_mod_<mod> = false        - whether <mod> is to be loaded in this world
   auth_backend = files          - which DB backend to use for authentication data
 
+For load_mod_<mod>, the possible values are:
+  false - Do not load the mod.
+  mods  - Load the mod with name <mod> from the mods folder itself, ignore others.
+  mods.<modpackname> - Load the mod from the specified modpack in the mods folder.
+  true  - Load the mod from wherever it is found (may cause conflicts if the
+          same mod appears also in some other place).
+
 Player File Format
 ===================
 

src/content/mods.cpp

@@ -44,7 +44,17 @@ bool parseDependsString(std::string &dep, std::unordered_set<char> &symbols)
 	return !dep.empty();
 }
 
-void parseModContents(ModSpec &spec)
+/*
+ * Build a tree of mods, where the first level of children is an array of all
+ * the mods and modpacks in the given path, and subsequent levels are the
+ * modpack contents and get stored in the corresponding child's
+ * ModSpec.modpack_content.
+ */
+static std::map<std::string, ModSpec> getModsInPath(const std::string &path,
+	const std::string &modpack_path = MODPACK_ROOT_MODS);
+
+// Retrieves depends, optdepends, is_modpack and modpack_content
+void parseModContents(ModSpec &spec, const std::string &modpack_path)
 {
 	// NOTE: this function works in mutual recursion with getModsInPath
 	Settings info;
@@ -69,7 +79,9 @@ void parseModContents(ModSpec &spec)
 	if (modpack_is.good()) {    // a modpack, recursively get the mods in it
 		modpack_is.close(); // We don't actually need the file
 		spec.is_modpack = true;
-		spec.modpack_content = getModsInPath(spec.path, true);
+		if (!modpack_path.empty())
+			spec.modpack_content = getModsInPath(spec.path,
+					modpack_path + MODPACK_PATH_SEP + spec.name);
 		// modpacks have no dependencies; they are defined and
 		// tracked separately for each mod in the modpack
 
@@ -134,8 +146,8 @@ void parseModContents(ModSpec &spec)
 	}
 }
 
-std::map<std::string, ModSpec> getModsInPath(
-		const std::string &path, bool part_of_modpack)
+static std::map<std::string, ModSpec> getModsInPath(
+		const std::string &path, const std::string &modpack_path)
 {
 	// NOTE: this function works in mutual recursion with parseModContents
 
@@ -156,14 +168,18 @@ std::map<std::string, ModSpec> getModsInPath(
 		modpath.clear();
 		modpath.append(path).append(DIR_DELIM).append(modname);
 
-		ModSpec spec(modname, modpath, part_of_modpack);
-		parseModContents(spec);
+		ModSpec spec(modname, modpath, modpack_path);
+		parseModContents(spec, modpack_path);
 		result.insert(std::make_pair(modname, spec));
 	}
 	return result;
 }
 
-std::vector<ModSpec> flattenMods(std::map<std::string, ModSpec> mods)
+/*
+ * Flatten the tree obtained with getModsInPath, to a one-dimensional array
+ * where only mods and not modpacks are present.
+ */
+static std::vector<ModSpec> flattenMods(const std::map<std::string, ModSpec> &mods)
 {
 	std::vector<ModSpec> result;
 	for (const auto &it : mods) {
@@ -196,9 +212,9 @@ void ModConfiguration::printUnsatisfiedModsError() const
 	}
 }
 
-void ModConfiguration::addModsInPath(const std::string &path)
+void ModConfiguration::addModsInPath(const std::string &path, const std::string mp_root)
 {
-	addMods(flattenMods(getModsInPath(path)));
+	addMods(flattenMods(getModsInPath(path, mp_root)));
 }
 
 void ModConfiguration::addMods(const std::vector<ModSpec> &new_mods)
@@ -220,7 +236,9 @@ 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 != (bool)want_from_modpack)
+			bool part_of_modpack = (mod.modpack_path.find(MODPACK_PATH_SEP) !=
+					std::string::npos);
+			if (part_of_modpack != (bool)want_from_modpack)
 				continue;
 
 			if (existing_mods.count(mod.name) == 0) {
@@ -265,25 +283,34 @@ void ModConfiguration::addModsFromConfig(
 		const std::string &settings_path, const std::set<std::string> &mods)
 {
 	Settings conf;
-	std::set<std::string> load_mod_names;
+	std::unordered_map<std::string, std::string> load_mod_names;
 
 	conf.readConfigFile(settings_path.c_str());
 	std::vector<std::string> names = conf.getNames();
 	for (const std::string &name : names) {
-		if (name.compare(0, 9, "load_mod_") == 0 && conf.getBool(name))
-			load_mod_names.insert(name.substr(9));
+		if (name.compare(0, 9, "load_mod_") == 0) {
+			const std::string &modpack_path = conf.get(name);
+			if (modpack_path != "false" && modpack_path != "nil") {
+				load_mod_names.insert(std::make_pair(name.substr(9), modpack_path));
+			}
+		}
 	}
 
 	std::vector<ModSpec> addon_mods;
 	for (const std::string &i : mods) {
-		std::vector<ModSpec> addon_mods_in_path = flattenMods(getModsInPath(i));
-		for (std::vector<ModSpec>::const_iterator it = addon_mods_in_path.begin();
-				it != addon_mods_in_path.end(); ++it) {
-			const ModSpec &mod = *it;
-			if (load_mod_names.count(mod.name) != 0)
-				addon_mods.push_back(mod);
-			else
-				conf.setBool("load_mod_" + mod.name, false);
+		std::vector<ModSpec> addon_mods_in_path = flattenMods(getModsInPath(i,
+				MODPACK_ROOT_MODS));
+		for (const ModSpec &mod : addon_mods_in_path) {
+			auto i = load_mod_names.find(mod.name);
+			if (i != load_mod_names.end()) {
+				const std::string &modpack_path = i->second;
+				if (is_yes(modpack_path)  // old search method
+						|| modpack_path == mod.modpack_path) {
+					addon_mods.push_back(mod);
+				}
+			} else {
+				conf.set("load_mod_" + mod.name, "false");
+			}
 		}
 	}
 	conf.updateConfigFile(settings_path.c_str());
@@ -302,8 +329,9 @@ void ModConfiguration::addModsFromConfig(
 
 	if (!load_mod_names.empty()) {
 		errorstream << "The following mods could not be found:";
-		for (const std::string &mod : load_mod_names)
-			errorstream << " \"" << mod << "\"";
+		for (const std::pair<std::string, std::string> &mod : load_mod_names)
+			errorstream << " \"" << mod.first << "\" from modpack \"" <<
+				mod.second << "\"";
 		errorstream << std::endl;
 	}
 }

src/content/mods.h

@@ -33,6 +33,16 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #define MODNAME_ALLOWED_CHARS "abcdefghijklmnopqrstuvwxyz0123456789_"
 
+// Modpack path root names for the locations where mods are. These roots are
+// not real filesystem paths, just names to distinguish the origin of the
+// mods. Modpack paths are separated by dots, e.g.
+//    mods.my_modpack.my_nested_modpack
+#define MODPACK_ROOT_GAME  "game"           // unimplemented
+#define MODPACK_ROOT_WORLD "world"          // unimplemented
+#define MODPACK_ROOT_MODS  "mods"
+#define MODPACK_ROOT_CLIENT_MODS "cmods"    // unimplemented
+#define MODPACK_PATH_SEP   "."
+
 struct ModSpec
 {
 	std::string name;
@@ -46,7 +56,7 @@ struct ModSpec
 	std::unordered_set<std::string> optdepends;
 	std::unordered_set<std::string> unsatisfied_depends;
 
-	bool part_of_modpack = false;
+	std::string modpack_path;
 	bool is_modpack = false;
 
 	// if modpack:
@@ -55,20 +65,16 @@ struct ModSpec
 			name(name), path(path)
 	{
 	}
-	ModSpec(const std::string &name, const std::string &path, bool part_of_modpack) :
-			name(name), path(path), part_of_modpack(part_of_modpack)
+	ModSpec(const std::string &name, const std::string &path,
+			const std::string &modpack_path) :
+			name(name), path(path), modpack_path(modpack_path)
 	{
 	}
 };
 
-// Retrieves depends, optdepends, is_modpack and modpack_content
-void parseModContents(ModSpec &mod);
-
-std::map<std::string, ModSpec> getModsInPath(
-		const std::string &path, bool part_of_modpack = false);
-
-// replaces modpack Modspecs with their content
-std::vector<ModSpec> flattenMods(std::map<std::string, ModSpec> mods);
+// Retrieves depends, optdepends, is_modpack and modpack_content.
+// Empty modpack_path means the caller is not interested in modpack contents.
+void parseModContents(ModSpec &spec, const std::string &modpack_path = "");
 
 // a ModConfiguration is a subset of installed mods, expected to have
 // all dependencies fullfilled, so it can be used as a list of mods to
@@ -92,7 +98,7 @@ protected:
 	ModConfiguration(const std::string &worldpath);
 	// adds all mods in the given path. used for games, modpacks
 	// and world-specific mods (worldmods-folders)
-	void addModsInPath(const std::string &path);
+	void addModsInPath(const std::string &path, const std::string mp_root);
 
 	// adds all mods in the set.
 	void addMods(const std::vector<ModSpec> &new_mods);

src/server/mods.cpp

@@ -39,8 +39,8 @@ ServerModManager::ServerModManager(const std::string &worldpath) :
 	SubgameSpec gamespec = findWorldSubgame(worldpath);
 
 	// Add all game mods and all world mods
-	addModsInPath(gamespec.gamemods_path);
-	addModsInPath(worldpath + DIR_DELIM + "worldmods");
+	addModsInPath(gamespec.gamemods_path, MODPACK_ROOT_GAME);
+	addModsInPath(worldpath + DIR_DELIM + "worldmods", MODPACK_ROOT_WORLD);
 
 	// Load normal mods
 	std::string worldmt = worldpath + DIR_DELIM + "world.mt";