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. _______________________________________________ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
