← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/bug-1731652-worker-plant-attribute into lp:widelands

 

Review: Approve

Code LGTM, just one small question in diff.

Diff comments:

> 
> === added file 'data/scripting/help.lua'
> --- data/scripting/help.lua	1970-01-01 00:00:00 +0000
> +++ data/scripting/help.lua	2017-12-03 10:26:49 +0000
> @@ -0,0 +1,54 @@
> +-- RST
> +-- help.lua
> +-- --------
> +--
> +-- This script contains functions that are used both in the Tribal Encyclopedia
> +-- and the Editor help.
> +
> +include "scripting/formatting.lua"
> +
> +
> +-- RST
> +-- .. function:: terrain_affinity_help(immovable_description)
> +--
> +--    Returns a formatted help string for the terrains that the given
> +--    immovable is most likely to grow on.
> +--
> +--    :arg immovable_description: The immovable type that we want the information for.
> +--    :type immovable_description: :class:`LuaImmovableDescription`
> +--    :returns: a richtext-formatted list of terrain images, terrain names and probabilities.
> +--
> +function terrain_affinity_help(immovable_description)
> +   set_textdomain("widelands")
> +   local world = wl.World();
> +   local result = ""
> +   terrain_list = {}
> +   for i, terrain in ipairs(world.terrain_descriptions) do
> +      local probability = immovable_description:probability_to_grow(terrain)
> +      if (probability > 0.01) then
> +         -- sort the terrains by percentage
> +         i = 1
> +         while (terrain_list[i] and (terrain_list[i].probability > probability)) do
> +            i = i + 1
> +         end
> +
> +         for j = #terrain_list, i, -1 do
> +            terrain_list[j+1] = terrain_list[j]
> +         end
> +         terrain_list[i] = {terrain = terrain, probability = probability}
> +      end
> +   end
> +
> +   for k,v in ipairs(terrain_list) do
> +      if (k <= 10 or v.probability > 0.25) then

Perhaps you meant ' and ' above.... Is it possible that there are immovables that dont have at least one terrain with probability 25% and more?

> +         result = result .. picture_li(v.terrain.representative_image,
> +               -- TRANSLATORS: Terrain name (Climate)
> +               (_"%1% (%2%)"):bformat(v.terrain.descname, v.terrain.editor_category.descname) .. "<br>" ..
> +               -- TRANSLATORS: Help text - Probability to grow for an immovable
> +               (_("%2.1f%%")):bformat(100 * v.probability)
> +            ) .. spacer()
> +      end
> +   end
> +   return result
> +end
> +


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1731652-worker-plant-attribute/+merge/333934
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1731652-worker-plant-attribute.


References