widelands-dev team mailing list archive

Thread
Date
Re: [Merge] lp:~widelands-dev/widelands/bug-1322473 into lp:widelands

To: mp+221235@xxxxxxxxxxxxxxxxxx
From: SirVer <SirVer@xxxxxx>
Date: Mon, 02 Jun 2014 05:47:54 -0000
In-reply-to: <20140528134335.30494.57942.launchpad@ackee.canonical.com>
Reply-to: mp+221235@xxxxxxxxxxxxxxxxxx
Sender: bounces@xxxxxxxxxxxxx
Review: Needs Fixing

some comments

Diff comments:

> === modified file 'maps/MP Scenarios/Island Hopping.wmf/scripting/multiplayer_init.lua'
> --- maps/MP Scenarios/Island Hopping.wmf/scripting/multiplayer_init.lua	2014-03-25 06:18:48 +0000
> +++ maps/MP Scenarios/Island Hopping.wmf/scripting/multiplayer_init.lua	2014-05-28 13:43:10 +0000
> @@ -239,6 +239,9 @@
>     disable_unused_buildings()
>  
>     send_to_all(welcome_msg)

should this not be deleted then?

> +   -- set the objective with the game type for all players
> +   -- TODO change this to a broadcast once individual game objectives have been implementes

implementes -> implemented

> +   game.players[1]:add_objective("win_conditions", _"Rules", welcome_msg)
>  
>     for idx,plr in ipairs(game.players) do
>        run(function() run_island(plr, 1) end)
> 
> === modified file 'maps/MP Scenarios/Smugglers.wmf/scripting/multiplayer_init.lua'
> --- maps/MP Scenarios/Smugglers.wmf/scripting/multiplayer_init.lua	2014-03-25 06:18:48 +0000
> +++ maps/MP Scenarios/Smugglers.wmf/scripting/multiplayer_init.lua	2014-05-28 13:43:10 +0000
> @@ -140,7 +140,9 @@
>     end
>  
>     send_to_all(welcome_msg:format((ngettext("%i point", "%i points", points_to_win)):format(points_to_win)))
> -
> +   -- set the objective with the game type for all players
> +   -- TODO change this to a broadcast once individual game objectives have been implementes
> +   game.players[1]:add_objective("win_conditions", _"Rules", welcome_msg:format((ngettext("%i point", "%i points", points_to_win)):format(points_to_win)))
>  
>     for idx,descr in ipairs(route_descrs) do
>        run(wait_for_established_route, descr)
> 
> === modified file 'maps/Plateau.wmf/scripting/init.lua'
> --- maps/Plateau.wmf/scripting/init.lua	2014-03-25 06:18:48 +0000
> +++ maps/Plateau.wmf/scripting/init.lua	2014-05-28 13:43:10 +0000
> @@ -22,6 +22,7 @@
>     sleep(300)
>  
>     send_msg(briefing_1_the_forbidden_island)
> +   local o = add_obj(obj_forbidden_island)
>  
>     local map = wl.Game().map
>     while not p1:seen_field(map:get_field(5,8)) do sleep(2345) end
> @@ -47,7 +48,8 @@
>     scroll_smoothly_to(castle)
>  
>     send_msg(briefing_2_found_ancient_castle)
> -   local o = add_obj(obj_capture_ancient_castle)
> +   o.done = true

o.done = true? is that correct here?

> +   o = add_obj(obj_capture_ancient_castle)
>  
>     -- Wait till we conquered the castle
>     while #p1:get_buildings"castle.atlanteans" < 1 do sleep(2345) end
> 
> === modified file 'maps/Plateau.wmf/scripting/texts.lua'
> --- maps/Plateau.wmf/scripting/texts.lua	2014-04-09 17:50:02 +0000
> +++ maps/Plateau.wmf/scripting/texts.lua	2014-05-28 13:43:10 +0000
> @@ -23,6 +23,19 @@
>     return p1:add_objective(o.name, o.title, _obj(reflow(o.body)), o)
>  end
>  
> +obj_forbidden_island = {
> +   name = "forbidden_island",
> +   title =_"The Forbidden Island",

be consistent with spaces. We use '= _"text"' in most places I think, you sometimes use '=_' and sometimes use '= _ "'

> +   body = _(
> +[[Finally! We have just taken our first step towards victory! ]] ..

Didn't you want to make one line per sentence and tag them individually for translation?

> +[[Last night, we landed on the forbidden island and defeated the few guards that were watching the ]] ..
> +[[north-western part of the island. ]] ..
> +[[I am quite sure that none of the other warlords has detected us so far, ]] ..
> +[[so we should keep quiet and build up our infrastructure. Soon we will be strong enough to raid their positions.]]),
> +}
> +
> +
> +
>  obj_capture_ancient_castle = {
>     name = "capture_ancient_castle",
>     title =_ "Capture the Ancient Castle",
> 
> === modified file 'scripting/win_condition_functions.lua'
> --- scripting/win_condition_functions.lua	2013-08-07 16:58:48 +0000
> +++ scripting/win_condition_functions.lua	2014-05-28 13:43:10 +0000
> @@ -125,3 +125,19 @@
>         wl.game.report_result(p, 0, make_extra_data(p, wc_name, wc_ver, extra))
>     end
>  end
> +
> +
> +-- RST
> +-- .. function:: broadcast_objective(plrs, header, msg, body)
> +--
> +--    :arg plrs:    This should be wl.Game().players

no need to pass it in then - you can get that in the method just as well.

> +--    :arg name:    A unique name for the objective

I would probably just pass in the table - we keep the data of objectives around in tables of the form anyways. Your call.

> +--    :arg title:   The title to be displayed for the objective
> +--    :arg body:    The content text to be displayed for the objective
> +--
> +--    broadcast an objective to all players

The description usually comes before the arguments.

> +--    technically, it is assigned to player1, because all players will see all objectives
> +function broadcast_objective(plrs, name, title, body)
> +	plrs[1]:add_objective(name, title, body)
> +end
> +
> 
> === modified file 'scripting/win_conditions/00_endless_game.lua'
> --- scripting/win_conditions/00_endless_game.lua	2014-03-25 06:18:48 +0000
> +++ scripting/win_conditions/00_endless_game.lua	2014-05-28 13:43:10 +0000
> @@ -18,7 +18,8 @@
>  	func = function()
>  		local plrs = wl.Game().players
>  
> -		broadcast(plrs, wc_name, wc_desc)
> +		-- set the objective with the game type for all players
> +		broadcast_objective(plrs, "win_condition", wc_name, wc_desc)
>  
>  		-- Iterate all players, if one is defeated, remove him
>  		-- from the list, send him a defeated message and give him full vision
> 
> === modified file 'scripting/win_conditions/01_defeat_all.lua'
> --- scripting/win_conditions/01_defeat_all.lua	2014-03-25 06:18:48 +0000
> +++ scripting/win_conditions/01_defeat_all.lua	2014-05-28 13:43:10 +0000
> @@ -18,7 +18,8 @@
>  	func = function()
>  		local plrs = wl.Game().players
>  
> -		broadcast(plrs, wc_name, wc_desc)
> +		-- set the objective with the game type for all players
> +		broadcast_objective(plrs, "win_condition", wc_name, wc_desc)
>  
>  		-- Iterate all players, if one is defeated, remove him
>  		-- from the list, send him a defeated message and give him full vision
> 
> === modified file 'scripting/win_conditions/02_collectors.lua'
> --- scripting/win_conditions/02_collectors.lua	2014-03-25 06:18:48 +0000
> +++ scripting/win_conditions/02_collectors.lua	2014-05-28 13:43:10 +0000
> @@ -22,6 +22,10 @@
>  	name = wc_name,
>  	description = wc_desc,
>  	func = function()
> +		local plrs = wl.Game().players
> +
> +		-- set the objective with the game type for all players
> +		broadcast_objective(plrs, "win_condition", wc_name, wc_desc)
>  
>  -- Simple flowing text. One Paragraph
>  local function p(s)
> @@ -167,12 +171,6 @@
>  sleep(1000)
>  
>  local remaining_time = 60 * 4
> -local plrs = wl.Game().players
> -
> --- send a message with the game type to all players
> -for idx, p in ipairs(plrs) do
> -	p:send_message(wc_name, wc_desc)
> -end
>  
>  -- Endless loop
>  while true do
> 
> === modified file 'scripting/win_conditions/03_territorial_lord.lua'
> --- scripting/win_conditions/03_territorial_lord.lua	2014-03-25 06:18:48 +0000
> +++ scripting/win_conditions/03_territorial_lord.lua	2014-05-28 13:43:10 +0000
> @@ -21,6 +21,10 @@
>  	name = wc_name,
>  	description = wc_desc,
>  	func = function()
> +		local plrs = wl.Game().players
> +
> +		-- set the objective with the game type for all players
> +		broadcast_objective(plrs, "win_condition", wc_name, wc_desc)
>  
>  		-- Get all valueable fields of the map
>  		local fields = {}
> @@ -50,12 +54,6 @@
>  		local candidateisteam = false
>  		local remaining_time = 10 -- (dummy) -- time in secs, if == 0 -> victory
>  
> -		-- Find all valid players
> -      local plrs = wl.Game().players
> -
> -		-- send a message with the game type to all players
> -		broadcast(plrs, wc_name, wc_desc)
> -
>  		-- Find all valid teams
>  		local teamnumbers = {} -- array with team numbers
>  		for idx,p in ipairs(plrs) do
> 
> === modified file 'scripting/win_conditions/03_territorial_time.lua'
> --- scripting/win_conditions/03_territorial_time.lua	2014-03-25 06:18:48 +0000
> +++ scripting/win_conditions/03_territorial_time.lua	2014-05-28 13:43:10 +0000
> @@ -26,6 +26,10 @@
>  	name = wc_name,
>  	description = wc_desc,
>  	func = function()
> +		local plrs = wl.Game().players
> +
> +		-- set the objective with the game type for all players
> +		broadcast_objective(plrs, "win_condition", wc_name, wc_desc)
>  
>  		-- Get all valueable fields of the map
>  		local fields = {}
> @@ -58,12 +62,6 @@
>  		local candidateisteam = false
>  		local remaining_time = 10 -- (dummy) -- time in secs, if == 0 -> victory
>  
> -		-- Find all valid players
> -      local plrs = wl.Game().players
> -
> -		-- send a message with the game type to all players
> -		broadcast(plrs, wc_name, wc_desc)
> -
>  		-- Find all valid teams
>  		local teamnumbers = {} -- array with team numbers
>  		for idx,p in ipairs(plrs) do
> 
> === modified file 'scripting/win_conditions/04_wood_gnome.lua'
> --- scripting/win_conditions/04_wood_gnome.lua	2014-03-25 06:18:48 +0000
> +++ scripting/win_conditions/04_wood_gnome.lua	2014-05-28 13:43:10 +0000
> @@ -21,10 +21,10 @@
>  	name = wc_name,
>  	description = wc_desc,
>  	func = function()
> -	local plrs = wl.Game().players
> +		local plrs = wl.Game().players

indentation?

>  
> -	-- send a message with the game type to all players
> -	broadcast(plrs, wc_name, wc_desc)
> +		-- set the objective with the game type for all players

indentation?

> +		broadcast_objective(plrs, "win_condition", wc_name, wc_desc)
>  
>  	local remaining_time = 4 * 60 -- 4 hours
>  
> 
> === modified file 'scripting/win_conditions/05_endless_game_fogless.lua'
> --- scripting/win_conditions/05_endless_game_fogless.lua	2014-03-25 06:18:48 +0000
> +++ scripting/win_conditions/05_endless_game_fogless.lua	2014-05-28 13:43:10 +0000
> @@ -18,7 +18,8 @@
>  	func = function()
>  		local plrs = wl.Game().players
>  
> -		broadcast(plrs, wc_name, wc_desc)
> +		-- set the objective with the game type for all players
> +		broadcast_objective(plrs, "win_condition", wc_name, wc_desc)
>  
>  		-- reveal the whole map for every player
>  		local game = wl.Game()
> 
> === modified file 'src/logic/cmd_queue.h'
> --- src/logic/cmd_queue.h	2014-05-11 12:29:55 +0000
> +++ src/logic/cmd_queue.h	2014-05-28 13:43:10 +0000
> @@ -22,6 +22,7 @@
>  
>  #include <memory>
>  #include <queue>
> +#include <stdint.h>
>  
>  #include "logic/queue_cmd_ids.h"
>  
> 
> === modified file 'src/wui/interactive_player.cc'
> --- src/wui/interactive_player.cc	2014-05-27 11:01:15 +0000
> +++ src/wui/interactive_player.cc	2014-05-28 13:43:10 +0000
> @@ -155,10 +155,7 @@
>  	}
>  
>  	m_toolbar.add(&m_toggle_help,            UI::Box::AlignLeft);
> -	if (not scenario)
> -		m_toggle_objectives.set_visible(false);
> -	else
> -		m_toolbar.add(&m_toggle_objectives,      UI::Box::AlignLeft);
> +	m_toolbar.add(&m_toggle_objectives,      UI::Box::AlignLeft);
>  	m_toolbar.add(&m_toggle_message_menu,    UI::Box::AlignLeft);
>  
>  	set_player_number(plyn);
> 


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

[Merge] lp:~widelands-dev/widelands/bug-1322473 into lp:widelands
From: GunChleoc, 2014-05-28