This obviously isn't finished yet. I wanted to get some feedback before I proceeded.

Is there a trac ticket already open for this?

Would you want the strings declared once in some type of data structure to avoid spelling errors when duplicating? for example, the strings (command line options) are used in the help output, and also later, e.g.

	const bool isVisualReplay = args.Has("replay-visual");
	const bool isNonVisualReplay = args.Has("replay");
	const bool isNonVisual = args.Has("autostart-nonvisual");
	const bool isUsingRLInterface = args.Has("rl-interface");

Did you see https://code.wildfiregames.com/D2432

IMHO the right approach is to change CmdLineArgs from an minimalist class to a proper command line parser where you register available options, does validation and has a help string for each.

I guess the issue was that we didn't want to duplicate the Readme inside the game's code. D4232 used a rather clever trick for that.

You won't need the readme.txt anymore if you have support for help, worst case just run "pyrogenesis -help > readme.txt" as a postbuild command. So no duplication required.

I'm not sure debug_printf works at all on Windows. You need to have SysInternals dbgview in order to be able to see such output.

In D4283#182511, @Stan wrote:

Did you see https://code.wildfiregames.com/D2432

Thanks @Stan , now I do :)

Are you planning to use some form of D2432 or do you want to go with some form of this patch?

If so, what should I use instead of debug_printf()?

In D4283#182523, @sera wrote:

You won't need the readme.txt anymore if you have support for help, worst case just run "pyrogenesis -help > readme.txt" as a postbuild command. So no duplication required.

I see. And we could also use help2man to generate a man page.

What would you think about something like this?

enum {
  WRITABLE_ROOT,
  REPLAY,
  UNIQUE_LOGS,
  ...
  NULL
};


struct  arg {
  int enum_value;
  const char *option;
  const char *description;
};


struct arg args[] = {
  {WRITABLE_ROOT, "writableRoot", "store runtime game data in root data directory"},
  {REPLAY, "replay=PATH", "non-visual replay of a previous game, used for analysis purposes"},
  {UNIQUE_LOGS", "unique-logs", "adds unix timestamp and process id to the filename of mainlog.html, interestinglog.html"},
  ...,
 {NULL, "", ""}

};


while (args[i].enum_value != NULL)
{
  printf ("%s%s", args[i].option, args[i].description);
  i++;
}

And then we'd use the strings elsewhere. ex:

        const bool isVisualReplay = args.Has(args[REPLAY_VISUAL].option);
	const bool isNonVisualReplay = args.Has(args[REPLAY].option);
	const bool isNonVisual = args.Has(args[AUTOSTART_NONVISUAL].option);

If so, what should I use instead of debug_printf()?

std::cout

What would you think about something like this?

Use C++ instead of C :)

Basically create a parser object (Parser), create option objects (Parser::Option) and add them via parser.add(option), then call parser.parse(argv). parse() validates argv against added options and by mismatch prints either usage or help and exits. Now you can query the parser or a returned options object for whether an option is set or not and what parameter were given.

Anyway, a good cmdline parser isn't easy to design/write, you may find inspiration with pythons argparse for instance or documentation of c++ parsers like cxxopts. Beware to not read the code or properly attribute it, Maybe you even find one that is a good fit for 0ad that can be used as a dependency (I have some doubt that such exists). I don't know if such is within your capability / you are willing to spend the effort, so a heredoc approach would still get a pass from me as it's already a step up from the current situation.

In D4283#182533, @sera wrote:

std::cout

Currently std::cout is not a standard way to write to output and logs.

Basically create a parser object (Parser), create option objects (Parser::Option) and add them via parser.add(option), then call parser.parse(argv). parse() validates argv against added options and by mismatch prints either usage or help and exits. Now you can query the parser or a returned options object for whether an option is set or not and what parameter were given.

Do you need to call parse multiple times? If not why do you need the method?

Anyway, a good cmdline parser isn't easy to design/write, you may find inspiration with pythons argparse for instance or documentation of c++ parsers like cxxopts.

+

I understand you'd rather I use C++ code. I'm not famillar with C++ actually, so I'll probably not do anymore with this diff.

I'm not sure why you've mentioned implementing a command line parser. 0ad already has this. I just mentioned replacing the strings with some variables and adding "-help" output.

Do you need to call parse multiple times? If not why do you need the method?

It's not a requirement but neither poor design.

I'm not sure why you've mentioned implementing a command line parser. 0ad already has this.

A poor man solution only.

I just mentioned replacing the strings with some variables and adding "-help" output.

A better parser with validation is just the cherry on top solution which seems out of reach, I thought I was clear enough but apparently not, copy paste the readme.txt into a single c++ raw string (heredoc approach) and printing it given h/help is already good enough to treat it as an improvement. So go with that.

In D4283#182540, @sera wrote:

I'm not sure why you've mentioned implementing a command line parser. 0ad already has this.

A poor man solution only.

Do you happen to know if there Is an existing ticket for redesigning/implementing a new parser? Has using boost.program_options been considered?

I just mentioned replacing the strings with some variables and adding "-help" output.

A better parser with validation is just the cherry on top solution which seems out of reach, I thought I was clear enough but apparently not, copy paste the readme.txt into a single c++ raw string (heredoc approach) and printing it given h/help is already good enough to treat it as an improvement. So go with that.

Ok, I'll close this out then in favor of D2432