widelands-dev team mailing list archive

[Klaus Halfmann <klaus.halfmann@xxxxxxxx>]

Review: Needs Fixing review, compile

Going to compile and tesplay this now ...

For the Review I find:
* Renamed: show_work_area -> show_workarea, Same for hide_work_area -> hide_workarea
* Option show_workarea_preview_ / "Show buildings area preview" removed
* "workareapreview" not checked any longer
* Added new: is_warping_ - no idea what this indicates?

Some questions inline.

There is a bug in src/wui/interactive_player.cc:335:20: error: use of undeclared
      identifier 'work_area_overlays'; did you mean 'workarea_overlays'?
                        const auto it = work_area_overlays.find(f->fcoords);
                                        ^~~~~~~~~~~~~~~~~~
                                        workarea_overlays

(travis faild as of the bug in trunk we had before)

Shall I try to fix this?

Otherwise code LGTM.

Diff comments:

> 
> === modified file 'src/wui/buildingwindow.cc'
> --- src/wui/buildingwindow.cc	2018-04-27 06:11:05 +0000
> +++ src/wui/buildingwindow.cc	2018-07-22 19:53:33 +0000
> @@ -58,6 +58,7 @@
>       building_position_(b.get_position()),
>       showing_workarea_(false),
>       avoid_fastclick_(avoid_fastclick),
> +	 is_warping_(false),

How is this related to the workarea? What will be the effect?

>       expeditionbtn_(nullptr) {
>  	buildingnotes_subscriber_ = Notifications::subscribe<Widelands::NoteBuilding>(
>  	   [this](const Widelands::NoteBuilding& note) { on_building_note(note); });
> 
> === modified file 'src/wui/buildingwindow.h'
> --- src/wui/buildingwindow.h	2018-04-27 06:11:05 +0000
> +++ src/wui/buildingwindow.h	2018-07-22 19:53:33 +0000
> @@ -129,6 +129,7 @@
>  
>  	bool showing_workarea_;
>  	bool avoid_fastclick_;
> +	bool is_warping_;

please add comment like: warping means: objets will move faster than light  ;-)

>  
>  	UI::Button* expeditionbtn_;
>  	std::unique_ptr<Notifications::Subscriber<Widelands::NoteExpeditionCanceled>>


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1619402-port-work-area-on-expedition/+merge/349594
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1619402-port-work-area-on-expedition.