KYRA: (EOB) - change behavior of safe game file importing code

- The initial import of original save files will now use the next free available slots instead of moving the original files to the top of the save file list
- add error check to loading routine for original saves
- also limit debug console command to main menu, since it causes issues when used during the game or during character generation

6 files changed, 35 insertions, 64 deletions

diff --git a/engines/kyra/debugger.cpp b/engines/kyra/debugger.cpp
index 2c9f9f2eea..c0a91ac098 100644
--- a/engines/kyra/debugger.cpp
+++ b/engines/kyra/debugger.cpp
@@ -486,6 +486,11 @@ void Debugger_EoB::initialize() {
 }
 
 bool Debugger_EoB::cmd_importSaveFile(int argc, const char **argv) {
+	if (!_vm->_allowImport) {
+		DebugPrintf("This command may only be used from the main menu.\n");
+		return true;
+	}
+
 	if (argc == 3) {
 		int slot = atoi(argv[1]);
 		if (slot < -1 || slot > 989) {
@@ -493,16 +498,10 @@ bool Debugger_EoB::cmd_importSaveFile(int argc, const char **argv) {
 			return true;
 		}
 
-		::GUI::MessageDialog dialog("Your current game (if any) will be lost if you continue. Make sure to save your game first.\n\n", "Continue", "Cancel");
-		if (!dialog.runModal()) {
-			DebugPrintf("Cancelled.\n");
-			return true;
-		}
-
 		DebugPrintf(_vm->importOriginalSaveFile(slot, argv[2]) ? "Success.\n" : "Failure.\n");
 		_vm->loadItemDefs();
 	} else {
-		DebugPrintf("Syntax:   import_savefile <dest slot> <source file>\n              (Imports source save game file to dest slot.)\n          import_savefile -1\n              (Imports all original save game files found and pushs them to the top of the save game list.)\n\n");
+		DebugPrintf("Syntax:   import_savefile <dest slot> <source file>\n              (Imports source save game file to dest slot.)\n          import_savefile -1\n              (Imports all original save game files found and puts them into the first available slots.)\n\n");
 	}
 
 	return true;
diff --git a/engines/kyra/eobcommon.cpp b/engines/kyra/eobcommon.cpp
index 8e99c6f1b3..7f35b12123 100644
--- a/engines/kyra/eobcommon.cpp
+++ b/engines/kyra/eobcommon.cpp
@@ -148,6 +148,7 @@ EoBCoreEngine::EoBCoreEngine(OSystem *system, const GameFlags &flags)
 	_clericSpellOffset = 0;
 	_restPartyElapsedTime = 0;
 	_allowSkip = false;
+	_allowImport = false;
 
 	_wallsOfForce = 0;
 
diff --git a/engines/kyra/eobcommon.h b/engines/kyra/eobcommon.h
index f17cf05ee5..e3ea533c63 100644
--- a/engines/kyra/eobcommon.h
+++ b/engines/kyra/eobcommon.h
@@ -857,6 +857,7 @@ protected:
 	int8 _rrId[10];
 
 	bool _allowSkip;
+	bool _allowImport;
 
 	Screen_EoB *_screen;
 	GUI_EoB *_gui;
diff --git a/engines/kyra/saveload_eob.cpp b/engines/kyra/saveload_eob.cpp
index 4edd46ffa2..8884765937 100644
--- a/engines/kyra/saveload_eob.cpp
+++ b/engines/kyra/saveload_eob.cpp
@@ -530,6 +530,7 @@ Common::Error EoBCoreEngine::saveGameStateIntern(int slot, const char *saveName,
 
 bool EoBCoreEngine::importOriginalSaveFile(int destSlot, const char *sourceFile) {
 	Common::Array<Common::String> origFiles;
+	Common::Array<int> newSlots;
 
 	if (sourceFile) {
 		// If a source file is specified via the console command we just check whether it exists.
@@ -564,63 +565,26 @@ bool EoBCoreEngine::importOriginalSaveFile(int destSlot, const char *sourceFile)
 	if (!numFilesFound)
 		return false;
 
-	int moveUpSlots = 0;
 	_gui->updateSaveSlotsList(_targetName, true);
 
-	// Check whether ScummVM save files exist and how many slots they need to be moved up to make space for the original save files
+	// Find free save slots for the original save files
 	if (destSlot == -1) {
-		for (int i = 0; i < numFilesFound; ++i) {
-			if (Common::find(_gui->_saveSlots.begin(), _gui->_saveSlots.end(), i) != _gui->_saveSlots.end())
-				moveUpSlots++;
+		int assignedSlots = 0;
+		for (int testSlot = 0; testSlot < 990 && assignedSlots < numFilesFound; testSlot++) {
+			if (Common::find(_gui->_saveSlots.begin(), _gui->_saveSlots.end(), testSlot) == _gui->_saveSlots.end()) {
+				newSlots.push_back(testSlot);
+				assignedSlots++;
+			}
 		}
-	}
-
-	// "move up" existing save files
-	if (moveUpSlots) {
-		Common::sort(_gui->_saveSlots.begin(), _gui->_saveSlots.end(), Common::Greater<int>());
-		for (Common::Array<int>::iterator i = _gui->_saveSlots.begin(); i != _gui->_saveSlots.end(); ++i) {
-			int newIndex = *i + moveUpSlots;
-
-			if (*i >= 990) {
-				// Skip quick save slots
-				continue;
-			} else if (*i >= (990 - moveUpSlots)) {
-				// Try to squeeze occupied slots together by finding and occupying empty slots before the current one if necessary
-				int missingSlots = moveUpSlots - (989 - *i);
-				int missingLeft = missingSlots;
-				int missingLast = missingLeft;
-				Common::Array<int>::iterator squeezeStart = i + 1;
-
-				for (; squeezeStart != _gui->_saveSlots.end() && missingLeft; ++squeezeStart) {
-					int diff = *(squeezeStart - 1) - *squeezeStart - 1;
-					missingLast = MIN(missingLeft, diff);
-					missingLeft -= missingLast;
-				}
-
-				// This will probably never happen, since we do have 990 save slots
-				if (missingLeft)
-					warning("%d original save files could not be converted due to missing save game slots", missingLeft);
-
-				int indexS = *(squeezeStart - 2) - missingLast;
-				for (Common::Array<int>::iterator ii = squeezeStart - 2; ii != i - 1; --ii)
-					_saveFileMan->renameSavefile(getSavegameFilename(*ii), getSavegameFilename(indexS++));
 
-				int newPos = *i -= (missingSlots - missingLeft);
+		// This will probably never happen, since we do have 990 save slots
+		if (assignedSlots != numFilesFound)
+			warning("%d original save files could not be converted due to missing save game slots", numFilesFound - assignedSlots);
 
-				_gui->updateSaveSlotsList(_targetName, true);
-				Common::sort(_gui->_saveSlots.begin(), _gui->_saveSlots.end(), Common::Greater<int>());
-				i = Common::find(_gui->_saveSlots.begin(), _gui->_saveSlots.end(), newPos);
-				if (i == _gui->_saveSlots.end())
-					error("EoBCoreEngine::importOriginalSaveFile(): Unknown error");
-				newIndex = *i + moveUpSlots;
-			}
-
-			_saveFileMan->renameSavefile(getSavegameFilename(*i), getSavegameFilename(newIndex));
-		}
+	} else {
+		newSlots.push_back(destSlot);
 	}
 
-	int curSlot = MAX(destSlot, 0);
-
 	if (destSlot != -1) {
 		if (Common::find(_gui->_saveSlots.begin(), _gui->_saveSlots.end(), destSlot) != _gui->_saveSlots.end()) {
 			::GUI::MessageDialog dialog(Common::String::format(_("A save game file was found in the specified slot %d. Overwrite?\n\n"), destSlot), _("Yes"), _("No"));
@@ -629,17 +593,19 @@ bool EoBCoreEngine::importOriginalSaveFile(int destSlot, const char *sourceFile)
 		}
 	}
 
-	for (Common::Array<Common::String>::iterator i = origFiles.begin(); i != origFiles.end(); ++i) {
-		Common::String desc = readOriginalSaveFile(*i);
+	int importedCount = 0;
+	for (int i = 0; i < numFilesFound; i++) {
+		Common::String desc = readOriginalSaveFile(origFiles[i]);
 		if (desc.empty()) {
-			warning("Unable to import original save file '%s'", i->c_str());
+			warning("Unable to import original save file '%s'", origFiles[i].c_str());
 		} else {
 			// We can't make thumbnails here, since we do not want to load all the level data, monsters, etc. for each save we convert.
 			// Instead, we use an empty surface to avoid that createThumbnailFromScreen() makes a completely pointless thumbnail from
 			// whatever screen that is currently shown when this function is called.
 			Graphics::Surface dummy;
-			saveGameStateIntern(curSlot++, desc.c_str(), &dummy);
-			warning("Imported original save file '%s' ('%s')", i->c_str(), desc.c_str());
+			saveGameStateIntern(newSlots[i], desc.c_str(), &dummy);
+			warning("Imported original save file '%s' ('%s')", origFiles[i].c_str(), desc.c_str());
+			importedCount++;
 		}
 	}
 
@@ -653,8 +619,8 @@ bool EoBCoreEngine::importOriginalSaveFile(int destSlot, const char *sourceFile)
 	memset(_characters, 0, sizeof(EoBCharacter) * 6);
 	_inf->reset();
 
-	if (destSlot == -1 && curSlot) {
-		::GUI::MessageDialog dialog(_("One or more original save game files have been successfully imported into\nScummVM. If you want to manually import original save game files later you will\nneed to open the ScummVM debug console and use the command 'import_savefile'.\n\n"));
+	if (destSlot == -1 && importedCount) {
+		::GUI::MessageDialog dialog(Common::String::format(_("%d original save game files have been successfully imported into\nScummVM. If you want to manually import original save game files later you will\nneed to open the ScummVM debug console and use the command 'import_savefile'.\n\n"), importedCount));
 		dialog.runModal();
 	}
 
@@ -899,7 +865,7 @@ Common::String EoBCoreEngine::readOriginalSaveFile(Common::String &file) {
 		t->extraProperties = in.readUint16();
 	}
 
-	return desc;
+	return in.err() ? Common::String() : desc;
 }
 
 void *EoBCoreEngine::generateMonsterTempData(LevelTempData *tmp) {
diff --git a/engines/kyra/sequences_darkmoon.cpp b/engines/kyra/sequences_darkmoon.cpp
index 516c71e626..f0d4a01486 100644
--- a/engines/kyra/sequences_darkmoon.cpp
+++ b/engines/kyra/sequences_darkmoon.cpp
@@ -116,7 +116,9 @@ int DarkMoonEngine::mainMenu() {
 			_screen->_curPage = op;
 			_screen->copyRegion(0, 0, 0, 0, 320, 200, 2, 0, Screen::CR_NO_P_CHECK);
 			_screen->updateScreen();
+			_allowImport = true;
 			menuChoice = mainMenuLoop();
+			_allowImport = false;
 		} break;
 
 		case 1:
diff --git a/engines/kyra/sequences_eob.cpp b/engines/kyra/sequences_eob.cpp
index 9f54306d59..64f88e95ce 100644
--- a/engines/kyra/sequences_eob.cpp
+++ b/engines/kyra/sequences_eob.cpp
@@ -56,7 +56,9 @@ int EoBEngine::mainMenu() {
 			_screen->_curPage = 0;
 			_screen->copyRegion(0, 0, 0, 0, 320, 200, 2, 0, Screen::CR_NO_P_CHECK);
 			_screen->updateScreen();
+			_allowImport = true;
 			menuChoice = mainMenuLoop();
+			_allowImport = false;
 			} break;
 
 		case 1: