Join GitHub today
GitHub is home to over 36 million developers working together to host and review code, manage projects, and build software together.
Sign upTranslations bugfix and update #13247
Conversation
VlasovVitaly
added some commits
Aug 11, 2015
BevapDin
reviewed
Aug 11, 2015
| @@ -1076,34 +1078,82 @@ std::string item::info(bool showtext, std::vector<iteminfo> &dump_ref) const | |||
| it_tool* tool = dynamic_cast<it_tool*>(type); | |||
|
|
|||
| if ((tool->max_charges)!=0) { | |||
| std::string charges_line = _("Charges"); //; | |||
| int t_max; | |||
| const std::string t_ammo_name = _(ammo_name(tool->ammo_id).c_str()); | |||
This comment has been minimized.
This comment has been minimized.
BevapDin
Aug 11, 2015
Contributor
ammo_name should probably do the translation. I see at least 3 further calls to it where the result is not translated:
src/game.cpp:
add_msg(m_info, _("Out of %s!"), ammo_name(tool->ammo_id).c_str());src/npctalk.cpp:
phrase.replace(fa, l, ammo_name(me->weapon.ammo_type()) );src/player.cpp:
add_msg(m_info, _("That %s cannot be used on a %s."), used->tname().c_str(),
ammo_name(guntype->ammo).c_str());That would also be kind of consistent with how item::tname, monster::name, etc. return a translated name.
This comment has been minimized.
This comment has been minimized.
VlasovVitaly
Aug 11, 2015
Author
Contributor
I need to check these cases. Fixes (if they need) will in separate PR.
BevapDin
reviewed
Aug 11, 2015
| int t_max; | ||
| const std::string t_ammo_name = _(ammo_name(tool->ammo_id).c_str()); | ||
| std::string temp_fmt; | ||
| std::string charges_line = _("Charges"); | ||
| dump->push_back(iteminfo("TOOL",charges_line+ ": " + to_string(charges))); |
This comment has been minimized.
This comment has been minimized.
BevapDin
Aug 11, 2015
Contributor
You could also use string_format directly here and get rid of the charges_line variable:
string_format( _("Charges: %d"), charges)
This comment has been minimized.
This comment has been minimized.
BevapDin
reviewed
Aug 11, 2015
| @@ -1076,34 +1078,82 @@ std::string item::info(bool showtext, std::vector<iteminfo> &dump_ref) const | |||
| it_tool* tool = dynamic_cast<it_tool*>(type); | |||
|
|
|||
| if ((tool->max_charges)!=0) { | |||
| std::string charges_line = _("Charges"); //; | |||
| int t_max; | |||
This comment has been minimized.
This comment has been minimized.
BevapDin
Aug 11, 2015
Contributor
Declaring the variables t_max, temp_fmt etc. outside of the if-blocks where they are used makes the impressions there is some connection between the logic in the block and the code afterwards. I expected something like this:
int f_max;
// set f_max depending on various conditions:
if( condition1 ) {
f_max = value1;
} else {
f_max = value2;
}
print( "max: %d", f_max );But here, there is not such connection, each block uses the variable separately. Therefor the variables should be declared in each block (and you can actually make them const there):
if( condition1 ) {
const int f_max = value1;
print( "max: %d", f_max );
} else {
const int f_max = value2;
print( "max: %d", f_max );
}Or maybe just move the dump->push_back(iteminfo( line out of the blocks and at the end as it is common to all blocks.
This comment has been minimized.
This comment has been minimized.
VlasovVitaly
Aug 11, 2015
Author
Contributor
Both variants has strengths and weaknesses. I like first one, but this is not obvious behaviour. Second is obvious, but require many lines in every logic block...
Guess, i will choose moving call of dump->push_back() to end of logic. This is not so strange.
VlasovVitaly commentedAug 11, 2015
Bug fixed

Ammunition type was not translated.
Improved code readability in this piece of file.
Added lots of helping comments for translators.