[Widelands-dev] [Merge] lp:~widelands-dev/widelands/collectors_notification into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/collectors_notification into lp:widelands has been updated. Commit message changed to: - unified some functions for wood_gnome and collectors - removed duplicated code and moved it to win_condition_functions - notify only every hour, 10 minutes in the last 30 - move _game_over function out of the main loop. - don't show points when game ended by military defeat - main loop ends when time is over or only one faction is left - check every second for defeated players For more details, see: 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/collectors_notification into lp:widelands
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
> +
> +++ data/scripting/win_conditions/collectors.lua 2019-01-02 13:47:40
> +
> @@ -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 +
> +++ data/scripting/win_conditions/win_condition_functions.lua 2019-01-02
> 13:47:40 +
> @@ -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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bar01_fix_reveal_campaign into lp:widelands
Thanks :-) -- https://code.launchpad.net/~widelands-dev/widelands/bar01_fix_reveal_campaign/+merge/361480 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bar01_fix_reveal_campaign. ___ 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/bug-1810062-territorial-calculations into lp:widelands
> but we should have this code as well to provide backwards compatibility for > old maps without having to reedit them in the mapeditor. >From my understanding the code for calculating the fields a player could reach >is executed automatically whenever a map is saved. So loading and resaving all >shipped maps would do the trick and there is no need to edit a map at all. >Except external maps, like we have on the website. Not sure what you meant with 'backwards compatibility'. Anyway, we should discuss this on the bug report. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands. ___ 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/bug-1810062-territorial-calculations into lp:widelands
+1 for calculating these amounts in the editor but we should have this code as well to provide backwards compatibility for old maps without having to reedit them in the mapeditor. as far as I understand this code the algorithm isn't exactly correct as it checks a quadratic pattern whether a building can be build there to calculate accesability but the conquer function is circular so for the following pattern the o's will be falsely declared positive possibilities to reach the center field (c) with a conquer radius of 1. o x x o x c x o x x o -- https://code.launchpad.net/~widelands-dev/widelands/bug-1810062-territorial-calculations/+merge/361366 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1810062-territorial-calculations into lp:widelands. ___ 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/bar01_fix_reveal_campaign into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bar01_fix_reveal_campaign into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bar01_fix_reveal_campaign/+merge/361480 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bar01_fix_reveal_campaign. ___ 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/bar01_fix_reveal_campaign into lp:widelands
Review: Approve Good spot. Yes it seems to be a copy paste error. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bar01_fix_reveal_campaign/+merge/361480 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bar01_fix_reveal_campaign. ___ 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/bar01_fix_reveal_campaign into lp:widelands
Continuous integration builds have changed state: Travis build 4388. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/476741251. Appveyor build 4179. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bar01_fix_reveal_campaign-4179. -- https://code.launchpad.net/~widelands-dev/widelands/bar01_fix_reveal_campaign/+merge/361480 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bar01_fix_reveal_campaign into lp:widelands. ___ 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/bar01_fix_reveal_campaign into lp:widelands
kaputtnik has proposed merging
lp:~widelands-dev/widelands/bar01_fix_reveal_campaign into lp:widelands.
Commit message:
Fixed reveal_campaign in bar_01
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bar01_fix_reveal_campaign/+merge/361480
I guess this was a copy and paste error, introduced in
https://code.launchpad.net/~widelands-dev/widelands/bug-1803602-reveal-frisian-campaign/+merge/358894
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bar01_fix_reveal_campaign into lp:widelands.
=== modified file 'data/campaigns/bar01.wmf/scripting/mission_thread.lua'
--- data/campaigns/bar01.wmf/scripting/mission_thread.lua 2018-11-19 08:09:41 +
+++ data/campaigns/bar01.wmf/scripting/mission_thread.lua 2019-01-08 08:04:32 +
@@ -328,7 +328,7 @@
message_box_objective(plr, msg_mission_complete)
plr:reveal_scenario("barbariantut01")
- p1:reveal_campaign("campsect1")
+ plr:reveal_campaign("campsect1")
end
___
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
