Merge pull request #252 from DrMcCoy/detector_public_reportUnknown

DETECTOR: Make reportUnknown() accessible to inherited AdvancedMetaEngine classes

7 files changed, 98 insertions, 65 deletions

diff --git a/engines/advancedDetector.cpp b/engines/advancedDetector.cpp
index ac06e74e0a..9beba6ce4a 100644
--- a/engines/advancedDetector.cpp
+++ b/engines/advancedDetector.cpp
@@ -22,7 +22,6 @@
 
 #include "common/debug.h"
 #include "common/util.h"
-#include "common/hash-str.h"
 #include "common/file.h"
 #include "common/macresman.h"
 #include "common/md5.h"
@@ -308,14 +307,7 @@ Common::Error AdvancedMetaEngine::createInstance(OSystem *syst, Engine **engine)
 		return Common::kNoError;
 }
 
-struct SizeMD5 {
-	int size;
-	Common::String md5;
-};
-
-typedef Common::HashMap<Common::String, SizeMD5, Common::IgnoreCase_Hash, Common::IgnoreCase_EqualTo> SizeMD5Map;
-
-static void reportUnknown(const Common::FSNode &path, const SizeMD5Map &filesSizeMD5) {
+void AdvancedMetaEngine::reportUnknown(const Common::FSNode &path, const ADFilePropertiesMap &filesProps) const {
 	// TODO: This message should be cleaned up / made more specific.
 	// For example, we should specify at least which engine triggered this.
 	//
@@ -327,7 +319,7 @@ static void reportUnknown(const Common::FSNode &path, const SizeMD5Map &filesSiz
 	report += _("of the game you tried to add and its version/language/etc.:");
 	report += "\n";
 
-	for (SizeMD5Map::const_iterator file = filesSizeMD5.begin(); file != filesSizeMD5.end(); ++file)
+	for (ADFilePropertiesMap::const_iterator file = filesProps.begin(); file != filesProps.end(); ++file)
 		report += Common::String::format("  {\"%s\", 0, \"%s\", %d},\n", file->_key.c_str(), file->_value.md5.c_str(), file->_value.size);
 
 	report += "\n";
@@ -375,8 +367,36 @@ void AdvancedMetaEngine::composeFileHashMap(FileMap &allFiles, const Common::FSL
 	}
 }
 
+bool AdvancedMetaEngine::getFileProperties(const Common::FSNode &parent, const FileMap &allFiles, const ADGameDescription &game, const Common::String fname, ADFileProperties &fileProps) const {
+	// FIXME/TODO: We don't handle the case that a file is listed as a regular
+	// file and as one with resource fork.
+
+	if (game.flags & ADGF_MACRESFORK) {
+		Common::MacResManager macResMan;
+
+		if (!macResMan.open(parent, fname))
+			return false;
+
+		fileProps.md5 = macResMan.computeResForkMD5AsString(_md5Bytes);
+		fileProps.size = macResMan.getResForkDataSize();
+		return true;
+	}
+
+	if (!allFiles.contains(fname))
+		return false;
+
+	Common::File testFile;
+
+	if (!testFile.open(allFiles[fname]))
+		return false;
+
+	fileProps.size = (int32)testFile.size();
+	fileProps.md5 = Common::computeStreamMD5AsString(testFile, _md5Bytes);
+	return true;
+}
+
 ADGameDescList AdvancedMetaEngine::detectGame(const Common::FSNode &parent, const FileMap &allFiles, Common::Language language, Common::Platform platform, const Common::String &extra) const {
-	SizeMD5Map filesSizeMD5;
+	ADFilePropertiesMap filesProps;
 
 	const ADGameFileDescription *fileDesc;
 	const ADGameDescription *g;
@@ -391,39 +411,14 @@ ADGameDescList AdvancedMetaEngine::detectGame(const Common::FSNode &parent, cons
 
 		for (fileDesc = g->filesDescriptions; fileDesc->fileName; fileDesc++) {
 			Common::String fname = fileDesc->fileName;
-			SizeMD5 tmp;
+			ADFileProperties tmp;
 
-			if (filesSizeMD5.contains(fname))
+			if (filesProps.contains(fname))
 				continue;
 
-			// FIXME/TODO: We don't handle the case that a file is listed as a regular
-			// file and as one with resource fork.
-
-			if (g->flags & ADGF_MACRESFORK) {
-				Common::MacResManager macResMan;
-
-				if (macResMan.open(parent, fname)) {
-					tmp.md5 = macResMan.computeResForkMD5AsString(_md5Bytes);
-					tmp.size = macResMan.getResForkDataSize();
-					debug(3, "> '%s': '%s'", fname.c_str(), tmp.md5.c_str());
-					filesSizeMD5[fname] = tmp;
-				}
-			} else {
-				if (allFiles.contains(fname)) {
-					debug(3, "+ %s", fname.c_str());
-
-					Common::File testFile;
-
-					if (testFile.open(allFiles[fname])) {
-						tmp.size = (int32)testFile.size();
-						tmp.md5 = Common::computeStreamMD5AsString(testFile, _md5Bytes);
-					} else {
-						tmp.size = -1;
-					}
-
-					debug(3, "> '%s': '%s'", fname.c_str(), tmp.md5.c_str());
-					filesSizeMD5[fname] = tmp;
-				}
+			if (getFileProperties(parent, allFiles, *g, fname, tmp)) {
+				debug(3, "> '%s': '%s'", fname.c_str(), tmp.md5.c_str());
+				filesProps[fname] = tmp;
 			}
 		}
 	}
@@ -456,19 +451,19 @@ ADGameDescList AdvancedMetaEngine::detectGame(const Common::FSNode &parent, cons
 		for (fileDesc = g->filesDescriptions; fileDesc->fileName; fileDesc++) {
 			Common::String tstr = fileDesc->fileName;
 
-			if (!filesSizeMD5.contains(tstr)) {
+			if (!filesProps.contains(tstr)) {
 				fileMissing = true;
 				allFilesPresent = false;
 				break;
 			}
 
-			if (fileDesc->md5 != NULL && fileDesc->md5 != filesSizeMD5[tstr].md5) {
-				debug(3, "MD5 Mismatch. Skipping (%s) (%s)", fileDesc->md5, filesSizeMD5[tstr].md5.c_str());
+			if (fileDesc->md5 != NULL && fileDesc->md5 != filesProps[tstr].md5) {
+				debug(3, "MD5 Mismatch. Skipping (%s) (%s)", fileDesc->md5, filesProps[tstr].md5.c_str());
 				fileMissing = true;
 				break;
 			}
 
-			if (fileDesc->fileSize != -1 && fileDesc->fileSize != filesSizeMD5[tstr].size) {
+			if (fileDesc->fileSize != -1 && fileDesc->fileSize != filesProps[tstr].size) {
 				debug(3, "Size Mismatch. Skipping");
 				fileMissing = true;
 				break;
@@ -514,8 +509,8 @@ ADGameDescList AdvancedMetaEngine::detectGame(const Common::FSNode &parent, cons
 
 	// We didn't find a match
 	if (matched.empty()) {
-		if (!filesSizeMD5.empty() && gotAnyMatchesWithAllFiles) {
-			reportUnknown(parent, filesSizeMD5);
+		if (!filesProps.empty() && gotAnyMatchesWithAllFiles) {
+			reportUnknown(parent, filesProps);
 		}
 
 		// Filename based fallback
@@ -524,7 +519,7 @@ ADGameDescList AdvancedMetaEngine::detectGame(const Common::FSNode &parent, cons
 	return matched;
 }
 
-const ADGameDescription *AdvancedMetaEngine::detectGameFilebased(const FileMap &allFiles, const ADFileBasedFallback *fileBasedFallback) const {
+const ADGameDescription *AdvancedMetaEngine::detectGameFilebased(const FileMap &allFiles, const Common::FSList &fslist, const ADFileBasedFallback *fileBasedFallback, ADFilePropertiesMap *filesProps) const {
 	const ADFileBasedFallback *ptr;
 	const char* const* filenames;
 
@@ -554,6 +549,16 @@ const ADGameDescription *AdvancedMetaEngine::detectGameFilebased(const FileMap &
 				maxNumMatchedFiles = numMatchedFiles;
 
 				debug(4, "and overridden");
+
+				if (filesProps) {
+					for (filenames = ptr->filenames; *filenames; ++filenames) {
+						ADFileProperties tmp;
+
+						if (getFileProperties(fslist.begin()->getParent(), allFiles, *agdesc, *filenames, tmp))
+							(*filesProps)[*filenames] = tmp;
+					}
+				}
+
 			}
 		}
 	}
diff --git a/engines/advancedDetector.h b/engines/advancedDetector.h
index 45a9f183e8..8c19d03691 100644
--- a/engines/advancedDetector.h
+++ b/engines/advancedDetector.h
@@ -26,6 +26,8 @@
 #include "engines/metaengine.h"
 #include "engines/engine.h"
 
+#include "common/hash-str.h"
+
 #include "common/gui_options.h" // FIXME: Temporary hack?
 
 namespace Common {
@@ -46,6 +48,20 @@ struct ADGameFileDescription {
 };
 
 /**
+ * A record describing the properties of a file. Used on the existing
+ * files while detecting a game.
+ */
+struct ADFileProperties {
+	int32 size;
+	Common::String md5;
+};
+
+/**
+ * A map of all relevant existing files in a game directory while detecting.
+ */
+typedef Common::HashMap<Common::String, ADFileProperties, Common::IgnoreCase_Hash, Common::IgnoreCase_EqualTo> ADFilePropertiesMap;
+
+/**
  * A shortcut to produce an empty ADGameFileDescription record. Used to mark
  * the end of a list of these.
  */
@@ -286,9 +302,17 @@ protected:
 	 * In case of a tie, the entry coming first in the list is chosen.
 	 *
 	 * @param allFiles	a map describing all present files
+	 * @param fslist	a list of nodes for all present files
 	 * @param fileBasedFallback	a list of ADFileBasedFallback records, zero-terminated
+	 * @param filesProps	if not 0, return a map of properties for all detected files here
+	 */
+	const ADGameDescription *detectGameFilebased(const FileMap &allFiles, const Common::FSList &fslist, const ADFileBasedFallback *fileBasedFallback, ADFilePropertiesMap *filesProps = 0) const;
+
+	/**
+	 * Log and print a report that we found an unknown game variant, together with the file
+	 * names, sizes and MD5 sums.
 	 */
-	const ADGameDescription *detectGameFilebased(const FileMap &allFiles, const ADFileBasedFallback *fileBasedFallback) const;
+	void reportUnknown(const Common::FSNode &path, const ADFilePropertiesMap &filesProps) const;
 
 	// TODO
 	void updateGameDescriptor(GameDescriptor &desc, const ADGameDescription *realDesc) const;
@@ -298,6 +322,9 @@ protected:
 	 * Includes nifty stuff like removing trailing dots and ignoring case.
 	 */
 	void composeFileHashMap(FileMap &allFiles, const Common::FSList &fslist, int depth) const;
+
+	/** Get the properties (size and MD5) of this file. */
+	bool getFileProperties(const Common::FSNode &parent, const FileMap &allFiles, const ADGameDescription &game, const Common::String fname, ADFileProperties &fileProps) const;
 };
 
 #endif
diff --git a/engines/cge/detection.cpp b/engines/cge/detection.cpp
index d16b682501..2e04b82026 100644
--- a/engines/cge/detection.cpp
+++ b/engines/cge/detection.cpp
@@ -108,7 +108,7 @@ public:
 	}
 
 	virtual const ADGameDescription *fallbackDetect(const FileMap &allFiles, const Common::FSList &fslist) const {
-		return detectGameFilebased(allFiles, CGE::fileBasedFallback);
+		return detectGameFilebased(allFiles, fslist, CGE::fileBasedFallback);
 	}
 
 	virtual const char *getName() const {
diff --git a/engines/gob/detection/detection.cpp b/engines/gob/detection/detection.cpp
index bcfd5dacfa..14da54637b 100644
--- a/engines/gob/detection/detection.cpp
+++ b/engines/gob/detection/detection.cpp
@@ -40,7 +40,14 @@ public:
 	}
 
 	virtual const ADGameDescription *fallbackDetect(const FileMap &allFiles, const Common::FSList &fslist) const {
-		return detectGameFilebased(allFiles, Gob::fileBased);
+		ADFilePropertiesMap filesProps;
+
+		const ADGameDescription *game = detectGameFilebased(allFiles, fslist, Gob::fileBased, &filesProps);
+		if (!game)
+			return 0;
+
+		reportUnknown(fslist.begin()->getParent(), filesProps);
+		return game;
 	}
 
 	virtual const char *getName() const {
diff --git a/engines/mohawk/detection.cpp b/engines/mohawk/detection.cpp
index f0c657897d..5664929948 100644
--- a/engines/mohawk/detection.cpp
+++ b/engines/mohawk/detection.cpp
@@ -167,7 +167,7 @@ public:
 	}
 
 	virtual const ADGameDescription *fallbackDetect(const FileMap &allFiles, const Common::FSList &fslist) const {
-		return detectGameFilebased(allFiles, Mohawk::fileBased);
+		return detectGameFilebased(allFiles, fslist, Mohawk::fileBased);
 	}
 
 	virtual const char *getName() const {
diff --git a/engines/toon/detection.cpp b/engines/toon/detection.cpp
index 0cafee1475..3877fa2a6c 100644
--- a/engines/toon/detection.cpp
+++ b/engines/toon/detection.cpp
@@ -133,7 +133,7 @@ public:
 	}
 
 	virtual const ADGameDescription *fallbackDetect(const FileMap &allFiles, const Common::FSList &fslist) const {
-		return detectGameFilebased(allFiles, Toon::fileBasedFallback);
+		return detectGameFilebased(allFiles, fslist, Toon::fileBasedFallback);
 	}
 
 	virtual const char *getName() const {
diff --git a/engines/touche/detection.cpp b/engines/touche/detection.cpp
index 35dd54776f..e4bbe0c4c1 100644
--- a/engines/touche/detection.cpp
+++ b/engines/touche/detection.cpp
@@ -135,19 +135,13 @@ public:
 	}
 
 	virtual const ADGameDescription *fallbackDetect(const FileMap &allFiles, const Common::FSList &fslist) const {
-		const ADGameDescription *matchedDesc = detectGameFilebased(allFiles, Touche::fileBasedFallback);
-
-		if (matchedDesc) { // We got a match
-			Common::String report = Common::String::format(_("Your game version has been detected using "
-				"filename matching as a variant of %s."), matchedDesc->gameid);
-			report += "\n";
-			report += _("If this is an original and unmodified version, please report any");
-			report += "\n";
-			report += _("information previously printed by ScummVM to the team.");
-			report += "\n";
-			g_system->logMessage(LogMessageType::kInfo, report.c_str());
-		}
+		ADFilePropertiesMap filesProps;
+
+		const ADGameDescription *matchedDesc = detectGameFilebased(allFiles, fslist, Touche::fileBasedFallback, &filesProps);
+		if (!matchedDesc)
+			return 0;
 
+		reportUnknown(fslist.begin()->getParent(), filesProps);
 		return matchedDesc;
 	}