ENGINES: Change targets to have an 'engine ID'

The engine ID identifies which engine should be used to launch the target.
Also remove the 'single ID' system. Different games from engines that used
that system now have different game IDs.

Also-By: Matthew Hoops <clone2727@gmail.com>

4 files changed, 160 insertions, 94 deletions

diff --git a/base/commandLine.cpp b/base/commandLine.cpp
index b4a9174802..74a5982f32 100644
--- a/base/commandLine.cpp
+++ b/base/commandLine.cpp
@@ -705,9 +705,8 @@ static void listTargets() {
 	printf("Target               Description                                           \n"
 	       "-------------------- ------------------------------------------------------\n");
 
-	using namespace Common;
-	const ConfigManager::DomainMap &domains = ConfMan.getGameDomains();
-	ConfigManager::DomainMap::const_iterator iter;
+	const Common::ConfigManager::DomainMap &domains = ConfMan.getGameDomains();
+	Common::ConfigManager::DomainMap::const_iterator iter;
 
 	Common::Array<Common::String> targets;
 	targets.reserve(domains.size());
@@ -716,15 +715,15 @@ static void listTargets() {
 		Common::String name(iter->_key);
 		Common::String description(iter->_value.getVal("description"));
 
+		// If there's no description, fallback on the default description.
 		if (description.empty()) {
-			// FIXME: At this point, we should check for a "gameid" override
-			// to find the proper desc. In fact, the platform probably should
-			// be taken into account, too.
-			const Common::String &gameid = name;
-			PlainGameDescriptor g = EngineMan.findGame(gameid);
+			PlainGameDescriptor g = EngineMan.findTarget(name);
 			if (g.description)
 				description = g.description;
 		}
+		// If there's still no description, we cannot come up with one. Insert some dummy text.
+		if (description.empty())
+			description = "<Unknown game>";
 
 		targets.push_back(Common::String::format("%-20s %s", name.c_str(), description.c_str()));
 	}
@@ -766,24 +765,21 @@ static Common::Error listSaves(const Common::String &target) {
 		// target specific savepath will be checked
 		ConfMan.setActiveDomain(*i);
 
-		// Grab the gameid from the domain resp. use the target as gameid
-		Common::String gameid;
-		if (domain)
-			gameid = domain->getVal("gameid");
-		if (gameid.empty())
-			gameid = *i;
-		gameid.toLowercase(); // Normalize it to lower case
-
-		// Find the plugin that will handle the specified gameid
+		// Look for a game matching the target
 		const Plugin *plugin = nullptr;
-		EngineMan.findGame(gameid, &plugin);
+		PlainGameDescriptor game;
+		if (domain) {
+			game = EngineMan.findTarget(target, &plugin);
+		} else {
+			game = EngineMan.findGame(target, &plugin);
+		}
 
 		if (!plugin) {
 			// If the target was specified, treat this as an error, and otherwise skip it.
 			if (!target.empty())
 				return Common::Error(Common::kEnginePluginNotFound,
-				                     Common::String::format("target '%s', gameid '%s", i->c_str(), gameid.c_str()));
-			printf("Plugin could not be loaded for target '%s', gameid '%s", i->c_str(), gameid.c_str());
+				                     Common::String::format("target '%s', gameid '%s", i->c_str(), game.gameId));
+			printf("Plugin could not be loaded for target '%s', gameid '%s", i->c_str(), game.gameId);
 			continue;
 		}
 
@@ -794,7 +790,7 @@ static Common::Error listSaves(const Common::String &target) {
 			if (!target.empty())
 				// TODO: Include more info about the target (desc, engine name, ...) ???
 				return Common::Error(Common::kEnginePluginNotSupportSaves,
-				                     Common::String::format("target '%s', gameid '%s", i->c_str(), gameid.c_str()));
+				                     Common::String::format("target '%s', gameid '%s", i->c_str(), game.gameId));
 			continue;
 		}
 
@@ -805,7 +801,7 @@ static Common::Error listSaves(const Common::String &target) {
 			// TODO: Include more info about the target (desc, engine name, ...) ???
 			if (atLeastOneFound)
 				printf("\n");
-			printf("Save states for target '%s' (gameid '%s'):\n", i->c_str(), gameid.c_str());
+			printf("Save states for target '%s' (gameid '%s'):\n", i->c_str(), game.gameId);
 			printf("  Slot Description                                           \n"
 					   "  ---- ------------------------------------------------------\n");
 
@@ -817,7 +813,7 @@ static Common::Error listSaves(const Common::String &target) {
 		} else {
 			// If the target was specified, indicate no save games were found for it. Otherwise just skip it.
 			if (!target.empty())
-				printf("There are no save states for target '%s' (gameid '%s'):\n", i->c_str(), gameid.c_str());
+				printf("There are no save states for target '%s' (gameid '%s'):\n", i->c_str(), game.gameId);
 		}
 	}
 
@@ -916,10 +912,14 @@ static Common::String detectGames(const Common::String &path, const Common::Stri
 		return Common::String();
 	}
 	// TODO this is not especially pretty
-	printf("ID             Description                                                Full Path\n");
-	printf("-------------- ---------------------------------------------------------- ---------------------------------------------------------\n");
+	printf("EngineID       GameID         Description                                                Full Path\n");
+	printf("-------------- -------------- ---------------------------------------------------------- ---------------------------------------------------------\n");
 	for (DetectedGames::const_iterator v = candidates.begin(); v != candidates.end(); ++v) {
-		printf("%-14s %-58s %s\n", v->gameId.c_str(), v->description.c_str(), v->path.c_str());
+		printf("%-14s %-14s %-58s %s\n",
+		       v->engineId.c_str(),
+		       v->gameId.c_str(),
+		       v->description.c_str(),
+		       v->path.c_str());
 	}
 
 	return candidates[0].gameId;
@@ -1010,7 +1010,7 @@ static void runDetectorTest() {
 		bool gameidDiffers = false;
 		DetectedGames::const_iterator x;
 		for (x = candidates.begin(); x != candidates.end(); ++x) {
-			gameidDiffers |= (scumm_stricmp(gameid.c_str(), x->gameId.c_str()) != 0);
+			gameidDiffers |= !gameid.equalsIgnoreCase(x->gameId);
 		}
 
 		if (candidates.empty()) {
@@ -1120,7 +1120,8 @@ void upgradeTargets() {
 		// At this point, g points to a GameDescriptor which we can use to update
 		// the target referred to by dom. We update several things
 
-		// Always set the gameid explicitly (in case of legacy targets)
+		// Always set the engine ID and game ID explicitly (in case of legacy targets)
+		dom["engineid"] = g->engineId;
 		dom["gameid"] = g->gameId;
 
 		// Always set the GUI options. The user should not modify them, and engines might
@@ -1149,7 +1150,7 @@ void upgradeTargets() {
 		// ScummVM still generates an incorrect description string. So, the description
 		// should only be updated if the user explicitly requests this.
 #if 0
-		if (desc != g->description()) {
+		if (desc != g->description) {
 			printf("  -> update desc from '%s' to\n                      '%s' ?\n", desc.c_str(), g->description.c_str());
 			dom["description"] = g->description;
 		}
@@ -1248,9 +1249,17 @@ bool processSettings(Common::String &command, Common::StringMap &settings, Commo
 	// domain (i.e. a target) matching this argument, or alternatively
 	// whether there is a gameid matching that name.
 	if (!command.empty()) {
-		PlainGameDescriptor gd = EngineMan.findGame(command);
-		if (ConfMan.hasGameDomain(command) || gd.gameId) {
-			bool idCameFromCommandLine = false;
+		PlainGameDescriptor gd;
+		const Plugin *plugin = nullptr;
+		if (ConfMan.hasGameDomain(command)) {
+			// Command is a known target
+			ConfMan.setActiveDomain(command);
+		} else if (gd = EngineMan.findGame(command, &plugin), gd.gameId) {
+			// Command is a known game ID
+			ConfMan.setActiveDomain(command);
+
+			ConfMan.set("gameid", gd.gameId);
+			ConfMan.set("engineid", plugin->get<MetaEngine>().getEngineId());
 
 			// WORKAROUND: Fix for bug #1719463: "DETECTOR: Launching
 			// undefined target adds launcher entry"
@@ -1258,15 +1267,7 @@ bool processSettings(Common::String &command, Common::StringMap &settings, Commo
 			// We designate gameids which come strictly from command line
 			// so AdvancedDetector will not save config file with invalid
 			// gameid in case target autoupgrade was performed
-			if (!ConfMan.hasGameDomain(command)) {
-				idCameFromCommandLine = true;
-			}
-
-			ConfMan.setActiveDomain(command);
-
-			if (idCameFromCommandLine)
-				ConfMan.set("id_came_from_command_line", "1");
-
+			ConfMan.set("id_came_from_command_line", "1");
 		} else {
 #ifndef DISABLE_COMMAND_LINE
 			usage("Unrecognized game target '%s'", command.c_str());
diff --git a/base/main.cpp b/base/main.cpp
index 750e09ee87..45c8e393b7 100644
--- a/base/main.cpp
+++ b/base/main.cpp
@@ -108,39 +108,50 @@ static bool launcherDialog() {
 }
 
 static const Plugin *detectPlugin() {
-	const Plugin *plugin = nullptr;
+	// Figure out the engine ID and game ID
+	Common::String engineId = ConfMan.get("engineid");
+	Common::String gameId = ConfMan.get("gameid");
 
-	// Make sure the gameid is set in the config manager, and that it is lowercase.
-	Common::String gameid(ConfMan.getActiveDomainName());
-	assert(!gameid.empty());
-	if (ConfMan.hasKey("gameid")) {
-		gameid = ConfMan.get("gameid");
+	// Print text saying what's going on
+	printf("User picked target '%s' (engine ID '%s', game ID '%s')...\n", ConfMan.getActiveDomainName().c_str(), engineId.c_str(), gameId.c_str());
 
-		// Set last selected game, that the game will be highlighted
-		// on RTL
-		ConfMan.set("lastselectedgame", ConfMan.getActiveDomainName(), Common::ConfigManager::kApplicationDomain);
-		ConfMan.flushToDisk();
+	// At this point the engine ID and game ID must be known
+	if (engineId.empty()) {
+		warning("The engine ID is not set for target '%s'", ConfMan.getActiveDomainName().c_str());
+		return 0;
 	}
 
-	gameid.toLowercase();
-	ConfMan.set("gameid", gameid);
-
-	// Query the plugins and find one that will handle the specified gameid
-	printf("User picked target '%s' (gameid '%s')...\n", ConfMan.getActiveDomainName().c_str(), gameid.c_str());
-	printf("  Looking for a plugin supporting this gameid... ");
+	if (gameId.empty()) {
+		warning("The game ID is not set for target '%s'", ConfMan.getActiveDomainName().c_str());
+		return 0;
+	}
 
-	PlainGameDescriptor game = EngineMan.findGame(gameid, &plugin);
+	const Plugin *plugin = EngineMan.findPlugin(engineId);
+	if (!plugin) {
+		warning("'%s' is an invalid engine ID. Use the --list-engines command to list supported engine IDs", engineId.c_str());
+		return 0;
+	}
 
-	if (plugin == 0) {
-		printf("failed\n");
-		warning("%s is an invalid gameid. Use the --list-games option to list supported gameid", gameid.c_str());
-	} else {
-		printf("%s\n  Starting '%s'\n", plugin->getName(), game.description);
+	// Query the plugin for the game descriptor
+	printf("   Looking for a plugin supporting this target... %s\n", plugin->getName());
+	PlainGameDescriptor game = plugin->get<MetaEngine>().findGame(gameId.c_str());
+	if (!game.gameId) {
+		warning("'%s' is an invalid game ID for the engine '%s'. Use the --list-games option to list supported game IDs", gameId.c_str(), engineId.c_str());
+		return 0;
 	}
 
 	return plugin;
 }
 
+void saveLastLaunchedTarget(const Common::String &target) {
+	if (ConfMan.hasGameDomain(target)) {
+		// Set the last selected game, so the game will be highlighted next time the user
+		// returns to the launcher.
+		ConfMan.set("lastselectedgame", target, Common::ConfigManager::kApplicationDomain);
+		ConfMan.flushToDisk();
+	}
+}
+
 // TODO: specify the possible return values here
 static Common::Error runGame(const Plugin *plugin, OSystem &system, const Common::String &edebuglevels) {
 	// Determine the game data path, for validation and error messages
@@ -208,7 +219,7 @@ static Common::Error runGame(const Plugin *plugin, OSystem &system, const Common
 	Common::String caption(ConfMan.get("description"));
 
 	if (caption.empty()) {
-		PlainGameDescriptor game = EngineMan.findGame(ConfMan.get("gameid"));
+		PlainGameDescriptor game = EngineMan.findTarget(ConfMan.getActiveDomainName());
 		if (game.description) {
 			caption = game.description;
 		}
@@ -532,6 +543,8 @@ extern "C" int scummvm_main(int argc, const char * const argv[]) {
 	// work as well as it should. In theory everything should be destroyed
 	// cleanly, so this is now enabled to encourage people to fix bits :)
 	while (0 != ConfMan.getActiveDomain()) {
+		saveLastLaunchedTarget(ConfMan.getActiveDomainName());
+
 		// Try to find a plugin which feels responsible for the specified game.
 		const Plugin *plugin = detectPlugin();
 		if (plugin) {
diff --git a/base/plugins.cpp b/base/plugins.cpp
index ac217ab960..a64dbdd65c 100644
--- a/base/plugins.cpp
+++ b/base/plugins.cpp
@@ -287,14 +287,14 @@ void PluginManagerUncached::init() {
 
 /**
  * Try to load the plugin by searching in the ConfigManager for a matching
- * gameId under the domain 'plugin_files'.
+ * engine ID under the domain 'engine_plugin_files'.
  **/
-bool PluginManagerUncached::loadPluginFromGameId(const Common::String &gameId) {
-	Common::ConfigManager::Domain *domain = ConfMan.getDomain("plugin_files");
+bool PluginManagerUncached::loadPluginFromEngineId(const Common::String &engineId) {
+	Common::ConfigManager::Domain *domain = ConfMan.getDomain("engine_plugin_files");
 
 	if (domain) {
-		if (domain->contains(gameId)) {
-			Common::String filename = (*domain)[gameId];
+		if (domain->contains(engineId)) {
+			Common::String filename = (*domain)[engineId];
 
 			if (loadPluginByFileName(filename)) {
 				return true;
@@ -326,17 +326,17 @@ bool PluginManagerUncached::loadPluginByFileName(const Common::String &filename)
 
 /**
  * Update the config manager with a plugin file name that we found can handle
- * the game.
+ * the engine.
  **/
-void PluginManagerUncached::updateConfigWithFileName(const Common::String &gameId) {
+void PluginManagerUncached::updateConfigWithFileName(const Common::String &engineId) {
 	// Check if we have a filename for the current plugin
 	if ((*_currentPlugin)->getFileName()) {
-		if (!ConfMan.hasMiscDomain("plugin_files"))
-			ConfMan.addMiscDomain("plugin_files");
+		if (!ConfMan.hasMiscDomain("engine_plugin_files"))
+			ConfMan.addMiscDomain("engine_plugin_files");
 
-		Common::ConfigManager::Domain *domain = ConfMan.getDomain("plugin_files");
+		Common::ConfigManager::Domain *domain = ConfMan.getDomain("engine_plugin_files");
 		assert(domain);
-		(*domain)[gameId] = (*_currentPlugin)->getFileName();
+		(*domain)[engineId] = (*_currentPlugin)->getFileName();
 
 		ConfMan.flushToDisk();
 	}
@@ -490,24 +490,12 @@ PlainGameDescriptor EngineManager::findGame(const Common::String &gameName, cons
 		return result;
 	}
 
-	// Now look for the game using the gameId. This is much faster than scanning plugin
-	// by plugin
-	if (PluginMan.loadPluginFromGameId(gameName))  {
-		result = findGameInLoadedPlugins(gameName, plugin);
-		if (result.gameId) {
-			return result;
-		}
-	}
-
-	// We failed to find it using the gameid. Scan the list of plugins
+	// We failed to find it in memory. Scan the list of plugins
 	PluginMan.loadFirstPlugin();
 	do {
 		result = findGameInLoadedPlugins(gameName, plugin);
-		if (result.gameId) {
-			// Update with new plugin file name
-			PluginMan.updateConfigWithFileName(gameName);
+		if (result.gameId)
 			break;
-		}
 	} while (PluginMan.loadNextPlugin());
 
 	return result;
@@ -551,7 +539,6 @@ DetectionResults EngineManager::detectGames(const Common::FSList &fslist) const
 			DetectedGames engineCandidates = metaEngine.detectGames(fslist);
 
 			for (uint i = 0; i < engineCandidates.size(); i++) {
-				engineCandidates[i].engineName = metaEngine.getName();
 				engineCandidates[i].path = fslist.begin()->getParent().getPath();
 				engineCandidates[i].shortPath = fslist.begin()->getParent().getDisplayName();
 				candidates.push_back(engineCandidates[i]);
@@ -597,6 +584,7 @@ Common::String EngineManager::createTargetForGame(const DetectedGame &game) {
 	ConfMan.addGameDomain(domain);
 
 	// Copy all non-empty relevant values into the new domain
+	addStringToConf("engineid", game.engineId, domain);
 	addStringToConf("gameid", game.gameId, domain);
 	addStringToConf("description", game.description, domain);
 	addStringToConf("language", Common::getLanguageCode(game.language), domain);
@@ -623,6 +611,70 @@ Common::String EngineManager::createTargetForGame(const DetectedGame &game) {
 	return domain;
 }
 
+const Plugin *EngineManager::findLoadedPlugin(const Common::String &engineId) const {
+	const PluginList &plugins = getPlugins();
+
+	for (PluginList::const_iterator iter = plugins.begin(); iter != plugins.end(); iter++)
+		if (engineId == (*iter)->get<MetaEngine>().getEngineId())
+			return *iter;
+
+	return 0;
+}
+
+const Plugin *EngineManager::findPlugin(const Common::String &engineId) const {
+	// First look for the game using the plugins in memory. This is critical
+	// for calls coming from inside games
+	const Plugin *plugin = findLoadedPlugin(engineId);
+	if (plugin)
+		return plugin;
+
+	// Now look for the plugin using the engine ID. This is much faster than scanning plugin
+	// by plugin
+	if (PluginMan.loadPluginFromEngineId(engineId))  {
+		plugin = findLoadedPlugin(engineId);
+		if (plugin)
+			return plugin;
+	}
+
+	// We failed to find it using the engine ID. Scan the list of plugins
+	PluginMan.loadFirstPlugin();
+	do {
+		plugin = findLoadedPlugin(engineId);
+		if (plugin) {
+			// Update with new plugin file name
+			PluginMan.updateConfigWithFileName(engineId);
+			return plugin;
+		}
+	} while (PluginMan.loadNextPlugin());
+
+	return 0;
+}
+
+PlainGameDescriptor EngineManager::findTarget(const Common::String &target, const Plugin **plugin) const {
+	// Ignore empty targets
+	if (target.empty())
+		return PlainGameDescriptor();
+
+	// Lookup the domain. If we have no domain, fallback on the old function [ultra-deprecated].
+	const Common::ConfigManager::Domain *domain = ConfMan.getDomain(target);
+	if (!domain || !domain->contains("gameid") || !domain->contains("engineid"))
+		return PlainGameDescriptor();
+
+	// Look for the engine ID
+	const Plugin *foundPlugin = findPlugin(domain->getVal("engineid"));
+	if (!foundPlugin) {
+		return PlainGameDescriptor();
+	}
+
+	// Make sure it does support the game ID
+	PlainGameDescriptor desc = foundPlugin->get<MetaEngine>().findGame(domain->getVal("gameid").c_str());
+
+	if (desc.gameId && plugin)
+		*plugin = foundPlugin;
+
+	return desc;
+}
+
 // Music plugins
 
 #include "audio/musicplugin.h"
diff --git a/base/plugins.h b/base/plugins.h
index 1daa426202..ccb1acf3fb 100644
--- a/base/plugins.h
+++ b/base/plugins.h
@@ -315,8 +315,8 @@ public:
 	virtual void init()	{}
 	virtual void loadFirstPlugin() {}
 	virtual bool loadNextPlugin() { return false; }
-	virtual bool loadPluginFromGameId(const Common::String &gameId) { return false; }
-	virtual void updateConfigWithFileName(const Common::String &gameId) {}
+	virtual bool loadPluginFromEngineId(const Common::String &engineId) { return false; }
+	virtual void updateConfigWithFileName(const Common::String &engineId) {}
 
 	// Functions used only by the cached PluginManager
 	virtual void loadAllPlugins();
@@ -345,8 +345,8 @@ public:
 	virtual void init();
 	virtual void loadFirstPlugin();
 	virtual bool loadNextPlugin();
-	virtual bool loadPluginFromGameId(const Common::String &gameId);
-	virtual void updateConfigWithFileName(const Common::String &gameId);
+	virtual bool loadPluginFromEngineId(const Common::String &engineId);
+	virtual void updateConfigWithFileName(const Common::String &engineId);
 
 	virtual void loadAllPlugins() {} 	// we don't allow these
 	virtual void loadAllPluginsOfType(PluginType type) {}