Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/fri-01-portspace into lp:widelands

2018-07-25 Thread GunChleoc
Review: Approve Code LGTM. Should still be tested though, there is always something that could go wrong. -- https://code.launchpad.net/~widelands-dev/widelands/fri-01-portspace/+merge/350729 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fri-01-portspace.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/no-hardcoded-resources into lp:widelands

2018-07-25 Thread GunChleoc
I like the implementation :) - some nits though. There are 2 more things that we need to look at: 1. Double-check the worker programs to see how the geologists distinguish between searching for water vs. mountain resources, to make sure that this is not hard-coded on the resource type. Maybe

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/no-hardcoded-resources into lp:widelands

2018-07-25 Thread Benedikt Straub
I implemented your renaming suggestions. The resource Fish doesn´t need a timeout because it´s used only for geologist messages, not by buildings that mine this resource. timeout is therefore ignored for resources with detectable=false. 1) This is defined in Worker::geologist_update(). The

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1619402-port-work-area-on-expedition into lp:widelands

2018-07-25 Thread Klaus Halfmann
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

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/no-hardcoded-resources into lp:widelands

2018-07-25 Thread bunnybot
Continuous integration builds have changed state: Travis build 3722. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/407977888. Appveyor build 3522. State: failed. Details:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1736901-shift-market into lp:widelands

2018-07-25 Thread kaputtnik
Review: Approve LGTM :-) -- https://code.launchpad.net/~widelands-dev/widelands/bug-1736901-shift-market/+merge/349630 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1736901-shift-market. ___ Mailing list:

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/no-hardcoded-resources into lp:widelands

2018-07-25 Thread bunnybot
Continuous integration builds have changed state: Travis build 3723. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/408099801. Appveyor build 3523. State: success. Details:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/mines-worldsavior into lp:widelands

2018-07-25 Thread hessenfarmer
Review: Resubmit Ok yesterday I fixed all diff comments from Nordfriese and me. (Hope I caught all of them) Did a testrun of 4 Ai (one of each tribe) on Full moon map. No anomalies showed up in the first 3 hours. Although no mine got depleted so I couldn't get the stats of a depleted mine. Ai