WowAce

What steps will reproduce the problem?
1. Try to fix a localization.
2. Notice that changes in the "Exclude" table (and many other tables, if not all) don't have any affect ingame.

What is the expected output? What do you see instead?
I expect the localizations to be correctly used.

What version of the product are you using?
r157

Do you have an error log of what happened?
-

Please provide any additional information below.
Seems like it was broken by gathirer in r145. If i remove the lines 371 to 391 in LibStatLogic-1.2.lua everything works again.

Yeah.

He tried to implement a great idea, using the GlobalStrings in WoW to help LibStatLogic already be localized.

i went back and fixed as much of enUS as i could find was broken; and a lot of those regressions will have to be fixed in the other locales.

He made so many changes, and a lot of them right and good, that i don't want to revert. But i also don't want to have to go through the other locales and fix them all. :/

The problem is not the locale file. I've fixed the deDE file so that it works with the mentioned lines removed. But if I don't remove those lines, the locale file isn't loaded/used correctly.

I don't understand what this whole code block is for. The comment says it's not ok to fallback to enUS (and I agree!), but then there is code that seems to copy everything to enUS and then always use enUS?

<<reply 980060="">>

I don't understand what this whole code block is for. The comment says it's not ok to fallback to enUS (and I agree!), but then there is code that seems to copy everything to enUS and then always use enUS?

What happened what that he changed almost all the text strings that were in the localizations to instead use constants from GlobalStrings.lua. The idea was, hopefully, that you could use the already translated text from Blizzard.

The problem with that is that LibStatLogic cannot use most of those strings as is. The Blizzard-supplied GlobalStrings contain patterns for output formatting:

Increases your armor by %d.

LibStatLogic would never be able to match the string Increases your armor by %d. with anything, and so would always fail. He then tried to write code that would try to intelligiently strip the %codes out of the GlobalStrings.

That's why now there is a StatIDLookupLong, and then code that tries to filter them as it copies them into the expected location: StatIDLookup.

The problem with that is that:

• it didn't work

He couldn't get a set of functions that could strip the GlobalStrings into a format needed by LibStatLogic. In fact, if you look at localization files, you see were Whitetooth includes with most entries a comment that is a matching line from globalstrings - but not using the actual constant from globalstrings.

So gathirer then tried to mimic what "DisplayLocale" table, and things like AceLocale do. We start with a "base" locale. And in the absence of any translated entry in the current locale (e.g. deDE) it will fallback to the "base" en-US entry.

This might be fine if we were able to use all, or nearly all, strings directly from GlobalStrings.lua. In that case our "base" enUS locale is actually mostly pre-translated to the current locale.

Except it doesn't work.

i then added the comment you mention (it's not okay, or conceptually correct to fallback to enUS on other clients*), but i left all the code.

Instead i sort of made things worse. Garither's code now puts all the localization strings into StatIDLookupLong, and then tries to run them through a stripper as it copies them into StatIDLookup.

StatIDLookupLong -> filter -> StatIDLookup

in order to restore things to basic functionality, i now also copy the original entries from StatIDLookupLong into StatIDLookup:

StatIDLookupLong -> filter -> StatIDLookup
StatIDLookupLong ----------> StatIDLookup

This nearly doubles the size of StatIDLookup, but at least it works.

Like i said, i didn't want to rip out everything Gathirer did - especially after he put so much (useful) effort into updating all the class talents in LibStatLogic.

And like i said. He sort of created a mess. Turned off warnings, so he had no idea of the mess that was left, and now it's a mess that i really don't want to have to look at. People bitched, and bitched, and bitched, and BITCHED about warnings from LibStatLogic, that he has them turned off by default.

i at least changed the code so that i see them:

if (BNConnected()) then
    local _, battleTag = BNGetInfo();
    if battleTag == "Pauladin#1741" then --us developers need to see warnings - otherwise we'd have no idea things are broken
        ItemNotRecognizedWarningDefault = true;
    end;
end

If you like, i suppose you can add yourself to the "default good guy" list.

Edit: If you're so inclined, please feel free to rollback as much or as little of it as you can stand. i really don't want to have to do it.

Thanks for the detailed answer, but it wasn't necessary. I already knew most of what you wrote.
I assumed you wanted to fix that, but I can do it too if we agree on a solution.

How about this:

We create three new tables:
1) One that contains all values of GlobalStrings constants that can be used in the Exclude table.
2) One that contains all values of GlobalStrings constants that can be used exactly as they are defined by Blizzard in the StatIDLookup table.
3) One that contains all values of GlobalStrings constants that contain placeholders like %d and can only be used in the StatIDLookup table after some processing.

Ad 1) All entries of table 1 would be copied to the Exclude table of the currently loaded localization on startup.
Ad 2) Same for table 2 and the StatIDLookup table.
Ad 3) The values in table 3 will also be copied to the StatIDLookup table, but only after they ran through the logic that correctly removes the placeholders.
This logic might need to be different based on the localization, so it must be overrideable in the localization files. I think the easiest way would be to define it as a function in the localization files and just call the enUS function if the localized one is nil.

This solution will also reduce the problem with the double sized StatIDLookup table.

If you agree on that solution I'll start working on it tomorrow. Of course suggestions are welcome!

I've now commited the changes. Could you please test if I broke anything for the enUS client? deDE works fine, but I couldn't test enUS.

Please also make sure that "green" stats like haste and crit are matched by SingleEquipStatCheck. Previously they only matched with a DeepScanPattern (at least with deDE) because "by" and "by up to" wasn't removed from the strings.

<<reply 980464="">>

This solution will also reduce the problem with the double sized StatIDLookup table.

If you agree on that solution I'll start working on it tomorrow. Of course suggestions are welcome!

i'm really don't like the idea of trying to process blizzard's localized string constants and turn them into something else. It's just going to be frought with errors, bugs, and edge cases. We already have enough edge cases trying to process item text.

i really don't want to have to debug code that is turning one string constant into another. Just localize the damn thing once and be done with it!

But isn't it better to fight these problems just once instead of changing the strings each time they are changed by Blizzard? The processing really isn't that complicated. And now it's already done ;-)

If you find something that can't be processed easily you can still add the string to the StatIDLookup table directly, no change there. It will then just be one wrong/unneeded entry in the StatIDLookup table (better than before my commit when there were many wrong/unneeded entries).