-
Notifications
You must be signed in to change notification settings - Fork 109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🔧 Added class for parsing command line parameters #941
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -166,6 +166,15 @@ struct SettingsData | |
|
||
/// Write errors to a log file | ||
bool writeErrorLogFile; | ||
|
||
// ================================== | ||
// Command line options | ||
// ================================== | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you extend the comment, that these settings are special ones and will not be preserved in between runs? |
||
|
||
/// Sets a different video driver | ||
std::string videoDriver = "Default"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you really need save it all game? it just option on start, dont pay for unused variables There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will video driver handling be added later in a different PR? I guess I'm also just wondering why this parameter is necessary, like I don't know how many people use different drivers for different programs. But then again, I don't normally run Cytopia from the command line. |
||
|
||
bool skipMenu = false; | ||
}; | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,11 +6,13 @@ | |
#include "LOG.hxx" | ||
#include "engine/WindowManager.hxx" | ||
#include <UIManager.hxx> | ||
#include "SimpleOpt.h" | ||
#include "ParseCli.hxx" | ||
|
||
#include <SDL.h> | ||
#include <SDL_ttf.h> | ||
|
||
bool initialize(const char *videoDriver) | ||
bool initialize() | ||
{ | ||
if (SDL_Init(0) != 0) | ||
{ | ||
|
@@ -19,6 +21,10 @@ bool initialize(const char *videoDriver) | |
return false; | ||
} | ||
|
||
const char *videoDriver = nullptr; | ||
if (Settings::instance().videoDriver != "Default") | ||
videoDriver = Settings::instance().videoDriver.c_str(); | ||
|
||
if (SDL_VideoInit(videoDriver) != 0) | ||
{ | ||
LOG(LOG_ERROR) << "Unknown video driver " << videoDriver; | ||
|
@@ -47,51 +53,31 @@ bool initialize(const char *videoDriver) | |
|
||
int protected_main(int argc, char **argv) | ||
{ | ||
(void)argc; | ||
(void)argv; | ||
|
||
LOG(LOG_INFO) << VERSION; | ||
|
||
// add commandline parameter to skipMenu | ||
auto has_args = [argv, argc](const std::string ¶m) | ||
{ | ||
for (int i = 1; i < argc; ++i) | ||
if (param == argv[i]) | ||
return i; | ||
|
||
LOG(LOG_DEBUG) << "Unknown game option " << param; | ||
return 0; | ||
}; | ||
|
||
bool skipMenu = has_args("--skipMenu"); | ||
uint32_t videoOpt = has_args("--video"); | ||
const char *videoDriver = nullptr; | ||
if (videoOpt) | ||
{ | ||
videoDriver = argv[videoOpt + 1]; | ||
} | ||
ParseCli cli; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. parser cli? |
||
if (!cli.ProcessCommandLine(argc, argv)) | ||
return EXIT_FAILURE; | ||
|
||
LOG(LOG_DEBUG) << "Launching Cytopia"; | ||
|
||
Cytopia::Game game; | ||
|
||
LOG(LOG_DEBUG) << "Initializing Cytopia"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. too much words? debug () << text, debug( "%s", text )? |
||
|
||
if (!initialize(videoDriver)) | ||
if (!initialize()) | ||
return EXIT_FAILURE; | ||
else | ||
LOG(LOG_DEBUG) << "DONE Cytopia"; | ||
|
||
bool startGame = true; | ||
if (!skipMenu) | ||
if (!Settings::instance().skipMenu) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. again temp var in persisten place |
||
{ | ||
startGame = mainMenu(); | ||
} | ||
|
||
if (startGame) | ||
{ | ||
LOG(LOG_DEBUG) << "Running the Game"; | ||
game.run(skipMenu); | ||
game.run(); | ||
} | ||
|
||
LOG(LOG_DEBUG) << "Closing the Game"; | ||
|
@@ -118,4 +104,4 @@ int main(int argc, char **argv) | |
} | ||
|
||
return EXIT_FAILURE; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
#include "ParseCli.hxx" | ||
#include "LOG.hxx" | ||
#include "Singleton.hxx" | ||
#include "Settings.hxx" | ||
|
||
// option identifiers | ||
enum | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this enum have a name? |
||
{ | ||
OPT_HELP, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. better user bane enum opt_id { |
||
OPT_SKIPMENU, | ||
OPT_VIDEODRIVER | ||
}; | ||
|
||
// option array | ||
CSimpleOpt::SOption cmdline_options[] = {{OPT_SKIPMENU, ("--skipMenu"), SO_NONE}, | ||
{OPT_VIDEODRIVER, ("--video"), SO_REQ_SEP}, | ||
{OPT_HELP, ("--help"), SO_NONE}, | ||
{OPT_HELP, ("-h"), SO_NONE}, | ||
SO_END_OF_OPTIONS}; | ||
|
||
bool ParseCli::ProcessCommandLine(int argc, char *argv[]) | ||
{ | ||
CSimpleOpt args(argc, argv, cmdline_options); | ||
bool success = true; | ||
while (args.Next()) | ||
{ | ||
if (args.LastError() != SO_SUCCESS) | ||
{ | ||
LOG(LOG_ERROR) << GetLastErrorText(args.LastError()) << " " << args.OptionText(); | ||
success = false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should it return false immediately? |
||
} | ||
|
||
switch (args.OptionId()) | ||
{ | ||
case OPT_HELP: | ||
ShowUsage(); | ||
return false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. empty line after return |
||
case OPT_SKIPMENU: | ||
Settings::instance().skipMenu = true; | ||
break; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. empty line after break better |
||
case OPT_VIDEODRIVER: | ||
if (args.OptionArg()) | ||
{ | ||
Settings::instance().videoDriver = args.OptionArg(); | ||
} | ||
else | ||
{ | ||
LOG(LOG_ERROR) << "videoDriver not set"; | ||
ShowUsage(); | ||
return false; | ||
} | ||
break; | ||
default: | ||
ShowUsage(); | ||
} | ||
} | ||
|
||
return success; | ||
} | ||
|
||
void ParseCli::ShowUsage() | ||
{ | ||
LOG(LOG_INFO) << "Usage: Cytopia [OPTIONS]"; | ||
LOG(LOG_INFO) << "\t--help (this)"; | ||
LOG(LOG_INFO) << "\t--skipMenu (Skips the main menu)"; | ||
LOG(LOG_INFO) << "\t--video <videoDriver> (Sets a different video driver)"; | ||
} | ||
|
||
std::string ParseCli::GetLastErrorText(int a_nError) | ||
{ | ||
switch (a_nError) | ||
{ | ||
case SO_SUCCESS: | ||
return "Success"; | ||
case SO_OPT_INVALID: | ||
return "Unrecognized option"; | ||
case SO_OPT_MULTIPLE: | ||
return "Option matched multiple strings"; | ||
case SO_ARG_INVALID: | ||
return "Option does not accept argument"; | ||
case SO_ARG_INVALID_TYPE: | ||
return "Invalid argument format"; | ||
case SO_ARG_MISSING: | ||
return "Required argument is missing"; | ||
case SO_ARG_INVALID_DATA: | ||
return "Invalid argument data"; | ||
default: | ||
return "Unknown error"; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
#ifndef CYTOPIA_PARSECLI_HXX | ||
#define CYTOPIA_PARSECLI_HXX | ||
|
||
#include "SimpleOpt.h" | ||
#include <string> | ||
|
||
class ParseCli | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ParserCli? |
||
{ | ||
public: | ||
bool ProcessCommandLine(int argc, char **argv); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. once function in class, it may be static or ctor |
||
|
||
private: | ||
std::string GetLastErrorText(int a_nError); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because SonarCloud has infiltrated my head: I think this could be made const. |
||
|
||
void ShowUsage(); | ||
}; | ||
|
||
#endif //CYTOPIA_PARSECLI_HXX |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why it virtual, we planed have same modes in game?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that was changed in this PR/commits.