GLK: Detect game Id clashes across different sub-engines

10 files changed, 102 insertions, 37 deletions

diff --git a/engines/glk/detection.cpp b/engines/glk/detection.cpp
index d4773c250b..1e1eb10a8f 100644
--- a/engines/glk/detection.cpp
+++ b/engines/glk/detection.cpp
@@ -82,6 +82,11 @@ public:
 	 * Query the engine for a PlainGameDescriptor for the specified gameid, if any.
 	 */
 	virtual PlainGameDescriptor findGame(const char *gameId) const override;
+
+	/**
+	 * Calls each sub-engine in turn to ensure no game Id accidentally shares the same Id
+	 */
+	void detectClashes() const;
 };
 
 bool GlkMetaEngine::hasFeature(MetaEngineFeature f) const {
@@ -204,6 +209,9 @@ PlainGameDescriptor GlkMetaEngine::findGame(const char *gameId) const {
 }
 
 DetectedGames GlkMetaEngine::detectGames(const Common::FSList &fslist) const {
+	// This is as good a place as any to detect multiple sub-engines using the same Ids
+	detectClashes();
+
 	DetectedGames detectedGames;
 	Glk::Frotz::FrotzMetaEngine::detectGames(fslist, detectedGames);
 	Glk::Glulxe::GlulxeMetaEngine::detectGames(fslist, detectedGames);
@@ -213,6 +221,14 @@ DetectedGames GlkMetaEngine::detectGames(const Common::FSList &fslist) const {
 	return detectedGames;
 }
 
+void GlkMetaEngine::detectClashes() const {
+	Common::StringMap map;
+	Glk::Frotz::FrotzMetaEngine::detectClashes(map);
+	Glk::Glulxe::GlulxeMetaEngine::detectClashes(map);
+	Glk::Scott::ScottMetaEngine::detectClashes(map);
+	Glk::TADS::TADSMetaEngine::detectClashes(map);
+}
+
 SaveStateList GlkMetaEngine::listSaves(const char *target) const {
 	Common::SaveFileManager *saveFileMan = g_system->getSavefileManager();
 	Common::StringArray filenames;
diff --git a/engines/glk/frotz/detection.cpp b/engines/glk/frotz/detection.cpp
index d7725e72a4..2ec48f9087 100644
--- a/engines/glk/frotz/detection.cpp
+++ b/engines/glk/frotz/detection.cpp
@@ -134,6 +134,14 @@ bool FrotzMetaEngine::detectGames(const Common::FSList &fslist, DetectedGames &g
 	return !gameList.empty();
 }
 
+void FrotzMetaEngine::detectClashes(Common::StringMap &map) {
+	for (const PlainGameDescriptor *pd = FROTZ_GAME_LIST; pd->gameId; ++pd) {
+		if (map.contains(pd->gameId))
+			error("Duplicate game Id found - %s", pd->gameId);
+		map[pd->gameId] = "";
+	}
+}
+
 bool FrotzMetaEngine::readSavegameHeader(Common::SeekableReadStream *stream, Glk::SavegameHeader &header) {
 	stream->seek(0);
 	if (stream->readUint32BE() != ID_FORM)
diff --git a/engines/glk/frotz/detection.h b/engines/glk/frotz/detection.h
index 4fa555bfa0..6cc72aed38 100644
--- a/engines/glk/frotz/detection.h
+++ b/engines/glk/frotz/detection.h
@@ -24,6 +24,7 @@
 #define GLK_FROTZ_DETECTION
 
 #include "common/fs.h"
+#include "common/hash-str.h"
 #include "engines/game.h"
 #include "glk/streams.h"
 
@@ -49,6 +50,11 @@ public:
 	static bool detectGames(const Common::FSList &fslist, DetectedGames &gameList);
 
 	/**
+	 * Check for game Id clashes with other sub-engines
+	 */
+	static void detectClashes(Common::StringMap &map);
+
+	/**
 	 * Check a passed stream for a Quetzal save, and if so, get header information
 	 */
 	static bool readSavegameHeader(Common::SeekableReadStream *stream, Glk::SavegameHeader &header);
diff --git a/engines/glk/frotz/detection_tables.h b/engines/glk/frotz/detection_tables.h
index 5d3fa77694..77e673923e 100644
--- a/engines/glk/frotz/detection_tables.h
+++ b/engines/glk/frotz/detection_tables.h
@@ -83,26 +83,6 @@ const PlainGameDescriptor FROTZ_GAME_LIST[] = {
 	{ "zork3", "Zork III: The Dungeon Master" },
 	{ "ztuu", "Zork: The Undiscovered Underground" },
 
-	// Converted Scott Adams games
-	{ "adventureland",     "Adventureland" },
-	{ "pirateadventure",   "Pirate Adventure" },
-	{ "missionimpossible", "Mission Impossible" },
-	{ "voodoocastle",      "Voodoo Castle" },
-	{ "thecount",          "The Count" },
-	{ "strangeodyssey",    "Strange Odyssey" },
-	{ "mysteryfunhouse",   "Mystery Fun House" },
-	{ "pyramidofdoom",     "Pyramid Of Doom" },
-	{ "ghosttown",         "Ghost Town" },
-	{ "savageisland1",     "Savage Island, Part 1" },
-	{ "savageisland2",     "Savage Island, Part 2" },
-	{ "goldenvoyage",      "The Golden Voyage" },
-	{ "adventure13",       "Adventure 13" },
-	{ "adventure14",       "Adventure 14" },
-	{ "buckaroobanzai",    "Buckaroo Banzai" },
-	{ "marveladventure",   "Marvel Adventure #1" },
-	{ "questprobe2",       "Questprobe 2: Spiderman" },
-	{ "scottsampler",      "Adventure International's Mini-Adventure Sampler" },
-
 	// English games
 	{ "404life",           "404 - Life not found" },
 	{ "69105keys",         "69,105 Keys" },
@@ -787,7 +767,6 @@ const PlainGameDescriptor FROTZ_GAME_LIST[] = {
 	{ "zsnake",            "Z-Snake" },
 	{ "ztornado",          "Z-Tornado" },
 	{ "ztrek",             "Super Z Trek" },
-	{ "ztuu",              "Zork: The Undiscovered Underground" },
 	{ "zugzwang",          "Zugzwang: The Interactive Life of a Chess Piece" },
 
 	// Painfull Little Stupid Games
@@ -802,18 +781,38 @@ const PlainGameDescriptor FROTZ_GAME_LIST[] = {
 	{ "plsg9",             "Zork LXIX: Painless Little Stupid Games #9" },
 	{ "plsg10",            "The Valley House: Painless Little Stupid Games #10" },
 
+	// Converted Scott Adams games
+	{ "adventurelandi5",     "Adventureland" },
+	{ "pirateadventurei5",   "Pirate Adventure" },
+	{ "missionimpossiblei5", "Mission Impossible" },
+	{ "voodoocastlei5",      "Voodoo Castle" },
+	{ "thecounti5",          "The Count" },
+	{ "strangeodysseyi5",    "Strange Odyssey" },
+	{ "mysteryfunhousei5",   "Mystery Fun House" },
+	{ "pyramidofdoomi5",     "Pyramid Of Doom" },
+	{ "ghosttowni5",         "Ghost Town" },
+	{ "savageisland1i5",     "Savage Island, Part 1" },
+	{ "savageisland2i5",     "Savage Island, Part 2" },
+	{ "goldenvoyagei5",      "The Golden Voyage" },
+	{ "adventure13i5",       "Adventure 13" },
+	{ "adventure14i5",       "Adventure 14" },
+	{ "buckaroobanzaii5",    "Buckaroo Banzai" },
+	{ "marveladventurei5",   "Marvel Adventure #1" },
+	{ "questprobe2i5",       "Questprobe 2: Spiderman" },
+	{ "scottsampleri5",      "Adventure International's Mini-Adventure Sampler" },
+
 	// Mysterious Adventures by Brian Howarth
-	{ "goldenbaton", "Mysterious Adventures 1: The Golden Baton" },
-	{ "timemachine", "Mysterious Adventures 2: The Time Machine" },
-	{ "arrowofdeath1", "Mysterious Adventures 3: Arrow of Death Part 1" },
-	{ "arrowofdeath2", "Mysterious Adventures 4: Arrow of Death Part 2" },
-	{ "pulsar7", "Mysterious Adventures 5: Escape from Pulsar 7" },
-	{ "circus", "Mysterious Adventures 6: Circus" },
-	{ "feasibility", "Mysterious Adventures 7: Feasibility Experiment" },
-	{ "akyrz", "Mysterious Adventures 8: The Wizard of Akyrz" },
-	{ "perseus", "Mysterious Adventures 9: Perseus and Andromeda" },
-	{ "10indians", "Mysterious Adventures 10: Ten Little Indians" },
-	{ "waxworks", "Mysterious Adventures 11: Waxworks" },
+	{ "goldenbatoni5", "Mysterious Adventures 1: The Golden Baton" },
+	{ "timemachinei5", "Mysterious Adventures 2: The Time Machine" },
+	{ "arrowofdeath1i5", "Mysterious Adventures 3: Arrow of Death Part 1" },
+	{ "arrowofdeath2i5", "Mysterious Adventures 4: Arrow of Death Part 2" },
+	{ "pulsar7i5", "Mysterious Adventures 5: Escape from Pulsar 7" },
+	{ "circusi5", "Mysterious Adventures 6: Circus" },
+	{ "feasibilityi5", "Mysterious Adventures 7: Feasibility Experiment" },
+	{ "akyrzi5", "Mysterious Adventures 8: The Wizard of Akyrz" },
+	{ "perseusi5", "Mysterious Adventures 9: Perseus and Andromeda" },
+	{ "10indiansi5", "Mysterious Adventures 10: Ten Little Indians" },
+	{ "waxworksi5", "Mysterious Adventures 11: Waxworks" },
 
 	// 1992 album Apollo 18, by They Might be Giants
 	{ "apollo1", "Apollo 18 01: Dig My Grave" },
@@ -897,7 +896,6 @@ const PlainGameDescriptor FROTZ_GAME_LIST[] = {
 	{ "ayon", "Nel Mondo di Ayon" },
 	{ "darkiss", "Darkiss! Il bacio del vampiro" },
 	{ "darkiss2", "Darkiss! Il bacio del vampiro. Capitolo 2" },
-	{ "enigma", "Enigma" },
 	{ "filamit", "Filaments" },
 	{ "flamel", "Flamel" },
 	{ "giardino", "Il giardino incantato" },
@@ -905,7 +903,6 @@ const PlainGameDescriptor FROTZ_GAME_LIST[] = {
 	{ "koohinoor", "Kooh-I-Noor" },
 	{ "luna", "La Pietra della Luna" },
 	{ "poesia", "Manca solo un verso a quella poesia" },
-	{ "poesia", "Manca solo un verso a quella poesia" },
 	{ "oldwest1", "Pecos Town, Old West Episode I" },
 	{ "rovo", "Il rovo" },
 	{ "scarafaggio", "Lo Scarafaggio" },
@@ -926,7 +923,6 @@ const PlainGameDescriptor FROTZ_GAME_LIST[] = {
 	{ "absolutos", "Los sonidos absolutos" },
 	{ "afuera", "Afuera" },
 	{ "amanda", "Amanda" },
-	{ "amanda", "Amanda" },
 	{ "aque1", "Aquelarre" },
 	{ "casi", "Casi Muerto" },
 	{ "Celos", "Un Asunto de Celos" },
@@ -954,7 +950,6 @@ const PlainGameDescriptor FROTZ_GAME_LIST[] = {
 	{ "kunelar", "El ascenso de Kunelar" },
 	{ "lamansion", "La Mansion" },
 	{ "lldc", "La Llamada de Cthulhu" },
-	{ "megara", "Los placeres de Megara" },
 	{ "macetas", "Macetas" },
 	{ "meeva", "La mediana evasion" },
 	{ "megara", "Los placeres de Megara" },
@@ -977,7 +972,6 @@ const PlainGameDescriptor FROTZ_GAME_LIST[] = {
 	{ "sombras", "Sombras de Moria" },
 	{ "tokland", "La isla de Tokland" },
 	{ "torre", "Misterio en la torre" },
-	{ "tuuli", "Tuuli" },
 
 	// Swedish games
 	{ "aventyr", "Aventyr" },
diff --git a/engines/glk/glulxe/detection.cpp b/engines/glk/glulxe/detection.cpp
index c2e19de4e9..fb701131ef 100644
--- a/engines/glk/glulxe/detection.cpp
+++ b/engines/glk/glulxe/detection.cpp
@@ -106,5 +106,13 @@ bool GlulxeMetaEngine::detectGames(const Common::FSList &fslist, DetectedGames &
 	return !gameList.empty();
 }
 
+void GlulxeMetaEngine::detectClashes(Common::StringMap &map) {
+	for (const GlulxeDescriptor *pd = GLULXE_GAME_LIST; pd->gameId; ++pd) {
+		if (map.contains(pd->gameId))
+			error("Duplicate game Id found - %s", pd->gameId);
+		map[pd->gameId] = "";
+	}
+}
+
 } // End of namespace Glulxe
 } // End of namespace Glk
diff --git a/engines/glk/glulxe/detection.h b/engines/glk/glulxe/detection.h
index d6d5c6a9ad..654977a0b2 100644
--- a/engines/glk/glulxe/detection.h
+++ b/engines/glk/glulxe/detection.h
@@ -24,6 +24,7 @@
 #define GLK_GLULXE_DETECTION
 
 #include "common/fs.h"
+#include "common/hash-str.h"
 #include "engines/game.h"
 
 namespace Glk {
@@ -63,6 +64,11 @@ public:
 	 * Detect supported games
 	 */
 	static bool detectGames(const Common::FSList &fslist, DetectedGames &gameList);
+
+	/**
+	 * Check for game Id clashes with other sub-engines
+	 */
+	static void detectClashes(Common::StringMap &map);
 };
 
 } // End of namespace Glulxe
diff --git a/engines/glk/scott/detection.cpp b/engines/glk/scott/detection.cpp
index a02e3c56ad..6c04a25646 100644
--- a/engines/glk/scott/detection.cpp
+++ b/engines/glk/scott/detection.cpp
@@ -77,5 +77,13 @@ bool ScottMetaEngine::detectGames(const Common::FSList &fslist, DetectedGames &g
 	return !gameList.empty();
 }
 
+void ScottMetaEngine::detectClashes(Common::StringMap &map) {
+	for (const PlainGameDescriptor *pd = SCOTT_GAME_LIST; pd->gameId; ++pd) {
+		if (map.contains(pd->gameId))
+			error("Duplicate game Id found - %s", pd->gameId);
+		map[pd->gameId] = "";
+	}
+}
+
 } // End of namespace Scott
 } // End of namespace Glk
diff --git a/engines/glk/scott/detection.h b/engines/glk/scott/detection.h
index 384197c467..709b7de1d9 100644
--- a/engines/glk/scott/detection.h
+++ b/engines/glk/scott/detection.h
@@ -24,6 +24,7 @@
 #define GLK_SCOTT_DETECTION
 
 #include "common/fs.h"
+#include "common/hash-str.h"
 #include "engines/game.h"
 
 namespace Glk {
@@ -45,6 +46,11 @@ public:
 	 * Detect supported games
 	 */
 	static bool detectGames(const Common::FSList &fslist, DetectedGames &gameList);
+
+	/**
+	 * Check for game Id clashes with other sub-engines
+	 */
+	static void detectClashes(Common::StringMap &map);
 };
 
 } // End of namespace Scott
diff --git a/engines/glk/tads/detection.cpp b/engines/glk/tads/detection.cpp
index df7eee1585..95f7330d65 100644
--- a/engines/glk/tads/detection.cpp
+++ b/engines/glk/tads/detection.cpp
@@ -96,5 +96,13 @@ bool TADSMetaEngine::detectGames(const Common::FSList &fslist, DetectedGames &ga
 	return !gameList.empty();
 }
 
+void TADSMetaEngine::detectClashes(Common::StringMap &map) {
+	for (const TADSDescriptor *pd = TADS_GAME_LIST; pd->gameId; ++pd) {
+		if (map.contains(pd->gameId))
+			error("Duplicate game Id found - %s", pd->gameId);
+		map[pd->gameId] = "";
+	}
+}
+
 } // End of namespace TADS
 } // End of namespace Glk
diff --git a/engines/glk/tads/detection.h b/engines/glk/tads/detection.h
index 3e4d3e3a00..f876cb2cf2 100644
--- a/engines/glk/tads/detection.h
+++ b/engines/glk/tads/detection.h
@@ -64,6 +64,11 @@ public:
 	 * Detect supported games
 	 */
 	static bool detectGames(const Common::FSList &fslist, DetectedGames &gameList);
+
+	/**
+	 * Check for game Id clashes with other sub-engines
+	 */
+	static void detectClashes(Common::StringMap &map);
 };
 
 } // End of namespace TADS