BASE: Change the command line interface to use engine-qualified game names

Qualified game names have the following form: engineId:gameId.
Unqualified game names are still supported as long as they are not
ambiguous. However they are considered deprecated and are no longer
displayed by the --list-games command.

1 files changed, 145 insertions, 70 deletions

diff --git a/base/commandLine.cpp b/base/commandLine.cpp
index 692d8bb571..bcc4304d83 100644
--- a/base/commandLine.cpp
+++ b/base/commandLine.cpp
@@ -35,9 +35,9 @@
 #include "common/config-manager.h"
 #include "common/fs.h"
 #include "common/rendermode.h"
-#include "common/stack.h"
 #include "common/system.h"
 #include "common/textconsole.h"
+#include "common/tokenizer.h"
 
 #include "gui/ThemeEngine.h"
 
@@ -203,6 +203,10 @@ static void ensureFirstCommand(const Common::String &existingCommand, const char
 		usage("--%s: Cannot accept more than one command (already found --%s).", newCommand, existingCommand.c_str());
 }
 
+static Common::String buildQualifiedGameName(const Common::String &engineId, const Common::String &gameId) {
+	return Common::String::format("%s:%s", engineId.c_str(), gameId.c_str());
+}
+
 #endif // DISABLE_COMMAND_LINE
 
 
@@ -309,6 +313,70 @@ void registerDefaults() {
 #endif
 }
 
+static bool parseGameName(const Common::String &gameName, Common::String &engineId, Common::String &gameId) {
+	Common::StringTokenizer tokenizer(gameName, ":");
+	Common::String token1, token2;
+
+	if (!tokenizer.empty()) {
+		token1 = tokenizer.nextToken();
+	}
+
+	if (!tokenizer.empty()) {
+		token2 = tokenizer.nextToken();
+	}
+
+	if (!tokenizer.empty()) {
+		return false; // Stray colon
+	}
+
+	if (!token1.empty() && !token2.empty()) {
+		engineId = token1;
+		gameId = token2;
+		return true;
+	} else if (!token1.empty()) {
+		engineId.clear();
+		gameId = token1;
+		return true;
+	}
+
+	return false;
+}
+
+static QualifiedGameDescriptor findGameMatchingName(const Common::String &name) {
+	if (name.empty()) {
+		return QualifiedGameDescriptor();
+	}
+
+	Common::String engineId;
+	Common::String gameId;
+	if (!parseGameName(name, engineId, gameId)) {
+		return QualifiedGameDescriptor(); // Invalid name format
+	}
+
+	// Check if the name is a known game id
+	QualifiedGameList games = EngineMan.findGamesMatching(engineId, gameId);
+	if (games.size() != 1) {
+		// Game not found or ambiguous name
+		return QualifiedGameDescriptor();
+	}
+
+	return games[0];
+}
+
+static Common::String createTemporaryTarget(const Common::String &engineId, const Common::String &gameId) {
+	Common::String domainName = gameId;
+
+	ConfMan.addGameDomain(domainName);
+	ConfMan.set("engineid", engineId, domainName);
+	ConfMan.set("gameid", gameId, domainName);
+
+	// We designate targets which come strictly from command line
+	// so ConfMan will not save them to the configuration file.
+	ConfMan.set("id_came_from_command_line", "1", domainName);
+
+	return domainName;
+}
+
 //
 // Various macros used by the command line parser.
 //
@@ -692,18 +760,18 @@ unknownOption:
 
 /** List all supported game IDs, i.e. all games which any loaded plugin supports. */
 static void listGames() {
-	printf("Engine ID       Game ID              Full Title                                            \n"
-	       "--------------- -------------------- ------------------------------------------------------\n");
+	printf("Game ID                        Full Title                                                 \n"
+	       "------------------------------ -----------------------------------------------------------\n");
 
 	Common::Array<Common::String> games;
 
 	const PluginList &plugins = EngineMan.getPlugins();
 	for (PluginList::const_iterator iter = plugins.begin(); iter != plugins.end(); ++iter) {
-		MetaEngine &metaengine = (*iter)->get<MetaEngine>();
+		const MetaEngine &metaengine = (*iter)->get<MetaEngine>();
 
 		PlainGameList list = metaengine.getSupportedGames();
 		for (PlainGameList::const_iterator v = list.begin(); v != list.end(); ++v) {
-			games.push_back(Common::String::format("%-15s %-20s %s", metaengine.getEngineId(), v->gameId, v->description));
+			games.push_back(Common::String::format("%-30s %s", buildQualifiedGameName(metaengine.getEngineId(), v->gameId).c_str(), v->description));
 		}
 	}
 
@@ -749,7 +817,7 @@ static void listTargets() {
 
 		// If there's no description, fallback on the default description.
 		if (description.empty()) {
-			PlainGameDescriptor g = EngineMan.findTarget(name);
+			QualifiedGameDescriptor g = EngineMan.findTarget(name);
 			if (g.description)
 				description = g.description;
 		}
@@ -767,13 +835,13 @@ static void listTargets() {
 }
 
 /** List all saves states for the given target. */
-static Common::Error listSaves(const Common::String &target) {
+static Common::Error listSaves(const Common::String &singleTarget) {
 	Common::Error result = Common::kNoError;
 
 	// If no target is specified, list save games for all known targets
 	Common::Array<Common::String> targets;
-	if (!target.empty())
-		targets.push_back(target);
+	if (!singleTarget.empty())
+		targets.push_back(singleTarget);
 	else {
 		const Common::ConfigManager::DomainMap &domains = ConfMan.getGameDomains();
 		Common::ConfigManager::DomainMap::const_iterator iter;
@@ -790,51 +858,56 @@ static Common::Error listSaves(const Common::String &target) {
 
 	bool atLeastOneFound = false;
 	for (Common::Array<Common::String>::const_iterator i = targets.begin(), end = targets.end(); i != end; ++i) {
-		// Grab the "target" domain, if any
-		const Common::ConfigManager::Domain *domain = ConfMan.getDomain(*i);
-
-		// Set up the game domain as newly active domain, so
-		// target specific savepath will be checked
-		ConfMan.setActiveDomain(*i);
-
-		// Look for a game matching the target
+		// Check whether there is either a game domain (i.e. a target) matching
+		// the specified game name, or alternatively whether there is a matching game id.
+		Common::String currentTarget;
+		QualifiedGameDescriptor game;
 		const Plugin *plugin = nullptr;
-		PlainGameDescriptor game;
-		if (domain) {
-			EngineMan.upgradeTargetIfNecessary(target);
-			game = EngineMan.findTarget(target, &plugin);
+		if (ConfMan.hasGameDomain(*i)) {
+			// The name is a known target
+			currentTarget = *i;
+			EngineMan.upgradeTargetIfNecessary(*i);
+			game = EngineMan.findTarget(*i, &plugin);
+		} else if (game = findGameMatchingName(*i), game.gameId) {
+			// The name is a known game id
+			plugin = EngineMan.findPlugin(game.engineId);
+			currentTarget = createTemporaryTarget(game.engineId, game.gameId);
 		} else {
-			game = EngineMan.findGame(target, &plugin);
+			return Common::Error(Common::kEnginePluginNotFound, Common::String::format("target '%s'", singleTarget.c_str()));
 		}
 
+		// If we actually found a domain, we're going to change the domain
+		ConfMan.setActiveDomain(currentTarget);
+
 		if (!plugin) {
 			// If the target was specified, treat this as an error, and otherwise skip it.
-			if (!target.empty())
+			if (!singleTarget.empty())
 				return Common::Error(Common::kEnginePluginNotFound,
-				                     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);
+				                     Common::String::format("target '%s'", i->c_str()));
+			printf("Plugin could not be loaded for target '%s'\n", i->c_str());
 			continue;
 		}
 
 		const MetaEngine &metaEngine = plugin->get<MetaEngine>();
+		Common::String qualifiedGameId = buildQualifiedGameName(game.engineId, game.gameId);
 
 		if (!metaEngine.hasFeature(MetaEngine::kSupportsListSaves)) {
 			// If the target was specified, treat this as an error, and otherwise skip it.
-			if (!target.empty())
+			if (!singleTarget.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(), game.gameId));
+				                     Common::String::format("target '%s', gameid '%s'", i->c_str(), qualifiedGameId.c_str()));
 			continue;
 		}
 
 		// Query the plugin for a list of saved games
 		SaveStateList saveList = metaEngine.listSaves(i->c_str());
 
-		if (saveList.size() > 0) {
+		if (!saveList.empty()) {
 			// 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(), game.gameId);
+			printf("Save states for target '%s' (gameid '%s'):\n", i->c_str(), qualifiedGameId.c_str());
 			printf("  Slot Description                                           \n"
 					   "  ---- ------------------------------------------------------\n");
 
@@ -845,15 +918,15 @@ static Common::Error listSaves(const Common::String &target) {
 			atLeastOneFound = true;
 		} 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(), game.gameId);
+			if (!singleTarget.empty())
+				printf("There are no save states for target '%s' (gameid '%s'):\n", i->c_str(), qualifiedGameId.c_str());
 		}
 	}
 
 	// Revert to the old active domain
 	ConfMan.setActiveDomain(oldDomain);
 
-	if (!atLeastOneFound && target.empty())
+	if (!atLeastOneFound && singleTarget.empty())
 		printf("No save states could be found.\n");
 
 	return result;
@@ -909,16 +982,17 @@ static DetectedGames getGameList(const Common::FSNode &dir) {
 	return detectionResults.listRecognizedGames();
 }
 
-static DetectedGames recListGames(const Common::FSNode &dir, const Common::String &gameId, bool recursive) {
+static DetectedGames recListGames(const Common::FSNode &dir, const Common::String &engineId, const Common::String &gameId, bool recursive) {
 	DetectedGames list = getGameList(dir);
 
 	if (recursive) {
 		Common::FSList files;
 		dir.getChildren(files, Common::FSNode::kListDirectoriesOnly);
 		for (Common::FSList::const_iterator file = files.begin(); file != files.end(); ++file) {
-			DetectedGames rec = recListGames(*file, gameId, recursive);
+			DetectedGames rec = recListGames(*file, engineId, gameId, recursive);
 			for (DetectedGames::const_iterator game = rec.begin(); game != rec.end(); ++game) {
-				if (gameId.empty() || game->gameId == gameId)
+				if ((game->engineId == engineId && game->gameId == gameId)
+				    || gameId.empty())
 					list.push_back(*game);
 			}
 		}
@@ -928,11 +1002,11 @@ static DetectedGames recListGames(const Common::FSNode &dir, const Common::Strin
 }
 
 /** Display all games in the given directory, return ID of first detected game */
-static Common::String detectGames(const Common::String &path, const Common::String &gameId, bool recursive) {
+static Common::String detectGames(const Common::String &path, const Common::String &engineId, const Common::String &gameId, bool recursive) {
 	bool noPath = path.empty();
 	//Current directory
 	Common::FSNode dir(path);
-	DetectedGames candidates = recListGames(dir, gameId, recursive);
+	DetectedGames candidates = recListGames(dir, engineId, gameId, recursive);
 
 	if (candidates.empty()) {
 		printf("WARNING: ScummVM could not find any game in %s\n", dir.getPath().c_str());
@@ -945,28 +1019,31 @@ static Common::String detectGames(const Common::String &path, const Common::Stri
 		return Common::String();
 	}
 	// TODO this is not especially pretty
-	printf("EngineID       GameID         Description                                                Full Path\n");
-	printf("-------------- -------------- ---------------------------------------------------------- ---------------------------------------------------------\n");
+	printf("GameID                         Description                                                Full Path\n");
+	printf("------------------------------ ---------------------------------------------------------- ---------------------------------------------------------\n");
 	for (DetectedGames::const_iterator v = candidates.begin(); v != candidates.end(); ++v) {
-		printf("%-14s %-14s %-58s %s\n",
-		       v->engineId.c_str(),
-		       v->gameId.c_str(),
+		printf("%-30s %-58s %s\n",
+		       buildQualifiedGameName(v->engineId, v->gameId).c_str(),
 		       v->description.c_str(),
 		       v->path.c_str());
 	}
 
-	return candidates[0].gameId;
+	return buildQualifiedGameName(candidates[0].engineId, candidates[0].gameId);
 }
 
-static int recAddGames(const Common::FSNode &dir, const Common::String &game, bool recursive) {
+static int recAddGames(const Common::FSNode &dir, const Common::String &engineId, const Common::String &gameId, bool recursive) {
 	int count = 0;
 	DetectedGames list = getGameList(dir);
 	for (DetectedGames::const_iterator v = list.begin(); v != list.end(); ++v) {
-		if (v->gameId != game && !game.empty()) {
-			printf("Found %s, only adding %s per --game option, ignoring...\n", v->gameId.c_str(), game.c_str());
+		if ((v->engineId != engineId || v->gameId != gameId)
+		    && !gameId.empty()) {
+			printf("Found %s, only adding %s per --game option, ignoring...\n",
+			       buildQualifiedGameName(v->engineId, v->gameId).c_str(),
+			       buildQualifiedGameName(engineId, gameId).c_str());
 		} else if (ConfMan.hasGameDomain(v->preferredTarget)) {
 			// TODO Better check for game already added?
-			printf("Found %s, but has already been added, skipping\n", v->gameId.c_str());
+			printf("Found %s, but has already been added, skipping\n",
+			       buildQualifiedGameName(v->engineId, v->gameId).c_str());
 		} else {
 			Common::String target = EngineMan.createTargetForGame(*v);
 			count++;
@@ -974,7 +1051,7 @@ static int recAddGames(const Common::FSNode &dir, const Common::String &game, bo
 			// Display added game info
 			printf("Game Added: \n  Target:   %s\n  GameID:   %s\n  Name:     %s\n  Language: %s\n  Platform: %s\n",
 			       target.c_str(),
-			       v->gameId.c_str(),
+			       buildQualifiedGameName(v->engineId, v->gameId).c_str(),
 			       v->description.c_str(),
 			       Common::getLanguageDescription(v->language),
 			       Common::getPlatformDescription(v->platform)
@@ -986,7 +1063,7 @@ static int recAddGames(const Common::FSNode &dir, const Common::String &game, bo
 		Common::FSList files;
 		if (dir.getChildren(files, Common::FSNode::kListDirectoriesOnly)) {
 			for (Common::FSList::const_iterator file = files.begin(); file != files.end(); ++file) {
-				count += recAddGames(*file, game, recursive);
+				count += recAddGames(*file, engineId, gameId, recursive);
 			}
 		}
 	}
@@ -994,10 +1071,10 @@ static int recAddGames(const Common::FSNode &dir, const Common::String &game, bo
 	return count;
 }
 
-static bool addGames(const Common::String &path, const Common::String &game, bool recursive) {
+static bool addGames(const Common::String &path, const Common::String &engineId, const Common::String &gameId, bool recursive) {
 	//Current directory
 	Common::FSNode dir(path);
-	int added = recAddGames(dir, game, recursive);
+	int added = recAddGames(dir, engineId, gameId, recursive);
 	printf("Added %d games\n", added);
 	if (added == 0 && !recursive) {
 		printf("Consider using --recursive to search inside subdirectories\n");
@@ -1211,6 +1288,15 @@ bool processSettings(Common::String &command, Common::StringMap &settings, Commo
 
 #ifndef DISABLE_COMMAND_LINE
 
+	// Check the --game argument refers to a known game id.
+	QualifiedGameDescriptor gameOption;
+	if (settings.contains("game")) {
+		gameOption = findGameMatchingName(settings["game"]);
+		if (!gameOption.gameId) {
+			usage("Unrecognized game '%s'. Use the --list-games command for a list of accepted values.\n", settings["game"].c_str());
+		}
+	}
+
 	// Handle commands passed via the command line (like --list-targets and
 	// --list-games). This must be done after the config file and the plugins
 	// have been loaded.
@@ -1252,17 +1338,17 @@ bool processSettings(Common::String &command, Common::StringMap &settings, Commo
 			// From an UX point of view, however, it might get confusing.
 			// Consider removing this if consensus says otherwise.
 		} else {
-			command = detectGames(settings["path"], settings["game"], resursive);
+			command = detectGames(settings["path"], gameOption.engineId, gameOption.gameId, resursive);
 			if (command.empty()) {
 				err = Common::kNoGameDataFoundError;
 				return true;
 			}
 		}
 	} else if (command == "detect") {
-		detectGames(settings["path"], settings["game"], settings["recursive"] == "true");
+		detectGames(settings["path"], gameOption.engineId, gameOption.gameId, settings["recursive"] == "true");
 		return true;
 	} else if (command == "add") {
-		addGames(settings["path"], settings["game"], settings["recursive"] == "true");
+		addGames(settings["path"], gameOption.engineId, gameOption.gameId, settings["recursive"] == "true");
 		return true;
 	}
 #ifdef DETECTOR_TESTING_HACK
@@ -1285,28 +1371,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;
-		const Plugin *plugin = nullptr;
+		QualifiedGameDescriptor gd;
 		if (ConfMan.hasGameDomain(command)) {
 			// Command is a known target
 			ConfMan.setActiveDomain(command);
-		} else if (gd = EngineMan.findGame(command, &plugin), gd.gameId) {
+		} else if (gd = findGameMatchingName(command), 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"
-			//
-			// 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
-			ConfMan.set("id_came_from_command_line", "1");
+			Common::String domainName = createTemporaryTarget(gd.engineId, gd.gameId);
+			ConfMan.setActiveDomain(domainName);
 		} else {
 #ifndef DISABLE_COMMAND_LINE
-			usage("Unrecognized game target '%s'", command.c_str());
+			usage("Unrecognized game '%s'. Use the --list-targets and --list-games commands for a list of accepted values.", command.c_str());
 #endif // DISABLE_COMMAND_LINE
 		}
 	}