From d848543ee432a11c11968af59dd1caa362a2021d Mon Sep 17 00:00:00 2001 From: Max Horn Date: Tue, 29 Jul 2008 00:50:12 +0000 Subject: Changed advanced detector to *always* use the FSNode API for detection (i.e. killed second code path which used File::open trial&error directory 'scanning') svn-id: r33388 --- common/advancedDetector.cpp | 82 ++++++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 45 deletions(-) (limited to 'common/advancedDetector.cpp') diff --git a/common/advancedDetector.cpp b/common/advancedDetector.cpp index 4387bd199e..e328cf7787 100644 --- a/common/advancedDetector.cpp +++ b/common/advancedDetector.cpp @@ -48,7 +48,7 @@ using namespace AdvancedDetector; * @param platform restrict results to specified platform only * @return list of ADGameDescription (or subclass) pointers corresponding to matched games */ -static ADGameDescList detectGame(const FSList *fslist, const Common::ADParams ¶ms, Language language, Platform platform, const Common::String extra); +static ADGameDescList detectGame(const FSList &fslist, const Common::ADParams ¶ms, Language language, Platform platform, const Common::String extra); /** @@ -194,7 +194,7 @@ static void updateGameDescriptor(GameDescriptor &desc, const ADGameDescription * } GameList AdvancedMetaEngine::detectGames(const FSList &fslist) const { - ADGameDescList matches = detectGame(&fslist, params, Common::UNK_LANG, Common::kPlatformUnknown, ""); + ADGameDescList matches = detectGame(fslist, params, Common::UNK_LANG, Common::kPlatformUnknown, ""); GameList detectedGames; // Use fallback detector if there were no matches by other means @@ -233,7 +233,21 @@ PluginError AdvancedMetaEngine::createInstance(OSystem *syst, Engine **engine) c Common::String gameid = ConfMan.get("gameid"); - ADGameDescList matches = detectGame(0, params, language, platform, extra); + Common::String path; + if (ConfMan.hasKey("path")) { + path = ConfMan.get("path"); + } else { + path = "."; + warning("No path was provided. Assuming the data files are in the current directory"); + } + FilesystemNode dir(path); + FSList files; + if (!dir.isDirectory() || !dir.getChildren(files, FilesystemNode::kListAll)) { + warning("Game data path does not exist or is not a directory (%s)", path.c_str()); + return kNoGameDataFoundError; + } + + ADGameDescList matches = detectGame(files, params, language, platform, extra); if (params.singleid == NULL) { for (uint i = 0; i < matches.size(); i++) { @@ -287,7 +301,7 @@ static void reportUnknown(StringMap &filesMD5, IntMap &filesSize) { printf("\n"); } -static ADGameDescList detectGame(const FSList *fslist, const Common::ADParams ¶ms, Language language, Platform platform, const Common::String extra) { +static ADGameDescList detectGame(const FSList &fslist, const Common::ADParams ¶ms, Language language, Platform platform, const Common::String extra) { StringSet filesList; StringMap filesMD5; @@ -319,57 +333,35 @@ static ADGameDescList detectGame(const FSList *fslist, const Common::ADParams &p } } - // TODO/FIXME: Fingolfin says: It's not good that we have two different code paths here, - // one using a FSList, one using File::open, as that will lead to discrepancies and subtle - // problems caused by those. - if (fslist != 0) { - // Get the information of the existing files - for (FSList::const_iterator file = fslist->begin(); file != fslist->end(); ++file) { - if (file->isDirectory()) continue; - tstr = file->getName(); - tstr.toLowercase(); - - // Strip any trailing dot - if (tstr.lastChar() == '.') - tstr.deleteLastChar(); + // Get the information of the existing files + for (FSList::const_iterator file = fslist.begin(); file != fslist.end(); ++file) { + if (file->isDirectory()) continue; + tstr = file->getName(); + tstr.toLowercase(); - allFiles[tstr] = true; + // Strip any trailing dot + if (tstr.lastChar() == '.') + tstr.deleteLastChar(); - debug(3, "+ %s", tstr.c_str()); + allFiles[tstr] = true; - if (!filesList.contains(tstr)) continue; + debug(3, "+ %s", tstr.c_str()); - if (!md5_file_string(*file, md5str, params.md5Bytes)) - continue; - filesMD5[tstr] = md5str; + if (!filesList.contains(tstr)) continue; - debug(3, "> %s: %s", tstr.c_str(), md5str); - - if (testFile.open(file->getPath())) { - filesSize[tstr] = (int32)testFile.size(); - testFile.close(); - } - } - } else { - // Get the information of the requested files - for (StringSet::const_iterator file = filesList.begin(); file != filesList.end(); ++file) { - tstr = file->_key; + if (!md5_file_string(*file, md5str, params.md5Bytes)) + continue; + filesMD5[tstr] = md5str; - debug(3, "+ %s", tstr.c_str()); - if (!filesMD5.contains(tstr)) { - if (testFile.open(tstr) || testFile.open(tstr + ".")) { - filesSize[tstr] = (int32)testFile.size(); - testFile.close(); + debug(3, "> %s: %s", tstr.c_str(), md5str); - if (md5_file_string(file->_key.c_str(), md5str, params.md5Bytes)) { - filesMD5[tstr] = md5str; - debug(3, "> %s: %s", tstr.c_str(), md5str); - } - } - } + if (testFile.open(file->getPath())) { + filesSize[tstr] = (int32)testFile.size(); + testFile.close(); } } + ADGameDescList matched; int maxFilesMatched = 0; -- cgit v1.2.3 From 04c05d3ca0717aea07fd3872b4ce54220a45b78f Mon Sep 17 00:00:00 2001 From: Max Horn Date: Wed, 30 Jul 2008 15:16:57 +0000 Subject: Advanced detector: split out part of detectGame into a new function detectGameFilebased; some cleanup svn-id: r33453 --- common/advancedDetector.cpp | 200 +++++++++++++++++++++----------------------- 1 file changed, 97 insertions(+), 103 deletions(-) (limited to 'common/advancedDetector.cpp') diff --git a/common/advancedDetector.cpp b/common/advancedDetector.cpp index e328cf7787..6496315e2e 100644 --- a/common/advancedDetector.cpp +++ b/common/advancedDetector.cpp @@ -34,7 +34,11 @@ namespace Common { -using namespace AdvancedDetector; +/** + * A list of pointers to ADGameDescription structs (or subclasses thereof). + */ +typedef Array ADGameDescList; + /** * Detect games in specified directory. @@ -282,10 +286,10 @@ PluginError AdvancedMetaEngine::createInstance(OSystem *syst, Engine **engine) c return kNoError; } -typedef HashMap StringSet; -typedef HashMap IntMap; +typedef HashMap StringSet; +typedef HashMap IntMap; -static void reportUnknown(StringMap &filesMD5, IntMap &filesSize) { +static void reportUnknown(const StringMap &filesMD5, const IntMap &filesSize) { // TODO: This message should be cleaned up / made more specific. // For example, we should specify at least which engine triggered this. // @@ -301,21 +305,14 @@ static void reportUnknown(StringMap &filesMD5, IntMap &filesSize) { printf("\n"); } +static ADGameDescList detectGameFilebased(const FSList &fslist, const Common::ADParams ¶ms, IntMap &allFiles); + static ADGameDescList detectGame(const FSList &fslist, const Common::ADParams ¶ms, Language language, Platform platform, const Common::String extra) { StringSet filesList; - StringMap filesMD5; IntMap filesSize; IntMap allFiles; - File testFile; - - String tstr; - - uint i; - char md5str[32+1]; - - bool fileMissing; const ADGameFileDescription *fileDesc; const ADGameDescription *g; const byte *descPtr; @@ -327,34 +324,36 @@ static ADGameDescList detectGame(const FSList &fslist, const Common::ADParams &p g = (const ADGameDescription *)descPtr; for (fileDesc = g->filesDescriptions; fileDesc->fileName; fileDesc++) { - tstr = String(fileDesc->fileName); - tstr.toLowercase(); - filesList[tstr] = true; + filesList[String(fileDesc->fileName)] = true; } } // Get the information of the existing files for (FSList::const_iterator file = fslist.begin(); file != fslist.end(); ++file) { - if (file->isDirectory()) continue; - tstr = file->getName(); - tstr.toLowercase(); + if (file->isDirectory()) + continue; + + String tstr = file->getName(); // Strip any trailing dot if (tstr.lastChar() == '.') tstr.deleteLastChar(); - allFiles[tstr] = true; + allFiles[tstr] = true; // Record the presence of this file debug(3, "+ %s", tstr.c_str()); - if (!filesList.contains(tstr)) continue; + if (!filesList.contains(tstr)) + continue; + char md5str[32+1]; if (!md5_file_string(*file, md5str, params.md5Bytes)) continue; filesMD5[tstr] = md5str; debug(3, "> %s: %s", tstr.c_str(), md5str); + File testFile; if (testFile.open(file->getPath())) { filesSize[tstr] = (int32)testFile.size(); testFile.close(); @@ -366,9 +365,10 @@ static ADGameDescList detectGame(const FSList &fslist, const Common::ADParams &p int maxFilesMatched = 0; // MD5 based matching + uint i; for (i = 0, descPtr = params.descs; ((const ADGameDescription *)descPtr)->gameid != 0; descPtr += params.descItemSize, ++i) { g = (const ADGameDescription *)descPtr; - fileMissing = false; + bool fileMissing = false; // Do not even bother to look at entries which do not have matching // language and platform (if specified). @@ -377,32 +377,28 @@ static ADGameDescList detectGame(const FSList &fslist, const Common::ADParams &p continue; } - if ((params.flags & kADFlagUseExtraAsHint) && extra != "" && g->extra != extra) + if ((params.flags & kADFlagUseExtraAsHint) && !extra.empty() && g->extra != extra) continue; // Try to match all files for this game for (fileDesc = g->filesDescriptions; fileDesc->fileName; fileDesc++) { - tstr = fileDesc->fileName; - tstr.toLowercase(); + String tstr = fileDesc->fileName; if (!filesMD5.contains(tstr)) { fileMissing = true; break; } - if (fileDesc->md5 != NULL) { - if (fileDesc->md5 != filesMD5[tstr]) { - debug(3, "MD5 Mismatch. Skipping (%s) (%s)", fileDesc->md5, filesMD5[tstr].c_str()); - fileMissing = true; - break; - } + + if (fileDesc->md5 != NULL && fileDesc->md5 != filesMD5[tstr]) { + debug(3, "MD5 Mismatch. Skipping (%s) (%s)", fileDesc->md5, filesMD5[tstr].c_str()); + fileMissing = true; + break; } - if (fileDesc->fileSize != -1) { - if (fileDesc->fileSize != filesSize[tstr]) { - debug(3, "Size Mismatch. Skipping"); - fileMissing = true; - break; - } + if (fileDesc->fileSize != -1 && fileDesc->fileSize != filesSize[tstr]) { + debug(3, "Size Mismatch. Skipping"); + fileMissing = true; + break; } debug(3, "Matched file: %s", tstr.c_str()); @@ -440,85 +436,83 @@ static ADGameDescList detectGame(const FSList &fslist, const Common::ADParams &p } } - // We've found a match - if (!matched.empty()) - return matched; - - if (!filesMD5.empty()) - reportUnknown(filesMD5, filesSize); - - // Filename based fallback - if (params.fileBasedFallback != 0) { - const ADFileBasedFallback *ptr = params.fileBasedFallback; - const char* const* filenames = 0; - - // First we create list of files required for detection. - // The filenames can be different than the MD5 based match ones. - for (; ptr->desc; ptr++) { - filenames = ptr->filenames; - for (; *filenames; filenames++) { - tstr = String(*filenames); - tstr.toLowercase(); - - if (!allFiles.contains(tstr)) { - if (testFile.open(tstr) || testFile.open(tstr + ".")) { - allFiles[tstr] = true; - testFile.close(); - } + // We didn't find a match + if (matched.empty()) { + if (!filesMD5.empty()) + reportUnknown(filesMD5, filesSize); + + // Filename based fallback + if (params.fileBasedFallback != 0) + matched = detectGameFilebased(fslist, params, allFiles); + } + + return matched; +} + +static ADGameDescList detectGameFilebased(const FSList &fslist, const Common::ADParams ¶ms, IntMap &allFiles) { + const ADFileBasedFallback *ptr; + const char* const* filenames; + + // First we create list of files required for detection. + // The filenames can be different than the MD5 based match ones. + for (ptr = params.fileBasedFallback; ptr->desc; ++ptr) { + for (filenames = ptr->filenames; *filenames; ++filenames) { + String tstr = String(*filenames); + + if (!allFiles.contains(tstr)) { + File testFile; + if (testFile.open(tstr) || testFile.open(tstr + ".")) { + allFiles[tstr] = true; + testFile.close(); } } } + } - // Then we perform the actual filename matching. If there are - // several matches, only the one with the maximum numbers of - // files is considered. - int maxNumMatchedFiles = 0; - const ADGameDescription *matchedDesc = 0; - - ptr = params.fileBasedFallback; + // Then we perform the actual filename matching. If there are + // several matches, only the one with the maximum numbers of + // files is considered. + int maxNumMatchedFiles = 0; + const ADGameDescription *matchedDesc = 0; - for (; ptr->desc; ptr++) { - const ADGameDescription *agdesc = (const ADGameDescription *)ptr->desc; - int numMatchedFiles = 0; - fileMissing = false; + for (ptr = params.fileBasedFallback; ptr->desc; ++ptr) { + const ADGameDescription *agdesc = (const ADGameDescription *)ptr->desc; + int numMatchedFiles = 0; + bool fileMissing = false; - filenames = ptr->filenames; - for (; *filenames; filenames++) { - if (fileMissing) { - continue; - } + for (filenames = ptr->filenames; *filenames; ++filenames) { + if (fileMissing) + continue; - tstr = String(*filenames); - tstr.toLowercase(); - - debug(3, "++ %s", *filenames); - if (!allFiles.contains(tstr)) { - fileMissing = true; - continue; - } - - numMatchedFiles++; + debug(3, "++ %s", *filenames); + if (!allFiles.contains(*filenames)) { + fileMissing = true; + continue; } - if (!fileMissing) - debug(4, "Matched: %s", agdesc->gameid); + numMatchedFiles++; + } - if (!fileMissing && numMatchedFiles > maxNumMatchedFiles) { - matchedDesc = agdesc; - maxNumMatchedFiles = numMatchedFiles; + if (!fileMissing) + debug(4, "Matched: %s", agdesc->gameid); - debug(4, "and overriden"); - } + if (!fileMissing && numMatchedFiles > maxNumMatchedFiles) { + matchedDesc = agdesc; + maxNumMatchedFiles = numMatchedFiles; + + debug(4, "and overriden"); } + } - if (matchedDesc) { // We got a match - matched.push_back(matchedDesc); - if (params.flags & kADFlagPrintWarningOnFileBasedFallback) { - printf("Your game version has been detected using filename matching as a\n"); - printf("variant of %s.\n", matchedDesc->gameid); - printf("If this is an original and unmodified version, please report any\n"); - printf("information previously printed by ScummVM to the team.\n"); - } + ADGameDescList matched; + + if (matchedDesc) { // We got a match + matched.push_back(matchedDesc); + if (params.flags & kADFlagPrintWarningOnFileBasedFallback) { + printf("Your game version has been detected using filename matching as a\n"); + printf("variant of %s.\n", matchedDesc->gameid); + printf("If this is an original and unmodified version, please report any\n"); + printf("information previously printed by ScummVM to the team.\n"); } } -- cgit v1.2.3 From fbe4f0dd48c290701532e39502551cd07d881dcb Mon Sep 17 00:00:00 2001 From: Max Horn Date: Wed, 30 Jul 2008 15:38:42 +0000 Subject: Simplified advanced detector file sys scanning code svn-id: r33455 --- common/advancedDetector.cpp | 71 ++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 40 deletions(-) (limited to 'common/advancedDetector.cpp') diff --git a/common/advancedDetector.cpp b/common/advancedDetector.cpp index 6496315e2e..adc6f9e327 100644 --- a/common/advancedDetector.cpp +++ b/common/advancedDetector.cpp @@ -305,13 +305,14 @@ static void reportUnknown(const StringMap &filesMD5, const IntMap &filesSize) { printf("\n"); } -static ADGameDescList detectGameFilebased(const FSList &fslist, const Common::ADParams ¶ms, IntMap &allFiles); +static ADGameDescList detectGameFilebased(const StringMap &allFiles, const Common::ADParams ¶ms); static ADGameDescList detectGame(const FSList &fslist, const Common::ADParams ¶ms, Language language, Platform platform, const Common::String extra) { - StringSet filesList; + StringMap allFiles; + + StringSet detectFiles; StringMap filesMD5; IntMap filesSize; - IntMap allFiles; const ADGameFileDescription *fileDesc; const ADGameDescription *g; @@ -319,16 +320,8 @@ static ADGameDescList detectGame(const FSList &fslist, const Common::ADParams &p debug(3, "Starting detection"); - // First we compose list of files which we need MD5s for - for (descPtr = params.descs; ((const ADGameDescription *)descPtr)->gameid != 0; descPtr += params.descItemSize) { - g = (const ADGameDescription *)descPtr; - - for (fileDesc = g->filesDescriptions; fileDesc->fileName; fileDesc++) { - filesList[String(fileDesc->fileName)] = true; - } - } - - // Get the information of the existing files + // First we compose an efficient to query set of all files in fslist. + // Includes nifty stuff like removing trailing dots and ignoring case. for (FSList::const_iterator file = fslist.begin(); file != fslist.end(); ++file) { if (file->isDirectory()) continue; @@ -339,23 +332,37 @@ static ADGameDescList detectGame(const FSList &fslist, const Common::ADParams &p if (tstr.lastChar() == '.') tstr.deleteLastChar(); - allFiles[tstr] = true; // Record the presence of this file + allFiles[tstr] = file->getPath(); // Record the presence of this file + } - debug(3, "+ %s", tstr.c_str()); + // Compute the set of files for which we need MD5s for. I.e. files which are + // included in some ADGameDescription *and* present in fslist. + for (descPtr = params.descs; ((const ADGameDescription *)descPtr)->gameid != 0; descPtr += params.descItemSize) { + g = (const ADGameDescription *)descPtr; - if (!filesList.contains(tstr)) - continue; + for (fileDesc = g->filesDescriptions; fileDesc->fileName; fileDesc++) { + String tstr = fileDesc->fileName; + if (allFiles.contains(tstr)) + detectFiles[tstr] = true; + } + } + + // Get the information for all detection files, if they exist + for (StringSet::const_iterator file = detectFiles.begin(); file != detectFiles.end(); ++file) { + String fname = file->_key; + + debug(3, "+ %s", fname.c_str()); char md5str[32+1]; - if (!md5_file_string(*file, md5str, params.md5Bytes)) + if (!md5_file_string(allFiles[fname].c_str(), md5str, params.md5Bytes)) continue; - filesMD5[tstr] = md5str; + filesMD5[fname] = md5str; - debug(3, "> %s: %s", tstr.c_str(), md5str); + debug(3, "> %s: %s", fname.c_str(), md5str); File testFile; - if (testFile.open(file->getPath())) { - filesSize[tstr] = (int32)testFile.size(); + if (testFile.open(allFiles[fname])) { + filesSize[fname] = (int32)testFile.size(); testFile.close(); } } @@ -443,32 +450,16 @@ static ADGameDescList detectGame(const FSList &fslist, const Common::ADParams &p // Filename based fallback if (params.fileBasedFallback != 0) - matched = detectGameFilebased(fslist, params, allFiles); + matched = detectGameFilebased(allFiles, params); } return matched; } -static ADGameDescList detectGameFilebased(const FSList &fslist, const Common::ADParams ¶ms, IntMap &allFiles) { +static ADGameDescList detectGameFilebased(const StringMap &allFiles, const Common::ADParams ¶ms) { const ADFileBasedFallback *ptr; const char* const* filenames; - // First we create list of files required for detection. - // The filenames can be different than the MD5 based match ones. - for (ptr = params.fileBasedFallback; ptr->desc; ++ptr) { - for (filenames = ptr->filenames; *filenames; ++filenames) { - String tstr = String(*filenames); - - if (!allFiles.contains(tstr)) { - File testFile; - if (testFile.open(tstr) || testFile.open(tstr + ".")) { - allFiles[tstr] = true; - testFile.close(); - } - } - } - } - // Then we perform the actual filename matching. If there are // several matches, only the one with the maximum numbers of // files is considered. -- cgit v1.2.3 From 9e4bc568619567d1e49a8928cec61c93a6b3d860 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Wed, 30 Jul 2008 15:48:16 +0000 Subject: Simplify/optimize/cleanup detectGameFilebased further svn-id: r33457 --- common/advancedDetector.cpp | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) (limited to 'common/advancedDetector.cpp') diff --git a/common/advancedDetector.cpp b/common/advancedDetector.cpp index adc6f9e327..8bd019018a 100644 --- a/common/advancedDetector.cpp +++ b/common/advancedDetector.cpp @@ -456,13 +456,16 @@ static ADGameDescList detectGame(const FSList &fslist, const Common::ADParams &p return matched; } +/** + * Check for each ADFileBasedFallback record whether all files listed + * in it are present. If multiple pass this test, we pick the one with + * the maximal number of matching files. In case of a tie, the entry + * coming first in the list is chosen. + */ static ADGameDescList detectGameFilebased(const StringMap &allFiles, const Common::ADParams ¶ms) { const ADFileBasedFallback *ptr; const char* const* filenames; - // Then we perform the actual filename matching. If there are - // several matches, only the one with the maximum numbers of - // files is considered. int maxNumMatchedFiles = 0; const ADGameDescription *matchedDesc = 0; @@ -472,26 +475,24 @@ static ADGameDescList detectGameFilebased(const StringMap &allFiles, const Commo bool fileMissing = false; for (filenames = ptr->filenames; *filenames; ++filenames) { - if (fileMissing) - continue; - debug(3, "++ %s", *filenames); if (!allFiles.contains(*filenames)) { fileMissing = true; - continue; + break; } numMatchedFiles++; } - if (!fileMissing) + if (!fileMissing) { debug(4, "Matched: %s", agdesc->gameid); - - if (!fileMissing && numMatchedFiles > maxNumMatchedFiles) { - matchedDesc = agdesc; - maxNumMatchedFiles = numMatchedFiles; - - debug(4, "and overriden"); + + if (numMatchedFiles > maxNumMatchedFiles) { + matchedDesc = agdesc; + maxNumMatchedFiles = numMatchedFiles; + + debug(4, "and overriden"); + } } } -- cgit v1.2.3