L-Sherry / Localize-me

CCLoader mod to add locales
MIT License
5 stars 3 forks source link

Localize Me 0.6.0 crashes when running under CCLoader 3, specifically when running mod titles and descriptions through text_filter in patch_ccloader3_mods #8

Closed dmitmel closed 4 years ago

dmitmel commented 4 years ago

After upgrading Localize Me to 0.6.0 I noticed that the game crashes with an error Error: Loading of a language file failed. during loading of data/lang/sc/gui.en_US.json when crosscode-ru is active. Unfortunately ig.Lang as well as a ton of other loadable classes don't care about logging errors, so I had to do that manually, but the extracted stack trace looks like:

TypeError: text_filter is not a function
    at localize_field (chrome-extension://lmepkikdgdbfdpjokdmnnanopegnpjda/assets/mods3/Localize-me/mod.js:695:12)
    at window.modloader.loadedMods.forEach (chrome-extension://lmepkikdgdbfdpjokdmnnanopegnpjda/assets/mods3/Localize-me/mod.js:718:18)
    at Map.forEach (<anonymous>)
    at JSONPatcher.patch_ccloader3_mods (chrome-extension://lmepkikdgdbfdpjokdmnnanopegnpjda/assets/mods3/Localize-me/mod.js:708:31)
    at JSONPatcher.patch_langfile (chrome-extension://lmepkikdgdbfdpjokdmnnanopegnpjda/assets/mods3/Localize-me/mod.js:751:10)

The block which is responsible for this crash is:

https://github.com/L-Sherry/Localize-me/blob/94759dc0f0d92ee70d5e070eeb3fecee6338c545/mod.js#L690-L696

See, crosscode-ru assigns text_filter to null with default options, which obviously causes the crash. The question is: why not use this.get_text_to_display with the skip_missing parameter set to true in this case? It handles non-existent text_filters just fine and you call it in this function later anyway.

L-Sherry commented 4 years ago

I didn't use get_text_to_display because i made that patch in the v1.x branch where get_text_to_display doesn't do the same thing. I should probably split it and fix this.

But right now i'm in vacation. expect some delay, especially since i'll have trouble testing it.

L-Sherry commented 4 years ago

I still can't get crosscode-ru's ccloader3 branch to run, however. Especially when using localize-me's v1.x branch

dmitmel commented 4 years ago

Ok. I'll try to submit a patch.

dmitmel commented 4 years ago

Interesting. I've finally gotten to testing this bug (I've been using my fork of Localize Me this entire time), and strangely enough I don't observe any crashes with the mainline Localize Me installed.

L-Sherry commented 4 years ago

I've been using my fork of Localize Me this entire time

The one that doesn't work with french-cc ? :stuck_out_tongue:

I don't observe any crashes with the mainland Localize Me installed.

I was going to ask "mainline master or motherland v1.x ?", but i checked both of those and none of them actually work. I don't understand why it could work for you. ccloader3 doesn't hook xhr (as used in the master branch), fetch (as used in v1.x) and crosscode-ru uses ru-translation-tool/localize-me-mapping.json as map_file, for files which are actually in assets/mods/something/assets...

dmitmel commented 4 years ago

Ah yes. Sh*t. See, in my development setup ru-transation-tool isn't contained inside the mod. It is placed into the game's assets dir, therefore it doesn't rely on URL rewriting performed by CCLoader. I can see what you are talking about now.

dmitmel commented 4 years ago

The one that doesn't work with french-cc ?

Dunno what are you talking about. I specifically tested it with French-CC to ensure compatibility of CCLoader 3 and Localize Me+French-CC.

By the way, can you please change the reference to ccmod3 in French-CC to just ccmod? This was a political change, ccmod was the initial name for the namespace, after a long discussion I agreed to change it to ccmod3 which was then reverted after another long discussion back. Please don't tell me that I'm breaking compatibility though - you are technically relying on unreleased yet functionality. As for when CCLoader 3 will be released: definitely before the end of the summer, no ETA though - I'm currently busy upgrading the Russian translation to 1.3.0, specifically helping our editor. Do you need any help with that? I've been asked by another person from another localization project to perform the same task, they are editing translation packs by hand, so I might as well give you my program for rebuilding a bunch of translation packs to another CC version (when it is written) or vise-versa (i.e. I'd be grateful if you have an existing solution).

L-Sherry commented 4 years ago

Last time i tried it wanted to load something like http://localhost:8000/assets/localhost:8000/assets/French/packs/[...] ... And in your pull request you mention another change to french-cc...

Also, for migration, i usually start one with a yes "" | tools/jsontr.py continue, or, if the amount of change is small (as reported by tools/jsontr.py count), i skip the yes "" | part and review everything manually. it still has the major drawback to not remove stale translations for texts that do not exist anymore, so i still have to manually prune them afterward (as reported by jsontr.py check).

But as a general migration tool it isn't quite practical in the general case. If the translation isn't complete, then it will also default-translate everything that wasn't already translated, unless one uses the filtering capabilities. And it wants everything in one pack file because that's what jsontr.py creates (and it's easier to maintain with a simple text editor with search/replace).

I have some ideas about something better... maybe i'll start something during this scorched heat... But frankly i haven't even touched 1.3.0 right now. I'll want to play it before spoiling myself with the new texts, before i translate it...

dmitmel commented 4 years ago

And in your pull request you mention another change to french-cc...

If that PR gets merged. Actually, let me see if French-CC worked with the combination of Localize Me and French-CC I used.

dmitmel commented 4 years ago

Localize Me commit adc0f49fbae86752d2509303c0265a1ca7397e44 (basically the v1.x branch before you migrated to CCLoader 3) + French-CC 0.7.0 with a patch to replace document.currentScript with import.meta.url (because I got rid of module: false after you did this change on my request): works

Localize Me 0.5.3 (which technically doesn't rely on module: false by itself) + the same version of French-CC: works

The latest Localize Me and French-CC versions work as well.

Tried running all configurations on a web server and from regular nw.js with IG_ROOT set to http://localhost:8080/.

Neither work with with crosscode-ru, probably, but you shouldn't worry about that. Btw: we are releasing the update to 1.3.0 sometime during the next week (my optimistic estimate) and I'm planning to distribute the v1.x branch of Localize Me. Do you have any objections to that? I hope that I'll be able to finish the virtual console (i.e. those warning/error popups) for CCLoader 3, so I'm planning to distribute an alpha-beta-gamma version of it as well with crosscode-ru very soon.

L-Sherry commented 4 years ago

I guess i should stop trying to delay localize-me v1.x and release it, even if it doesn't have the change i want.

Anyway, i pushed some alpha code to migrate stuff. This was easier than expected, so it's probably broken, but still. It should work as follow:

# assuming $PWD is Localize-Me-Tools
# fetches all lang labels and cache them
./jsontr.py --gamedir path/to/CrosscOld/assets --string-cache old_cache.json save_cache
./jsontr.py --gamedir path/to/NewCrossCode/assets --string-cache new_cache.json save_cache
# analyze and generate a migration plan
# (remove no-file-move if files were renamed by the update)
./packfile.py calcmigration --no-file-move old_cache.json new_cache.json migration_plan.json
# look at migration_plan.json to see if it suits your needs.
# It's almost designed to be tweakable and shared to anyone

# apply the changes from migration_plan.json to old_packs/
# and write new_packs/
./packfile.py --string-cache new_cache.json migrate migration_plan.json old_packs/ new_packs/

Haven't tried it on v1.3 (because i'll need some faster internet to download it), but i tried it on a v1.1 to v1.2 migration, and it was acceptable. If it's broken, feel free to open bugs in localize-me-tools.