widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

Excellent, I didn't know that OPtr could do that.

Code LGTM, 1 nit.

Not tested.

This will probably also fix

https://bugs.launchpad.net/widelands/+bug/1735090

and

https://bugs.launchpad.net/widelands/+bug/1619970


Diff comments:

> 
> === modified file 'src/wui/buildingwindow.cc'
> --- src/wui/buildingwindow.cc	2017-11-29 07:59:49 +0000
> +++ src/wui/buildingwindow.cc	2017-11-30 11:02:11 +0000
> @@ -381,8 +392,13 @@
>  ===============
>  */
>  void BuildingWindow::act_start_stop() {
> -	if (dynamic_cast<const Widelands::ProductionSite*>(&building_)) {
> -		igbase()->game().send_player_start_stop_building(building_);
> +	Widelands::Building* building = building_.get(parent_->egbase());
> +	if (building == nullptr) {
> +		return;

no die() here? How about having a private function get_building_or_die(), since we keep repeating this code?

Or even better protected, but then we'd have to template this or use dynamic cast  not sure if it's worth the effort.

> +	}
> +
> +	if (dynamic_cast<const Widelands::ProductionSite*>(building)) {
> +		igbase()->game().send_player_start_stop_building(*building);
>  	}
>  }
>  


-- 
https://code.launchpad.net/~widelands-dev/widelands/keep_optr_in_ui/+merge/334524
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/keep_optr_in_ui into lp:widelands.