widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

Review: Needs Fixing

Only reviewed the code that I did not write. Some comments inlined.

Diff comments:

> 
> === modified file 'src/editor/ui_menus/editor_main_menu_random_map.cc'
> --- src/editor/ui_menus/editor_main_menu_random_map.cc	2014-11-30 18:49:38 +0000
> +++ src/editor/ui_menus/editor_main_menu_random_map.cc	2015-07-27 11:00:34 +0000
> @@ -504,12 +504,13 @@
>  		<< "ID = " << m_idEditbox->text() << "\n";
>  
>  	MapGenerator gen(map, mapInfo, egbase);
> -	map.create_empty_map(egbase.world(),
> -	                     mapInfo.w,
> -	                     mapInfo.h,
> -	                     _("No Name"),
> -	                     g_options.pull_section("global").get_string("realname", _("Unknown")),
> -	                     sstrm.str().c_str());
> +	map.create_empty_map(

strange formatting

> +				egbase.world(),
> +				mapInfo.w,
> +				mapInfo.h,
> +				_("No Name"),
> +				g_options.pull_section("global").get_string("realname", pgettext("map_name", "Unknown")),
> +				sstrm.str().c_str());
>  	loader.step(_("Generating random map..."));
>  	gen.create_random_map();
>  
> 
> === modified file 'src/editor/ui_menus/editor_player_menu.cc'
> --- src/editor/ui_menus/editor_player_menu.cc	2015-06-05 19:13:06 +0000
> +++ src/editor/ui_menus/editor_player_menu.cc	2015-07-27 11:00:34 +0000
> @@ -207,7 +207,7 @@
>  			number += '0' + nr_players_10;
>  		number += '0' + nr_players % 10;
>  		/** TRANSLATORS: Default player name, e.g. Player 1 */
> -		const std::string name = (boost::format(_("Player %s")) % number).str();
> +		const std::string name = (boost::format(_("Player %u")) % nr_players).str();

make sure that nr_players is not a char/uint8_t. Otherwise this will not work properly iirc. Might need a static_cast<int>() in that case

>  		map.set_scenario_player_name(nr_players, name);
>  	}
>  	map.set_scenario_player_tribe(nr_players, m_tribes[0]);
> 
> === modified file 'src/scripting/persistence.cc'
> --- src/scripting/persistence.cc	2015-01-31 16:03:59 +0000
> +++ src/scripting/persistence.cc	2015-07-27 11:00:34 +0000
> @@ -165,7 +165,7 @@
>  	"package", "pairs", "pcall", "print", "rawequal", "rawget", "rawset",
>  	"rawlen", "require", "select", "setfenv", "setmetatable", "table",
>  	"tonumber", "tostring", "type", "unpack", "wl", "xpcall", "string",
> -	"_", "set_textdomain", "get_build_id", "coroutine.yield", "ngettext",
> +	"_", "set_textdomain", "get_build_id", "coroutine.yield", "ngettext", "pgettext",

The ordering of these matter. Please only append at the end, i.e. after "path" before nullptr

>  	"include", "path", nullptr
>  };
>  


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1357510/+merge/265949
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1357510.