Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'daze' affliction tooltip says 'defense' instead of 'defence' (English (GB)) #4034

Closed
Konrad22 opened this issue Apr 18, 2019 · 6 comments

Comments

Projects
None yet
4 participants
@Konrad22
Copy link
Contributor

commented Apr 18, 2019

@Wedge009

This comment has been minimized.

Copy link
Member

commented Apr 18, 2019

Problem:
Source - https://github.com/wesnoth/wesnoth/blob/1.14/data/campaigns/Under_the_Burning_Suns/lua/theme.lua#L26
Translation - https://github.com/wesnoth/wesnoth/blob/1.14/po/wesnoth-utbs/en_GB.po#L137

Working example:
Source - https://github.com/wesnoth/wesnoth/blob/1.14/data/campaigns/Under_the_Burning_Suns/utils/abilities.cfg#L119
Translation - https://github.com/wesnoth/wesnoth/blob/1.14/po/wesnoth-utbs/en_GB.po#L16154

Problem is with 1.14.7 release and current 1.14 branch. Is there something different with the way the Lua strings are handled that would make it appear to not honour translation texts?

@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

I'm not aware of any such difference... it's pretty weird that it's not working...

@Wedge009

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

Hence our confusion. At first I thought there might be something wrong with the mo files provided in the release but doing my own build in 1.14 confirms the original (en_US) source text is used and not the translation.

@jostephd

This comment has been minimized.

Copy link
Member

commented May 7, 2019

This patch fixes it:

diff --git a/src/display.cpp b/src/display.cpp
index 5ead295de73..e1023c08774 100644
--- a/src/display.cpp
+++ b/src/display.cpp
@@ -2956,10 +2956,10 @@ void display::refresh_report(const std::string& report_name, const config * new_
                }
 
                skip_element:
-               t = elements.front()["tooltip"].t_str().base_str();
+               t = elements.front()["tooltip"].t_str().c_str();
                if (!t.empty()) {
                        if (!used_ellipsis) {
-                               tooltips::add_tooltip(area, t, elements.front()["help"].t_str().base_str());
+                               tooltips::add_tooltip(area, t, elements.front()["help"].t_str().c_str());
                        } else {
                                // Collect all tooltips for the ellipsis.
                                // TODO: need a better separator

Is it good to commit?

How I found that patch

Works fine in the lua console:

$ wesnoth.textdomain("wesnoth-utbs")("dazed: This unit is dazed. It suffers a -10% penalty to both its defense and chance to hit (except for magical attacks).")
dazed: This unit is dazed. It suffers a -10% penalty to both its defence and chance to hit (except for magical attacks).

But if I use the very same expression in the code:

diff --git a/data/campaigns/Under_the_Burning_Suns/lua/theme.lua b/data/campaigns/Under_the_Burning_Suns/lua/theme.lua
index 75f34789151..c8a2b9e0859 100644
--- a/data/campaigns/Under_the_Burning_Suns/lua/theme.lua
+++ b/data/campaigns/Under_the_Burning_Suns/lua/theme.lua
@@ -23,7 +23,7 @@ function wesnoth.theme_items.unit_status()
 
        if u.status.dazed then
                table.insert(s, { "element", { image = "misc/dazed-status-icon.png",
-                       tooltip = _ "dazed: This unit is dazed. It suffers a -10% penalty to both its defense and chance to hit (except for magical attacks)."
+                       tooltip = wesnoth.textdomain("wesnoth-utbs")("dazed: This unit is dazed. It suffers a -10% penalty to both its defense and chance to hit (except for magical attacks).")
                } } )
        end
 

It still shows "defense" in the sidebar. In fact, even with this patch:

diff --git a/data/campaigns/Under_the_Burning_Suns/lua/theme.lua b/data/campaigns/Under_the_Burning_Suns/lua/theme.lua
index 75f34789151..377a3bb904b 100644
--- a/data/campaigns/Under_the_Burning_Suns/lua/theme.lua
+++ b/data/campaigns/Under_the_Burning_Suns/lua/theme.lua
@@ -1,6 +1,6 @@
--- #textdomain wesnoth-utbs
+-- #textdomain wesnoth-help
 
-local _ = wesnoth.textdomain "wesnoth-utbs"
+local _ = wesnoth.textdomain "wesnoth-help"
 local old_unit_status = wesnoth.theme_items.unit_status
 
 function wesnoth.theme_items.unit_status()
@@ -23,7 +23,7 @@ function wesnoth.theme_items.unit_status()
 
        if u.status.dazed then
                table.insert(s, { "element", { image = "misc/dazed-status-icon.png",
-                       tooltip = _ "dazed: This unit is dazed. It suffers a -10% penalty to both its defense and chance to hit (except for magical attacks)."
+                       tooltip = _ "dextrous"
                } } )
        end
 

the sidebar says "dextrous", not "dexterous" as it should. So it looks like gettext works but the theme always uses the source string.

@jostephd jostephd self-assigned this May 7, 2019

@jostephd jostephd added this to the 1.14.9 milestone May 7, 2019

@Wedge009

This comment has been minimized.

Copy link
Member

commented May 8, 2019

Fascinating story of how you worked it out. Thanks for your efforts. If someone else can independently confirm it works I'd say it's good to merge. I'll give it a go if no-one has done it by the end of next week - I'm away at the moment.

@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented May 8, 2019

The patch looks good to me - base_str should only ever be used for serialization purposes, so the fact that it's being used here is super-suspicious.

Wedge009 added a commit that referenced this issue May 17, 2019

@Wedge009 Wedge009 closed this in c2173b2 May 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.