MOHAWK: Switch to slot based save file naming for Myst

Myst previously used the user provided description as the save filename.
This was not working when the user entered chacacters forbidden by the
filesystem.

The actual save format has not changed. Old saves are still compatible,
but must be renamed to 'myst-###.mys' and 'myst-###.mym'.

Fixes 7106.

4 files changed, 90 insertions, 81 deletions

diff --git a/engines/mohawk/detection.cpp b/engines/mohawk/detection.cpp
index a64d7ff7df..7c202998eb 100644
--- a/engines/mohawk/detection.cpp
+++ b/engines/mohawk/detection.cpp
@@ -221,10 +221,25 @@ SaveStateList MohawkMetaEngine::listSaves(const char *target) const {
 	// Loading games is only supported in Myst/Riven currently.
 #ifdef ENABLE_MYST
 	if (strstr(target, "myst")) {
-		filenames = Mohawk::MystGameState::generateSaveGameList();
+		filenames = g_system->getSavefileManager()->listSavefiles("myst-###.mys");
+		size_t prefixLen = sizeof("myst") - 1;
+
+		for (Common::StringArray::const_iterator filename = filenames.begin(); filename != filenames.end(); ++filename) {
+			// Extract the slot number from the filename
+			char slot[4];
+			slot[0] = (*filename)[prefixLen + 1];
+			slot[1] = (*filename)[prefixLen + 2];
+			slot[2] = (*filename)[prefixLen + 3];
+			slot[3] = '\0';
+
+			int slotNum = atoi(slot);
+
+			// Read the description from the save
+			Common::String description = Mohawk::MystGameState::querySaveDescription(slotNum);
+			saveList.push_back(SaveStateDescriptor(slotNum, description));
+		}
 
-		for (uint32 i = 0; i < filenames.size(); i++)
-			saveList.push_back(SaveStateDescriptor(i, filenames[i]));
+		Common::sort(saveList.begin(), saveList.end(), SaveStateDescriptorSlotComparator());
 	} else
 #endif
 	if (strstr(target, "riven")) {
@@ -238,11 +253,11 @@ SaveStateList MohawkMetaEngine::listSaves(const char *target) const {
 }
 
 void MohawkMetaEngine::removeSaveState(const char *target, int slot) const {
+
 	// Removing saved games is only supported in Myst/Riven currently.
 #ifdef ENABLE_MYST
 	if (strstr(target, "myst")) {
-		Common::StringArray filenames = Mohawk::MystGameState::generateSaveGameList();
-		Mohawk::MystGameState::deleteSave(filenames[slot]);
+		Mohawk::MystGameState::deleteSave(slot);
 	} else
 #endif
 	if (strstr(target, "riven")) {
@@ -254,13 +269,7 @@ void MohawkMetaEngine::removeSaveState(const char *target, int slot) const {
 SaveStateDescriptor MohawkMetaEngine::querySaveMetaInfos(const char *target, int slot) const {
 #ifdef ENABLE_MYST
 	if (strstr(target, "myst")) {
-		Common::StringArray filenames = Mohawk::MystGameState::generateSaveGameList();
-
-		if (slot >= (int) filenames.size()) {
-			return SaveStateDescriptor();
-		}
-
-		return Mohawk::MystGameState::querySaveMetaInfos(filenames[slot]);
+		return Mohawk::MystGameState::querySaveMetaInfos(slot);
 	} else
 #endif
 	{
diff --git a/engines/mohawk/myst.cpp b/engines/mohawk/myst.cpp
index c16fab9131..e2bc88ebf6 100644
--- a/engines/mohawk/myst.cpp
+++ b/engines/mohawk/myst.cpp
@@ -231,11 +231,9 @@ Common::Error MohawkEngine_Myst::run() {
 
 	// Load game from launcher/command line if requested
 	if (ConfMan.hasKey("save_slot") && hasGameSaveSupport()) {
-		uint32 gameToLoad = ConfMan.getInt("save_slot");
-		Common::StringArray savedGamesList = MystGameState::generateSaveGameList();
-		if (gameToLoad > savedGamesList.size())
-			error ("Could not find saved game");
-		_gameState->load(savedGamesList[gameToLoad]);
+		int saveSlot = ConfMan.getInt("save_slot");
+		if (!_gameState->load(saveSlot))
+			error("Failed to load save game from slot %i", saveSlot);
 	} else {
 		// Start us on the first stack.
 		if (getGameType() == GType_MAKINGOF)
@@ -1083,19 +1081,14 @@ void MohawkEngine_Myst::loadResources() {
 }
 
 Common::Error MohawkEngine_Myst::loadGameState(int slot) {
-	if (_gameState->load(MystGameState::generateSaveGameList()[slot]))
+	if (_gameState->load(slot))
 		return Common::kNoError;
 
 	return Common::kUnknownError;
 }
 
 Common::Error MohawkEngine_Myst::saveGameState(int slot, const Common::String &desc) {
-	Common::StringArray saveList = MystGameState::generateSaveGameList();
-
-	if ((uint)slot < saveList.size())
-		MystGameState::deleteSave(saveList[slot]);
-
-	return _gameState->save(desc) ? Common::kNoError : Common::kUnknownError;
+	return _gameState->save(slot, desc) ? Common::kNoError : Common::kUnknownError;
 }
 
 bool MohawkEngine_Myst::hasGameSaveSupport() const {
diff --git a/engines/mohawk/myst_state.cpp b/engines/mohawk/myst_state.cpp
index 06cd69b23c..4324d6bde5 100644
--- a/engines/mohawk/myst_state.cpp
+++ b/engines/mohawk/myst_state.cpp
@@ -106,16 +106,12 @@ MystGameState::MystGameState(MohawkEngine_Myst *vm, Common::SaveFileManager *sav
 MystGameState::~MystGameState() {
 }
 
-Common::StringArray MystGameState::generateSaveGameList() {
-	return g_system->getSavefileManager()->listSavefiles("*.mys");
-}
-
-bool MystGameState::load(const Common::String &filename) {
-	if (!loadState(filename)) {
+bool MystGameState::load(int slot) {
+	if (!loadState(slot)) {
 		return false;
 	}
 
-	loadMetadata(filename);
+	loadMetadata(slot);
 
 	// Set Channelwood elevator state to down, because we start on the lower level
 	_channelwood.elevatorState = 0;
@@ -136,7 +132,8 @@ bool MystGameState::load(const Common::String &filename) {
 	return true;
 }
 
-bool MystGameState::loadState(const Common::String &filename) {
+bool MystGameState::loadState(int slot) {
+	Common::String filename = buildSaveFilename(slot);
 	Common::InSaveFile *loadFile = _saveFileMan->openForLoading(filename);
 	if (!loadFile) {
 		return false;
@@ -160,9 +157,10 @@ bool MystGameState::loadState(const Common::String &filename) {
 	return true;
 }
 
-void MystGameState::loadMetadata(const Common::String &filename) {
+void MystGameState::loadMetadata(int slot) {
 	// Open the metadata file
-	Common::InSaveFile *metadataFile = openMetadataFile(filename);
+	Common::String filename = buildMetadataFilename(slot);
+	Common::InSaveFile *metadataFile = _vm->getSaveFileManager()->openForLoading(filename);
 	if (!metadataFile) {
 		return;
 	}
@@ -179,25 +177,19 @@ void MystGameState::loadMetadata(const Common::String &filename) {
 	delete metadataFile;
 }
 
-bool MystGameState::save(const Common::String &filename) {
-	// Make sure the description does not have an extension
-	Common::String desc = filename;
-	if (filename.hasSuffix(".mys") || filename.hasSuffix(".MYS")) {
-		desc = removeExtension(filename);
-	}
-
-	if (!saveState(desc)) {
+bool MystGameState::save(int slot, const Common::String &desc) {
+	if (!saveState(slot)) {
 		return false;
 	}
 
 	updateMetadateForSaving(desc);
 
-	return saveMetadata(desc);
+	return saveMetadata(slot);
 }
 
-bool MystGameState::saveState(const Common::String &desc) {
+bool MystGameState::saveState(int slot) {
 	// Make sure we have the right extension
-	Common::String filename = desc + ".mys";
+	Common::String filename = buildSaveFilename(slot);
 	Common::OutSaveFile *saveFile = _saveFileMan->openForSaving(filename);
 	if (!saveFile) {
 		return false;
@@ -213,6 +205,14 @@ bool MystGameState::saveState(const Common::String &desc) {
 	return true;
 }
 
+Common::String MystGameState::buildSaveFilename(int slot) {
+	return Common::String::format("myst-%03d.mys", slot);
+}
+
+Common::String MystGameState::buildMetadataFilename(int slot) {
+	return Common::String::format("myst-%03d.mym", slot);
+}
+
 void MystGameState::updateMetadateForSaving(const Common::String &desc) {
 	// Update save creation info
 	TimeDate t;
@@ -226,10 +226,10 @@ void MystGameState::updateMetadateForSaving(const Common::String &desc) {
 	_metadata.totalPlayTime = _vm->getTotalPlayTime();
 }
 
-bool MystGameState::saveMetadata(const Common::String &desc) {
+bool MystGameState::saveMetadata(int slot) {
 	// Write the metadata to a separate file so that the save files
 	// are still compatible with the original engine
-	Common::String metadataFilename = desc + ".mym";
+	Common::String metadataFilename = buildMetadataFilename(slot);
 	Common::OutSaveFile *metadataFile = _saveFileMan->openForSaving(metadataFilename);
 	if (!metadataFile) {
 		return false;
@@ -248,14 +248,12 @@ bool MystGameState::saveMetadata(const Common::String &desc) {
 	return true;
 }
 
-SaveStateDescriptor MystGameState::querySaveMetaInfos(const Common::String filename) {
-	SaveStateDescriptor desc;
-	desc.setDescription(filename);
-
+SaveStateDescriptor MystGameState::querySaveMetaInfos(int slot) {
 	// Open the metadata file
-	Common::InSaveFile *metadataFile = openMetadataFile(filename);
+	Common::String filename = buildMetadataFilename(slot);
+	Common::InSaveFile *metadataFile = g_system->getSavefileManager()->openForLoading(filename);
 	if (!metadataFile) {
-		return desc;
+		return SaveStateDescriptor();
 	}
 
 	Common::Serializer m(metadataFile, nullptr);
@@ -264,10 +262,11 @@ SaveStateDescriptor MystGameState::querySaveMetaInfos(const Common::String filen
 	Mohawk::MystSaveMetadata metadata;
 	if (!metadata.sync(m)) {
 		delete metadataFile;
-		return desc;
+		return SaveStateDescriptor();
 	}
 
 	// Set the save description
+	SaveStateDescriptor desc;
 	desc.setDescription(metadata.saveDescription);
 	desc.setSaveDate(metadata.saveYear, metadata.saveMonth, metadata.saveDay);
 	desc.setSaveTime(metadata.saveHour, metadata.saveMinute);
@@ -279,20 +278,26 @@ SaveStateDescriptor MystGameState::querySaveMetaInfos(const Common::String filen
 	return desc;
 }
 
-Common::InSaveFile *MystGameState::openMetadataFile(const Common::String &filename) {
-	// Remove the extension
-	Common::String baseName = removeExtension(filename);
-
+Common::String MystGameState::querySaveDescription(int slot) {
 	// Open the metadata file
-	return g_system->getSavefileManager()->openForLoading(baseName + ".mym");
-}
+	Common::String filename = buildMetadataFilename(slot);
+	Common::InSaveFile *metadataFile = g_system->getSavefileManager()->openForLoading(filename);
+	if (!metadataFile) {
+		return "";
+	}
+
+	Common::Serializer m(metadataFile, nullptr);
 
-Common::String MystGameState::removeExtension(const Common::String &filename) {
-	Common::String baseName = filename;
-	for (uint i = 0; i < 4; i++) {
-		baseName.deleteLastChar();
+	// Read the metadata file
+	Mohawk::MystSaveMetadata metadata;
+	if (!metadata.sync(m)) {
+		delete metadataFile;
+		return "";
 	}
-	return baseName;
+
+	delete metadataFile;
+
+	return metadata.saveDescription;
 }
 
 void MystGameState::syncGameState(Common::Serializer &s, bool isME) {
@@ -471,12 +476,14 @@ void MystGameState::syncGameState(Common::Serializer &s, bool isME) {
 		warning("Unexpected File Position 0x%03X At End of Save/Load", s.bytesSynced());
 }
 
-void MystGameState::deleteSave(const Common::String &saveName) {
-	debugC(kDebugSaveLoad, "Deleting save file \'%s\'", saveName.c_str());
-	Common::String basename = removeExtension(saveName);
+void MystGameState::deleteSave(int slot) {
+	Common::String filename = buildSaveFilename(slot);
+	Common::String metadataFilename = buildMetadataFilename(slot);
+
+	debugC(kDebugSaveLoad, "Deleting save file \'%s\'", filename.c_str());
 
-	g_system->getSavefileManager()->removeSavefile(saveName);
-	g_system->getSavefileManager()->removeSavefile(basename + ".mym");
+	g_system->getSavefileManager()->removeSavefile(filename);
+	g_system->getSavefileManager()->removeSavefile(metadataFilename);
 }
 
 void MystGameState::addZipDest(uint16 stack, uint16 view) {
diff --git a/engines/mohawk/myst_state.h b/engines/mohawk/myst_state.h
index 50359a5b52..7d5f3f7102 100644
--- a/engines/mohawk/myst_state.h
+++ b/engines/mohawk/myst_state.h
@@ -58,12 +58,12 @@ public:
 	MystGameState(MohawkEngine_Myst*, Common::SaveFileManager*);
 	~MystGameState();
 
-	static Common::StringArray generateSaveGameList();
-	static SaveStateDescriptor querySaveMetaInfos(const Common::String filename);
+	static SaveStateDescriptor querySaveMetaInfos(int slot);
+	static Common::String querySaveDescription(int slot);
 
-	bool load(const Common::String &filename);
-	bool save(const Common::String &filename);
-	static void deleteSave(const Common::String &saveName);
+	bool load(int slot);
+	bool save(int slot, const Common::String &desc);
+	static void deleteSave(int slot);
 
 	void addZipDest(uint16 stack, uint16 view);
 	bool isReachableZipDest(uint16 stack, uint16 view);
@@ -292,13 +292,13 @@ public:
 
 private:
 	void syncGameState(Common::Serializer &s, bool isME);
-	static Common::InSaveFile *openMetadataFile(const Common::String &filename);
-	bool loadState(const Common::String &filename);
-	void loadMetadata(const Common::String &filename);
-	bool saveState(const Common::String &desc);
+	static Common::String buildSaveFilename(int slot);
+	static Common::String buildMetadataFilename(int slot);
+	bool loadState(int slot);
+	void loadMetadata(int slot);
+	bool saveState(int slot);
 	void updateMetadateForSaving(const Common::String &desc);
-	bool saveMetadata(const Common::String &desc);
-	static Common::String removeExtension(const Common::String &filename);
+	bool saveMetadata(int slot);
 
 	// The values in these regions are lists of VIEW resources
 	// which correspond to visited zip destinations