Merge pull request OpenXcom#1009 from myk002/badmods

disable mods that have bad rulesets

Author: SupSuper

src/Engine/FileMap.cpp

@@ -28,10 +28,10 @@ namespace OpenXcom
 namespace FileMap
 {
 
-static std::vector< std::vector<std::string> >        _rulesets;
-static std::map<std::string, std::string>             _resources;
+static std::vector<std::pair<std::string, std::vector<std::string> > > _rulesets;
+static std::map<std::string, std::string> _resources;
 static std::map< std::string, std::set<std::string> > _vdirs;
-static std::set<std::string>                          _emptySet;
+static std::set<std::string> _emptySet;
 
 static std::string _canonicalize(const std::string &in)
 {
@@ -91,7 +91,7 @@ std::set<std::string> _filterFiles(const T &files, const std::string &ext)
 std::set<std::string> filterFiles(const std::vector<std::string> &files, const std::string &ext) { return _filterFiles(files, ext); }
 std::set<std::string> filterFiles(const std::set<std::string>    &files, const std::string &ext) { return _filterFiles(files, ext); }
 
-const std::vector< std::vector<std::string> > &getRulesets()
+const std::vector<std::pair<std::string, std::vector<std::string> > > &getRulesets()
 {
 	return _rulesets;
 }
@@ -107,20 +107,21 @@ static std::string _combinePath(const std::string &prefixPath, const std::string
 	return ret;
 }
 
-static void _mapFiles(const std::string &basePath, const std::string &relPath, bool ignoreRulesets)
+static void _mapFiles(const std::string &modId, const std::string &basePath,
+		      const std::string &relPath, bool ignoreRulesets)
 {
 	std::string fullDir = basePath + (relPath.length() ? "/" + relPath : "");
 	std::vector<std::string> files = CrossPlatform::getFolderContents(fullDir);
 	std::set<std::string> rulesetFiles = _filterFiles(files, "rul");
 
 	if (!ignoreRulesets && rulesetFiles.size())
 	{
-		_rulesets.insert(_rulesets.begin(), std::vector<std::string>());
+		_rulesets.insert(_rulesets.begin(), std::pair<std::string, std::vector<std::string> >(modId, std::vector<std::string>()));
 		for (std::set<std::string>::iterator i = rulesetFiles.begin(); i != rulesetFiles.end(); ++i)
 		{
 			std::string fullpath = fullDir + "/" + *i;
 			Log(LOG_DEBUG) << "  recording ruleset: " << fullpath;
-			_rulesets.front().push_back(fullpath);
+			_rulesets.front().second.push_back(fullpath);
 		}
 	}
 
@@ -143,7 +144,7 @@ static void _mapFiles(const std::string &basePath, const std::string &relPath, b
 			// record ruleset files in that subdirectory, otherwise ignore them
 			bool ignoreRulesetsRecurse =
 				!rulesetFiles.empty() || !relPath.empty() || _canonicalize(*i) != "ruleset";
-			_mapFiles(basePath, _combinePath(relPath, *i), ignoreRulesetsRecurse);
+			_mapFiles(modId, basePath, _combinePath(relPath, *i), ignoreRulesetsRecurse);
 			continue;
 		}
 
@@ -179,10 +180,10 @@ void clear()
 	_vdirs.clear();
 }
 
-void load(const std::string &path, bool ignoreRulesets)
+void load(const std::string &modId, const std::string &path, bool ignoreRulesets)
 {
 	Log(LOG_INFO) << "  mapping resources in: " << path;
-	_mapFiles(path, "", ignoreRulesets);
+	_mapFiles(modId, path, "", ignoreRulesets);
 }
 
 }

src/Engine/FileMap.h

@@ -46,18 +46,18 @@ namespace FileMap
 	std::set<std::string> filterFiles(const std::vector<std::string> &files, const std::string &ext);
 	std::set<std::string> filterFiles(const std::set<std::string>    &files, const std::string &ext);
 
-	/// Returns the ruleset files found, grouped by mod, while mapping resources.  The highest-prioirity mod
+	/// Returns the ruleset files found, grouped by mod, while mapping resources.  The highest-priority mod
 	/// will be last in the returned vector.
-	const std::vector< std::vector<std::string> > &getRulesets();
+	const std::vector<std::pair<std::string, std::vector<std::string> > > &getRulesets();
 
 	/// clears FileMap state
 	void clear();
 
 	/// Scans a directory tree rooted at the specified filesystem path.  Any files it encounters that have already
 	/// been mapped will be ignored.  Therefore, load files from mods with the highest priority first.  If
-	/// ignoreRulesets is false (the default), it will add any rulesets it finds to the front of the vector
+	/// ignoreRulesets is false, it will add any rulesets it finds to the front of the vector
 	/// returned by getRulesets().
-	void load(const std::string &path, bool ignoreRulesets = false);
+	void load(const std::string &modId, const std::string &path, bool ignoreRulesets);
 }
 
 }

src/Engine/Game.cpp

@@ -534,16 +534,36 @@ void Game::loadRulesets()
 	Ruleset::resetGlobalStatics();
 	delete _rules;
 	_rules = new Ruleset();
-	const std::vector< std::vector<std::string> > &rulesets(FileMap::getRulesets());
+	const std::vector<std::pair<std::string, std::vector<std::string> > > &rulesets(FileMap::getRulesets());
 	for (int i = 0; rulesets.size() > i; ++i)
 	{
 		try
 		{
-			_rules->loadModRulesets(rulesets[i], i);
+			_rules->loadModRulesets(rulesets[i].second, i);
 		}
 		catch (YAML::Exception &e)
 		{
-			throw Exception("failed to load ruleset: " + std::string(e.what()));
+			const std::string &modId = rulesets[i].first;
+			Log(LOG_WARNING) << "disabling mod with invalid ruleset: " << modId;
+			std::vector<std::pair<std::string, bool> >::iterator it =
+				std::find(Options::mods.begin(), Options::mods.end(),
+					  std::pair<std::string, bool>(modId, true));
+			if (it == Options::mods.end())
+			{
+				Log(LOG_ERROR) << "cannot find broken mod in mods list: " << modId;
+				Log(LOG_ERROR) << "clearing mods list";
+				Options::mods.clear();
+			}
+			else
+			{
+				it->second = false;
+			}
+			Options::save();
+
+			throw Exception("failed to load ruleset from mod '" +
+				Options::getModInfos().at(modId).getName() +
+				"' (" + std::string(e.what()) +
+				"); disabling mod for next startup");
 		}
 	}
 	_rules->sortLists();

src/Engine/Options.cpp

@@ -575,7 +575,9 @@ bool init(int argc, char *argv[])
 					}
 					else
 					{
-						if (inactiveMaster.empty())
+						// prefer activating standard masters over a possibly broken
+						// third party master
+						if (inactiveMaster.empty() || j->first == "xcom1" || j->first == "xcom2")
 						{
 							inactiveMaster = j->first;
 						}
@@ -662,11 +664,11 @@ static void _loadMod(const ModInfo &modInfo, std::set<std::string> circDepCheck)
 		return;
 	}
 
-	FileMap::load(modInfo.getPath());
+	FileMap::load(modInfo.getId(), modInfo.getPath(), false);
 	for (std::vector<std::string>::const_iterator i = modInfo.getExternalResourceDirs().begin(); i != modInfo.getExternalResourceDirs().end(); ++i)
 	{
 		// always ignore ruleset files in external resource dirs
-		FileMap::load(CrossPlatform::searchDataFolder(*i), true);
+		FileMap::load(modInfo.getId(), CrossPlatform::searchDataFolder(*i), true);
 	}
 
 	// if this is a master but it has a master of its own, allow it to
@@ -705,7 +707,7 @@ void mapResources()
 		_loadMod(modInfo, circDepCheck);
 	}
 	// pick up stuff in common
-	FileMap::load(CrossPlatform::searchDataFolder("common"), true);
+	FileMap::load("", CrossPlatform::searchDataFolder("common"), true);
 }
 
 /**