Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
959 changes: 959 additions & 0 deletions external/header_only/SimpleGlob.h

Large diffs are not rendered by default.

1,046 changes: 1,046 additions & 0 deletions external/header_only/SimpleOpt.h

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ set(SOURCE_FILES
util/SignalMediator.hxx
util/Singleton.hxx
util/Meta.hxx
util/ParseCli.{hxx,cxx}
util/Exception.{hxx,cxx}
util/OSystem.{hxx,cxx}
services/Randomizer.{hxx,cxx}
Expand Down
2 changes: 1 addition & 1 deletion src/Game.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ void Game::initialize()
LOG(LOG_DEBUG) << "Initialized Game Object";
}

void Game::run(bool SkipMenu)
void Game::run()
{
Camera::instance().centerScreenOnMapCenter();

Expand Down
2 changes: 1 addition & 1 deletion src/Game.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public:
* @details starts running the game
* @param SkipMenu if the main menu should be skipped or not
*/
virtual void run(bool SkipMenu = false);
virtual void run();
Copy link
Contributor

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?

Copy link
Collaborator

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.


/// ends the game
virtual void shutdown();
Expand Down
9 changes: 9 additions & 0 deletions src/engine/basics/Settings.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,15 @@ struct SettingsData

/// Write errors to a log file
bool writeErrorLogFile;

// ==================================
// Command line options
// ==================================
Copy link
Member

Choose a reason for hiding this comment

The 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";
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Collaborator

Choose a reason for hiding this comment

The 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;
};

/**
Expand Down
42 changes: 14 additions & 28 deletions src/main.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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;
Expand Down Expand Up @@ -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 &param)
{
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;
Copy link
Contributor

Choose a reason for hiding this comment

The 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";
Copy link
Contributor

Choose a reason for hiding this comment

The 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)
Copy link
Contributor

Choose a reason for hiding this comment

The 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";
Expand All @@ -118,4 +104,4 @@ int main(int argc, char **argv)
}

return EXIT_FAILURE;
}
}
90 changes: 90 additions & 0 deletions src/util/ParseCli.cxx
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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this enum have a name?

{
OPT_HELP,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better user bane enum opt_id {
e_opt_help
....
}
OPT_HELP looks like macros

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;
Copy link
Member

Choose a reason for hiding this comment

The 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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

empty line after return

case OPT_SKIPMENU:
Settings::instance().skipMenu = true;
break;
Copy link
Contributor

Choose a reason for hiding this comment

The 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";
}
}
18 changes: 18 additions & 0 deletions src/util/ParseCli.hxx
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ParserCli?

{
public:
bool ProcessCommandLine(int argc, char **argv);
Copy link
Contributor

Choose a reason for hiding this comment

The 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);
Copy link
Collaborator

Choose a reason for hiding this comment

The 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