From 47fd38c5c0c8bbf7cc76e81ae7f7f2a2fec2beda Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Fri, 29 Dec 2023 12:06:55 +0000 Subject: [PATCH] review feedback --- fabric-installer-native-bootstrap.vcxproj | 2 + src/Bootstrap.cpp | 18 ++++----- src/Bootstrap.h | 2 +- src/ISystemHelper.h | 48 ++++++----------------- src/SystemHelper.cpp | 48 +++++++++++------------ src/SystemHelper.h | 20 +++++----- 6 files changed, 55 insertions(+), 83 deletions(-) diff --git a/fabric-installer-native-bootstrap.vcxproj b/fabric-installer-native-bootstrap.vcxproj index 02dd096..c89ab9a 100644 --- a/fabric-installer-native-bootstrap.vcxproj +++ b/fabric-installer-native-bootstrap.vcxproj @@ -261,12 +261,14 @@ + + diff --git a/src/Bootstrap.cpp b/src/Bootstrap.cpp index d09d38b..88e3a12 100644 --- a/src/Bootstrap.cpp +++ b/src/Bootstrap.cpp @@ -39,8 +39,8 @@ void Bootstrap::launch() { } bool Bootstrap::launchMinecraftLauncher() { - const HostArchitecture::Value hostArch = systemHelper.getHostArchitecture(); - logger.log(L"Host archiecture: " + HostArchitecture::AsString(hostArch)); + const Architecture::Value hostArch = systemHelper.getHostArchitecture(); + logger.log(L"Host archiecture: " + Architecture::AsString(hostArch)); const auto javaPaths = getMinecraftJavaPaths(hostArch); @@ -140,31 +140,31 @@ bool Bootstrap::attemptLaunch(const std::wstring& path, bool checkExists) { } // Return all of the possible java paths, starting with the newest on the host platform, down to the oldest on the none host platforms. -const std::vector Bootstrap::getMinecraftJavaPaths(const HostArchitecture::Value& hostArch) { +const std::vector Bootstrap::getMinecraftJavaPaths(const Architecture::Value& hostArch) { std::vector paths; - for (const HostArchitecture::Value& arch : HostArchitecture::VALUES) { - if (arch == HostArchitecture::UNKNOWN) { + for (const Architecture::Value& arch : Architecture::VALUES) { + if (arch == Architecture::UNKNOWN) { continue; } if (arch < hostArch) { // Skip arches that the host does not support. // E.g: On x64 there is no need to go looking for arm64 JDKs as its never going to run. - logger.log(L"Arch not supported: " + HostArchitecture::AsString(arch)); + logger.log(L"Arch not supported: " + Architecture::AsString(arch)); continue; } std::wstring javaName; switch (arch) { - case HostArchitecture::X64: + case Architecture::X64: javaName = L"windows-x64"; break; - case HostArchitecture::ARM64: + case Architecture::ARM64: javaName = L"windows-arm64"; break; - case HostArchitecture::X86: + case Architecture::X86: javaName = L"windows-x86"; break; default: diff --git a/src/Bootstrap.h b/src/Bootstrap.h index 676bc72..5939381 100644 --- a/src/Bootstrap.h +++ b/src/Bootstrap.h @@ -20,7 +20,7 @@ class Bootstrap { void showErrorMessage(); - const std::vector getMinecraftJavaPaths(const HostArchitecture::Value& hostArch); + const std::vector getMinecraftJavaPaths(const Architecture::Value& hostArch); private: ISystemHelper& systemHelper; diff --git a/src/ISystemHelper.h b/src/ISystemHelper.h index cb850d2..f0668d4 100644 --- a/src/ISystemHelper.h +++ b/src/ISystemHelper.h @@ -6,44 +6,18 @@ #include #include -namespace HostArchitecture { - // Ordered to ensure that we only check for JVMs that will run on the host arch. - // On an x64 host only check for x64 and x86 as arm will never run. - // On x86 there is no need to try any other arch as it wont run. - enum Value { - UNKNOWN, - ARM64, - X64, - X86, - }; - - constexpr Value VALUES[] = { UNKNOWN, ARM64, X64, X86 }; - - inline std::wstring AsString(const Value& arch) { - switch (arch) { - case X64: - return L"x64"; - case ARM64: - return L"arm64"; - case X86: - return L"x86"; - case UNKNOWN: - default: - return L"unknown"; - } - } -} +#include "Architecture.h" class ISystemHelper { public: - virtual std::optional getRegValue(HKEY hive, const std::wstring& path, const std::wstring& key) = 0; - virtual std::optional getEnvVar(const std::wstring& key) = 0; - virtual void showErrorMessage(const std::wstring& title, const std::wstring& message, const std::wstring& url) = 0; - virtual DWORD createProcess(std::vector args) = 0; - virtual bool fileExists(const std::wstring& path) = 0; - virtual bool dirExists(const std::wstring& path) = 0; - virtual std::wstring getBootstrapFilename() = 0; - virtual std::wstring getTempDir() = 0; - virtual HostArchitecture::Value getHostArchitecture() = 0; - virtual long long getEpochTime() = 0; + virtual std::optional getRegValue(HKEY hive, const std::wstring& path, const std::wstring& key) const = 0; + virtual std::optional getEnvVar(const std::wstring& key) const = 0; + virtual void showErrorMessage(const std::wstring& title, const std::wstring& message, const std::wstring& url) const = 0; + virtual DWORD createProcess(std::vector args) const = 0; + virtual bool fileExists(const std::wstring& path) const = 0; + virtual bool dirExists(const std::wstring& path) const = 0; + virtual std::wstring getBootstrapFilename() const = 0; + virtual std::wstring getTempDir() const = 0; + virtual Architecture::Value getHostArchitecture() const = 0; + virtual int64_t getEpochTime() const = 0; }; \ No newline at end of file diff --git a/src/SystemHelper.cpp b/src/SystemHelper.cpp index 7a4e788..91d177d 100644 --- a/src/SystemHelper.cpp +++ b/src/SystemHelper.cpp @@ -4,7 +4,7 @@ #include #include -std::optional SystemHelper::getRegValue(HKEY hive, const std::wstring& path, const std::wstring& key) { +std::optional SystemHelper::getRegValue(HKEY hive, const std::wstring& path, const std::wstring& key) const { DWORD dataSize{}; LONG retCode = ::RegGetValueW( hive, @@ -43,8 +43,7 @@ std::optional SystemHelper::getRegValue(HKEY hive, const std::wstr return value; } -std::optional SystemHelper::getEnvVar(const std::wstring& key) -{ +std::optional SystemHelper::getEnvVar(const std::wstring& key) const { // Read the size of the env var DWORD size = ::GetEnvironmentVariableW(key.c_str(), nullptr, 0); if (!size || ::GetLastError() == ERROR_ENVVAR_NOT_FOUND) { @@ -62,7 +61,7 @@ std::optional SystemHelper::getEnvVar(const std::wstring& key) return value; } -void SystemHelper::showErrorMessage(const std::wstring& title, const std::wstring& message, const std::wstring& url) { +void SystemHelper::showErrorMessage(const std::wstring& title, const std::wstring& message, const std::wstring& url) const { const int result = ::MessageBoxW( nullptr, message.c_str(), @@ -75,8 +74,7 @@ void SystemHelper::showErrorMessage(const std::wstring& title, const std::wstrin } } -DWORD SystemHelper::createProcess(std::vector args) -{ +DWORD SystemHelper::createProcess(std::vector args) const { STARTUPINFOW info; PROCESS_INFORMATION processInfo; @@ -122,24 +120,23 @@ DWORD SystemHelper::createProcess(std::vector args) } } -bool SystemHelper::fileExists(const std::wstring& path) { +bool SystemHelper::fileExists(const std::wstring& path) const { const auto attributes = ::GetFileAttributesW(path.c_str()); return (attributes != INVALID_FILE_ATTRIBUTES && !(attributes & FILE_ATTRIBUTE_DIRECTORY)); } -bool SystemHelper::dirExists(const std::wstring& path) { +bool SystemHelper::dirExists(const std::wstring& path) const { const auto attributes = ::GetFileAttributesW(path.c_str()); return (attributes != INVALID_FILE_ATTRIBUTES && (attributes & FILE_ATTRIBUTE_DIRECTORY)); } -std::wstring SystemHelper::getBootstrapFilename() { +std::wstring SystemHelper::getBootstrapFilename() const { wchar_t moduleFileName[MAX_PATH] = { 0 }; ::GetModuleFileNameW(nullptr, moduleFileName, MAX_PATH); return moduleFileName; } -std::wstring SystemHelper::getTempDir() -{ +std::wstring SystemHelper::getTempDir() const { std::wstring tempDir; DWORD size = ::GetTempPath(0, nullptr); @@ -147,7 +144,7 @@ std::wstring SystemHelper::getTempDir() throw std::runtime_error("Failed to get temp path"); } - tempDir.resize(size + 1); + tempDir.resize((size_t) size + 1); size = ::GetTempPath(size + 1, tempDir.data()); if (!size || size >= tempDir.size()) { @@ -159,31 +156,31 @@ std::wstring SystemHelper::getTempDir() } // A windows 7 compatible version of getHostArchitecture, must either be x64 or x86 -HostArchitecture::Value getLegacyHostArchitecture() { +Architecture::Value getLegacyHostArchitecture() { #if defined(_M_X64) // x64 bin will only run on x64 Windows 7 & 8 - return HostArchitecture::X64; + return Architecture::X64; #else BOOL isWow64 = FALSE; if (!::IsWow64Process(::GetCurrentProcess(), &isWow64)) { - return HostArchitecture::Value::UNKNOWN; + return Architecture::Value::UNKNOWN; } if (isWow64) { - return HostArchitecture::Value::X64; + return Architecture::Value::X64; } - return HostArchitecture::Value::X86; + return Architecture::Value::X86; # endif } // https://devblogs.microsoft.com/oldnewthing/20220209-00/?p=106239 // Slightly fun as we are almost always ran though emulation -HostArchitecture::Value SystemHelper::getHostArchitecture() { +Architecture::Value SystemHelper::getHostArchitecture() const { #if defined(_M_ARM64) // ARM64 bin will only run on ARM64. - return HostArchitecture::ARM64; + return Architecture::ARM64; #else const auto kernel32Handle = ::GetModuleHandle(TEXT("kernel32.dll")); @@ -206,24 +203,23 @@ HostArchitecture::Value SystemHelper::getHostArchitecture() { if (result == 0) { // Error/Unknown - return HostArchitecture::Value::UNKNOWN; + return Architecture::Value::UNKNOWN; } switch (native_machine) { case IMAGE_FILE_MACHINE_ARM64: - return HostArchitecture::Value::ARM64; + return Architecture::Value::ARM64; case IMAGE_FILE_MACHINE_AMD64: - return HostArchitecture::Value::X64; + return Architecture::Value::X64; case IMAGE_FILE_MACHINE_I386: - return HostArchitecture::Value::X86; + return Architecture::Value::X86; default: - return HostArchitecture::Value::UNKNOWN; + return Architecture::Value::UNKNOWN; } # endif } -long long SystemHelper::getEpochTime() -{ +int64_t SystemHelper::getEpochTime() const { const auto now = std::chrono::system_clock::now(); return std::chrono::duration_cast(now.time_since_epoch()).count(); } diff --git a/src/SystemHelper.h b/src/SystemHelper.h index 3c5b8a9..1b67147 100644 --- a/src/SystemHelper.h +++ b/src/SystemHelper.h @@ -4,14 +4,14 @@ class SystemHelper : public ISystemHelper { public: - std::optional getRegValue(HKEY hive, const std::wstring& path, const std::wstring& key) override; - std::optional getEnvVar(const std::wstring& key) override; - void showErrorMessage(const std::wstring& title, const std::wstring& message, const std::wstring& url) override; - DWORD createProcess(std::vector args) override; - bool fileExists(const std::wstring& path) override; - bool dirExists(const std::wstring& path) override; - std::wstring getBootstrapFilename() override; - std::wstring getTempDir() override; - HostArchitecture::Value getHostArchitecture() override; - long long getEpochTime() override; + std::optional getRegValue(HKEY hive, const std::wstring& path, const std::wstring& key) const override; + std::optional getEnvVar(const std::wstring& key) const override; + void showErrorMessage(const std::wstring& title, const std::wstring& message, const std::wstring& url) const override; + DWORD createProcess(std::vector args) const override; + bool fileExists(const std::wstring& path) const override; + bool dirExists(const std::wstring& path) const override; + std::wstring getBootstrapFilename() const override; + std::wstring getTempDir() const override; + Architecture::Value getHostArchitecture() const override; + int64_t getEpochTime() const override; }; \ No newline at end of file