From e7e7aa0558f14edd0ddc3057f8ecfec22863e9ea Mon Sep 17 00:00:00 2001 From: Johannes Schickel Date: Thu, 25 Feb 2016 17:02:22 +0100 Subject: COMMON: Beautify SaveFileManager documentation. --- common/savefile.h | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/common/savefile.h b/common/savefile.h index b0c4d31f53..f2205e2f4f 100644 --- a/common/savefile.h +++ b/common/savefile.h @@ -115,49 +115,56 @@ public: * exports from the Quest for Glory series. QfG5 is a 3D game and won't be * supported by ScummVM. * - * @param name the name of the savefile - * @param compress toggles whether to compress the resulting save file - * (default) or not. - * @return pointer to an OutSaveFile, or NULL if an error occurred. + * @param name The name of the savefile. + * @param compress Toggles whether to compress the resulting save file + * (default) or not. + * @return Pointer to an OutSaveFile, or NULL if an error occurred. */ virtual OutSaveFile *openForSaving(const String &name, bool compress = true) = 0; /** * Open the file with the specified name in the given directory for loading. - * @param name the name of the savefile - * @return pointer to an InSaveFile, or NULL if an error occurred. + * + * @param name The name of the savefile. + * @return Pointer to an InSaveFile, or NULL if an error occurred. */ virtual InSaveFile *openForLoading(const String &name) = 0; /** * Removes the given savefile from the system. - * @param name the name of the savefile to be removed. + * + * @param name The name of the savefile to be removed. * @return true if no error occurred, false otherwise. */ virtual bool removeSavefile(const String &name) = 0; /** * Renames the given savefile. - * @param oldName Old name. - * @param newName New name. + * + * @param oldName Old name. + * @param newName New name. * @return true if no error occurred. false otherwise. */ virtual bool renameSavefile(const String &oldName, const String &newName); /** * Copy the given savefile. - * @param oldName Old name. - * @param newName New name. + * + * @param oldName Old name. + * @param newName New name. * @return true if no error occurred. false otherwise. */ virtual bool copySavefile(const String &oldName, const String &newName); /** - * Request a list of available savegames with a given DOS-style pattern, - * also known as "glob" in the POSIX world. Refer to the Common::matchString() - * function to learn about the precise pattern format. - * @param pattern Pattern to match. Wildcards like * or ? are available. - * @return list of strings for all present file names. + * List available savegames matching a given pattern. + * + * Our pattern format is based on DOS paterns, also known as "glob" in the + * POSIX world. Please refer to the Common::matchString() function to learn + * about the precise pattern format. + * + * @param pattern Pattern to match. Wildcards like * or ? are available. + * @return List of strings for all present file names. * @see Common::matchString() */ virtual StringArray listSavefiles(const String &pattern) = 0; -- cgit v1.2.3 From 8c5931bca4b85c8779bf69aeb04bad5ed4191b79 Mon Sep 17 00:00:00 2001 From: Johannes Schickel Date: Thu, 25 Feb 2016 17:16:25 +0100 Subject: COMMON: Add documentation about savefile names. --- common/savefile.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/common/savefile.h b/common/savefile.h index f2205e2f4f..9fca07f9d5 100644 --- a/common/savefile.h +++ b/common/savefile.h @@ -56,6 +56,12 @@ typedef WriteStream OutSaveFile; * i.e. typically save states, but also configuration files and similar * things. * + * Savefile names represent SaveFiles. These names are case insensitive, that + * means a name of "Kq1.000" represents the same savefile as "kq1.000". In + * addition, SaveFileManager does not allow for names which contain path + * separators like '/' or '\'. This is because we do not support directories + * in SaveFileManager. + * * While not declared as a singleton, it is effectively used as such, * with OSystem::getSavefileManager returning a pointer to the single * SaveFileManager instances to be used. -- cgit v1.2.3 From d6d63a16e2c34d3697a26aff97aadf76f1ab4c68 Mon Sep 17 00:00:00 2001 From: Johannes Schickel Date: Thu, 25 Feb 2016 18:33:33 +0100 Subject: BACKENDS: Make DefaultSaveFileManager case insensitive. For this we introduce a file cache inside DefaultSaveFileManager similar to what we use inside FSDirectory. However, we only do small updates for newly created saves (via openForSaving) or for removed saves (via removeSavefile). Re-caching is done whenever the savepath changes. Tizen changes have not been tested. --- backends/platform/tizen/system.cpp | 49 +++++----- backends/saves/default/default-saves.cpp | 155 ++++++++++++++++++++----------- backends/saves/default/default-saves.h | 25 +++++ 3 files changed, 152 insertions(+), 77 deletions(-) diff --git a/backends/platform/tizen/system.cpp b/backends/platform/tizen/system.cpp index a235456670..1820a28791 100644 --- a/backends/platform/tizen/system.cpp +++ b/backends/platform/tizen/system.cpp @@ -81,36 +81,41 @@ struct TizenSaveFileManager : public DefaultSaveFileManager { }; bool TizenSaveFileManager::removeSavefile(const Common::String &filename) { - Common::String savePathName = getSavePath(); + // Assure the savefile name cache is up-to-date. + assureCached(getSavePath()); + if (getError().getCode() != Common::kNoError) + return false; - checkPath(Common::FSNode(savePathName)); - if (getError().getCode() != Common::kNoError) { + // Obtain node if exists. + SaveFileCache::const_iterator file = _saveFileCache.find(filename); + if (file == _saveFileCache.end()) { return false; - } + } else { + const Common::FSNode fileNode = file->_value; + // Remove from cache, this invalidates the 'file' iterator. + _saveFileCache.erase(file); + file = _saveFileCache.end(); - // recreate FSNode since checkPath may have changed/created the directory - Common::FSNode savePath(savePathName); - Common::FSNode file = savePath.getChild(filename); + String unicodeFileName; + StringUtil::Utf8ToString(fileNode.getPath().c_str(), unicodeFileName); - String unicodeFileName; - StringUtil::Utf8ToString(file.getPath().c_str(), unicodeFileName); + switch (Tizen::Io::File::Remove(unicodeFileName)) { + case E_SUCCESS: + return true; - switch (Tizen::Io::File::Remove(unicodeFileName)) { - case E_SUCCESS: - return true; + case E_ILLEGAL_ACCESS: + setError(Common::kWritePermissionDenied, "Search or write permission denied: " + + file.getName()); + break; - case E_ILLEGAL_ACCESS: - setError(Common::kWritePermissionDenied, "Search or write permission denied: " + - file.getName()); - break; + default: + setError(Common::kPathDoesNotExist, "removeSavefile: '" + file.getName() + + "' does not exist or path is invalid"); + break; + } - default: - setError(Common::kPathDoesNotExist, "removeSavefile: '" + file.getName() + - "' does not exist or path is invalid"); - break; + return false; } - - return false; } // diff --git a/backends/saves/default/default-saves.cpp b/backends/saves/default/default-saves.cpp index 4f7013724a..1f8b511a9c 100644 --- a/backends/saves/default/default-saves.cpp +++ b/backends/saves/default/default-saves.cpp @@ -60,22 +60,15 @@ void DefaultSaveFileManager::checkPath(const Common::FSNode &dir) { } Common::StringArray DefaultSaveFileManager::listSavefiles(const Common::String &pattern) { - Common::String savePathName = getSavePath(); - checkPath(Common::FSNode(savePathName)); + // Assure the savefile name cache is up-to-date. + assureCached(getSavePath()); if (getError().getCode() != Common::kNoError) return Common::StringArray(); - // recreate FSNode since checkPath may have changed/created the directory - Common::FSNode savePath(savePathName); - - Common::FSDirectory dir(savePath); - Common::ArchiveMemberList savefiles; Common::StringArray results; - Common::String search(pattern); - - if (dir.listMatchingMembers(savefiles, search) > 0) { - for (Common::ArchiveMemberList::const_iterator file = savefiles.begin(); file != savefiles.end(); ++file) { - results.push_back((*file)->getName()); + for (SaveFileCache::const_iterator file = _saveFileCache.begin(), end = _saveFileCache.end(); file != end; ++file) { + if (file->_key.matchString(pattern, true)) { + results.push_back(file->_key); } } @@ -83,68 +76,81 @@ Common::StringArray DefaultSaveFileManager::listSavefiles(const Common::String & } Common::InSaveFile *DefaultSaveFileManager::openForLoading(const Common::String &filename) { - // Ensure that the savepath is valid. If not, generate an appropriate error. - Common::String savePathName = getSavePath(); - checkPath(Common::FSNode(savePathName)); + // Assure the savefile name cache is up-to-date. + assureCached(getSavePath()); if (getError().getCode() != Common::kNoError) - return 0; - - // recreate FSNode since checkPath may have changed/created the directory - Common::FSNode savePath(savePathName); + return nullptr; - Common::FSNode file = savePath.getChild(filename); - if (!file.exists()) - return 0; - - // Open the file for reading - Common::SeekableReadStream *sf = file.createReadStream(); - - return Common::wrapCompressedReadStream(sf); + SaveFileCache::const_iterator file = _saveFileCache.find(filename); + if (file == _saveFileCache.end()) { + return nullptr; + } else { + // Open the file for loading. + Common::SeekableReadStream *sf = file->_value.createReadStream(); + return Common::wrapCompressedReadStream(sf); + } } Common::OutSaveFile *DefaultSaveFileManager::openForSaving(const Common::String &filename, bool compress) { - // Ensure that the savepath is valid. If not, generate an appropriate error. - Common::String savePathName = getSavePath(); - checkPath(Common::FSNode(savePathName)); + // Assure the savefile name cache is up-to-date. + const Common::String savePathName = getSavePath(); + assureCached(savePathName); if (getError().getCode() != Common::kNoError) - return 0; + return nullptr; - // recreate FSNode since checkPath may have changed/created the directory - Common::FSNode savePath(savePathName); + // Obtain node. + SaveFileCache::const_iterator file = _saveFileCache.find(filename); + Common::FSNode fileNode; - Common::FSNode file = savePath.getChild(filename); + // If the file did not exist before, we add it to the cache. + if (file == _saveFileCache.end()) { + const Common::FSNode savePath(savePathName); + fileNode = savePath.getChild(filename); + } else { + fileNode = file->_value; + } - // Open the file for saving - Common::WriteStream *sf = file.createWriteStream(); + // Open the file for saving. + Common::WriteStream *const sf = fileNode.createWriteStream(); + Common::OutSaveFile *const result = compress ? Common::wrapCompressedWriteStream(sf) : sf; - return compress ? Common::wrapCompressedWriteStream(sf) : sf; + // Add file to cache now that it exists. + _saveFileCache[filename] = Common::FSNode(fileNode.getPath()); + + return result; } bool DefaultSaveFileManager::removeSavefile(const Common::String &filename) { - Common::String savePathName = getSavePath(); - checkPath(Common::FSNode(savePathName)); + // Assure the savefile name cache is up-to-date. + assureCached(getSavePath()); if (getError().getCode() != Common::kNoError) return false; - // recreate FSNode since checkPath may have changed/created the directory - Common::FSNode savePath(savePathName); - - Common::FSNode file = savePath.getChild(filename); - - // FIXME: remove does not exist on all systems. If your port fails to - // compile because of this, please let us know (scummvm-devel or Fingolfin). - // There is a nicely portable workaround, too: Make this method overloadable. - if (remove(file.getPath().c_str()) != 0) { + // Obtain node if exists. + SaveFileCache::const_iterator file = _saveFileCache.find(filename); + if (file == _saveFileCache.end()) { + return false; + } else { + const Common::FSNode fileNode = file->_value; + // Remove from cache, this invalidates the 'file' iterator. + _saveFileCache.erase(file); + file = _saveFileCache.end(); + + // FIXME: remove does not exist on all systems. If your port fails to + // compile because of this, please let us know (scummvm-devel or Fingolfin). + // There is a nicely portable workaround, too: Make this method overloadable. + if (remove(fileNode.getPath().c_str()) != 0) { #ifndef _WIN32_WCE - if (errno == EACCES) - setError(Common::kWritePermissionDenied, "Search or write permission denied: "+file.getName()); + if (errno == EACCES) + setError(Common::kWritePermissionDenied, "Search or write permission denied: "+fileNode.getName()); - if (errno == ENOENT) - setError(Common::kPathDoesNotExist, "removeSavefile: '"+file.getName()+"' does not exist or path is invalid"); + if (errno == ENOENT) + setError(Common::kPathDoesNotExist, "removeSavefile: '"+fileNode.getName()+"' does not exist or path is invalid"); #endif - return false; - } else { - return true; + return false; + } else { + return true; + } } } @@ -171,4 +177,43 @@ Common::String DefaultSaveFileManager::getSavePath() const { return dir; } +void DefaultSaveFileManager::assureCached(const Common::String &savePathName) { + // Check that path exists and is usable. + checkPath(Common::FSNode(savePathName)); + + if (_cachedDirectory == savePathName) { + return; + } + + _saveFileCache.clear(); + _cachedDirectory.clear(); + + if (getError().getCode() != Common::kNoError) { + warning("DefaultSaveFileManager::assureCached: Can not cache path '%s': '%s'", savePathName.c_str(), getErrorDesc().c_str()); + return; + } + + // FSNode can cache its members, thus create it after checkPath to reflect + // actual file system state. + const Common::FSNode savePath(savePathName); + + Common::FSList children; + if (!savePath.getChildren(children, Common::FSNode::kListFilesOnly)) { + return; + } + + // Build the savefile name cache. + for (Common::FSList::const_iterator file = children.begin(), end = children.end(); file != end; ++file) { + if (_saveFileCache.contains(file->getName())) { + warning("DefaultSaveFileManager::assureCached: Name clash when building cache, ignoring file '%s'", file->getName().c_str()); + } else { + _saveFileCache[file->getName()] = *file; + } + } + + // Only now store that we cached 'savePathName' to indicate we successfully + // cached the directory. + _cachedDirectory = savePathName; +} + #endif // !defined(DISABLE_DEFAULT_SAVEFILEMANAGER) diff --git a/backends/saves/default/default-saves.h b/backends/saves/default/default-saves.h index 81f45f96b8..bf4ca0229d 100644 --- a/backends/saves/default/default-saves.h +++ b/backends/saves/default/default-saves.h @@ -27,6 +27,7 @@ #include "common/savefile.h" #include "common/str.h" #include "common/fs.h" +#include "common/hashmap.h" /** * Provides a default savefile manager implementation for common platforms. @@ -54,6 +55,30 @@ protected: * Sets the internal error and error message accordingly. */ virtual void checkPath(const Common::FSNode &dir); + + /** + * Assure that the given save path is cached. + * + * @param savePathName String representation of save path to cache. + */ + void assureCached(const Common::String &savePathName); + + typedef Common::HashMap SaveFileCache; + + /** + * Cache of all the save files in the currently cached directory. + * + * Modify with caution because we only re-cache when the save path changed! + * This needs to be updated inside at least openForSaving and + * removeSavefile. + */ + SaveFileCache _saveFileCache; + +private: + /** + * The currently cached directory. + */ + Common::String _cachedDirectory; }; #endif -- cgit v1.2.3 From 12046ea0d7ac9723ee9ae9047fbb39ccbbb17a1b Mon Sep 17 00:00:00 2001 From: Johannes Schickel Date: Sun, 28 Feb 2016 07:48:12 +0100 Subject: BACKENDS: Remove request to mail Fingolfin. --- backends/saves/default/default-saves.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backends/saves/default/default-saves.cpp b/backends/saves/default/default-saves.cpp index 1f8b511a9c..daec36ae72 100644 --- a/backends/saves/default/default-saves.cpp +++ b/backends/saves/default/default-saves.cpp @@ -137,7 +137,7 @@ bool DefaultSaveFileManager::removeSavefile(const Common::String &filename) { file = _saveFileCache.end(); // FIXME: remove does not exist on all systems. If your port fails to - // compile because of this, please let us know (scummvm-devel or Fingolfin). + // compile because of this, please let us know (scummvm-devel). // There is a nicely portable workaround, too: Make this method overloadable. if (remove(fileNode.getPath().c_str()) != 0) { #ifndef _WIN32_WCE -- cgit v1.2.3