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

@@ -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

Source/engine/load_file.cpp

@@ -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

Source/engine/load_file.hpp

@@ -1,24 +1,78 @@
 #pragma once
 
 #include <array>
+#include <cstddef>
+#include <cstdint>
 #include <memory>
 
 #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 <typename T>
-void LoadFileInMem(const char *path, T *data, std::size_t count = 0)
+void LoadFileInMem(const char *path, T *data)
 {
-	if (count == 0)
+	SFile file { path };
-		count = GetFileSize(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<byte *>(data), fileLen);
+}
 
-	LoadFileData(path, reinterpret_cast<byte *>(data), count * sizeof(T));
+template <typename T>
+void LoadFileInMem(const char *path, T *data, std::size_t count)
+{
+	SFile file { path };
+	if (!file.Ok())
+		return;
+	file.Read(reinterpret_cast<byte *>(data), count * sizeof(T));
 }
 
 template <typename T, std::size_t N>
@@ -30,24 +84,24 @@ void LoadFileInMem(const char *path, std::array<T, N> &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 <typename T = byte>
-std::unique_ptr<T[]> LoadFileInMem(const char *path, size_t *elements = nullptr)
+std::unique_ptr<T[]> 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)
+	if (numRead != nullptr)
-		*elements = fileLen / sizeof(T);
+		*numRead = fileLen / sizeof(T);
 
 	std::unique_ptr<T[]> buf { new T[fileLen / sizeof(T)] };
-
+	file.Read(reinterpret_cast<byte *>(buf.get()), fileLen);
-	LoadFileData(path, reinterpret_cast<byte *>(buf.get()), fileLen);
-
 	return buf;
 }
 

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<byte[]> &data, std::array<std::optional<CelSprite>, 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)