SCI: Implement the file operations needed for the save dialog in Phantasmagoria

Phantasmagoria's scripts keep polling for the existence of the savegame
index file and request to read and write it using the same parameters
when opening it. The index file is closed and reopened for every save
slot, which is slow and can be much slower on non-desktop devices.
Also, the game scripts request seeking in writable streams and request
to expand the existing index file.

To provide this functionality and to reduce constant slow file opening
and closing, this virtual class has been introduced

engines/sci/engine/file.cpp

@@ -0,0 +1,139 @@
+/* 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 "common/savefile.h"
+#include "common/stream.h"
+
+#include "sci/sci.h"
+#include "sci/engine/file.h"
+
+namespace Sci {
+
+#ifdef ENABLE_SCI32
+
+VirtualIndexFile::VirtualIndexFile(Common::String fileName) : _fileName(fileName), _changed(false) {
+	Common::SeekableReadStream *inFile = g_sci->getSaveFileManager()->openForLoading(fileName);
+
+	_bufferSize = inFile->size();
+	_buffer = new char[_bufferSize];
+	inFile->read(_buffer, _bufferSize);
+	_ptr = _buffer;
+	delete inFile;
+}
+
+VirtualIndexFile::~VirtualIndexFile() {
+	close();
+
+	_bufferSize = 0;
+	delete[] _buffer;
+	_buffer = 0;
+}
+
+uint32 VirtualIndexFile::read(char *buffer, uint32 size) {
+	uint32 curPos = _ptr - _buffer;
+	uint32 finalSize = MIN<uint32>(size, _bufferSize - curPos);
+	char *localPtr = buffer;
+
+	for (uint32 i = 0; i < finalSize; i++)
+		*localPtr++ = *_ptr++;
+
+	return finalSize;
+}
+
+uint32 VirtualIndexFile::write(const char *buffer, uint32 size) {
+	_changed = true;
+	uint32 curPos = _ptr - _buffer;
+	
+	// Check if the buffer needs to be resized
+	if (curPos + size >= _bufferSize) {
+		_bufferSize = curPos + size + 1;
+		char *tmp = _buffer;
+		_buffer = new char[_bufferSize];
+		_ptr = _buffer + curPos;
+		memcpy(_buffer, tmp, _bufferSize);
+		delete[] tmp;
+	}
+
+	for (uint32 i = 0; i < size; i++)
+		*_ptr++ = *buffer++;
+
+	return size;
+}
+
+uint32 VirtualIndexFile::readLine(char *buffer, uint32 size) {
+	uint32 startPos = _ptr - _buffer;
+	uint32 bytesRead = 0;
+	char *localPtr = buffer;
+
+	// This is not a full-blown implementation of readLine, but it
+	// suffices for Phantasmagoria
+	while (startPos + bytesRead < size) {
+		bytesRead++;
+
+		if (*_ptr == 0 || *_ptr == 0x0A) {
+			_ptr++;
+			*localPtr = 0;
+			return bytesRead;
+		} else {
+			*localPtr++ = *_ptr++;
+		}
+	}
+
+	return bytesRead;
+}
+
+bool VirtualIndexFile::seek(int32 offset, int whence) {
+	uint32 startPos = _ptr - _buffer;
+	assert(offset >= 0);
+
+	switch (whence) {
+	case SEEK_CUR:
+		assert(startPos + offset < _bufferSize);
+		_ptr += offset;
+		break;
+	case SEEK_SET:
+		assert(offset < _bufferSize);
+		_ptr = _buffer + offset;
+		break;
+	case SEEK_END:
+		assert(_bufferSize - offset >= 0);
+		_ptr = _buffer + (_bufferSize - offset);
+		break;
+	}
+
+	return true;
+}
+
+void VirtualIndexFile::close() {
+	if (_changed) {
+		Common::WriteStream *outFile = g_sci->getSaveFileManager()->openForSaving(_fileName);
+		outFile->write(_buffer, _bufferSize);
+		delete outFile;
+	}
+
+	// Maintain the buffer, and seek to the beginning of it
+	_ptr = _buffer;
+}
+
+#endif
+
+} // End of namespace Sci

engines/sci/engine/file.h

@@ -0,0 +1,75 @@
+/* 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 SCI_ENGINE_FILE_H
+#define SCI_ENGINE_FILE_H
+
+#include "common/scummsys.h"
+
+namespace Sci {
+
+#ifdef ENABLE_SCI32
+
+/**
+ * An implementation of a virtual file that supports basic read and write
+ * operations simultaneously.
+ *
+ * This class has been initially implemented for Phantasmagoria, which has its
+ * own custom save/load code. The load code keeps checking for the existence
+ * of the save index file and keeps closing and reopening it for each save
+ * slot. This is notoriously slow and clumsy, and introduces noticeable delays,
+ * especially for non-desktop systems. Also, its game scripts request to open
+ * the index file for reading and writing with the same parameters
+ * (SaveManager::setCurrentSave and SaveManager::getCurrentSave). Moreover,
+ * the game scripts reopen the index file for writing in order to update it
+ * and seek within it. We do not support seeking in writeable streams, and the
+ * fact that our saved games are ZIP files makes this operation even more
+ * expensive. Finally, the savegame index file is supposed to be expanded when
+ * a new save slot is added.
+ * For the aforementioned reasons, this class has been implemented, which offers
+ * the basic functionality needed by the game scripts in Phantasmagoria.
+ */
+class VirtualIndexFile {
+public:
+	VirtualIndexFile(Common::String fileName);
+	~VirtualIndexFile();
+
+	uint32 read(char *buffer, uint32 size);
+	uint32 readLine(char *buffer, uint32 size);
+	uint32 write(const char *buffer, uint32 size);
+	bool seek(int32 offset, int whence);
+	void close();
+
+private:
+	char *_buffer;
+	uint32 _bufferSize;
+	char *_ptr;
+
+	Common::String _fileName;
+	bool _changed;
+};
+
+#endif
+
+} // End of namespace Sci
+
+#endif // SCI_ENGINE_FILE_H

engines/sci/engine/kfile.cpp

@@ -33,6 +33,7 @@
 #include "gui/saveload.h"
 
 #include "sci/sci.h"
+#include "sci/engine/file.h"
 #include "sci/engine/state.h"
 #include "sci/engine/kernel.h"
 #include "sci/engine/savegame.h"
@@ -72,8 +73,6 @@ struct SavegameDesc {
  * for reading only.
  */
 
-
-
 FileHandle::FileHandle() : _in(0), _out(0) {
 }
 
@@ -93,15 +92,14 @@ bool FileHandle::isOpen() const {
 	return _in || _out;
 }
 
-
-
 enum {
 	_K_FILE_MODE_OPEN_OR_CREATE = 0,
 	_K_FILE_MODE_OPEN_OR_FAIL = 1,
 	_K_FILE_MODE_CREATE = 2
 };
 
-
+#define VIRTUALFILE_HANDLE 200
+#define PHANTASMAGORIA_SAVEGAME_INDEX "phantsg.dir"
 
 reg_t file_open(EngineState *s, const Common::String &filename, int mode, bool unwrapFilename) {
 	Common::String englishName = g_sci->getSciLanguageString(filename, K_LANG_ENGLISH);
@@ -130,6 +128,7 @@ reg_t file_open(EngineState *s, const Common::String &filename, int mode, bool u
 		outFile = saveFileMan->openForSaving(wrappedName);
 		if (!outFile)
 			debugC(kDebugLevelFile, "  -> file_open(_K_FILE_MODE_CREATE): failed to create file '%s'", englishName.c_str());
+		
 		// QfG1 opens the character export file with _K_FILE_MODE_CREATE first,
 		// closes it immediately and opens it again with this here. Perhaps
 		// other games use this for read access as well. I guess changing this
@@ -171,8 +170,8 @@ reg_t kFOpen(EngineState *s, int argc, reg_t *argv) {
 }
 
 static FileHandle *getFileFromHandle(EngineState *s, uint handle) {
-	if (handle == 0) {
-		error("Attempt to use file handle 0");
+	if (handle == 0 || handle == VIRTUALFILE_HANDLE) {
+		error("Attempt to use invalid file handle (%d)", handle);
 		return 0;
 	}
 
@@ -738,6 +737,21 @@ reg_t kFileIO(EngineState *s, int argc, reg_t *argv) {
 reg_t kFileIOOpen(EngineState *s, int argc, reg_t *argv) {
 	Common::String name = s->_segMan->getString(argv[0]);
 
+#ifdef ENABLE_SCI32
+	if (name == PHANTASMAGORIA_SAVEGAME_INDEX) {
+		if (s->_virtualIndexFile) {
+			return make_reg(0, VIRTUALFILE_HANDLE);
+		} else {
+			Common::String englishName = g_sci->getSciLanguageString(name, K_LANG_ENGLISH);
+			Common::String wrappedName = g_sci->wrapFilename(englishName);
+			if (!g_sci->getSaveFileManager()->listSavefiles(wrappedName).empty()) {
+				s->_virtualIndexFile = new VirtualIndexFile(wrappedName);
+				return make_reg(0, VIRTUALFILE_HANDLE);
+			}
+		}
+	}
+#endif
+
 	// SCI32 can call K_FILEIO_OPEN with only one argument. It seems to
 	// just be checking if it exists.
 	int mode = (argc < 2) ? (int)_K_FILE_MODE_OPEN_OR_FAIL : argv[1].toUint16();
@@ -780,7 +794,16 @@ reg_t kFileIOOpen(EngineState *s, int argc, reg_t *argv) {
 reg_t kFileIOClose(EngineState *s, int argc, reg_t *argv) {
 	debugC(kDebugLevelFile, "kFileIO(close): %d", argv[0].toUint16());
 
-	FileHandle *f = getFileFromHandle(s, argv[0].toUint16());
+	uint16 handle = argv[0].toUint16();
+
+#ifdef ENABLE_SCI32
+	if (handle == VIRTUALFILE_HANDLE) {
+		s->_virtualIndexFile->close();
+		return SIGNAL_REG;
+	}
+#endif
+
+	FileHandle *f = getFileFromHandle(s, handle);
 	if (f) {
 		f->close();
 		return SIGNAL_REG;
@@ -789,39 +812,56 @@ reg_t kFileIOClose(EngineState *s, int argc, reg_t *argv) {
 }
 
 reg_t kFileIOReadRaw(EngineState *s, int argc, reg_t *argv) {
-	int handle = argv[0].toUint16();
-	int size = argv[2].toUint16();
+	uint16 handle = argv[0].toUint16();
+	uint16 size = argv[2].toUint16();
 	int bytesRead = 0;
 	char *buf = new char[size];
 	debugC(kDebugLevelFile, "kFileIO(readRaw): %d, %d", handle, size);
 
+#ifdef ENABLE_SCI32
+	if (handle == VIRTUALFILE_HANDLE) {
+		bytesRead = s->_virtualIndexFile->read(buf, size);
+	} else {
+#endif
 		FileHandle *f = getFileFromHandle(s, handle);
-	if (f) {
+		if (f)
 			bytesRead = f->_in->read(buf, size);
+#ifdef ENABLE_SCI32
+	}
+#endif
+
 	// TODO: What happens if less bytes are read than what has
 	// been requested? (i.e. if bytesRead is non-zero, but still
 	// less than size)
 	if (bytesRead > 0)
 		s->_segMan->memcpy(argv[1], (const byte*)buf, size);
-	}
 
 	delete[] buf;
 	return make_reg(0, bytesRead);
 }
 
 reg_t kFileIOWriteRaw(EngineState *s, int argc, reg_t *argv) {
-	int handle = argv[0].toUint16();
-	int size = argv[2].toUint16();
+	uint16 handle = argv[0].toUint16();
+	uint16 size = argv[2].toUint16();
 	char *buf = new char[size];
 	bool success = false;
 	s->_segMan->memcpy((byte *)buf, argv[1], size);
 	debugC(kDebugLevelFile, "kFileIO(writeRaw): %d, %d", handle, size);
 
+#ifdef ENABLE_SCI32
+	if (handle == VIRTUALFILE_HANDLE) {
+		s->_virtualIndexFile->write(buf, size);
+		success = true;
+	} else {
+#endif
 		FileHandle *f = getFileFromHandle(s, handle);
 		if (f) {
 			f->_out->write(buf, size);
 			success = true;
 		}
+#ifdef ENABLE_SCI32
+	}
+#endif
 
 	delete[] buf;
 	if (success)
@@ -854,9 +894,19 @@ reg_t kFileIOUnlink(EngineState *s, int argc, reg_t *argv) {
 		name = g_sci->getSavegameName(savedir_nr);
 		result = saveFileMan->removeSavefile(name);
 	} else if (getSciVersion() >= SCI_VERSION_2) {
-		// We don't need to wrap the filename in SCI32 games, as it's already
-		// constructed here
+		// The file name may be already wrapped, so check both cases
 		result = saveFileMan->removeSavefile(name);
+		if (!result) {
+			const Common::String wrappedName = g_sci->wrapFilename(name);
+			result = saveFileMan->removeSavefile(wrappedName);
+		}
+
+#ifdef ENABLE_SCI32
+		if (name == PHANTASMAGORIA_SAVEGAME_INDEX) {
+			delete s->_virtualIndexFile;
+			s->_virtualIndexFile = 0;
+		}
+#endif
 	} else {
 		const Common::String wrappedName = g_sci->wrapFilename(name);
 		result = saveFileMan->removeSavefile(wrappedName);
@@ -869,15 +919,22 @@ reg_t kFileIOUnlink(EngineState *s, int argc, reg_t *argv) {
 }
 
 reg_t kFileIOReadString(EngineState *s, int argc, reg_t *argv) {
-	int size = argv[1].toUint16();
+	uint16 size = argv[1].toUint16();
 	char *buf = new char[size];
-	int handle = argv[2].toUint16();
+	uint16 handle = argv[2].toUint16();
 	debugC(kDebugLevelFile, "kFileIO(readString): %d, %d", handle, size);
+	uint32 bytesRead;
+
+#ifdef ENABLE_SCI32
+	if (handle == VIRTUALFILE_HANDLE)
+		bytesRead = s->_virtualIndexFile->readLine(buf, size);
+	else
+#endif
+		bytesRead = fgets_wrapper(s, buf, size, handle);
 
-	int readBytes = fgets_wrapper(s, buf, size, handle);
 	s->_segMan->memcpy(argv[0], (const byte*)buf, size);
 	delete[] buf;
-	return readBytes ? argv[0] : NULL_REG;
+	return bytesRead ? argv[0] : NULL_REG;
 }
 
 reg_t kFileIOWriteString(EngineState *s, int argc, reg_t *argv) {
@@ -885,6 +942,13 @@ reg_t kFileIOWriteString(EngineState *s, int argc, reg_t *argv) {
 	Common::String str = s->_segMan->getString(argv[1]);
 	debugC(kDebugLevelFile, "kFileIO(writeString): %d", handle);
 
+#ifdef ENABLE_SCI32
+	if (handle == VIRTUALFILE_HANDLE) {
+		s->_virtualIndexFile->write(str.c_str(), str.size());
+		return NULL_REG;
+	}
+#endif
+
 	FileHandle *f = getFileFromHandle(s, handle);
 
 	if (f) {
@@ -896,15 +960,31 @@ reg_t kFileIOWriteString(EngineState *s, int argc, reg_t *argv) {
 }
 
 reg_t kFileIOSeek(EngineState *s, int argc, reg_t *argv) {
-	int handle = argv[0].toUint16();
-	int offset = argv[1].toUint16();
-	int whence = argv[2].toUint16();
+	uint16 handle = argv[0].toUint16();
+	uint16 offset = ABS<int16>(argv[1].toSint16());	// can be negative
+	uint16 whence = argv[2].toUint16();
 	debugC(kDebugLevelFile, "kFileIO(seek): %d, %d, %d", handle, offset, whence);
 
+#ifdef ENABLE_SCI32
+	if (handle == VIRTUALFILE_HANDLE)
+		return make_reg(0, s->_virtualIndexFile->seek(offset, whence));
+#endif
+
 	FileHandle *f = getFileFromHandle(s, handle);
 
-	if (f)
-		s->r_acc = make_reg(0, f->_in->seek(offset, whence));
+	if (f && f->_in) {
+		// Backward seeking isn't supported in zip file streams, thus adapt the
+		// parameters accordingly if games ask for such a seek mode. A known
+		// case where this is requested is the save file manager in Phantasmagoria
+		if (whence == SEEK_END) {
+			whence = SEEK_SET;
+			offset = f->_in->size() - offset;
+		}
+
+		return make_reg(0, f->_in->seek(offset, whence));
+	} else if (f && f->_out) {
+		error("kFileIOSeek: Unsupported seek operation on a writeable stream (offset: %d, whence: %d)", offset, whence);
+	}
 
 	return SIGNAL_REG;
 }
@@ -1027,6 +1107,14 @@ reg_t kFileIOFindNext(EngineState *s, int argc, reg_t *argv) {
 reg_t kFileIOExists(EngineState *s, int argc, reg_t *argv) {
 	Common::String name = s->_segMan->getString(argv[0]);
 
+#ifdef ENABLE_SCI32
+	// Cache the file existence result for the Phantasmagoria
+	// save index file, as the game scripts keep checking for
+	// its existence.
+	if (name == PHANTASMAGORIA_SAVEGAME_INDEX && s->_virtualIndexFile)
+		return TRUE_REG;
+#endif
+
 	bool exists = false;
 
 	// Check for regular file

engines/sci/engine/state.cpp

@@ -26,6 +26,7 @@
 #include "sci/debug.h"	// for g_debug_sleeptime_factor
 #include "sci/event.h"
 
+#include "sci/engine/file.h"
 #include "sci/engine/kernel.h"
 #include "sci/engine/state.h"
 #include "sci/engine/selector.h"
@@ -68,21 +69,26 @@ static const uint16 s_halfWidthSJISMap[256] = {
 };
 
 EngineState::EngineState(SegManager *segMan)
-: _segMan(segMan), _dirseeker() {
+: _segMan(segMan),
+#ifdef ENABLE_SCI32
+	_virtualIndexFile(0),
+#endif
+	_dirseeker() {
 
 	reset(false);
 }
 
 EngineState::~EngineState() {
 	delete _msgState;
+#ifdef ENABLE_SCI32
+	delete _virtualIndexFile;
+#endif
 }
 
 void EngineState::reset(bool isRestoring) {
 	if (!isRestoring) {
 		_memorySegmentSize = 0;
-
 		_fileHandles.resize(5);
-
 		abortScriptProcessing = kAbortNone;
 	}
 

engines/sci/engine/state.h

@@ -45,6 +45,7 @@ namespace Sci {
 class EventManager;
 class MessageState;
 class SoundCommandParser;
+class VirtualIndexFile;
 
 enum AbortGameState {
 	kAbortNone = 0,
@@ -163,6 +164,10 @@ public:
 	int16 _lastSaveVirtualId; // last virtual id fed to kSaveGame, if no kGetSaveFiles was called inbetween
 	int16 _lastSaveNewId;    // last newly created filename-id by kSaveGame
 
+#ifdef ENABLE_SCI32
+	VirtualIndexFile *_virtualIndexFile;
+#endif
+
 	uint _chosenQfGImportItem; // Remembers the item selected in QfG import rooms
 
 	bool _cursorWorkaroundActive; // ffs. GfxCursor::setPosition()

engines/sci/module.mk

@@ -10,6 +10,7 @@ MODULE_OBJS := \
 	sci.o \
 	util.o \
 	engine/features.o \
+	engine/file.o \
 	engine/gc.o \
 	engine/kernel.o \
 	engine/kevent.o \