widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing

The conditional statement can be more effective - see the diff comments.

You will also need to merge trunk to make Travis happy.

Diff comments:

> === modified file 'data/scripting/win_conditions/collectors.lua'
> --- data/scripting/win_conditions/collectors.lua	2016-09-12 09:57:16 +0000
> +++ data/scripting/win_conditions/collectors.lua	2016-11-29 20:59:38 +0000
> @@ -135,25 +135,31 @@
>  
>     -- Send all players the momentary game state
>     local function _send_state(remaining_time, plrs)
> -      set_textdomain("win_conditions")
> -      local h = math.floor(remaining_time / 60)
> -      local m = remaining_time % 60
> -      -- TRANSLATORS: Context: 'The game will end in (2 hours and) 30 minutes.'
> -      local time = ""
> -      if m > 0 then
> -         time = (ngettext("%i minute", "%i minutes", m)):format(m)
> -      end
> -      if h > 0 then
> +      local msg = ""
> +      if remaining_time <= 0 then
> +         set_textdomain("widelands")
> +         msg = p(_"Game over")

This is a nice thought, but our gettext string collection script will add this to the win_conditions textdomain anyway, so we can have just 1 set_textdomain("win_conditions") call.

> +      else
> +         set_textdomain("win_conditions")
> +         local h = math.floor(remaining_time / 60)
> +         local m = remaining_time % 60
> +         -- TRANSLATORS: Context: 'The game will end in (2 hours and) 30 minutes.'
> +         local time = ""
>           if m > 0 then

Shift this down as an "else if" after "if h > 0 then" - no need to execute this if h > 0. The "game over" can be the final else then.

> -            -- TRANSLATORS: Context: 'The game will end in 2 hours and 30 minutes.'
> -            time = (ngettext("%1% hour and %2%", "%1% hours and %2%", h)):bformat(h, time)
> -         else
> -            -- TRANSLATORS: Context: 'The game will end in 2 hours.'
> -            time = (ngettext("%1% hour", "%1% hours", h)):bformat(h)
> -         end
> +            time = (ngettext("%i minute", "%i minutes", m)):format(m)
> +         end
> +         if h > 0 then
> +            if m > 0 then
> +               -- TRANSLATORS: Context: 'The game will end in 2 hours and 30 minutes.'
> +               time = (ngettext("%1% hour and %2%", "%1% hours and %2%", h)):bformat(h, time)
> +            else
> +               -- TRANSLATORS: Context: 'The game will end in 2 hours.'
> +               time = (ngettext("%1% hour", "%1% hours", h)):bformat(h)
> +            end
> +         end
> +         -- TRANSLATORS: Context: 'The game will end in 2 hours and 30 minutes.'
> +         msg = p(_"The game will end in %s."):format(time)
>        end
> -      -- TRANSLATORS: Context: 'The game will end in 2 hours and 30 minutes.'
> -      local msg = p(_"The game will end in %s."):format(time)
>  
>        -- Points for players without team
>        for idx, plr in ipairs(plrs) do


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-collectors/+merge/312092
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-collectors.