Hello, I finished the move. All important points should be listed below.
Changes: * --ai_config, --new_storyscreens renamed to --ai-config, --new-storyscreens (consistency, no underscores were used except for here) * --no-delay renamed to --nodelay (consistency with nogui, nocache, nosound and nomusic) * --campaign[[<difficulty>] <id_c> [<id_s>]] split into --campaign, --campaign-difficulty and --campaign-scenario (KISS) * split optional comma-separated defines list from --preprocess= (or -p=) to --preprocess-defines= (KISS) * dropped --log= syntax. It was the same as --log-error. * added unit tests at src/tests/test_commandline_options.cpp. It's worth looking at them, since they show examples of working and tested syntax. Should probably be extended to test for exceptions and border cases as well. Benefits: * better code clarity (I hope) * centralized the commandline parsing instead of keeping it spread across 3 functions * delegating the parsing code to an external library, which guarantees consistency * as grzywacz pointed out, easy access to commandline arguments at any point: the bug with bpp going back to default regardless of commandline parameters after changing resolution should now be an easy fix * unit testing Undocumented before: * --proxy* options * --label - what is its exactly use? is my description accurate? Issues: * some help messages are now a bit more clumsy due to this: https://svn.boost.org/trac/boost/ticket/4781 . As you could see, I worked a bit around this by describing <arg> in command description. However, after contacting author of Boost::program_options he said that wontfix doesn't mean it isn't wanted, but only that he hasn't time for it, so hopefully it will be solved by a future release of boost. * As Boost::program_options allows option grouping, I grouped the options for more clarity. If some options are unclear. * At a few points I used syntax which feels a bit clumsy. If somebody has better ideas how to do these (without too much work, hopefully), please do/tell. These "clumsy" points: * 'two_strings' local class in commandline_options.cpp to accomodate for boost's tuple operator>> and program_options relationship. Please note the comment in code. * parse_to_uint_string_tuples_() and parse_to_uint_string_string_tuples_() in commandline_options.cpp - a bit of code repetition, but I haven't had better idea how to do this. I couldn't have just made one function, since the tuple<int,str,str> and tuple<int,str> are different return types. * repetition of code reacting to options multiplayer_ai_config, multiplayer_algorithm, multiplayer_controller, multiplayer_side and to some extent multiplayer_parm in play_multiplayer_mode() in game_controller.cpp. It's not a big issue, but still feels a bit clumsy. Also, if somebody gets foreach(boost::tuple<int,string>(), foovector) to work instead of using vector iterators, please let me know. Comments/fixes/bug reports welcome. Cheers, Zaroth
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Wesnoth-dev mailing list [email protected] https://mail.gna.org/listinfo/wesnoth-dev
