← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe into lp:widelands


Review: Approve

Code is looking okay and the editor no longer crashes. Two observations:

- When loading the old/broken maps, we still get the "Tribe not known" message. Maybe only display a "note:" message and select a default tribe in that case? Having the wrong tribe is in most cases probably preferable to not being able to use the map at all. Unknown tribes in maps could also happen when "backporting" maps from (newer) game versions where more/custom tribes are available.

- When loading the "broken" map in the editor and saving again the map is still broken. This one is actually a bit strange, shouldn't the change in editorinteractive.cc avoid this as well?

Diff comments:

> === modified file 'src/editor/editorinteractive.cc'
> --- src/editor/editorinteractive.cc	2018-07-20 08:42:23 +0000
> +++ src/editor/editorinteractive.cc	2018-08-12 17:17:16 +0000
> @@ -180,7 +180,7 @@
>  	loader_ui.step(_("Creating players"));
>  	iterate_player_numbers(p, map->get_nrplayers()) {
>  		egbase().add_player(
> -		   p, 0, map->get_scenario_player_tribe(p), map->get_scenario_player_name(p));
> +		   p, 0, map->get_scenario_player_tribe(p).empty() ? Widelands::get_all_tribenames().front() : map->get_scenario_player_tribe(p), map->get_scenario_player_name(p));

Why no random selection here?

>  	}
>  	ml->load_map_complete(egbase(), Widelands::MapLoader::LoadType::kEditor);

Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe.