widelands-dev team mailing list archive

[Notabilis <notabilis27@xxxxxx>]

Review: Approve

Seems to be working as intended now, thanks. Code is looking good, too.

Reading the tribes from the map file will need some getting used to but I think it is the correct approach. At first, it is a bit strange that the tribe is "reset" when changing the map but AI difficulty and starting condition stay the same.

Diff comments:

> 
> === modified file 'src/ui_fsmenu/launch_spg.cc'
> --- src/ui_fsmenu/launch_spg.cc	2018-05-29 18:57:46 +0000
> +++ src/ui_fsmenu/launch_spg.cc	2018-07-06 08:37:59 +0000
> @@ -84,6 +84,21 @@
>  
>       // Variables and objects used in the menu
>       is_scenario_(false) {
> +	subscriber_ =
> +	   Notifications::subscribe<NoteGameSettings>([this](const NoteGameSettings& note) {
> +		switch (note.action) {
> +		case NoteGameSettings::Action::kMap:
> +			update(true);
> +			break;
> +		case NoteGameSettings::Action::kPlayer:
> +			update(false);
> +			break;
> +		case NoteGameSettings::Action::kUser:
> +			update(false);
> +			break;

Use fall-through for kPlayer and kUser?
Just a though, I am fine with this version.

> +		}
> +		});
> +
>  	ok_.set_pos(Vector2i(get_w() * 7 / 10, get_h() * 9 / 10));
>  	back_.set_pos(Vector2i(get_w() * 7 / 10, get_h() * 17 / 20));
>  	win_condition_dropdown_.set_pos(Vector2i(get_w() * 7 / 10, get_h() * 4 / 10 + buth_));


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1769344-sp-thinks-too-much/+merge/346911
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1769344-sp-thinks-too-much.