From b718fdf8ef034ac2defd37f0a2a12f9f12857450 Mon Sep 17 00:00:00 2001 From: Ben Hoffman Date: Sat, 25 May 2024 21:07:32 -0400 Subject: [PATCH 1/4] Initial reworking of cmd line args --- FlingEngine/Core/inc/Misc/CommandLine.h | 43 +++++++++++++++-------- FlingEngine/Core/src/Engine.cpp | 7 ++-- FlingEngine/Core/src/Misc/CommandLine.cpp | 30 +++++++++------- FlingTests/src/CommandLineTests.cpp | 32 +++++++++++++---- 4 files changed, 76 insertions(+), 36 deletions(-) diff --git a/FlingEngine/Core/inc/Misc/CommandLine.h b/FlingEngine/Core/inc/Misc/CommandLine.h index 25114502..925939c1 100644 --- a/FlingEngine/Core/inc/Misc/CommandLine.h +++ b/FlingEngine/Core/inc/Misc/CommandLine.h @@ -1,8 +1,14 @@ #pragma once #include // string, stoi, to_string +#include // std::string_view #include "FlingTypes.h" +// TODO: I think that we may gain a lot if we just use +// Boost. That certainly will have a better implementation then +// I can whip up, and I think has config file options as well. +// https://www.boost.org/doc/libs/1_85_0/doc/html/program_options.html + namespace Fling { /** @@ -12,28 +18,37 @@ namespace Fling class CommandLine { public: - - /** Sets the static command line */ - static void Set(const std::string& CmdLine); + /** + * @return Instance of the current command line that the application was started with. + */ + static CommandLine& Get(); /** - * Builds a string with a space in between each argument passed in via the command - * except for the first argument (the application name) + * Initalize the command line instance with the given application args. + * This will initalize the command line's internal data structure for keepting + * track of the data passed into the command line + * + * @paran ArgC The number of command line arguements provided + * @param ArgV The char values of those command line arguments + * @return True if successfully initalized */ - static std::string BuildFromArgs(int32 Argc, const char* ArgV[]); + bool Init(const int32 Argc, const char* ArgV[]); - static bool Parse(const std::string& InKey); - - static const std::string& Get() { return CurrentCommandLine; } + /** + * Returns true if the given param had a value passed in via command line + * @param Param + * @return + */ + [[nodiscard]] bool HasParam(const std::string_view Param) const; - /** Returns true if the given flag is set on the command line */ - static bool HasFlag(const std::string& Flag); - - static bool HasParam(const std::string& Param); + std::string_view GetCommandLineData() const; private: - static std::string CurrentCommandLine; + std::string CurrentCommandLineData; + + // TODO: A TMap of string_view's to some generic data container type + // which we can use for quick checking of flags. }; } // namespace Fling \ No newline at end of file diff --git a/FlingEngine/Core/src/Engine.cpp b/FlingEngine/Core/src/Engine.cpp index 24a2d335..08bc5ce0 100644 --- a/FlingEngine/Core/src/Engine.cpp +++ b/FlingEngine/Core/src/Engine.cpp @@ -12,9 +12,10 @@ namespace Fling { Random::Init(); Logger::Get().Init(); - - CommandLine::Set(CommandLine::BuildFromArgs(argc, argv)); - F_LOG_TRACE("Command line args: {}\t", CommandLine::Get()); + + // Initalize the command line + const bool bSuccessfulCommandLineInit = CommandLine::Get().Init(argc, argv); + F_LOG_TRACE("Command line args: {}\t", CommandLine::Get().GetCommandLineData()); ResourceManager::Get().Init(); Timing::Get().Init(); diff --git a/FlingEngine/Core/src/Misc/CommandLine.cpp b/FlingEngine/Core/src/Misc/CommandLine.cpp index abdef8c7..a5817377 100644 --- a/FlingEngine/Core/src/Misc/CommandLine.cpp +++ b/FlingEngine/Core/src/Misc/CommandLine.cpp @@ -3,14 +3,16 @@ namespace Fling { - std::string CommandLine::CurrentCommandLine; - - void CommandLine::Set(const std::string& CmdLine) + CommandLine& CommandLine::Get() { - CurrentCommandLine = CmdLine; + // the command line is a singleton... you can only pass + // in one command line instance for the application's lifetime + static Fling::CommandLine Instance = {}; + + return Instance; } - std::string CommandLine::BuildFromArgs(int32 argc, const char* argv[]) + bool CommandLine::Init(const int32 argc, const char* argv[]) { std::stringstream CmdStream; @@ -25,20 +27,22 @@ namespace Fling CmdStream << " "; } } - return CmdStream.str(); - } + CurrentCommandLineData = CmdStream.str(); - bool CommandLine::Parse(const std::string& InKey) - { - - return false; + // As long as our command line is not empty, we should be fine... + // TODO: make sure our command line map is the same size as argc + return !CurrentCommandLineData.empty(); } - bool CommandLine::HasParam(const std::string& Param) + bool CommandLine::HasParam(const std::string_view Param) const { - std::size_t found = CurrentCommandLine.find(Param); + std::size_t found = CurrentCommandLineData.find(Param); return found != std::string::npos; } + std::string_view CommandLine::GetCommandLineData() const + { + return CurrentCommandLineData; + } } // namespace Fling diff --git a/FlingTests/src/CommandLineTests.cpp b/FlingTests/src/CommandLineTests.cpp index a1e031b9..5c7be7d6 100644 --- a/FlingTests/src/CommandLineTests.cpp +++ b/FlingTests/src/CommandLineTests.cpp @@ -3,6 +3,7 @@ #include #include "Misc/CommandLine.h" +#include TEST_CASE("Command Line", "[Command Line]") { @@ -15,13 +16,14 @@ TEST_CASE("Command Line", "[Command Line]") "FlingEngine.exe", }; - const int32 ArgCount = sizeof(Args) / sizeof(char*); - CommandLine::Set(CommandLine::BuildFromArgs(ArgCount, Args)); + constexpr int32 ArgCount = sizeof(Args) / sizeof(char*); + const bool bWasInitalized = CommandLine::Get().Init(ArgCount, Args); + REQUIRE(bWasInitalized); // The command line should always ignore the first argument, which is // the application name - const std::string& CurCmdLine = CommandLine::Get(); - REQUIRE(CurCmdLine.length() == 0); + const std::string_view CurCmdLine = CommandLine::Get().GetCommandLineData(); + REQUIRE(CurCmdLine.empty()); } SECTION("Bool Flag") @@ -31,10 +33,28 @@ TEST_CASE("Command Line", "[Command Line]") "FlingEngine.exe", "-test=918" }; + constexpr int32 ArgCount = sizeof(Args) / sizeof(char*); + const bool bWasInitalized = CommandLine::Get().Init(ArgCount, Args); + REQUIRE(bWasInitalized); + + const bool bHasFlag = CommandLine::Get().HasParam("test"); + REQUIRE(bHasFlag); + } + + SECTION("Integer Flag") + { + const char* Args[] = + { + "FlingEngine.exe", + "-numFlag=776", + }; + + // Set the command line int32 ArgCount = sizeof(Args) / sizeof(char*); - CommandLine::Set(CommandLine::BuildFromArgs(ArgCount, Args)); + const bool bWasInitalized = CommandLine::Get().Init(ArgCount, Args); + REQUIRE(bWasInitalized); - const bool bHasFlag = CommandLine::HasParam("test"); + const bool bHasFlag = CommandLine::Get().HasParam("numFlag"); REQUIRE(bHasFlag); } } \ No newline at end of file From 3f4b4825baffd5d64598d3a0491ef5d073b8a27c Mon Sep 17 00:00:00 2001 From: Ben Hoffman Date: Sun, 16 Mar 2025 08:51:07 -0400 Subject: [PATCH 2/4] Some comments and whatnot, going to switch branches --- FlingEngine/Core/inc/Misc/CommandLine.h | 32 +++++++++++++++++++++++ FlingEngine/Core/src/Engine.cpp | 1 + FlingEngine/Core/src/Misc/CommandLine.cpp | 25 ++++++++++++++++++ FlingTests/src/CommandLineTests.cpp | 4 +-- 4 files changed, 60 insertions(+), 2 deletions(-) diff --git a/FlingEngine/Core/inc/Misc/CommandLine.h b/FlingEngine/Core/inc/Misc/CommandLine.h index 925939c1..991553f1 100644 --- a/FlingEngine/Core/inc/Misc/CommandLine.h +++ b/FlingEngine/Core/inc/Misc/CommandLine.h @@ -14,6 +14,13 @@ namespace Fling /** * Holds onto the command line arguments passed to this application * Can be used to parse arguments into different types + * + * The syntax for specifying a command line argument is a key-value pair with a "-". + * For example, to specify the "foo" option with a value of 7, you could use: + * + * -foo=7 + * + * on the command line. */ class CommandLine { @@ -41,6 +48,18 @@ namespace Fling */ [[nodiscard]] bool HasParam(const std::string_view Param) const; + /** + * Gets the value of the given param as the given type. + * + * If the value as not specified on the command line, + * then the given "Default" value will be returned. + */ + template + T& GetValueAs(const std::string_view Param, const T& Default) const; + + // TODO: make this a std::string_view + const char* GetValueAsString(const std::string_view Param) const; + std::string_view GetCommandLineData() const; private: @@ -51,4 +70,17 @@ namespace Fling // which we can use for quick checking of flags. }; + + template + T& CommandLine::GetValueAs(const std::string_view Param, const T& Default) const + { + const bool bWasOverriden = false; + if (bWasOverriden) + { + // return the value that you have overriden on command line + } + + // Otherwise, nothing was set on command line, so use the default value + return Default; + } } // namespace Fling \ No newline at end of file diff --git a/FlingEngine/Core/src/Engine.cpp b/FlingEngine/Core/src/Engine.cpp index 08bc5ce0..8bd72d06 100644 --- a/FlingEngine/Core/src/Engine.cpp +++ b/FlingEngine/Core/src/Engine.cpp @@ -15,6 +15,7 @@ namespace Fling // Initalize the command line const bool bSuccessfulCommandLineInit = CommandLine::Get().Init(argc, argv); + F_LOG_TRACE("Command line initaliziation: {}\t", bSuccessfulCommandLineInit ? "successful" : "failed"); F_LOG_TRACE("Command line args: {}\t", CommandLine::Get().GetCommandLineData()); ResourceManager::Get().Init(); diff --git a/FlingEngine/Core/src/Misc/CommandLine.cpp b/FlingEngine/Core/src/Misc/CommandLine.cpp index a5817377..8b4c0f4d 100644 --- a/FlingEngine/Core/src/Misc/CommandLine.cpp +++ b/FlingEngine/Core/src/Misc/CommandLine.cpp @@ -7,6 +7,10 @@ namespace Fling { // the command line is a singleton... you can only pass // in one command line instance for the application's lifetime + + // TODO: Maybe make this a singleton on the FlingEngine type, not + // on it's own, so that you can technically run multiple instances + // of the engine like if you have multi-window game previews? static Fling::CommandLine Instance = {}; return Instance; @@ -16,6 +20,9 @@ namespace Fling { std::stringstream CmdStream; + // TODO: For each argument, parse it into a map or something for quick lookup later + // where the key is the string after the "-" and the value is the string before the next " -" + // Start at 1 to exclude the first argument(the executable name) for(int32 i = 1; i < argc; ++i) { @@ -41,6 +48,24 @@ namespace Fling return found != std::string::npos; } + const char* CommandLine::GetValueAsString(const std::string_view Param) const + { + // TODO: Regex match for this param and get it's value as a string + // Some examples of allowed sytanx + + // -myFlag=false + // -myFlag=1 + // -stringFlag="this is a string flag" + // -numericValue=-1123.56 + + // So when you call this function, the "Param" is the key part of the command line argument. + // Such as "myFlag" or "stringValue" in the examples above. We are searching the command + // line that we have been given for this value. + + + return nullptr; + } + std::string_view CommandLine::GetCommandLineData() const { return CurrentCommandLineData; diff --git a/FlingTests/src/CommandLineTests.cpp b/FlingTests/src/CommandLineTests.cpp index 5c7be7d6..e09e5efc 100644 --- a/FlingTests/src/CommandLineTests.cpp +++ b/FlingTests/src/CommandLineTests.cpp @@ -26,12 +26,12 @@ TEST_CASE("Command Line", "[Command Line]") REQUIRE(CurCmdLine.empty()); } - SECTION("Bool Flag") + SECTION("Bool Flag - true") { const char* Args[] = { "FlingEngine.exe", - "-test=918" + "-test=true" }; constexpr int32 ArgCount = sizeof(Args) / sizeof(char*); const bool bWasInitalized = CommandLine::Get().Init(ArgCount, Args); From d9300201da32d72318dd08676053c8bb668628a3 Mon Sep 17 00:00:00 2001 From: Ben Hoffman Date: Fri, 7 Aug 2026 22:37:21 -0400 Subject: [PATCH 3/4] Implement command line argument parser with ini ConsoleVariables fallback Adds -key=value / --key=value / bare -flag parsing (issue #154), a shared StringUtils::ParseAs for bool/int/float/double/string conversion, and an ini "[ConsoleVariables]" section fallback so config files can supply defaults that command line args still override. Wires CommandLine::Get().LoadConfigFile into Engine::Startup alongside FlingConfig, and removes FlingConfig's old unused LoadCommandLineOpts stub now that CommandLine owns this responsibility. Co-Authored-By: Claude Sonnet 5 --- FlingEngine/Core/inc/Misc/CommandLine.h | 61 +++-- FlingEngine/Core/inc/Misc/StringUtils.h | 81 +++++++ FlingEngine/Core/src/Engine.cpp | 13 +- FlingEngine/Core/src/Misc/CommandLine.cpp | 187 ++++++++++++++-- FlingEngine/Resources/inc/FlingConfig.h | 13 +- FlingEngine/Resources/src/FlingConfig.cpp | 21 +- FlingTests/src/CommandLineTests.cpp | 260 ++++++++++++++++++++++ FlingTests/src/StringUtilsTests.cpp | 66 ++++++ Sandbox/Gameplay/src/SandboxGame.cpp | 15 ++ 9 files changed, 649 insertions(+), 68 deletions(-) create mode 100644 FlingEngine/Core/inc/Misc/StringUtils.h create mode 100644 FlingTests/src/StringUtilsTests.cpp diff --git a/FlingEngine/Core/inc/Misc/CommandLine.h b/FlingEngine/Core/inc/Misc/CommandLine.h index 991553f1..c6df0b00 100644 --- a/FlingEngine/Core/inc/Misc/CommandLine.h +++ b/FlingEngine/Core/inc/Misc/CommandLine.h @@ -2,7 +2,9 @@ #include // string, stoi, to_string #include // std::string_view +#include #include "FlingTypes.h" +#include "Misc/StringUtils.h" // TODO: I think that we may gain a lot if we just use // Boost. That certainly will have a better implementation then @@ -25,12 +27,15 @@ namespace Fling class CommandLine { public: + /** Maximum length, in characters, that a single command line argument may be. Any argument longer than this is ignored. */ + static constexpr std::size_t MaxArgLength = 256; + /** * @return Instance of the current command line that the application was started with. */ static CommandLine& Get(); - /** + /** * Initalize the command line instance with the given application args. * This will initalize the command line's internal data structure for keepting * track of the data passed into the command line @@ -50,37 +55,65 @@ namespace Fling /** * Gets the value of the given param as the given type. - * - * If the value as not specified on the command line, + * + * If the value as not specified on the command line, * then the given "Default" value will be returned. */ template - T& GetValueAs(const std::string_view Param, const T& Default) const; + T GetValueAs(const std::string_view Param, const T& Default) const; // TODO: make this a std::string_view const char* GetValueAsString(const std::string_view Param) const; - std::string_view GetCommandLineData() const; + [[nodiscard]] std::string_view GetCommandLineData() const; + + /** + * Loads console variables from the "[ConsoleVariables]" section of an ini-style + * config file, following the same "Key=Value" syntax used on the command line + * (e.g. UE's ini config variables). Values loaded this way act as a fallback: + * if the same key was also passed directly on the command line, the command + * line value always takes precedence. + * + * @param FilePath Path to the ini file to load + * @return True if the file was opened and parsed successfully + */ + bool LoadConfigFile(const std::string& FilePath); + + /** + * Same as LoadConfigFile, but parses the ini data directly out of the given + * string rather than from a file on disk. Exposed publicly so that this parsing + * logic can be unit tested without touching the file system. + * + * @param IniContent Contents of an ini file to parse + * @return True if the content was parsed successfully + */ + bool LoadConfigVarsFromString(const std::string_view IniContent); private: + /** Looks up a param, checking command line values before config file (ConsoleVariables) values. */ + const std::string* FindValue(const std::string_view Param) const; + std::string CurrentCommandLineData; - // TODO: A TMap of string_view's to some generic data container type - // which we can use for quick checking of flags. - + /** Key/value pairs parsed out of the command line, e.g. "-foo=7" becomes ParsedArgs["foo"] = "7" */ + std::unordered_map ParsedArgs; + + /** Key/value pairs parsed out of a config file's "[ConsoleVariables]" section. Lower priority than ParsedArgs. */ + std::unordered_map ConfigArgs; }; template - T& CommandLine::GetValueAs(const std::string_view Param, const T& Default) const + T CommandLine::GetValueAs(const std::string_view Param, const T& Default) const { - const bool bWasOverriden = false; - if (bWasOverriden) + const std::string* Value = FindValue(Param); + if (Value == nullptr) { - // return the value that you have overriden on command line + // Nothing was set on the command line or in a config file for this param, + // so use the default value + return Default; } - // Otherwise, nothing was set on command line, so use the default value - return Default; + return StringUtils::ParseAs(*Value, Default); } } // namespace Fling \ No newline at end of file diff --git a/FlingEngine/Core/inc/Misc/StringUtils.h b/FlingEngine/Core/inc/Misc/StringUtils.h new file mode 100644 index 00000000..733eb3de --- /dev/null +++ b/FlingEngine/Core/inc/Misc/StringUtils.h @@ -0,0 +1,81 @@ +#pragma once + +#include +#include +#include +#include +#include +#include "FlingTypes.h" + +namespace Fling +{ + namespace StringUtils + { + /** + * Attempts to parse the given string_view as an instance of T. + * + * This is the single place that knows how to turn a raw string into an engine + * type (bool, int, float, ...), so that command line args, ini config values, + * and any other string-serialized data all agree on what "true", "7", or + * "3.14" mean. + * + * @param Str The string to parse + * @param Default Value returned if Str is empty or cannot be converted to T + * @return The parsed value, or Default if Str could not be parsed as a T + */ + template + T ParseAs(const std::string_view Str, const T& Default = T{}) + { + if (Str.empty()) + { + return Default; + } + + if constexpr (std::is_same_v) + { + return std::string(Str); + } + else if constexpr (std::is_same_v) + { + if (Str == "true" || Str == "True" || Str == "TRUE" || Str == "1") + { + return true; + } + if (Str == "false" || Str == "False" || Str == "FALSE" || Str == "0") + { + return false; + } + return Default; + } + else if constexpr (std::is_integral_v) + { + const std::string Temp(Str); + char* End = nullptr; + const long long Result = std::strtoll(Temp.c_str(), &End, 10); + return (End != Temp.c_str()) ? static_cast(Result) : Default; + } + else if constexpr (std::is_same_v) + { + const std::string Temp(Str); + char* End = nullptr; + const float Result = std::strtof(Temp.c_str(), &End); + return (End != Temp.c_str()) ? Result : Default; + } + else if constexpr (std::is_same_v) + { + const std::string Temp(Str); + char* End = nullptr; + const double Result = std::strtod(Temp.c_str(), &End); + return (End != Temp.c_str()) ? Result : Default; + } + else + { + // Fallback for any other stream-extractable type + std::istringstream ValueStream{ std::string(Str) }; + T Value{}; + ValueStream >> Value; + return ValueStream.fail() ? Default : Value; + } + } + } // namespace StringUtils +} // namespace Fling diff --git a/FlingEngine/Core/src/Engine.cpp b/FlingEngine/Core/src/Engine.cpp index 8bd72d06..9e30dab5 100644 --- a/FlingEngine/Core/src/Engine.cpp +++ b/FlingEngine/Core/src/Engine.cpp @@ -18,9 +18,10 @@ namespace Fling F_LOG_TRACE("Command line initaliziation: {}\t", bSuccessfulCommandLineInit ? "successful" : "failed"); F_LOG_TRACE("Command line args: {}\t", CommandLine::Get().GetCommandLineData()); + FlingConfig::Get().Init(); + ResourceManager::Get().Init(); - Timing::Get().Init(); - FlingConfig::Get().Init(); + Timing::Get().Init(); Input::Init(); F_LOG_TRACE("Fling Engine Sourcedir: \t{}", Fling::FlingPaths::EngineSourceDir()); @@ -33,13 +34,19 @@ namespace Fling #endif // Load command line args and any ini files - bool ConfigLoaded = FlingConfig::Get().LoadConfigFile(FlingPaths::EngineConfigDir() + "/EngineConf.ini"); + const std::string EngineConfigPath = FlingPaths::EngineConfigDir() + "/EngineConf.ini"; + bool ConfigLoaded = FlingConfig::Get().LoadConfigFile(EngineConfigPath); if (!ConfigLoaded) { F_LOG_WARN("NO EngineConf.ini has been provided! This may result in unexpected behavior from Fling!"); } + // Let the command line consider the same ini's [ConsoleVariables] section as a + // fallback, so systems can query CommandLine::GetValueAs for values that were + // only set in the config file. Command line args passed directly still win. + CommandLine::Get().LoadConfigFile(EngineConfigPath); + VulkanApp::Get().Init( static_cast(PipelineFlags::DEFERRED | PipelineFlags::IMGUI), g_Registry, diff --git a/FlingEngine/Core/src/Misc/CommandLine.cpp b/FlingEngine/Core/src/Misc/CommandLine.cpp index 8b4c0f4d..aa12df85 100644 --- a/FlingEngine/Core/src/Misc/CommandLine.cpp +++ b/FlingEngine/Core/src/Misc/CommandLine.cpp @@ -1,8 +1,39 @@ #include "Misc/CommandLine.h" #include +#include namespace Fling { + namespace + { + /** The ini section that command line style console variables are read from, UE-style. */ + const std::string ConsoleVariablesSectionName = "ConsoleVariables"; + + /** Strips a single pair of surrounding double quotes from a value, e.g. "\"foo bar\"" -> "foo bar" */ + std::string StripSurroundingQuotes(const std::string& Value) + { + if (Value.size() >= 2 && Value.front() == '"' && Value.back() == '"') + { + return Value.substr(1, Value.size() - 2); + } + + return Value; + } + + /** Strips leading/trailing whitespace (spaces and tabs) from a string */ + std::string Trim(const std::string& Value) + { + const std::size_t First = Value.find_first_not_of(" \t"); + if (First == std::string::npos) + { + return ""; + } + + const std::size_t Last = Value.find_last_not_of(" \t"); + return Value.substr(First, Last - First + 1); + } + } + CommandLine& CommandLine::Get() { // the command line is a singleton... you can only pass @@ -18,56 +49,174 @@ namespace Fling bool CommandLine::Init(const int32 argc, const char* argv[]) { - std::stringstream CmdStream; + if (argc <= 0 || argv == nullptr) + { + return false; + } - // TODO: For each argument, parse it into a map or something for quick lookup later - // where the key is the string after the "-" and the value is the string before the next " -" + ParsedArgs.clear(); + ConfigArgs.clear(); + + std::stringstream CmdStream; // Start at 1 to exclude the first argument(the executable name) for(int32 i = 1; i < argc; ++i) { - CmdStream << argv[i]; + std::string Arg = argv[i]; + + CmdStream << Arg; // Add a space between each arg except for the last one if(i + 1 < argc) { CmdStream << " "; } + + // CommandLine::Init runs very early in Engine startup, before logging is + // guaranteed to be available, so oversized arguments are ignored silently + // rather than logged. + if (Arg.size() > MaxArgLength) + { + continue; + } + + // Args are expected to be prefixed with one or more "-", e.g. "-foo=7" or "--foo=7" + const std::size_t FirstNonDash = Arg.find_first_not_of('-'); + if (FirstNonDash != 0) + { + Arg.erase(0, FirstNonDash == std::string::npos ? Arg.size() : FirstNonDash); + } + + if (Arg.empty()) + { + continue; + } + + const std::size_t EqualsPos = Arg.find('='); + if (EqualsPos != std::string::npos) + { + const std::string Key = Arg.substr(0, EqualsPos); + const std::string Value = StripSurroundingQuotes(Arg.substr(EqualsPos + 1)); + ParsedArgs[Key] = Value; + } + else + { + // A flag with no explicit value, e.g. "-BoolFlag", is treated as true + ParsedArgs[Arg] = "true"; + } } CurrentCommandLineData = CmdStream.str(); - // As long as our command line is not empty, we should be fine... - // TODO: make sure our command line map is the same size as argc - return !CurrentCommandLineData.empty(); + return true; } bool CommandLine::HasParam(const std::string_view Param) const { - std::size_t found = CurrentCommandLineData.find(Param); - - return found != std::string::npos; + return FindValue(Param) != nullptr; } const char* CommandLine::GetValueAsString(const std::string_view Param) const { - // TODO: Regex match for this param and get it's value as a string - // Some examples of allowed sytanx - + // Some examples of allowed syntax: // -myFlag=false // -myFlag=1 // -stringFlag="this is a string flag" // -numericValue=-1123.56 - - // So when you call this function, the "Param" is the key part of the command line argument. - // Such as "myFlag" or "stringValue" in the examples above. We are searching the command - // line that we have been given for this value. - + // -BoolFlag (implicitly "true") - return nullptr; + const std::string* Value = FindValue(Param); + return Value != nullptr ? Value->c_str() : nullptr; } std::string_view CommandLine::GetCommandLineData() const { return CurrentCommandLineData; } + + const std::string* CommandLine::FindValue(const std::string_view Param) const + { + const std::string Key(Param); + + // Values passed directly on the command line always win over ones loaded from a config file + auto Iter = ParsedArgs.find(Key); + if (Iter != ParsedArgs.end()) + { + return &Iter->second; + } + + Iter = ConfigArgs.find(Key); + if (Iter != ConfigArgs.end()) + { + return &Iter->second; + } + + return nullptr; + } + + bool CommandLine::LoadConfigFile(const std::string& FilePath) + { + std::ifstream File(FilePath); + if (!File.is_open()) + { + return false; + } + + std::stringstream Buffer; + Buffer << File.rdbuf(); + + return LoadConfigVarsFromString(Buffer.str()); + } + + bool CommandLine::LoadConfigVarsFromString(const std::string_view IniContent) + { + ConfigArgs.clear(); + + std::string CurrentSection; + std::istringstream Stream{ std::string(IniContent) }; + std::string Line; + + while (std::getline(Stream, Line)) + { + // Trim a trailing carriage return in case this ini uses CRLF line endings + if (!Line.empty() && Line.back() == '\r') + { + Line.pop_back(); + } + + const std::string Trimmed = Trim(Line); + if (Trimmed.empty() || Trimmed.front() == ';' || Trimmed.front() == '#') + { + // Blank line or comment + continue; + } + + if (Trimmed.front() == '[' && Trimmed.back() == ']') + { + CurrentSection = Trimmed.substr(1, Trimmed.size() - 2); + continue; + } + + if (CurrentSection != ConsoleVariablesSectionName) + { + // Only console variables are pulled into the command line lookup + continue; + } + + const std::size_t EqualsPos = Trimmed.find('='); + if (EqualsPos == std::string::npos) + { + continue; + } + + const std::string Key = Trim(Trimmed.substr(0, EqualsPos)); + const std::string Value = StripSurroundingQuotes(Trim(Trimmed.substr(EqualsPos + 1))); + + if (!Key.empty()) + { + ConfigArgs[Key] = Value; + } + } + + return true; + } } // namespace Fling diff --git a/FlingEngine/Resources/inc/FlingConfig.h b/FlingEngine/Resources/inc/FlingConfig.h index 5659db6a..ada1409a 100644 --- a/FlingEngine/Resources/inc/FlingConfig.h +++ b/FlingEngine/Resources/inc/FlingConfig.h @@ -6,8 +6,7 @@ namespace Fling { /** - * Provide simple access to engine configuration options from an INI file - * #TODO Parse command line options as well + * Provide simple access to engine configuration options from an INI file. */ class FlingConfig : public Singleton { @@ -36,16 +35,6 @@ namespace Fling static float GetFloat(const std::string& t_Section, const std::string& t_Key, const float t_DefaultVal = 0.0f) { return FlingConfig::Get().GetFloatImpl(t_Section, t_Key); } static double GetDouble(const std::string& t_Section, const std::string& t_Key, const double t_DefaultVal = 0.0) { return FlingConfig::Get().GetDoubleImpl(t_Section, t_Key); } - - /** - * Load in the command line options and store them somewhere that is - * globally accessible - * - * @param argc Argument count - * @param argv Command line args - * @return Number of options loaded - */ - uint32 LoadCommandLineOpts( int argc, char* argv[] ); private: diff --git a/FlingEngine/Resources/src/FlingConfig.cpp b/FlingEngine/Resources/src/FlingConfig.cpp index 3a241b03..905ab184 100644 --- a/FlingEngine/Resources/src/FlingConfig.cpp +++ b/FlingEngine/Resources/src/FlingConfig.cpp @@ -57,25 +57,6 @@ namespace Fling double FlingConfig::GetDoubleImpl(const std::string& t_Section, const std::string& t_Key, const double t_DefaultVal /*= 0.0*/) const { return m_IniReader.GetReal(t_Section, t_Key, t_DefaultVal); - } - - ////////////////////////////////////////////////////////////////////////// - // Command line parsing - - uint32 FlingConfig::LoadCommandLineOpts(int argc, char* argv[]) - { - uint32 ArgsLoaded = 0; - - // TODO: Use regex to try and parse out if things are a key/val etc - - for (int i = 0; i < argc; ++i) - { - // Parse out if this is a key or not - std::string value = argv[i]; - (void)(value); - } - - return ArgsLoaded; - } + } } // namespace Fling \ No newline at end of file diff --git a/FlingTests/src/CommandLineTests.cpp b/FlingTests/src/CommandLineTests.cpp index e09e5efc..21f4f332 100644 --- a/FlingTests/src/CommandLineTests.cpp +++ b/FlingTests/src/CommandLineTests.cpp @@ -4,6 +4,8 @@ #include "Misc/CommandLine.h" #include +#include +#include TEST_CASE("Command Line", "[Command Line]") { @@ -56,5 +58,263 @@ TEST_CASE("Command Line", "[Command Line]") const bool bHasFlag = CommandLine::Get().HasParam("numFlag"); REQUIRE(bHasFlag); + + const int32 Value = CommandLine::Get().GetValueAs("numFlag", -1); + REQUIRE(Value == 776); + + // Should also be readable as a plain int + const int PlainIntValue = CommandLine::Get().GetValueAs("numFlag", -1); + REQUIRE(PlainIntValue == 776); + } + + SECTION("Double Flag") + { + const char* Args[] = + { + "FlingEngine.exe", + "-numericValue=-1123.56", + }; + + int32 ArgCount = sizeof(Args) / sizeof(char*); + const bool bWasInitalized = CommandLine::Get().Init(ArgCount, Args); + REQUIRE(bWasInitalized); + + const double Value = CommandLine::Get().GetValueAs("numericValue", 0.0); + REQUIRE(Value == Catch::Approx(-1123.56)); + } + + SECTION("Multiple flags must be separate argv entries") + { + // A real shell splits "-foo=1 -bar=2" into two argv entries before the program + // ever sees them. A launcher that instead hands both flags over as a single + // argv string (e.g. a misconfigured launch.json "args" entry) will not get + // split back apart here - the second flag's key/value just becomes part of + // the first flag's value. This test documents that gotcha. + const char* Args[] = + { + "FlingEngine.exe", + "-BenTest=false -TestDouble=420.69" + }; + constexpr int32 ArgCount = sizeof(Args) / sizeof(char*); + REQUIRE(CommandLine::Get().Init(ArgCount, Args)); + + REQUIRE(CommandLine::Get().HasParam("BenTest")); + REQUIRE_FALSE(CommandLine::Get().HasParam("TestDouble")); + + // Passed as two separate argv entries, both are parsed correctly + const char* CorrectArgs[] = + { + "FlingEngine.exe", + "-BenTest=false", + "-TestDouble=420.69" + }; + constexpr int32 CorrectArgCount = sizeof(CorrectArgs) / sizeof(char*); + REQUIRE(CommandLine::Get().Init(CorrectArgCount, CorrectArgs)); + + REQUIRE(CommandLine::Get().GetValueAs("BenTest", true) == false); + REQUIRE(CommandLine::Get().GetValueAs("TestDouble", -1.0) == Catch::Approx(420.69)); + } + + SECTION("Bool Flag - implicit true") + { + const char* Args[] = + { + "FlingEngine.exe", + "-BoolFlag" + }; + constexpr int32 ArgCount = sizeof(Args) / sizeof(char*); + const bool bWasInitalized = CommandLine::Get().Init(ArgCount, Args); + REQUIRE(bWasInitalized); + + REQUIRE(CommandLine::Get().HasParam("BoolFlag")); + REQUIRE(CommandLine::Get().GetValueAs("BoolFlag", false) == true); + } + + SECTION("Bool Flag - double dash implicit true") + { + const char* Args[] = + { + "FlingEngine.exe", + "--BoolFlag" + }; + constexpr int32 ArgCount = sizeof(Args) / sizeof(char*); + REQUIRE(CommandLine::Get().Init(ArgCount, Args)); + + REQUIRE(CommandLine::Get().HasParam("BoolFlag")); + REQUIRE(CommandLine::Get().GetValueAs("BoolFlag", false) == true); + } + + SECTION("Double dash key-value flag") + { + const char* Args[] = + { + "FlingEngine.exe", + "--resolutionWidth=1920" + }; + constexpr int32 ArgCount = sizeof(Args) / sizeof(char*); + REQUIRE(CommandLine::Get().Init(ArgCount, Args)); + + REQUIRE(CommandLine::Get().HasParam("resolutionWidth")); + REQUIRE(CommandLine::Get().GetValueAs("resolutionWidth", -1) == 1920); + } + + SECTION("Argument exactly at the max length is parsed") + { + const std::string Prefix = "-longFlag="; + const std::string ArgAtLimit = Prefix + std::string(CommandLine::MaxArgLength - Prefix.size(), 'a'); + REQUIRE(ArgAtLimit.size() == CommandLine::MaxArgLength); + + const char* Args[] = + { + "FlingEngine.exe", + ArgAtLimit.c_str() + }; + constexpr int32 ArgCount = sizeof(Args) / sizeof(char*); + REQUIRE(CommandLine::Get().Init(ArgCount, Args)); + + REQUIRE(CommandLine::Get().HasParam("longFlag")); + } + + SECTION("Argument over the max length is ignored") + { + const std::string Prefix = "-longFlag="; + const std::string ArgOverLimit = Prefix + std::string(CommandLine::MaxArgLength - Prefix.size() + 1, 'a'); + REQUIRE(ArgOverLimit.size() == CommandLine::MaxArgLength + 1); + + const char* Args[] = + { + "FlingEngine.exe", + ArgOverLimit.c_str() + }; + constexpr int32 ArgCount = sizeof(Args) / sizeof(char*); + REQUIRE(CommandLine::Get().Init(ArgCount, Args)); + + REQUIRE_FALSE(CommandLine::Get().HasParam("longFlag")); + } + + SECTION("String Flag - quoted value") + { + const char* Args[] = + { + "FlingEngine.exe", + "-stringFlag=\"I like this format\"" + }; + constexpr int32 ArgCount = sizeof(Args) / sizeof(char*); + const bool bWasInitalized = CommandLine::Get().Init(ArgCount, Args); + REQUIRE(bWasInitalized); + + REQUIRE(CommandLine::Get().HasParam("stringFlag")); + + const std::string_view Value = CommandLine::Get().GetValueAsString("stringFlag"); + REQUIRE(Value == "I like this format"); + } + + SECTION("Missing param falls back to default") + { + const char* Args[] = + { + "FlingEngine.exe", + "-numFlag=776" + }; + constexpr int32 ArgCount = sizeof(Args) / sizeof(char*); + const bool bWasInitalized = CommandLine::Get().Init(ArgCount, Args); + REQUIRE(bWasInitalized); + + REQUIRE_FALSE(CommandLine::Get().HasParam("notPresent")); + REQUIRE(CommandLine::Get().GetValueAsString("notPresent") == nullptr); + REQUIRE(CommandLine::Get().GetValueAs("notPresent", 42) == 42); + } + + SECTION("Console Variables - loaded from ini string") + { + const char* Args[] = + { + "FlingEngine.exe" + }; + constexpr int32 ArgCount = sizeof(Args) / sizeof(char*); + REQUIRE(CommandLine::Get().Init(ArgCount, Args)); + + const std::string IniContent = + "[Video]\n" + "resolutionWidth=1280\n" + "\n" + "[ConsoleVariables]\n" + "resolutionWidth=1920\n" + "fullscreen=true\n" + "windowTitle=\"Fling Engine\"\n"; + + REQUIRE(CommandLine::Get().LoadConfigVarsFromString(IniContent)); + + // Values from the [ConsoleVariables] section should be picked up as if + // they were passed on the command line + REQUIRE(CommandLine::Get().HasParam("resolutionWidth")); + REQUIRE(CommandLine::Get().GetValueAs("resolutionWidth", -1) == 1920); + REQUIRE(CommandLine::Get().GetValueAs("fullscreen", false) == true); + + const std::string_view Title = CommandLine::Get().GetValueAsString("windowTitle"); + REQUIRE(Title == "Fling Engine"); + + // Keys from other sections should not be picked up + REQUIRE_FALSE(CommandLine::Get().HasParam("nonConsoleVariableKey")); + } + + SECTION("Console Variables - command line takes precedence over ini file") + { + const char* Args[] = + { + "FlingEngine.exe", + "-windowTitle=FromCommandLine" + }; + constexpr int32 ArgCount = sizeof(Args) / sizeof(char*); + REQUIRE(CommandLine::Get().Init(ArgCount, Args)); + + const std::string IniContent = + "[ConsoleVariables]\n" + "windowTitle=FromConfigFile\n" + "onlyInConfig=42\n"; + + REQUIRE(CommandLine::Get().LoadConfigVarsFromString(IniContent)); + + // The command line explicitly set windowTitle, so it should win over the ini value + const std::string_view Title = CommandLine::Get().GetValueAsString("windowTitle"); + REQUIRE(Title == "FromCommandLine"); + + // Values only present in the ini file should still be picked up + REQUIRE(CommandLine::Get().HasParam("onlyInConfig")); + REQUIRE(CommandLine::Get().GetValueAs("onlyInConfig", -1) == 42); + } + + SECTION("Console Variables - loaded from an actual ini file on disk") + { + const char* Args[] = + { + "FlingEngine.exe" + }; + constexpr int32 ArgCount = sizeof(Args) / sizeof(char*); + REQUIRE(CommandLine::Get().Init(ArgCount, Args)); + + const std::string TempFilePath = "CommandLineTests_Temp.ini"; + { + std::ofstream OutFile(TempFilePath); + OutFile << "[ConsoleVariables]\n"; + OutFile << "numEnemies=17\n"; + } + + REQUIRE(CommandLine::Get().LoadConfigFile(TempFilePath)); + REQUIRE(CommandLine::Get().GetValueAs("numEnemies", -1) == 17); + + std::remove(TempFilePath.c_str()); + } + + SECTION("Console Variables - missing file fails to load") + { + const char* Args[] = + { + "FlingEngine.exe" + }; + constexpr int32 ArgCount = sizeof(Args) / sizeof(char*); + REQUIRE(CommandLine::Get().Init(ArgCount, Args)); + + REQUIRE_FALSE(CommandLine::Get().LoadConfigFile("ThisFileDoesNotExist.ini")); } } \ No newline at end of file diff --git a/FlingTests/src/StringUtilsTests.cpp b/FlingTests/src/StringUtilsTests.cpp new file mode 100644 index 00000000..a6009104 --- /dev/null +++ b/FlingTests/src/StringUtilsTests.cpp @@ -0,0 +1,66 @@ +#include + +#include "Misc/StringUtils.h" +#include +#include + +TEST_CASE("String Utils", "[String Utils]") +{ + using namespace Fling; + + SECTION("Parse bool") + { + REQUIRE(StringUtils::ParseAs("true", false) == true); + REQUIRE(StringUtils::ParseAs("1", false) == true); + REQUIRE(StringUtils::ParseAs("false", true) == false); + REQUIRE(StringUtils::ParseAs("0", true) == false); + + // Garbage input should fall back to the default + REQUIRE(StringUtils::ParseAs("notABool", true) == true); + } + + SECTION("Parse int") + { + REQUIRE(StringUtils::ParseAs("776", -1) == 776); + REQUIRE(StringUtils::ParseAs("-42", 0) == -42); + + // A leading zero should not be treated as an octal prefix + REQUIRE(StringUtils::ParseAs("010", -1) == 10); + + // Garbage input should fall back to the default + REQUIRE(StringUtils::ParseAs("notANumber", -1) == -1); + } + + SECTION("Parse int32") + { + REQUIRE(StringUtils::ParseAs("2147483647", 0) == 2147483647); + } + + SECTION("Parse float") + { + REQUIRE(StringUtils::ParseAs("3.14", 0.0f) == Catch::Approx(3.14f)); + + // Garbage input should fall back to the default + REQUIRE(StringUtils::ParseAs("notANumber", -1.0f) == Catch::Approx(-1.0f)); + } + + SECTION("Parse double") + { + REQUIRE(StringUtils::ParseAs("-1123.56", 0.0) == Catch::Approx(-1123.56)); + + // Garbage input should fall back to the default + REQUIRE(StringUtils::ParseAs("notANumber", -1.0) == Catch::Approx(-1.0)); + } + + SECTION("Parse string") + { + REQUIRE(StringUtils::ParseAs("I like this format", "default") == "I like this format"); + } + + SECTION("Empty input falls back to default") + { + REQUIRE(StringUtils::ParseAs("", 42) == 42); + REQUIRE(StringUtils::ParseAs("", 3.5) == Catch::Approx(3.5)); + REQUIRE(StringUtils::ParseAs("", "default") == "default"); + } +} diff --git a/Sandbox/Gameplay/src/SandboxGame.cpp b/Sandbox/Gameplay/src/SandboxGame.cpp index f5df82e2..660eb79b 100644 --- a/Sandbox/Gameplay/src/SandboxGame.cpp +++ b/Sandbox/Gameplay/src/SandboxGame.cpp @@ -13,6 +13,9 @@ #include "GeometrySubpass.h" #include "Mover.h" +// Test command line args +#include "Misc/CommandLine.h" + namespace Sandbox { using namespace Fling; @@ -32,6 +35,18 @@ namespace Sandbox Input::BindKeyPress<&Sandbox::Game::ToggleRotation>(KeyNames::FL_KEY_T, *this); Input::BindKeyPress<&Sandbox::Game::OnToggleMoveLights>(KeyNames::FL_KEY_SPACE, *this); Input::BindKeyPress<&Sandbox::Game::OnTestSpawn>(KeyNames::FL_KEY_0, *this); + + { + using namespace Fling; + const CommandLine& CmdLineOpts = CommandLine::Get(); + const bool bMyOption = CmdLineOpts.GetValueAs("BenTest", false); + F_LOG_TRACE("Sandbox:Ben Test : {}\t", bMyOption ? "true" : "false"); + + const double DoubleOpt = CmdLineOpts.GetValueAs("TestDouble", -1.0); + F_LOG_TRACE("Sandbox:Ben Test : {}\t", DoubleOpt); + } + + F_LOG_TRACE("Done cmd line testing"); } void Game::OnStartGame(entt::registry& t_Reg) From 176de7ea3c42752188ce8625104ce6af30adcda6 Mon Sep 17 00:00:00 2001 From: Ben Hoffman Date: Sat, 8 Aug 2026 12:03:15 -0400 Subject: [PATCH 4/4] Fix Windows CI: build Catch2 at C++20 to match the main project MSVC defaults to C++14 when no /std flag is passed, so the standalone Catch2 build step in the CI workflow compiled it without std::string_view support. FlingTests (built at C++20 like the rest of the project) then failed to link with an unresolved Catch::StringMaker::convert symbol. Linux passed only because GCC 11+/Clang already default to C++17+. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/build.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 4f1d5fd8..bc2232c7 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -71,6 +71,8 @@ jobs: cmake -S external/Catch2 -B external/Catch2/build \ -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} \ -DCMAKE_INSTALL_PREFIX="${{ github.workspace }}/.deps" \ + -DCMAKE_CXX_STANDARD=20 \ + -DCMAKE_CXX_STANDARD_REQUIRED=ON \ -DBUILD_TESTING=OFF cmake --build external/Catch2/build --config ${{ matrix.build_type }} --target install