Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug_1571009_work_area_radius into lp:widelands
Yes, inderectly: We have rolling builds on Appveyor. This means the moment you commit a new revision, the current build of the branch is cancelled. -- https://code.launchpad.net/~widelands-dev/widelands/bug_1571009_work_area_radius/+merge/292066 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug_1571009_work_area_radius. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug_1571009_work_area_radius into lp:widelands
Whats the Problem with Appveyor? I did not find the Problem. Did the build take to long? Did I cancel it? -- https://code.launchpad.net/~widelands-dev/widelands/bug_1571009_work_area_radius/+merge/292066 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug_1571009_work_area_radius. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug_1571009_work_area_radius into lp:widelands
Continuous integration builds have changed state: Travis build 1021. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/123665737. Appveyor build 853. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1571009_work_area_radius-853. -- https://code.launchpad.net/~widelands-dev/widelands/bug_1571009_work_area_radius/+merge/292066 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug_1571009_work_area_radius. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug_1571009_work_area_radius into lp:widelands
For merging, set a commit message, then add a comment with ATbunnybot merge in it. -- https://code.launchpad.net/~widelands-dev/widelands/bug_1571009_work_area_radius/+merge/292066 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug_1571009_work_area_radius. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug_1571009_work_area_radius into lp:widelands
Review: Approve -- https://code.launchpad.net/~widelands-dev/widelands/bug_1571009_work_area_radius/+merge/292066 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug_1571009_work_area_radius. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug_1571009_work_area_radius into lp:widelands
Review: Abstain Tested and working :) Please remove the TODO comment before merging. -- https://code.launchpad.net/~widelands-dev/widelands/bug_1571009_work_area_radius/+merge/292066 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug_1571009_work_area_radius. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug_1571009_work_area_radius into lp:widelands
> The Lua functions always return an int, that's how the interface works. Sure, but why are you bringing this up? Has anyone said otherwise? -- 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. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/fh1_multiline_textarea into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/fh1_multiline_textarea into lp:widelands has been updated. Status: Needs review => Work in progress For more details, see: https://code.launchpad.net/~widelands-dev/widelands/fh1_multiline_textarea/+merge/292033 -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/fh1_multiline_textarea 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug_1571009_work_area_radius into lp:widelands
Diff. Diff comments: > > === modified file 'src/scripting/lua_map.cc' > --- src/scripting/lua_map.cc 2016-04-11 06:45:29 + > +++ src/scripting/lua_map.cc 2016-04-17 06:41:21 + > @@ -1871,10 +1871,16 @@ > /* RST > .. attribute:: workarea_radius > > - (RO) the workarea_radius of the building as an int. > + (RO) the first workarea_radius of the building as an > int, > + nil in case bulding has no workareas > */ > int LuaBuildingDescription::get_workarea_radius(lua_State * L) { > - lua_pushinteger(L, get()->workarea_info_.begin()->first); > +const WorkareaInfo& workareaInfo = get()->workarea_info_; You missed this space indent here. > + if (!workareaInfo.empty()) { > + lua_pushinteger(L, workareaInfo.begin()->first); > + } else { > + lua_pushnil(L); > + } > return 1; > } > -- 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. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug_1571009_work_area_radius into lp:widelands
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 > + > +++ src/logic/map_objects/tribes/workarea_info.h 2016-04-17 06:41:21 > + > @@ -25,9 +25,23 @@ > #include > #include > > -// 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>; > > #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. ___ 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