From a7b52562122d4a3243fd7cdc3c902fd7c3f06e6d Mon Sep 17 00:00:00 2001 From: danij Date: Mon, 22 Oct 2012 18:23:27 +0100 Subject: [PATCH] Refactor|FileSys: Moved unload blocking of required game files out of de::FS1 The file system itself should not be concerned about higher level concepts such as whether it is "legal" to unload a file that is "required" by the currently loaded de::Game. This logic is now placed in dd_main.cpp and an isRequiredFile() method was added to de::Game. --- doomsday/engine/portable/include/fs_main.h | 12 ++--- doomsday/engine/portable/include/game.h | 15 ++++++ doomsday/engine/portable/src/dd_main.cpp | 38 ++++++++----- doomsday/engine/portable/src/fs_main.cpp | 62 +++------------------- doomsday/engine/portable/src/game.cpp | 31 +++++++++++ 5 files changed, 84 insertions(+), 74 deletions(-) diff --git a/doomsday/engine/portable/include/fs_main.h b/doomsday/engine/portable/include/fs_main.h index 9627264394..970c67808e 100644 --- a/doomsday/engine/portable/include/fs_main.h +++ b/doomsday/engine/portable/include/fs_main.h @@ -169,15 +169,12 @@ namespace de /** * Attempt to remove a file from the virtual file system. * - * @param permitRequired @c true= allow removal of resources marked as "required" - * by the currently loaded Game. - * * @return @c true if the operation is successful. * */ - bool removeFile(File1& file, bool permitRequired = false); + bool removeFile(File1& file); - int removeFiles(FileList& files, bool permitRequired = false); + int removeFiles(FileList& files); lumpnum_t lumpNumForName(char const* name, bool silent = true); @@ -360,7 +357,7 @@ namespace de */ void deindex(File1& file); - bool unloadFile(File1& file, bool permitRequired = false, bool quiet = false); + bool unloadFile(File1& file, bool quiet = false); public: /** @@ -411,8 +408,7 @@ int F_Access(char const* path); struct file1_s* F_AddFile2(char const* path, size_t baseOffset); struct file1_s* F_AddFile(char const* path/*, baseOffset = 0*/); -boolean F_RemoveFile2(char const* path, boolean permitRequired); -boolean F_RemoveFile(char const* path/*, permitRequired = false */); +boolean F_RemoveFile(char const* path); FileHandle* F_Open3(char const* path, char const* mode, size_t baseOffset, boolean allowDuplicate); FileHandle* F_Open2(char const* path, char const* mode, size_t baseOffset/*, allowDuplicate = true */); diff --git a/doomsday/engine/portable/include/game.h b/doomsday/engine/portable/include/game.h index bb5e3d3fde..22f06c1e28 100644 --- a/doomsday/engine/portable/include/game.h +++ b/doomsday/engine/portable/include/game.h @@ -54,6 +54,7 @@ struct gamedef_s; namespace de { +class File1; class GameCollection; /** @@ -140,6 +141,20 @@ class Game */ struct AbstractResource_s* const* resources(resourceclass_t rclass, int* count) const; + /** + * Is @a file required by this game? This decision is made by comparing the + * absolute path of the specified file to those in the list of located, startup + * resources for the game. If the file's path matches one of these it is therefore + * "required" by this game. + * + * @param file File to be tested for required-status. Can be a contained file + * (such as a lump from a Wad file), in which case the path of the + * root (i.e., outermost file) file is used for testing this status. + * + * @return @c true iff @a file is required by this game. + */ + bool isRequiredFile(File1& file); + // Static members ------------------------------------------------------------------ /** diff --git a/doomsday/engine/portable/src/dd_main.cpp b/doomsday/engine/portable/src/dd_main.cpp index 5175ff834a..8bc8d7e29f 100644 --- a/doomsday/engine/portable/src/dd_main.cpp +++ b/doomsday/engine/portable/src/dd_main.cpp @@ -2334,11 +2334,34 @@ D_CMD(Load) return (didLoadGame || didLoadResource); } +static void tryUnloadFile(char const* path, bool* didUnloadFiles = 0) +{ + try + { + de::File1& file = App_FileSystem()->find(path); + + // Do not attempt to unload a resource required by the current game. + if(games->currentGame().isRequiredFile(file)) + { + Con_Message("\"%s\" is required by the current game.\n" + "Required game files cannot be unloaded in isolation.\n", + F_PrettyPath(Str_Text(file.composePath()))); + return; + } + + if(App_FileSystem()->removeFile(file)) + { + if(didUnloadFiles) *didUnloadFiles = true; + } + } + catch(FS1::NotFoundError const&) + {} // Ignore. +} + D_CMD(Unload) { DENG_UNUSED(src); - boolean didUnloadFiles = false; ddstring_t searchPath; int i; @@ -2391,21 +2414,12 @@ D_CMD(Unload) } // Try the resource locator. + bool didUnloadFiles = false; for(i = 1; i < argc; ++i) { if(!F_FindResource2(RC_PACKAGE, argv[i], &searchPath)) continue; - try - { - de::File1& file = App_FileSystem()->find(Str_Text(&searchPath)); - if(App_FileSystem()->removeFile(file, false/*not required game resources*/)) - { - // Success! - didUnloadFiles = true; - } - } - catch(FS1::NotFoundError const&) - {} // Ignore. + tryUnloadFile(Str_Text(&searchPath), &didUnloadFiles); } Str_Free(&searchPath); diff --git a/doomsday/engine/portable/src/fs_main.cpp b/doomsday/engine/portable/src/fs_main.cpp index 0858ba1492..67e3299c96 100644 --- a/doomsday/engine/portable/src/fs_main.cpp +++ b/doomsday/engine/portable/src/fs_main.cpp @@ -542,52 +542,11 @@ de::File1& FS1::find(char const* path) return (*found)->file(); } -bool FS1::unloadFile(de::File1& file, bool permitRequired, bool quiet) +bool FS1::unloadFile(de::File1& file, bool quiet) { FileList::iterator found = findListFile(d->loadedFiles, file); if(found == d->loadedFiles.end()) return false; - // Do not attempt to unload a resource required by the current game. - if(!permitRequired) - { - bool isRequired = false; - - if(AbstractResource* const* records = reinterpret_cast(App_CurrentGame())->resources(RC_PACKAGE, 0)) - { - // If this resource is from a container we must use the path of the - // root file container instead. - de::File1& rootFile = file; - while(rootFile.isContained()) - { rootFile = rootFile.container(); } - - AutoStr* absolutePath = rootFile.composePath(); - - for(AbstractResource* const* i = records; *i; i++) - { - AbstractResource* record = *i; - if(!(AbstractResource_ResourceFlags(record) & RF_STARTUP)) continue; - - ddstring_t const* resolvedPath = AbstractResource_ResolvedPath(record, true); - if(resolvedPath && !Str_CompareIgnoreCase(resolvedPath, Str_Text(absolutePath))) - { - isRequired = true; - break; - } - } - } - - if(isRequired) - { - if(!quiet) - { - Con_Message("\"%s\" is required by the current game.\n" - "Required game files cannot be unloaded in isolation.\n", - F_PrettyPath(Str_Text(file.composePath()))); - } - return false; - } - } - AutoStr* absolutePath = file.composePath(); if(!quiet && verbose >= 1) { @@ -651,7 +610,7 @@ FS1& FS1::unloadAllNonStartupFiles(int* retNumUnloaded) File1& file = hndl.file(); if(file.hasStartup()) continue; - if(unloadFile(file, true/*allow unloading game resources*/, true/*quiet please*/)) + if(unloadFile(file, true/*quiet please*/)) { numUnloaded += 1; } @@ -1244,9 +1203,9 @@ int FS1::addFiles(char const* const* paths, int num) return addedFileCount; } -bool FS1::removeFile(de::File1& file, bool permitRequired) +bool FS1::removeFile(de::File1& file) { - bool didUnload = unloadFile(file, permitRequired); + bool didUnload = unloadFile(file); if(didUnload) { DD_UpdateEngineState(); @@ -1254,13 +1213,13 @@ bool FS1::removeFile(de::File1& file, bool permitRequired) return didUnload; } -int FS1::removeFiles(FileList& files, bool permitRequired) +int FS1::removeFiles(FileList& files) { int removedFileCount = 0; DENG2_FOR_EACH(i, files, FileList::const_iterator) { File1& file = (*i)->file(); - if(unloadFile(file, permitRequired)) + if(unloadFile(file)) { VERBOSE2( Con_Message("Done unloading %s\n", F_PrettyPath(Str_Text(file.composePath()))) ) removedFileCount += 1; @@ -1595,23 +1554,18 @@ struct file1_s* F_AddFile(char const* path) return F_AddFile2(path, 0); } -boolean F_RemoveFile2(char const* path, boolean permitRequired) +boolean F_RemoveFile(char const* path) { try { de::File1& file = App_FileSystem()->find(path); - return App_FileSystem()->removeFile(file, CPP_BOOL(permitRequired)); + return App_FileSystem()->removeFile(file); } catch(FS1::NotFoundError const&) {} // Ignore. return false; } -boolean F_RemoveFile(char const* path) -{ - return F_RemoveFile2(path, false); -} - void F_ReleaseFile(struct file1_s* file) { if(!file) return; diff --git a/doomsday/engine/portable/src/game.cpp b/doomsday/engine/portable/src/game.cpp index 9e092355c0..0760e0e7e1 100644 --- a/doomsday/engine/portable/src/game.cpp +++ b/doomsday/engine/portable/src/game.cpp @@ -233,6 +233,37 @@ AbstractResource* const* Game::resources(resourceclass_t rclass, int* count) con return d->requiredResources[rclass].records? d->requiredResources[rclass].records : 0; } +bool Game::isRequiredFile(File1& file) +{ + bool isRequired = false; + + if(AbstractResource* const* records = resources(RC_PACKAGE, 0)) + { + // If this resource is from a container we must use the path of the + // root file container instead. + File1& rootFile = file; + while(rootFile.isContained()) + { rootFile = rootFile.container(); } + + AutoStr* absolutePath = rootFile.composePath(); + + for(AbstractResource* const* i = records; *i; i++) + { + AbstractResource* record = *i; + if(!(AbstractResource_ResourceFlags(record) & RF_STARTUP)) continue; + + ddstring_t const* resolvedPath = AbstractResource_ResolvedPath(record, true); + if(resolvedPath && !Str_CompareIgnoreCase(resolvedPath, Str_Text(absolutePath))) + { + isRequired = true; + break; + } + } + } + + return isRequired; +} + Game* Game::fromDef(GameDef const& def) { AutoStr* dataPath = AutoStr_NewStd();