Merge lp:~qcumber-some/widelands/bugfix-768854 into lp:widelands

[Jens Beyer]

Bugfix for bug768854

I hope I didn't miss a function. Problem seems to be that lua does not translate from within inner (local) functions if you don't set the textdomain again inside that function.

Please review and consider other lua scripts as well where this may be a problem.

[SirVer (sirver)]

I think this solution is dangerous. What it does is changing the textdomain which will persist for all other lua functions called later on. This could mean that some other text is then not translated. I suggest moving all static text out of the inner functions into the global lua namespace so that there are translated only once on loading. This is similar to how it is done in the scenarios (where we often have a text.lua script that contains all the static text).

Could you try that out Jens?

review: Needs Fixing

[Jens Beyer (qcumber-some)]

> I think this solution is dangerous. What it does is changing the textdomain
> which will persist for all other lua functions called later on. This could
> mean that some other text is then not translated. I suggest moving all static
> text out of the inner functions into the global lua namespace so that there
> are translated only once on loading. This is similar to how it is done in the
> scenarios (where we often have a text.lua script that contains all the static
> text).
>
> Could you try that out Jens?

Fixed as requested. From what I found this works, hope I understood correctly what to do ;-)

Please review again.

[Jens Beyer (qcumber-some)]

To add, I've pushed the branch again, it does not contain the 'old' commit.

review: Needs Resubmitting

[SirVer (sirver)]

Looking good from a translator point of views. All strings should now be properly translated when the game starts. However, by skimming over I saw that in some logic formated strings are used, e.g.

if candidateisteam and currentcandidate == game_status_territoral_lord.team:format(p.team)

This will break the game logic when a player saves a game, switches locale and reloads. Even worse: when someone attaches a savegame to a bug we need to load it with the same locale or the logic fails. This needs fixing.

review: Needs Fixing

[Jens Beyer (qcumber-some)]

> Looking good from a translator point of views. All strings should now be
> properly translated when the game starts. However, by skimming over I saw that
> in some logic formated strings are used, e.g.
>
> if candidateisteam and currentcandidate ==
> game_status_territoral_lord.team:format(p.team)
>
> This will break the game logic when a player saves a game, switches locale and
> reloads. Even worse: when someone attaches a savegame to a bug we need to load
> it with the same locale or the logic fails. This needs fixing.

This also made me sceptical when I encountered that, but I couldn't pinpoint why, so I left it the way it was, because this already had been the way before, hasn't it?

This is the line you mentioned before my fix:

if candidateisteam and currentcandidate == _("Team %i"):format(p.team)

Isn't this the same problem? If so, I'd vote for keeping my changes to fix this bug and create a new bug addressing game logic breakage. I suspect I'm not fit enough in Lua to fix this.

[SirVer (sirver)]

>> This will break the game logic when a player saves a game, switches locale and
>> reloads. Even worse: when someone attaches a savegame to a bug we need to load
>> it with the same locale or the logic fails. This needs fixing.
>
>This also made me sceptical when I encountered that, but I couldn't pinpoint why, so I left it the way it was, because this already had been the way before, hasn't it?
>
>This is the line you mentioned before my fix:
>
>if candidateisteam and currentcandidate == _("Team %i"):format(p.team)
>
>Isn't this the same problem? If so, I'd vote for keeping my changes to fix this bug and create a new bug addressing game logic breakage. I suspect I'm not fit enough in Lua to fix this.

Yes that is the very same problem. This should not have slipped by :/.
I agree then, a bug report of its own is the way to go. I will merge
your work soonish (maybe tomorrow).

[David Allwicher (aber)]

qcumber-some is also member of widelands-dev ;)

[SirVer (sirver)]

Ooh shit... Jens, I dropped the ball on this and now there are some merge conflicts! Could you look over this branch again and resubmit. I am deeply sorry and I owe you a whisky for that.

Also... have we already opened the bug report for using translated strings in logic or have we not?

review: Needs Fixing

[Jens Beyer (qcumber-some)]

> Ooh shit... Jens, I dropped the ball on this and now there are some merge
> conflicts! Could you look over this branch again and resubmit. I am deeply
> sorry and I owe you a whisky for that.
>
> Also... have we already opened the bug report for using translated strings in
> logic or have we not?

I did a "manual merge" (read: reimplement) the same directly on trunk. Setting this branch to merged anyway.

=== added file 'scripting/win_condition_texts.lua'
--- scripting/win_condition_texts.lua	1970-01-01 00:00:00 +0000
+++ scripting/win_condition_texts.lua	2012-01-09 00:57:27 +0000
@@ -0,0 +1,50 @@
+won_game = {
+  title = _ "Congratulations!",
+  body = _ "You have won this game!"
+}
+
+lost_game = {
+  title = _ "You are defeated!",
+  body = _ "You have nothing to command left. If you want, you may continue as spectator."
+}
+
+won_game_over = {
+  title = _ "You won",
+  body = _"You are the winner!"
+}
+
+lost_game_over = {
+  title = _ "You lost",
+  body = _"You've lost this game!"
+}
+
+game_status = {
+  title = _ "Status",
+  body = _ "Player overview:"
+}
+
+game_status_woodgnome = {
+  end_in = _ "The game will end in %i minutes.",
+  trees = _ "%s has %i trees at the moment.",
+  overview = _ "Player overview:",
+  had1 = _ "%s had ",
+  had2 = _ "%i trees.",
+  winner1 = _ "The winner is %s ",
+  winner2 = _ "with %i trees.",
+  owned = _ "Trees owned"
+}
+
+game_status_collectors = {
+  title = _ "Status for %s<br>",
+  total = _ "Total: %i points",
+  end_in = _ "The game will end in %s.",
+  points = _ "Points"
+}
+
+game_status_territoral_lord = {
+  team = _ "Team %i",
+  other1 = _ "%s owns more than half of the maps area.",
+  other2 = _ "You still got %i minutes to prevent a victory.",
+  player1 = _ "You own more than half of the maps area.",
+  player2 = _ "Keep it for %i more minutes to win the game."
+}
\ No newline at end of file

=== modified file 'scripting/win_conditions/00_endless_game.lua'
--- scripting/win_conditions/00_endless_game.lua	2010-09-12 11:17:13 +0000
+++ scripting/win_conditions/00_endless_game.lua	2012-01-09 00:57:27 +0000
@@ -7,6 +7,8 @@
 
 set_textdomain("win_conditions")
 
+use("aux", "win_condition_texts")
+
 local wc_name = _ "Endless Game"
 local wc_desc = _"This is an endless game without rules."
 return {
@@ -21,9 +23,7 @@
 		-- from the list, send him a defeated message and give him full vision
 		repeat
 			sleep(5000)
-			check_player_defeated(plrs, _ "You are defeated!",
-				_ ("You have nothing to command left. If you want, you may " ..
-				   "continue as spectator."))
+			check_player_defeated(plrs, lost_game.title, lost_game.body)
 		until count_factions(plrs) < 1
 	
 	end

=== modified file 'scripting/win_conditions/01_defeat_all.lua'
--- scripting/win_conditions/01_defeat_all.lua	2010-09-12 11:17:13 +0000
+++ scripting/win_conditions/01_defeat_all.lua	2012-01-09 00:57:27 +0000
@@ -7,6 +7,8 @@
 
 set_textdomain("win_conditions")
 
+use("aux", "win_condition_texts")
+
 local wc_name = _ "Autocrat"
 local wc_desc = _ "The tribe or team that can defeat all others wins the game!"
 return {
@@ -21,15 +23,11 @@
 		-- from the list, send him a defeated message and give him full vision
 		repeat
 			sleep(5000)
-			check_player_defeated(plrs, _ "You are defeated!",
-				_ ("You have nothing to command left. If you want, you may " ..
-				   "continue as spectator."))
+			check_player_defeated(plrs, lost_game.title, lost_game.body)
 		until count_factions(plrs) <= 1
 
 		-- Send congratulations to all remaining players
-		broadcast(plrs, 
-				_ "Congratulations!",
-				_ "You have won this game!",
+		broadcast(plrs, won_game.title, won_game.body,
 				{popup = true}
 		)
 

=== modified file 'scripting/win_conditions/02_collectors.lua'
--- scripting/win_conditions/02_collectors.lua	2011-11-06 00:06:25 +0000
+++ scripting/win_conditions/02_collectors.lua	2012-01-09 00:57:27 +0000
@@ -8,6 +8,8 @@
 
 set_textdomain("win_conditions")
 
+use("aux", "win_condition_texts")
+
 local wc_name = _ "Collectors"
 local wc_desc = _ (
 	"You get points for precious wares in your warehouses, the player with " .. 
@@ -87,7 +89,7 @@
 	)
 
 	local points = 0
-	local descr = { _("Status for %s<br>"):format(plr.name) }
+	local descr = { game_status_collectors.title:format(plr.name) }
 	for idx, ware in ipairs(point_table[plr.tribe_name .. "_order"]) do
 		local value = point_table[plr.tribe_name][ware]
 		local count = 0
@@ -100,7 +102,7 @@
 			ware, value, count, lpoints
 		)
 	end
-	descr[#descr+1] = _("Total: %i points"):format(points)
+	descr[#descr+1] = game_status_collectors.total:format(points)
 
 	return points, p(table.concat(descr, "\n"))
 end
@@ -115,7 +117,7 @@
 	else
 		time = ("%i minutes"):format(m)
 	end
-	local msg = p(_"The game will end in %s."):format(time)
+	local msg = p(game_status_collectors.end_in):format(time)
 	msg = msg .. "\n\n"
 
 	for idx, plr in ipairs(plrs) do
@@ -125,7 +127,7 @@
 	end
 
 	for idx, plr in ipairs(plrs) do
-		plr:send_message(_ "Status", "<rt>" .. msg .. "</rt>")
+		plr:send_message(game_status.title, "<rt>" .. msg .. "</rt>")
 	end
 end
 
@@ -136,15 +138,15 @@
 	end
 	table.sort(points, function(a,b) return a[2] < b[2] end)
 	for i=1,#points-1 do
-		points[i][1]:send_message(_"You lost", _"You've lost this game!")
+		points[i][1]:send_message(lost_game_over.title, lost_game_over.body)
 	end
-	points[#points][1]:send_message(_"You won!", _"You are the winner!")
+	points[#points][1]:send_message(won_game_over.title, won_game_over.body)
 end
 
 -- Instantiate the hook to calculate points
 if hooks == nil then hooks = {} end
 hooks.custom_statistic = {
-	name = _ "Points",
+	name = game_status_collectors.points,
 	pic = "pics/genstats_points.png",
 	calculator = function(p) 
 		local pts = _calc_points(p)
@@ -176,9 +178,7 @@
 	local runs = 0
 	repeat
 		sleep(5000)
-		check_player_defeated(plrs, _ "You are defeated!",
-			_ ("You have nothing to command left. If you want, you may " ..
-			   "continue as spectator."))
+		check_player_defeated(plrs, lost_game.title, lost_game.body)
 		runs = runs + 1
 	until runs >= 120 -- 120 * 5000ms = 600000 ms = 10 minutes
 	remaining_time = remaining_time - 10

=== modified file 'scripting/win_conditions/03_territorial_lord.lua'
--- scripting/win_conditions/03_territorial_lord.lua	2010-11-12 14:16:36 +0000
+++ scripting/win_conditions/03_territorial_lord.lua	2012-01-09 00:57:27 +0000
@@ -8,6 +8,8 @@
 
 set_textdomain("win_conditions")
 
+use("aux", "win_condition_texts")
+
 local wc_name = _ "Territorial Lord"
 local wc_desc = _ (
 	"Each player or team tries to obtain more than half of the maps' " ..
@@ -126,10 +128,10 @@
 					if teampoints[t] > ( #fields / 2 ) then
 						-- this team owns more than half of the map's area
 						foundcandidate = true
-						if candidateisteam == true and currentcandidate == _("Team %i"):format(t) then
+						if candidateisteam == true and currentcandidate == game_status_territoral_lord.team:format(t) then
 							remaining_time = remaining_time - 30
 						else
-							currentcandidate = _("Team %i"):format(t)
+							currentcandidate = game_status_territoral_lord.team:format(t)
 							candidateisteam = true
 							remaining_time = 20 * 60 -- 20 minutes
 						end
@@ -142,20 +144,20 @@
 		end
 
 		function _send_state()
-			local msg1 = _("%s owns more than half of the maps area."):format(currentcandidate)
+			local msg1 = game_status_territoral_lord.other1:format(currentcandidate)
 			msg1 = msg1 .. "\n"
-			msg1 = msg1 .. _("You still got %i minutes to prevent a victory."):format(remaining_time / 60)
+			msg1 = msg1 .. game_status_territoral_lord.other2:format(remaining_time / 60)
 
-			local msg2 = _("You own more than half of the maps area.")
+			local msg2 = game_status_territoral_lord.player1
 			msg2 = msg2 .. "\n"
-			msg2 = msg2 .. _("Keep it for %i more minutes to win the game."):format(remaining_time / 60)
+			msg2 = msg2 .. game_status_territoral_lord.player2:format(remaining_time / 60)
 
 			for idx, p in ipairs(plrs) do
-				if candidateisteam and currentcandidate == _("Team %i"):format(p.team)
+				if candidateisteam and currentcandidate == game_status_territoral_lord.team:format(p.team)
 					or not candidateisteam and currentcandidate == p.name then
-					p:send_message(_ "Status", msg2, {popup = true})
+					p:send_message(game_status.title, msg2, {popup = true})
 				else
-					p:send_message(_ "Status", msg1, {popup = true})
+					p:send_message(game_status.title, msg1, {popup = true})
 				end
 			end
 		end
@@ -163,9 +165,7 @@
 		-- Start a new coroutine that checks for defeated players
 		run(function()
 			sleep(5000)
-			check_player_defeated(plrs, _ "You are defeated!",
-				_ ("You have nothing to command left. If you want, you may " ..
-		         "continue as spectator."))
+			check_player_defeated(plrs, lost_game.title, lost_game.body)
 		end)
 
 		-- here is the main loop!!!
@@ -180,11 +180,11 @@
 			if remaining_time == 0 then
 				for idx, p in ipairs(plrs) do
 					p.see_all = 1
-					if candidateisteam and currentcandidate == _("Team %i"):format(p.team)
+					if candidateisteam and currentcandidate == game_status_territoral_lord.team:format(p.team)
 						or not candidateisteam and currentcandidate == p.name then
-						p:send_message(_"You won!", _"You are the winner!", {popup = true})
+						p:send_message(won_game_over.title, won_game_over.body, {popup = true})
 					else
-						p:send_message(_"You lost", _"You've lost this game!", {popup = true})
+						p:send_message(lost_game_over.title, lost_game_over.body, {popup = true})
 					end
 				end
 				break

=== modified file 'scripting/win_conditions/04_wood_gnome.lua'
--- scripting/win_conditions/04_wood_gnome.lua	2011-11-06 00:06:25 +0000
+++ scripting/win_conditions/04_wood_gnome.lua	2012-01-09 00:57:27 +0000
@@ -8,6 +8,8 @@
 
 set_textdomain("win_conditions")
 
+use("aux", "win_condition_texts")
+
 local wc_name = _ "Wood Gnome"
 local wc_desc = _
 [[As wood gnome you like big forests, so your task is, to have more trees on
@@ -76,29 +78,27 @@
 
 	local function _send_state()
 		local playerpoints = _calc_points()
-		local msg = _("The game will end in %i minutes."):format(remaining_time)
+		local msg = game_status.body:format(remaining_time)
 		msg = msg .. "\n\n"
-		msg = msg .. _ ("Player overview:")
+		msg = msg .. game_status_woodgnome.end_in:format(remaining_time)
 		for idx,plr in ipairs(plrs) do
 			msg = msg .. "\n"
-			msg = msg .. _ ("%s has %i trees at the moment."):format(plr.name, playerpoints[plr.number])
+			msg = msg .. game_status_woodgnome.trees:format(plr.name, playerpoints[plr.number])
 		end
 
-		broadcast(plrs, _ "Status", msg)
+		broadcast(plrs, game_status.title, msg)
 	end
 
 	-- Start a new coroutine that checks for defeated players
 	run(function()
 		sleep(5000)
-		check_player_defeated(plrs, _ "You are defeated!",
-			_ ("You have nothing to command left. If you want, you may " ..
-				"continue as spectator."))
+		check_player_defeated(plrs, lost_game.title, lost_game.body)
 	end)
 
 	-- Install statistics hook
 	if hooks == nil then hooks = {} end
 	hooks.custom_statistic = {
-		name = _ "Trees owned",
+		name = game_status_woodgnome.owned,
 		pic = "pics/genstats_trees.png",
 		calculator = function(p) 
 			local pts = _calc_points(p)
@@ -130,23 +130,23 @@
 	table.sort(points, function(a,b) return a[2] < b[2] end)
 
 	local msg = "\n\n"
-	msg = msg .. _ ("Player overview:")
+	msg = msg .. game_status_woodgnome.overview
 	for idx,plr in ipairs(plrs) do
 		msg = msg .. "\n"
-		msg = msg .. _ ("%s had "):format(plr.name)
-		msg = msg .. _ ("%i trees."):format(playerpoints[plr.number])
+		msg = msg .. game_status_woodgnome.had1:format(plr.name)
+		msg = msg .. game_status_woodgnome.had2:format(playerpoints[plr.number])
 	end
 	msg = msg .. "\n\n"
-	msg = msg .. _ ("The winner is %s "):format(points[#points][1].name)
-	msg = msg .. _ ("with %i trees."):format(playerpoints[points[#points][1].number])
+	msg = msg .. game_status_woodgnome.winner1:format(points[#points][1].name)
+	msg = msg .. game_status_woodgnome.winner2:format(playerpoints[points[#points][1].number])
 	local privmsg = ""
 	for i=1,#points-1 do
-		privmsg = _ ("You have lost this game!")
+		privmsg = lost_game_over.body
 		privmsg = privmsg .. msg
-		points[i][1]:send_message(_"You lost!", privmsg, {popup = true})
+		points[i][1]:send_message(lost_game_over.title, privmsg, {popup = true})
 	end
-	privmsg = _ ("You are the winner of this game!")
+	privmsg = won_game_over.body
 	privmsg = privmsg .. msg
-	points[#points][1]:send_message(_"You won!", privmsg, {popup = true})
+	points[#points][1]:send_message(won_game_over.title, privmsg, {popup = true})
 end
 }

=== modified file 'scripting/win_conditions/05_endless_game_fogless.lua'
--- scripting/win_conditions/05_endless_game_fogless.lua	2011-08-21 12:02:42 +0000
+++ scripting/win_conditions/05_endless_game_fogless.lua	2012-01-09 00:57:27 +0000
@@ -7,6 +7,8 @@
 
 set_textdomain("win_conditions")
 
+use("aux", "win_condition_texts")
+
 local wc_name = _ "Endless Game (no fog)"
 local wc_desc = _"This is an endless game without rules. Fog of war is disabled."
 return {
@@ -36,9 +38,7 @@
 		-- from the list, send him a defeated message and give him full vision
 		repeat
 			sleep(5000)
-			check_player_defeated(plrs, _ "You are defeated!",
-				_ ("You have nothing to command left. If you want, you may " ..
-				   "continue as spectator."))
+			check_player_defeated(plrs, lost_game.title, lost_game.body)
 		until count_factions(plrs) < 1
 
 	end