search

CommunityDragon / CDTB

A library containing everything to extract files from client files.
GNU Lesser General Public License v3.0
119 stars 33 forks source link

Regex fix for new tft stringtables #101

Closed SamGuthrie closed 2 months ago

SamGuthrie commented 2 months ago

Correcting a slight issue in regex search for the stringtable paths, resulting in no valid stringtable lookups for the TFT data files

Also replaced \\ with / in paths from glob.glob (think this is only necessary as I'm running on windows, happy to remove it if you want)

Would be great to re-run the TFT data with these changes in place

Morilli commented 2 months ago

This windows path separator handling seems to have gotten lost in #95. The logic doesn't look too clean, but it's probably fine. This should be applied to the below re.search call as well though.

Also, is this doing anything besides the \ -> / conversion? As far as I can see the current logic is fine for non-windows platforms.

SamGuthrie commented 2 months ago

Updated the windows path separator logic for all the file paths

The main fix was to change the regex search prefix from f to rf

Just having f wasn't working and resulted in none of the translations being applied.

Ie. in https://raw.communitydragon.org/pbe/cdragon/tft/en_us.json -> the Portal trait has "tft_trait_description_Set12_Portal" instead of its actual description

Morilli commented 2 months ago

There is no character that needs to be escaped in the pattern (.._..)/data/menu/en_us/{game}.stringtable$, so adding r"" to the string should be unnecessary.

I have re-exported the pbe tft and arena files, so they should be correct now.

SamGuthrie commented 2 months ago

Hmm you're correct about it not needing r"", my mistake. It does seem that the reexported files are still incorrect however, so is there something else wrong?

Running this branch on a local export is generating a correct tft en_us.json file, so I don't know what the difference might be

EDIT: I had the old file cached, looks good now - thanks

benoitryder commented 2 months ago

I wasn't sure rf"" was accepted and thus decided to omit it, since it's not necessary anyway. Adding will not fix anything, but it's still a good idea: it's safer.

The logic doesn't look too clean, but it's probably fine.

We can create a small helper to factorize the for path in glob(...): path.replace(...) parts. It's fine as it is now though; no need to change that in this PR.

Running this branch on a local export is generating a correct tft en_us.json file, so I don't know what the difference might be

The path separator fix is a real one. If you are running the script on a Windows system without it, it'll fail.