Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/fix_infrastructure_return_values into lp:widelands
I started all the campaign and tutorial scenarios without problems, so it's OK. Thanks for fixing :) BTW we forgot to set a commit message ;) -- https://code.launchpad.net/~widelands-dev/widelands/fix_infrastructure_return_values/+merge/337364 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fix_infrastructure_return_values. ___ 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/fix_infrastructure_return_values into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/fix_infrastructure_return_values into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/fix_infrastructure_return_values/+merge/337364 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fix_infrastructure_return_values. ___ 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/fix_infrastructure_return_values into lp:widelands
Review: Approve SO actually you next branch will use and then test it. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/fix_infrastructure_return_values/+merge/337364 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fix_infrastructure_return_values. ___ 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/fix_infrastructure_return_values into lp:widelands
Klaus, this code is only used in scenarios or campaigns. I grep'd through the data directory and currently all campaigns which use prefilled_buildings() or place_building_in_region() didn't use any return values. I would have been surprised if they do :-D The branch linked in bug 1639514 uses the return value from prefilled_buildings(): https://bazaar.launchpad.net/~widelands-dev/widelands/bug-1639514_fix_yellow_player/view/head:/data/campaigns/bar02.wmf/scripting/starting_conditions.lua#L97 For testing one has to write a scenario map. I could do so if it is wanted :-) -- https://code.launchpad.net/~widelands-dev/widelands/fix_infrastructure_return_values/+merge/337364 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fix_infrastructure_return_values. ___ 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/fix_infrastructure_return_values into lp:widelands
Review: Approve Code LGTM, not tested. -- https://code.launchpad.net/~widelands-dev/widelands/fix_infrastructure_return_values/+merge/337364 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fix_infrastructure_return_values. ___ 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/fix_infrastructure_return_values into lp:widelands
There was no real problem, but wrong documentation. Nevertheless this change will make it possible to work with the returned building, e.g. using building.flag for creating roads or query building.descr. -- https://code.launchpad.net/~widelands-dev/widelands/fix_infrastructure_return_values/+merge/337364 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fix_infrastructure_return_values. ___ 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/fix_infrastructure_return_values into lp:widelands
kaputtnik whats the problem then? Is it worth, me taking a look? -- https://code.launchpad.net/~widelands-dev/widelands/fix_infrastructure_return_values/+merge/337364 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fix_infrastructure_return_values. ___ 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/fix_infrastructure_return_values into lp:widelands
Continuous integration builds have changed state: Travis build 3154. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/339079461. Appveyor build 2961. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_fix_infrastructure_return_values-2961. -- https://code.launchpad.net/~widelands-dev/widelands/fix_infrastructure_return_values/+merge/337364 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fix_infrastructure_return_values. ___ 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/fix_infrastructure_return_values into lp:widelands
Review: Needs Fixing Ups, that will not work... -- https://code.launchpad.net/~widelands-dev/widelands/fix_infrastructure_return_values/+merge/337364 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fix_infrastructure_return_values. ___ 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/fix_infrastructure_return_values into lp:widelands
kaputtnik has proposed merging
lp:~widelands-dev/widelands/fix_infrastructure_return_values into lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/fix_infrastructure_return_values/+merge/337364
While working on a fix of bug 1639514 i found that there was a return value for
place_building_in_region() is documented, but it does not return anything.
https://wl.widelands.org/docs/wl/autogen_auxiliary_infrastructure/#place_building_in_region
The problem was that place_building_in_region() calls prefilled_buildings()
which does not return the building created whereas this value is returned by
place_buildings()
Relevant code of place_building_in_region():
https://bazaar.launchpad.net/~widelands-dev/widelands/trunk/view/head:/data/scripting/infrastructure.lua#L154
prefilled_buildings() get the building of place_buildings(), but do not return
it:
https://bazaar.launchpad.net/~widelands-dev/widelands/trunk/view/head:/data/scripting/infrastructure.lua#L99
This branch provides a small change, so that both functions returns the placed
building. Also some small corrections related to RST.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/fix_infrastructure_return_values into lp:widelands.
=== modified file 'data/scripting/infrastructure.lua'
--- data/scripting/infrastructure.lua 2017-06-08 12:37:10 +
+++ data/scripting/infrastructure.lua 2018-02-08 14:34:18 +
@@ -32,6 +32,7 @@
--:arg create_carriers: If this is :const:`true` carriers are created for
-- the roads. Otherwise no carriers will be created.
--:type create_carriers: :class:`boolean`
+
function connected_road(p, start, cmd, g_create_carriers)
create_carriers = true
if g_create_carriers ~= nil then
@@ -96,9 +97,13 @@
-- :meth:`wl.map.Warehouse.set_workers`. Note that ProductionSites
-- are filled with workers by default.
--:type b1_descr: :class:`array`
+--
+--:returns: The building created
+
function prefilled_buildings(p, ...)
+ local b = nil
for idx,bdescr in ipairs({...}) do
- local b = p:place_building(bdescr[1], wl.Game().map:get_field(bdescr[2],bdescr[3]), false, true)
+ b = p:place_building(bdescr[1], wl.Game().map:get_field(bdescr[2],bdescr[3]), false, true)
-- Fill with workers
if b.valid_workers then b:set_workers(b.valid_workers) end
if bdescr.workers then b:set_workers(bdescr.workers) end
@@ -116,6 +121,7 @@
if bdescr.wares then b:set_wares(bdescr.wares) end
if bdescr.inputs then b:set_inputs(bdescr.inputs) end
end
+ return b
end
-- RST
@@ -132,12 +138,12 @@
--:type building: :class:`string`
--:arg region: The fields which are tested for suitability.
--:type region: :class:`array`
---:arg opts: a table with prefill information (wares, soldiers, workers,
--- see :func:`prefilled_buildings`) and the following options:
---
+--:arg opts: A table with prefill information (wares, soldiers, workers,
+-- see :func:`prefilled_buildings`)
--:type opts: :class:`table`
--
---:returns: the building created
+--:returns: The building created
+
function place_building_in_region(plr, building, fields, gargs)
local idx
local f
@@ -167,12 +173,13 @@
-- RST
-- .. function:: is_building(immovable)
--
---Checks whether an immpvable is a finished building, i.e. not
+--Checks whether an immovable is a finished building, i.e. not
--a construction site.
--
--:arg immovable: The immovable to test
--
--:returns: true if the immovable is a building
+
function is_building(immovable)
return immovable.descr.type_name == "productionsite" or
immovable.descr.type_name == "warehouse" or
___
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
