DETECTOR: Separate code for handling obsolete gameids from advanced detector

This includes a renaming of ADObsoleteGameID to Engine::ObsoleteGameID,
and AdvancedDetector::findGameID now is Engines::findGameID.

Doxygen comments were added or improved

engines/advancedDetector.cpp

@@ -32,68 +32,7 @@
 #include "common/translation.h"
 
 #include "engines/advancedDetector.h"
-
+#include "engines/obsolete.h"
-void AdvancedMetaEngine::upgradeTargetIfNecessary() const {
-	if (_obsoleteList == 0)
-		return;
-
-	Common::String gameid = ConfMan.get("gameid");
-
-	for (const ADObsoleteGameID *o = _obsoleteList; o->from; ++o) {
-		if (gameid.equalsIgnoreCase(o->from)) {
-			gameid = o->to;
-			ConfMan.set("gameid", gameid);
-
-			if (o->platform != Common::kPlatformUnknown)
-				ConfMan.set("platform", Common::getPlatformCode(o->platform));
-
-			warning("Target upgraded from %s to %s", o->from, o->to);
-
-			// WORKAROUND: Fix for bug #1719463: "DETECTOR: Launching
-			// undefined target adds launcher entry"
-			if (ConfMan.hasKey("id_came_from_command_line")) {
-				warning("Target came from command line. Skipping save");
-			} else {
-				ConfMan.flushToDisk();
-			}
-			break;
-		}
-	}
-}
-
-namespace AdvancedDetector {
-
-GameDescriptor findGameID(
-	const char *gameid,
-	const PlainGameDescriptor *gameids,
-	const ADObsoleteGameID *obsoleteList
-	) {
-	// First search the list of supported gameids for a match.
-	const PlainGameDescriptor *g = findPlainGameDescriptor(gameid, gameids);
-	if (g)
-		return GameDescriptor(*g);
-
-	// If we didn't find the gameid in the main list, check if it
-	// is an obsolete game id.
-	if (obsoleteList != 0) {
-		const ADObsoleteGameID *o = obsoleteList;
-		while (o->from) {
-			if (0 == scumm_stricmp(gameid, o->from)) {
-				g = findPlainGameDescriptor(o->to, gameids);
-				if (g && g->description)
-					return GameDescriptor(gameid, "Obsolete game ID (" + Common::String(g->description) + ")");
-				else
-					return GameDescriptor(gameid, "Obsolete game ID");
-			}
-			o++;
-		}
-	}
-
-	// No match found
-	return GameDescriptor();
-}
-
-}	// End of namespace AdvancedDetector
 
 static GameDescriptor toGameDescriptor(const ADGameDescription &g, const PlainGameDescriptor *sg) {
 	const char *title = 0;
@@ -218,7 +157,6 @@ GameList AdvancedMetaEngine::detectGames(const Common::FSList &fslist) const {
 
 Common::Error AdvancedMetaEngine::createInstance(OSystem *syst, Engine **engine) const {
 	assert(engine);
-	upgradeTargetIfNecessary();
 
 	const ADGameDescription *agdDesc = 0;
 	Common::Language language = Common::UNK_LANG;
@@ -605,14 +543,19 @@ GameList AdvancedMetaEngine::getSupportedGames() const {
 }
 
 GameDescriptor AdvancedMetaEngine::findGame(const char *gameid) const {
-	return AdvancedDetector::findGameID(gameid, _gameids, _obsoleteList);
+	// First search the list of supported gameids for a match.
+	const PlainGameDescriptor *g = findPlainGameDescriptor(gameid, _gameids);
+	if (g)
+		return GameDescriptor(*g);
+
+	// No match found
+	return GameDescriptor();
 }
 
 AdvancedMetaEngine::AdvancedMetaEngine(const void *descs, uint descItemSize, const PlainGameDescriptor *gameids)
 	: _gameDescriptors((const byte *)descs), _descItemSize(descItemSize), _gameids(gameids) {
 
 	_md5Bytes = 5000;
-	_obsoleteList = NULL;
 	_singleid = NULL;
 	_fileBasedFallback = NULL;
 	_flags = 0;

engines/advancedDetector.h

@@ -19,6 +19,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
  *
  */
+
 #ifndef ENGINES_ADVANCED_DETECTOR_H
 #define ENGINES_ADVANCED_DETECTOR_H
 
@@ -82,13 +83,6 @@ typedef Common::Array<const ADGameDescription*> ADGameDescList;
 #define AD_TABLE_END_MARKER	\
 	{ NULL, NULL, { { NULL, 0, NULL, 0 } }, Common::UNK_LANG, Common::kPlatformUnknown, ADGF_NO_FLAGS, Common::GUIO_NONE }
 
-
-struct ADObsoleteGameID {
-	const char *from;
-	const char *to;
-	Common::Platform platform;
-};
-
 struct ADFileBasedFallback {
 	/**
 	 * Pointer to an ADGameDescription or subclass thereof which will get
@@ -119,21 +113,6 @@ enum ADFlags {
 };
 
 
-namespace AdvancedDetector {
-
-/**
- * Scan through the game descriptors specified in params and search for
- * 'gameid' in there. If a match is found, returns a GameDescriptor
- * with gameid and description set.
- */
-GameDescriptor findGameID(
-	const char *gameid,
-	const PlainGameDescriptor *gameids,
-	const ADObsoleteGameID *obsoleteList = 0
-	);
-
-} // End of namespace AdvancedDetector
-
 /**
  * A MetaEngine implementation based around the advanced detector code.
  */
@@ -169,13 +148,6 @@ protected:
 	 */
 	const PlainGameDescriptor *_gameids;
 
-	/**
-	 * Structure for autoupgrading obsolete targets (optional).
-	 *
-	 * @todo Properly explain this.
-	 */
-	const ADObsoleteGameID *_obsoleteList;
-
 	/**
 	 * Name of single gameid (optional).
 	 *
@@ -274,8 +246,7 @@ protected:
 	 */
 	ADGameDescList detectGameFilebased(const FileMap &allFiles) const;
 
-	void upgradeTargetIfNecessary() const;
+	// TODO
-
 	void updateGameDescriptor(GameDescriptor &desc, const ADGameDescription *realDesc) const;
 
 	/**

engines/agos/detection.cpp

@@ -23,6 +23,7 @@
 #include "base/plugins.h"
 
 #include "engines/advancedDetector.h"
+#include "engines/obsolete.h"
 #include "common/config-manager.h"
 #include "common/savefile.h"
 #include "common/system.h"
@@ -48,7 +49,7 @@ struct AGOSGameDescription {
  * corresponding new target and platform combination.
  *
  */
-static const ADObsoleteGameID obsoleteGameIDsTable[] = {
+static const Engines::ObsoleteGameID obsoleteGameIDsTable[] = {
 	{"simon1acorn", "simon1", Common::kPlatformAcorn},
 	{"simon1amiga", "simon1", Common::kPlatformAmiga},
 	{"simon1cd32", "simon1", Common::kPlatformAmiga},
@@ -92,12 +93,15 @@ using namespace AGOS;
 class AgosMetaEngine : public AdvancedMetaEngine {
 public:
 	AgosMetaEngine() : AdvancedMetaEngine(AGOS::gameDescriptions, sizeof(AGOS::AGOSGameDescription), agosGames) {
-		_obsoleteList = obsoleteGameIDsTable;
 		_guioptions = Common::GUIO_NOLAUNCHLOAD;
 		_maxScanDepth = 2;
 		_directoryGlobs = directoryGlobs;
 	}
 
+	virtual GameDescriptor findGame(const char *gameid) const {
+		return Engines::findGameID(gameid, _gameids, obsoleteGameIDsTable);
+	}
+
 	virtual const char *getName() const {
 		return "AGOS";
 	}
@@ -107,7 +111,13 @@ public:
 	}
 
 	virtual bool hasFeature(MetaEngineFeature f) const;
+
+	virtual Common::Error createInstance(OSystem *syst, Engine **engine) const {
+		Engines::upgradeTargetIfNecessary(obsoleteGameIDsTable);
+		return AdvancedMetaEngine::createInstance(syst, engine);
+	}
 	virtual bool createInstance(OSystem *syst, Engine **engine, const ADGameDescription *desc) const;
+
 	virtual SaveStateList listSaves(const char *target) const;
 	virtual int getMaximumSaveSlot() const;
 };

engines/cine/detection.cpp

@@ -23,6 +23,7 @@
 #include "base/plugins.h"
 
 #include "engines/advancedDetector.h"
+#include "engines/obsolete.h"
 #include "common/system.h"
 #include "common/textconsole.h"
 
@@ -52,7 +53,7 @@ static const PlainGameDescriptor cineGames[] = {
 	{0, 0}
 };
 
-static const ADObsoleteGameID obsoleteGameIDsTable[] = {
+static const Engines::ObsoleteGameID obsoleteGameIDsTable[] = {
 	{"fw", "cine", Common::kPlatformUnknown},
 	{"os", "cine", Common::kPlatformUnknown},
 	{0, 0, Common::kPlatformUnknown}
@@ -63,11 +64,14 @@ static const ADObsoleteGameID obsoleteGameIDsTable[] = {
 class CineMetaEngine : public AdvancedMetaEngine {
 public:
 	CineMetaEngine() : AdvancedMetaEngine(Cine::gameDescriptions, sizeof(Cine::CINEGameDescription), cineGames) {
-		_obsoleteList = obsoleteGameIDsTable;
 		_singleid = "cine";
 		_guioptions = Common::GUIO_NOSPEECH | Common::GUIO_NOMIDI;
 	}
 
+	virtual GameDescriptor findGame(const char *gameid) const {
+		return Engines::findGameID(gameid, _gameids, obsoleteGameIDsTable);
+	}
+
 	virtual const char *getName() const {
 		return "Cine";
 	}
@@ -76,7 +80,12 @@ public:
 		return "Future Wars & Operation Stealth (C) Delphine Software";
 	}
 
+	virtual Common::Error createInstance(OSystem *syst, Engine **engine) const {
+		Engines::upgradeTargetIfNecessary(obsoleteGameIDsTable);
+		return AdvancedMetaEngine::createInstance(syst, engine);
+	}
 	virtual bool createInstance(OSystem *syst, Engine **engine, const ADGameDescription *desc) const;
+
 	virtual bool hasFeature(MetaEngineFeature f) const;
 	virtual SaveStateList listSaves(const char *target) const;
 	virtual int getMaximumSaveSlot() const;

engines/gob/detection.cpp

@@ -22,6 +22,7 @@
 
 #include "base/plugins.h"
 #include "engines/advancedDetector.h"
+#include "engines/obsolete.h"
 
 #include "gob/gob.h"
 
@@ -78,7 +79,7 @@ static const PlainGameDescriptor gobGames[] = {
 	{0, 0}
 };
 
-static const ADObsoleteGameID obsoleteGameIDsTable[] = {
+static const Engines::ObsoleteGameID obsoleteGameIDsTable[] = {
 	{"gob1", "gob", kPlatformUnknown},
 	{"gob2", "gob", kPlatformUnknown},
 	{0, 0, kPlatformUnknown}
@@ -89,12 +90,15 @@ static const ADObsoleteGameID obsoleteGameIDsTable[] = {
 class GobMetaEngine : public AdvancedMetaEngine {
 public:
 	GobMetaEngine() : AdvancedMetaEngine(Gob::gameDescriptions, sizeof(Gob::GOBGameDescription), gobGames) {
-		_obsoleteList = obsoleteGameIDsTable;
 		_singleid = "gob";
 		_fileBasedFallback = Gob::fileBased;
 		_guioptions = Common::GUIO_NOLAUNCHLOAD;
 	}
 
+	virtual GameDescriptor findGame(const char *gameid) const {
+		return Engines::findGameID(gameid, _gameids, obsoleteGameIDsTable);
+	}
+
 	virtual const char *getName() const {
 		return "Gob";
 	}
@@ -104,6 +108,11 @@ public:
 	}
 
 	virtual bool hasFeature(MetaEngineFeature f) const;
+
+	virtual Common::Error createInstance(OSystem *syst, Engine **engine) const {
+		Engines::upgradeTargetIfNecessary(obsoleteGameIDsTable);
+		return AdvancedMetaEngine::createInstance(syst, engine);
+	}
 	virtual bool createInstance(OSystem *syst, Engine **engine, const ADGameDescription *desc) const;
 };
 

engines/module.mk

@@ -5,6 +5,7 @@ MODULE_OBJS := \
 	dialogs.o \
 	engine.o \
 	game.o \
+	obsolete.o \
 	savestate.o
 
 # Include common rules

engines/obsolete.cpp

@@ -0,0 +1,88 @@
+/* ScummVM - Graphic Adventure Engine
+ *
+ * ScummVM is the legal property of its developers, whose names
+ * are too numerous to list here. Please refer to the COPYRIGHT
+ * file distributed with this source distribution.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ */
+
+#include "engines/obsolete.h"
+
+#include "common/config-manager.h"
+
+
+namespace Engines {
+
+void upgradeTargetIfNecessary(const ObsoleteGameID *obsoleteList) {
+	if (obsoleteList == 0)
+		return;
+
+	Common::String gameid = ConfMan.get("gameid");
+
+	for (const ObsoleteGameID *o = obsoleteList; o->from; ++o) {
+		if (gameid.equalsIgnoreCase(o->from)) {
+			gameid = o->to;
+			ConfMan.set("gameid", gameid);
+
+			if (o->platform != Common::kPlatformUnknown)
+				ConfMan.set("platform", Common::getPlatformCode(o->platform));
+
+			warning("Target upgraded from %s to %s", o->from, o->to);
+
+			// WORKAROUND: Fix for bug #1719463: "DETECTOR: Launching
+			// undefined target adds launcher entry"
+			if (ConfMan.hasKey("id_came_from_command_line")) {
+				warning("Target came from command line. Skipping save");
+			} else {
+				ConfMan.flushToDisk();
+			}
+			break;
+		}
+	}
+}
+
+GameDescriptor findGameID(
+	const char *gameid,
+	const PlainGameDescriptor *gameids,
+	const ObsoleteGameID *obsoleteList
+	) {
+	// First search the list of supported gameids for a match.
+	const PlainGameDescriptor *g = findPlainGameDescriptor(gameid, gameids);
+	if (g)
+		return GameDescriptor(*g);
+
+	// If we didn't find the gameid in the main list, check if it
+	// is an obsolete game id.
+	if (obsoleteList != 0) {
+		const ObsoleteGameID *o = obsoleteList;
+		while (o->from) {
+			if (0 == scumm_stricmp(gameid, o->from)) {
+				g = findPlainGameDescriptor(o->to, gameids);
+				if (g && g->description)
+					return GameDescriptor(gameid, "Obsolete game ID (" + Common::String(g->description) + ")");
+				else
+					return GameDescriptor(gameid, "Obsolete game ID");
+			}
+			o++;
+		}
+	}
+
+	// No match found
+	return GameDescriptor();
+}
+
+} // End of namespace Engines

engines/obsolete.h

@@ -0,0 +1,78 @@
+/* ScummVM - Graphic Adventure Engine
+ *
+ * ScummVM is the legal property of its developers, whose names
+ * are too numerous to list here. Please refer to the COPYRIGHT
+ * file distributed with this source distribution.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ */
+
+#ifndef ENGINES_OBSOLETE_H
+#define ENGINES_OBSOLETE_H
+
+#include "engines/game.h"
+
+namespace Engines {
+
+/**
+ * Structure for autoupgrading targets using an obsolete gameid
+ * to the correct new gameid.
+ */
+struct ObsoleteGameID {
+
+	/** Name of the obsolete gameid. */
+	const char *from;
+
+	/** Name of the corresponding new gameid. */
+	const char *to;
+
+	/**
+	 * If platform is set to a value different from Common::kPlatformUnknown,
+	 * then upgradeTargetIfNecessary() will use this value to set the platform
+	 * attribute of any target it updates using this ObsoleteGameID record.
+	 * This is useful when the old gameid encoded the target platform (e.g.
+	 * "zakTowns" for FM-TOWNS) while the new gameid does not (e.g. "zak").
+	 */
+	Common::Platform platform;
+};
+
+/**
+ * Check if the currently active game target has an obsolete gameid;
+ * if so, replace it by the correct new gameid.
+ * This function is typically invoked by a MetaEngine::createInstance
+ * implementation.
+ */
+void upgradeTargetIfNecessary(const ObsoleteGameID *obsoleteList);
+
+
+/**
+ * Scan through the given list of plain game descriptors specified and search
+ * for 'gameid' in there. If a match is found, returns a GameDescriptor
+ * with gameid and description set.
+ *
+ * Optionally can take a list of obsolete game ids into account in order
+ * to support obsolete gameids.
+ */
+GameDescriptor findGameID(
+	const char *gameid,
+	const PlainGameDescriptor *gameids,
+	const ObsoleteGameID *obsoleteList = 0
+	);
+
+
+} // End of namespace Engines
+
+#endif

engines/saga/detection.cpp

@@ -28,6 +28,7 @@
 
 #include "common/config-manager.h"
 #include "engines/advancedDetector.h"
+#include "engines/obsolete.h"
 #include "common/system.h"
 #include "graphics/thumbnail.h"
 
@@ -91,7 +92,7 @@ static const PlainGameDescriptor sagaGames[] = {
 	{0, 0}
 };
 
-static const ADObsoleteGameID obsoleteGameIDsTable[] = {
+static const Engines::ObsoleteGameID obsoleteGameIDsTable[] = {
 	{"ite", "saga", Common::kPlatformUnknown},
 	{"ihnm", "saga", Common::kPlatformUnknown},
 	{"dino", "saga", Common::kPlatformUnknown},
@@ -104,10 +105,13 @@ static const ADObsoleteGameID obsoleteGameIDsTable[] = {
 class SagaMetaEngine : public AdvancedMetaEngine {
 public:
 	SagaMetaEngine() : AdvancedMetaEngine(Saga::gameDescriptions, sizeof(Saga::SAGAGameDescription), sagaGames) {
-		_obsoleteList = obsoleteGameIDsTable;
 		_singleid = "saga";
 	}
 
+	virtual GameDescriptor findGame(const char *gameid) const {
+		return Engines::findGameID(gameid, _gameids, obsoleteGameIDsTable);
+	}
+
 	virtual const char *getName() const {
 		return "SAGA ["
 
@@ -135,7 +139,13 @@ public:
 	}
 
 	virtual bool hasFeature(MetaEngineFeature f) const;
+
+	virtual Common::Error createInstance(OSystem *syst, Engine **engine) const {
+		Engines::upgradeTargetIfNecessary(obsoleteGameIDsTable);
+		return AdvancedMetaEngine::createInstance(syst, engine);
+	}
 	virtual bool createInstance(OSystem *syst, Engine **engine, const ADGameDescription *desc) const;
+
 	virtual SaveStateList listSaves(const char *target) const;
 	virtual int getMaximumSaveSlot() const;
 	virtual void removeSaveState(const char *target, int slot) const;

engines/scumm/detection.cpp

@@ -882,7 +882,7 @@ GameList ScummMetaEngine::getSupportedGames() const {
 }
 
 GameDescriptor ScummMetaEngine::findGame(const char *gameid) const {
-	return AdvancedDetector::findGameID(gameid, gameDescriptions, obsoleteGameIDsTable);
+	return Engines::findGameID(gameid, gameDescriptions, obsoleteGameIDsTable);
 }
 
 static Common::String generatePreferredTarget(const DetectorResult &x) {
@@ -975,20 +975,7 @@ Common::Error ScummMetaEngine::createInstance(OSystem *syst, Engine **engine) co
 	// We start by checking whether the specified game ID is obsolete.
 	// If that is the case, we automatically upgrade the target to use
 	// the correct new game ID (and platform, if specified).
-	for (const ADObsoleteGameID *o = obsoleteGameIDsTable; o->from; ++o) {
+	Engines::upgradeTargetIfNecessary(obsoleteGameIDsTable);
-		if (!scumm_stricmp(gameid, o->from)) {
-			// Match found, perform upgrade
-			gameid = o->to;
-			ConfMan.set("gameid", o->to);
-
-			if (o->platform != Common::kPlatformUnknown)
-				ConfMan.set("platform", Common::getPlatformCode(o->platform));
-
-			warning("Target upgraded from game ID %s to %s", o->from, o->to);
-			ConfMan.flushToDisk();
-			break;
-		}
-	}
 
 	// Fetch the list of files in the current directory
 	Common::FSList fslist;

engines/scumm/detection_tables.h

@@ -23,7 +23,7 @@
 #ifndef SCUMM_DETECTION_TABLES_H
 #define SCUMM_DETECTION_TABLES_H
 
-#include "engines/advancedDetector.h"
+#include "engines/obsolete.h"
 #include "common/rect.h"
 #include "common/util.h"
 
@@ -145,7 +145,7 @@ static const PlainGameDescriptor gameDescriptions[] = {
  * Conversion table mapping old obsolete game IDs to the
  * corresponding new game ID and platform combination.
  */
-static const ADObsoleteGameID obsoleteGameIDsTable[] = {
+static const Engines::ObsoleteGameID obsoleteGameIDsTable[] = {
 	{"bluesabctimedemo", "bluesabctime", UNK},
 	{"BluesBirthdayDemo", "BluesBirthday", UNK},
 	{"comidemo", "comi", UNK},