From c142838122c49811a3b77c6909705aab7121c6ff Mon Sep 17 00:00:00 2001 From: Bastien Bouclet Date: Sun, 25 Sep 2016 08:45:42 +0200 Subject: 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. --- base/commandLine.cpp | 215 ++++++++++++++++++++++++++++++++++----------------- base/main.cpp | 2 +- base/plugins.cpp | 85 +++++++++++--------- 3 files changed, 194 insertions(+), 108 deletions(-) (limited to 'base') 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 games; const PluginList &plugins = EngineMan.getPlugins(); for (PluginList::const_iterator iter = plugins.begin(); iter != plugins.end(); ++iter) { - MetaEngine &metaengine = (*iter)->get(); + const MetaEngine &metaengine = (*iter)->get(); 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 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::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(); + 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().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 } } diff --git a/base/main.cpp b/base/main.cpp index 85fdbb2986..5df9fec22e 100644 --- a/base/main.cpp +++ b/base/main.cpp @@ -219,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.findTarget(ConfMan.getActiveDomainName()); + QualifiedGameDescriptor game = EngineMan.findTarget(ConfMan.getActiveDomainName()); if (game.description) { caption = game.description; } diff --git a/base/plugins.cpp b/base/plugins.cpp index ac98de4d15..6a24409929 100644 --- a/base/plugins.cpp +++ b/base/plugins.cpp @@ -479,50 +479,54 @@ DECLARE_SINGLETON(EngineManager); * This function works for both cached and uncached PluginManagers. * For the cached version, most of the logic here will short circuit. * - * For the uncached version, we first try to find the plugin using the gameId + * For the uncached version, we first try to find the plugin using the engineId * and only if we can't find it there, we loop through the plugins. **/ -PlainGameDescriptor EngineManager::findGame(const Common::String &gameName, const Plugin **plugin) const { - // First look for the game using the plugins in memory. This is critical - // for calls coming from inside games - PlainGameDescriptor result = findGameInLoadedPlugins(gameName, plugin); - if (result.gameId) { - return result; +QualifiedGameList EngineManager::findGamesMatching(const Common::String &engineId, const Common::String &gameId) const { + QualifiedGameList results; + + if (!engineId.empty()) { + // If we got an engine name, look for THE game only in that engine + const Plugin *p = EngineMan.findPlugin(engineId); + if (p) { + const MetaEngine &engine = p->get(); + + PlainGameDescriptor pluginResult = engine.findGame(gameId.c_str()); + if (pluginResult.gameId) { + results.push_back(QualifiedGameDescriptor(engine.getEngineId(), pluginResult)); + } + } + } else { + // This is a slow path, we have to scan the list of plugins + PluginMan.loadFirstPlugin(); + do { + results.push_back(findGameInLoadedPlugins(gameId)); + } while (PluginMan.loadNextPlugin()); } - // We failed to find it in memory. Scan the list of plugins - PluginMan.loadFirstPlugin(); - do { - result = findGameInLoadedPlugins(gameName, plugin); - if (result.gameId) - break; - } while (PluginMan.loadNextPlugin()); - - return result; + return results; } /** * Find the game within the plugins loaded in memory **/ -PlainGameDescriptor EngineManager::findGameInLoadedPlugins(const Common::String &gameName, const Plugin **plugin) const { +QualifiedGameList EngineManager::findGameInLoadedPlugins(const Common::String &gameId) const { // Find the GameDescriptor for this target const PluginList &plugins = getPlugins(); - if (plugin) - *plugin = 0; - + QualifiedGameList results; PluginList::const_iterator iter; for (iter = plugins.begin(); iter != plugins.end(); ++iter) { - PlainGameDescriptor pgd = (*iter)->get().findGame(gameName.c_str()); - if (pgd.gameId) { - if (plugin) - *plugin = *iter; - return pgd; + const MetaEngine &engine = (*iter)->get(); + PlainGameDescriptor pluginResult = engine.findGame(gameId.c_str()); + + if (pluginResult.gameId) { + results.push_back(QualifiedGameDescriptor(engine.getEngineId(), pluginResult)); } } - return PlainGameDescriptor::empty(); + return results; } DetectionResults EngineManager::detectGames(const Common::FSList &fslist) const { @@ -650,29 +654,33 @@ const Plugin *EngineManager::findPlugin(const Common::String &engineId) const { return 0; } -PlainGameDescriptor EngineManager::findTarget(const Common::String &target, const Plugin **plugin) const { +QualifiedGameDescriptor EngineManager::findTarget(const Common::String &target, const Plugin **plugin) const { // Ignore empty targets if (target.empty()) - return PlainGameDescriptor(); + return QualifiedGameDescriptor(); // 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(); + return QualifiedGameDescriptor(); // Look for the engine ID const Plugin *foundPlugin = findPlugin(domain->getVal("engineid")); if (!foundPlugin) { - return PlainGameDescriptor(); + return QualifiedGameDescriptor(); } // Make sure it does support the game ID - PlainGameDescriptor desc = foundPlugin->get().findGame(domain->getVal("gameid").c_str()); + const MetaEngine &engine = foundPlugin->get(); + PlainGameDescriptor desc = engine.findGame(domain->getVal("gameid").c_str()); + if (!desc.gameId) { + return QualifiedGameDescriptor(); + } - if (desc.gameId && plugin) + if (plugin) *plugin = foundPlugin; - return desc; + return QualifiedGameDescriptor(engine.getEngineId(), desc); } void EngineManager::upgradeTargetIfNecessary(const Common::String &target) const { @@ -734,10 +742,13 @@ void EngineManager::upgradeTargetForEngineId(const Common::String &target) const // Next, try to find an engine with the game ID in its supported games list if (engineId.empty()) { - PlainGameDescriptor pgd = findGame(oldGameId, &plugin); - if (plugin) { - engineId = plugin->get().getEngineId(); - newGameId = pgd.gameId; + QualifiedGameList candidates = findGamesMatching("", oldGameId); + if (candidates.size() > 1) { + warning("Multiple matching engines were found when upgrading target '%s'", target.c_str()); + return; + } else if (!candidates.empty()) { + engineId = candidates[0].engineId; + newGameId = candidates[0].gameId; } } -- cgit v1.2.3