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

name_inactive key of [specials] tag is ignored in attack dialog #4352

Closed
doofus-01 opened this issue Sep 15, 2019 · 5 comments
Assignees

Comments

@doofus-01
Copy link
Member

@doofus-01 doofus-01 commented Sep 15, 2019

Concerning the weapon specials
https://wiki.wesnoth.org/AbilitiesWML#The_.5Bspecials.5D_tag

The inactive name works for the side panel, but the attack dialog just uses the (active) name with gray font.

This is for BfW 1.14

@doofus-01 doofus-01 added UI WML labels Sep 15, 2019
@sevu sevu added the Bug label Sep 15, 2019
@jostephd

This comment has been minimized.

Copy link
Member

@jostephd jostephd commented Sep 17, 2019

Sidebar:

wesnoth/src/reports.cpp

Lines 943 to 953 in 8d576dd

const std::vector<std::pair<t_string, t_string>> &specials = at.special_tooltips(&active);
const std::size_t specials_size = specials.size();
for ( std::size_t i = 0; i != specials_size; ++i )
{
// Aliases for readability:
const t_string &name = specials[i].first;
const t_string &description = specials[i].second;
const color_t &details_color = active[i] ? font::weapon_details_color :
font::inactive_details_color;
str << span_color(details_color) << " " << " " << name << naps << '\n';

Attack Enemy dialog:

/**
* Returns a comma-separated string of active names for the specials of *this.
* Empty names are skipped.
*
* This excludes inactive specials if only_active is true. Whether or not a
* special is active depends on the current context (see set_specials_context)
* and the @a is_backstab parameter.
*/
std::string attack_type::weapon_specials(bool only_active, bool is_backstab) const
{
//log_scope("weapon_specials");
std::string res;
for (const config::any_child &sp : specials_.all_children_range())
{
const bool active = special_active(sp.cfg, AFFECT_EITHER, sp.key, is_backstab);
const std::string& name = sp.cfg["name"].str();
if (!name.empty()) {
if (!res.empty()) res += ", ";
if (only_active && !active) res += font::span_color(font::inactive_details_color);
res += name;
if (only_active && !active) res += "</span>";
}
}
return res;
}

Proposed fix:

diff --git a/src/units/abilities.cpp b/src/units/abilities.cpp
index 049c23f8575..c28749a9082 100644
--- a/src/units/abilities.cpp
+++ b/src/units/abilities.cpp
@@ -760,7 +760,10 @@ std::string attack_type::weapon_specials(bool only_active, bool is_backstab) con
 	{
 		const bool active = special_active(sp.cfg, AFFECT_EITHER, sp.key, is_backstab);
 
-		const std::string& name = sp.cfg["name"].str();
+		const std::string& name =
+			active
+			? sp.cfg["name"].str()
+			: default_value(sp.cfg, "name_inactive", "name").str();
 		if (!name.empty()) {
 			if (!res.empty()) res += ", ";
 			if (only_active && !active) res += font::span_color(font::inactive_details_color);

@doofus-01 This works in my testing; can you test it too?

@jostephd jostephd self-assigned this Sep 17, 2019
@jostephd

This comment has been minimized.

Copy link
Member

@jostephd jostephd commented Sep 17, 2019

Linking #3686 which introduced the gray.

@doofus-01

This comment has been minimized.

Copy link
Member Author

@doofus-01 doofus-01 commented Sep 18, 2019

@jostephd - I will test this this weekend, if no one else gets to it first. Thanks.

@doofus-01

This comment has been minimized.

Copy link
Member Author

@doofus-01 doofus-01 commented Sep 22, 2019

@jostephd - I tested your proposed fix and it works. Thank you. I'll leave this issue open for your fix to get committed.

jostephd added a commit that referenced this issue Sep 26, 2019
Fixes #4352

(cherry picked from commit e306af9)
@jostephd jostephd closed this in e306af9 Sep 26, 2019
jostephd added a commit that referenced this issue Sep 26, 2019
@jostephd

This comment has been minimized.

Copy link
Member

@jostephd jostephd commented Sep 26, 2019

Thanks for testing!

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