widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

The Lua functions always return an int, that's how the interface works.

I don't agree with your TODO comment - see diff comments.

Otherwise, code LGTM - I'm still waiting for the compiler for testing.

Diff comments:

> === modified file 'src/logic/map_objects/tribes/workarea_info.h'
> --- src/logic/map_objects/tribes/workarea_info.h	2015-11-28 22:29:26 +0000
> +++ src/logic/map_objects/tribes/workarea_info.h	2016-04-17 06:41:21 +0000
> @@ -25,9 +25,23 @@
>  #include <set>
>  #include <string>
>  
> -//  This type is used to store information about workareas. It stores radii and
> -//  for each radius a set of strings. Each string contains a description of an
> -//  activity (or similar) that can be performed within the radius.
> +/** The WorkareaInfo stores radii and for each radius a set of strings.
> + *
> + * A Workarea is a "circle" around a building that this building affects
> + * or is needed by this building, e.g. Areas for Mines, Fields of a Farm.
> + * Worareads are shown on the Map when clicking on a building.
> + *
> + * Each string contains a description of an  activity (or similar) i
> + * that can be performed within the radius.
> + */
> +
> +// TODO(Hasi50): In fact this complex idea of a workarea is not used.
> +// I do knot know of any building that has different sizes of workareas
> +// during its liftimer. LuaBuildingDescription::get_workarea_radiu does not use it
> +// and the GUI does not show it.
> +//
> +// So we should just use a simple unit8 perhaps?

Have a look at:

FieldOverlayManager::OverlayId InteractiveBase::show_work_area

In this function, multiple overlays are generated from the map. You can see multiple overlays on the same building if you build a Fortress. So, a map is needed.

> +
>  using WorkareaInfo = std::map<uint32_t, std::set<std::string>>;
>  
>  #endif  // end of include guard: WL_LOGIC_MAP_OBJECTS_TRIBES_WORKAREA_INFO_H


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