Review: Approve diff, testing Code is looking good and it seems to work as intended. A few possible improvements are in the diff.
I assume it is intentional that there is no message on game start? In the commit/merge message of this branch you could add that wood gnome is also affected. Diff comments: > === modified file 'data/scripting/win_conditions/collectors.lua' > --- data/scripting/win_conditions/collectors.lua 2018-10-13 08:51:51 > +0000 > +++ data/scripting/win_conditions/collectors.lua 2019-01-02 13:47:40 > +0000 > @@ -28,7 +28,11 @@ > > -- set the objective with the game type for all players > broadcast_objective("win_condition", wc_descname, wc_desc) > - > + > + -- set the maximum game time of 4 hours > + local max_time = 4 * 60 > + > + local game = wl.Game() > local plrs = wl.Game().players Suggestion: Use new "game" variable > local teams = {} > for idx,plr in ipairs(plrs) do > @@ -221,10 +199,14 @@ > lost_or_won = 1 > player:send_message(won_game_over.title, won_game_over.body) > end > - if (player.team == 0) then > - wl.game.report_result(player, lost_or_won, > make_extra_data(player, wc_descname, wc_version, {score=info[2]})) > + if (count_factions(plrs) > 1) then Suggestion: Call count_factions() only once before the loop? > + if (player.team == 0) then > + wl.game.report_result(player, lost_or_won, > make_extra_data(player, wc_descname, wc_version, {score=info[2]})) > + else > + wl.game.report_result(player, lost_or_won, > make_extra_data(player, wc_descname, wc_version, {score=info[3], > team_score=info[2]})) > + end > else > - wl.game.report_result(player, lost_or_won, > make_extra_data(player, wc_descname, wc_version, {score=info[3], > team_score=info[2]})) > + wl.game.report_result(player, lost_or_won) > end > end > end > > === modified file 'data/scripting/win_conditions/win_condition_functions.lua' > --- data/scripting/win_conditions/win_condition_functions.lua 2018-09-28 > 17:20:32 +0000 > +++ data/scripting/win_conditions/win_condition_functions.lua 2019-01-02 > 13:47:40 +0000 > @@ -237,3 +237,59 @@ > table.sort(ranked_players_and_teams, function(a,b) return a["points"] > > b["points"] end) > return ranked_players_and_teams > end > + > +-- RST > +-- .. function:: format_remaining_time(remaining_time) > +-- > +-- return a message that contains the remaining game time > +-- to be used when sending status meassages > +-- > +-- :arg remaining_time: The remaining game time in minutes > +function format_remaining_time(remaining_time) > + local h = 0 > + local m = 60 > + local time = "" > + set_textdomain("win_conditions") > + > + if (remaining_time ~= 60) then > + h = math.floor(remaining_time / 60) > + m = remaining_time % 60 > + end > + > + if ((h > 0) and (m > 0)) then > + -- TRANSLATORS: Context: 'The game will end in 2 hours and 30 minutes.' > + time = (ngettext("%1% hour and %2% minutes", "%1% hours and %2% > minutes", h, m)):bformat(h, m) > + elseif m > 0 then > + -- TRANSLATORS: Context: 'The game will end in 30 minutes.' > + time = (ngettext("%i minute", "%i minutes", m)):format(m) > + else > + -- TRANSLATORS: Context: 'The game will end in 2 hours.' > + time = (ngettext("%1% hour", "%1% hours", h)):bformat(h) > + end > + -- TRANSLATORS: Context: 'The game will end in (2 hours and) 30 minutes.' > + return p(_"The game will end in %s."):format(time) > +end > + > +-- RST > +-- .. function:: notification_remaining_time(max_time) > +-- > +-- calculate the remaining game time for notifications > +-- returns the remaining time and whether the notification should popup > +-- to be used when sending status meassages Typo "messages" > +-- status messages are to be send every 30 minutes and every 5 during the > last 30 minutes > +-- the message window pops up ever hour 30, 20 & 10 minutes berfore the > game ends. If I understand it right this method should only be called within coroutines and is blocking them. Add a commend about that to the documentation? Currently it reads as if this method only calculates something. > +-- > +-- :arg max_time: The time maximum game time in minutes > +function notification_remaining_time(max_time, remaining_time) > + local show_popup = false > + if (wl.Game().time < ((max_time - 30) * 60 * 1000)) then -- > + wake_me(wl.Game().time + (30 * 60 * 1000)) -- 30 minutes > + remaining_time = remaining_time - 30 > + if (remaining_time % 60 == 0) or (remaining_time == 30) then > show_popup = true end > + else > + wake_me(wl.Game().time + (300 * 1000)) --5 Minutes > + remaining_time = remaining_time - 5 > + if ((remaining_time ~= 0) and (remaining_time % 10 == 0)) then > show_popup = true end > + end > + return remaining_time, show_popup > +end > \ No newline at end of file -- https://code.launchpad.net/~widelands-dev/widelands/collectors_notification/+merge/361334 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/collectors_notification. _______________________________________________ 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