From 4e1ad7ab65720b11ee333c4fe9ce727e799fd77a Mon Sep 17 00:00:00 2001 From: Gleb Mazovetskiy Date: Thu, 16 Sep 2021 13:15:36 +0100 Subject: [PATCH] LoadFile: Avoid opening the file twice Previously we were passing the path to `GetFileSize`, which resulted in opening and closing the file twice. --- CMakeLists.txt | 1 - Source/engine/load_file.cpp | 38 ----------------- Source/engine/load_file.hpp | 84 ++++++++++++++++++++++++++++++------- Source/player.cpp | 2 + 4 files changed, 71 insertions(+), 54 deletions(-) delete mode 100644 Source/engine/load_file.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 60e40912a..698b09cb9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -420,7 +420,6 @@ set(libdevilutionx_SRCS Source/engine/animationinfo.cpp Source/engine/demomode.cpp Source/engine/load_cel.cpp - Source/engine/load_file.cpp Source/engine/random.cpp Source/engine/render/automap_render.cpp Source/engine/render/cel_render.cpp diff --git a/Source/engine/load_file.cpp b/Source/engine/load_file.cpp deleted file mode 100644 index 51aade796..000000000 --- a/Source/engine/load_file.cpp +++ /dev/null @@ -1,38 +0,0 @@ -#include "load_file.hpp" - -#include "diablo.h" -#include "storm/storm.h" - -namespace devilution { - -size_t GetFileSize(const char *pszName) -{ - HANDLE file; - if (!SFileOpenFile(pszName, &file)) { - if (!gbQuietMode) - app_fatal("GetFileSize - SFileOpenFile failed for file:\n%s", pszName); - return 0; - } - const size_t fileLen = SFileGetFileSize(file); - SFileCloseFileThreadSafe(file); - - return fileLen; -} - -void LoadFileData(const char *pszName, byte *buffer, size_t fileLen) -{ - HANDLE file; - if (!SFileOpenFile(pszName, &file)) { - if (!gbQuietMode) - app_fatal("LoadFileData - SFileOpenFile failed for file:\n%s", pszName); - return; - } - - if (fileLen == 0) - app_fatal("Zero length SFILE:\n%s", pszName); - - SFileReadFileThreadSafe(file, buffer, fileLen); - SFileCloseFileThreadSafe(file); -} - -} // namespace devilution diff --git a/Source/engine/load_file.hpp b/Source/engine/load_file.hpp index 47381c391..409619da6 100644 --- a/Source/engine/load_file.hpp +++ b/Source/engine/load_file.hpp @@ -1,24 +1,78 @@ #pragma once #include +#include +#include #include #include "appfat.h" +#include "diablo.h" +#include "storm/storm.h" #include "utils/stdcompat/cstddef.hpp" namespace devilution { -size_t GetFileSize(const char *pszName); +class SFile { +public: + explicit SFile(const char *path) + { + if (!SFileOpenFile(path, &handle_)) { + handle_ = nullptr; + if (!gbQuietMode) { + const std::uint32_t code = SErrGetLastError(); + if (code == STORM_ERROR_FILE_NOT_FOUND) { + app_fatal("Failed to open file:\n%s\n\nFile not found", path); + } else { + app_fatal("Failed to open file:\n%s\n\nError Code: %u", path, code); + } + } + } + } -void LoadFileData(const char *pszName, byte *buffer, size_t fileLen); + ~SFile() + { + if (handle_ != nullptr) + SFileCloseFileThreadSafe(handle_); + } + + [[nodiscard]] bool Ok() const + { + return handle_ != nullptr; + } + + [[nodiscard]] std::size_t Size() const + { + return SFileGetFileSize(handle_); + } + + bool Read(void *buffer, std::size_t len) const + { + return SFileReadFileThreadSafe(handle_, buffer, len); + } + +private: + HANDLE handle_; +}; template -void LoadFileInMem(const char *path, T *data, std::size_t count = 0) +void LoadFileInMem(const char *path, T *data) { - if (count == 0) - count = GetFileSize(path); + SFile file { path }; + if (!file.Ok()) + return; + const std::size_t fileLen = file.Size(); + if ((fileLen % sizeof(T)) != 0) + app_fatal("File size does not align with type\n%s", path); + file.Read(reinterpret_cast(data), fileLen); +} - LoadFileData(path, reinterpret_cast(data), count * sizeof(T)); +template +void LoadFileInMem(const char *path, T *data, std::size_t count) +{ + SFile file { path }; + if (!file.Ok()) + return; + file.Read(reinterpret_cast(data), count * sizeof(T)); } template @@ -30,24 +84,24 @@ void LoadFileInMem(const char *path, std::array &data) /** * @brief Load a file in to a buffer * @param path Path of file - * @param elements Number of T elements read + * @param numRead Number of T elements read * @return Buffer with content of file */ template -std::unique_ptr LoadFileInMem(const char *path, size_t *elements = nullptr) +std::unique_ptr LoadFileInMem(const char *path, std::size_t *numRead = nullptr) { - const size_t fileLen = GetFileSize(path); - + SFile file { path }; + if (!file.Ok()) + return nullptr; + const std::size_t fileLen = file.Size(); if ((fileLen % sizeof(T)) != 0) app_fatal("File size does not align with type\n%s", path); - if (elements != nullptr) - *elements = fileLen / sizeof(T); + if (numRead != nullptr) + *numRead = fileLen / sizeof(T); std::unique_ptr buf { new T[fileLen / sizeof(T)] }; - - LoadFileData(path, reinterpret_cast(buf.get()), fileLen); - + file.Read(reinterpret_cast(buf.get()), fileLen); return buf; } diff --git a/Source/player.cpp b/Source/player.cpp index fb4ef0fb0..53e216d01 100644 --- a/Source/player.cpp +++ b/Source/player.cpp @@ -395,12 +395,14 @@ void StartWalk(int pnum, Displacement vel, Direction dir, bool pmWillBeCalled) void SetPlayerGPtrs(const char *path, std::unique_ptr &data, std::array, 8> &anim, int width) { data = nullptr; +#ifndef RUN_TESTS data = LoadFileInMem(path); for (int i = 0; i < 8; i++) { byte *pCelStart = CelGetFrame(data.get(), i); anim[i].emplace(pCelStart, width); } +#endif } void ClearStateVariables(Player &player)