lua function unitName broken ?

[titi]

I am not sure if this is a bugreport or just something to discuss.

Currently the lua function "unitName" returns the currents units "displayname" which si for example translated too. Is this really meant like this or did this return the naem of the unit type in the past ?
I personally cannot really imagine why there should be a function unitName but no function returning the unitType .

[Omega]

Yes, that seems to be broken. It was changed by softcoder in 0ead6a36. As an aside, git blame makes it easy to figure out where a specific line of code changed.

Here's the relevant function:

Code: [Select]

const string World::getUnitName(int unitId) {
Unit* unit= findUnitById(unitId);
if(unit == NULL) {
throw megaglest_runtime_error("Can not find Faction unit to get position unitId = " + intToStr(unitId),true);
}
return unit->getFullName(game->showTranslatedTechTree());
}
This is what it used to be:

Code: [Select]

const string World::getUnitName(int unitId) {
Unit* unit= findUnitById(unitId);
if(unit == NULL) {
throw megaglest_runtime_error("Can not find Faction unit to get position unitId = " + intToStr(unitId),true);
}
return unit->getFullName();
}
As an aside, I haven't checked if the function is used elsewhere.

This is probably unintentional, since it prevents easy checking for what unit an ID corresponds to (eg, what type was the last unit that died?). Not to mention most of the changes in the revision in question are using the non-translated versions. However, I do think we need some way to get the translated value for output (since scenarios can be translated).

[Baŝto]

I think a techTreeTranslate(string) and scenarioTranslate(string[,string…]) would be in general handy.
I remember having trouble with outputting strings, some functions translated every string, others were not able to replace %s. I guess just outputting the plain string and using the functions mentioned above would be the easiest solution.

[titi_son]

i think this function is alright, but we need another one ( maybe called unitType() ) because you may want to have a the translated unitname. This may be useful for tutorials. So you can tell the exactly unit name to tell the player which unit to select. This will work also if he plays without translated techtree.

[GunChleoc]

We definitely need 2 functions here - one to return an internal name fr coding reference, and one to return the (translated) name that the player will see on screen.

[Omega]

As an aside, if we are using two functions (or have some optional boolean parameter that specifies whether to translate), then I think the current unitName should *probably* be the untranslated value, simply to fix any scenarios that might have been broken and gone unnoticed. Although I'm not sure if there are any such scenarios. If there is, though, it'd probably using the old behaviour could fix broken scenarios, while the worst case for it would be printing out a non-translated name when a translated name is expected.

[titi]

its not only about translation. Its also about a changed unit name when a unit name when the unit gets more advanced ( many kills ).

I fixed this now and added a new function "getUnitDisplayName()"
The old one "getUnitName" only returns the plain unit type name which fits to all the other name related lua functions.

https://github.com/MegaGlest/megaglest-source/commit/cf5da4d2b360ea49d96f67a00cb1d9abdcb2f90f