raethkcj / RatingBuster

An item comparison tool for WoW Classic.
GNU General Public License v2.0
57 stars 14 forks source link

Update frFR.lua #43

Closed Hantunaar closed 3 years ago

Hantunaar commented 3 years ago

Updated frFR localization. Tweaked it for some time now and I think it's ready despite some issues:

-I wasn't able to make "Show detailed conversions text" to work with expertise, it only affected the way resilience was displayed. Not sure if intended but as such, I worded it for resilience only -Wasn't able to find out what Max Weapon Damage was. Is it weapon damage bonus from items or just weapon damage from weapons? Either way it seems to be broken for french. -Stats summary in items tooltips (like Spell hit (%)) is still displayed half in english, half in french. The entries aren't in this file.

Thanks for resurrecting this addon. I remember using it for Cataclysm and I'm now very happy to have it for Classic TBC!

raethkcj commented 3 years ago

Thanks so much for your work! Before merging I want to try to address your bullet points. This addon is a little confusing because it relies on a library called StatLogic which is also included in this repo. Both RatingBuster and StatLogic have their own locale files that are used for different things, even though they feel similar. So there is another frFR file here: https://github.com/raethkcj/RatingBuster/blob/master/libs/StatLogic-1.0/frFR.lua

For your first two bullet points, I went ahead and added what I think are the patterns for Expertise and Weapon Damage; hopefully you can test those and then update the description strings if they work: https://github.com/raethkcj/RatingBuster/commit/b1024af10730adb08336895a2d4eb76a8f538c8d

(Weapon Max Damage is the top end of a weapon's damage range, commonly looked for by melee players): image

For the third bullet point, these description strings are all at the bottom of the StatLogic locale, and they try to use the constants that Blizzard provides if possible. So for example for Spell Hit, the line it uses is here, and you can rearrange/translate while still using the built-in constant PLAYERSTAT_SPELL_COMBAT: https://github.com/raethkcj/RatingBuster/blob/b1024af10730adb08336895a2d4eb76a8f538c8d/libs/StatLogic-1.0/frFR.lua#L532

Hantunaar commented 3 years ago

Expertise and Weapon damage are working now. Is there no setting for weapons DPS? I'm no melee player but would think it's another important part of the stats, or is it missing from french perhaps.

Anyway I'll need to reword "Show detailed conversions text" now it works and I spotted some typos. Is there a way I can make new changes without making a new pull request? Sorry I'm quite new to Github.

I'm trying things with the description strings. So far I've found spell haste isnt calculated in stats summary. missing spellhaste

raethkcj commented 3 years ago

As long as you commit to this patch-2 branch, changes will be added to this one!

As far as I know there's no setting for Weapon DPS. Somewhat of a relic of this addon being made for TBC, since a lot of melee care more about a weapon's average damage than DPS.

Normally when stuff isn't showing up in the summary, it's because the pattern is missing or incorrect in this locale file, but it seems to be good here, so I might need to dig a little more:

https://github.com/raethkcj/RatingBuster/blob/b1024af10730adb08336895a2d4eb76a8f538c8d/libs/StatLogic-1.0/frFR.lua#L416

Hantunaar commented 3 years ago

I've found this: https://www.townlong-yak.com/framexml/live/GlobalStrings.lua and was wondering, is there a reason why the addon uses PLAYERSTAT_SPELL_COMBAT and not SPELLS ? It seems shorter but maybe it has to be PLAYERSTAT_SPELL_COMBAT ?

Also while RATING is correct in english, in french they used it for arenas and the word doesn't make sense in stats summaries. I'm trying to find something better suited for french but have a few rewording to do.

raethkcj commented 3 years ago

In English at least, SPELLS is plural, which makes less sense in the summary. Other languages might be even less consistent.

image

Btw, if using the constants just makes it more difficult, feel free to ignore them, it seems like they're just intended as guidelines from this note in the same file:

-- Please localize these strings too, global strings were used in the enUS locale just to have minimum -- localization effect when a locale is not available for that language, you don't have to use global -- strings in your localization.

Hantunaar commented 3 years ago

I shared my files with some friends and got told some things weren't as clear as I thought. Doing a bit more rewording as they give me their feedback.

About the other file in StatLogic1-0, can I add it here somehow or do I have to make another pull request?

raethkcj commented 3 years ago

You can add it here for sure.

I'm not sure what you're using to edit the files, are you just using the web interface? If so you can do it here: https://github.com/Hantunaar/RatingBuster/blob/patch-2/libs/StatLogic-1.0/frFR.lua

Otherwise just edit locally and git add, git commit, git push as usual.

Hantunaar commented 3 years ago

That should be the final update to both files.

Edit: Found a fix to missing spell haste from stats summary as showed in the screenshot. Adding it to StatLogic1-0. (40aefee)

Edit2: Also fixed ZG shaman enchant not counting as spell damage. (69ceab2)

raethkcj commented 3 years ago

Merged as of 3f404c4543ce02bb04a58b155734c6049d96c40e!

Thanks so much for all your work on this, hopefully it will help many other French users!