-
Notifications
You must be signed in to change notification settings - Fork 42
Adding Swedish entries in various json #2715
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Adding Swedish entries in various json #2715
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me in principle.
Perhaps you can also create the same change for Ukrainian afterwards (in a separate change)?
"sourceList": [ | ||
"Data/Swedish/*.ini" | ||
], | ||
"registryList": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be removed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets see if i understand this, should i not add anything to this file or is it these specific lines i don't need here?
if removing line 407-410 how should it look to make sure its correct?
Example 1:
or should i use other added languages as template like Arabic if so the Swedish entry would look similar to the Arabic one like this
Example 2:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I mean is you can remove the
"registryList": [
"Resources/FileHashRegistry/Generals-108-GeneralsZH-104.csv"
]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When using what i have in the commit it will build but removing the lines you suggested the json decoder gives me errors.
So maybe using the Arabic snippet example 2 is better with a source and target? when tested this build as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to remove the comma above as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we don't want the translation to use the English folder for any added language in the long run, but i noticed that Russian and Arabic uses "setGameLanguageOnInstall": "English",
and "sourceTargetList"
While other language uses "sourceList":
So maybe my entry is all wrong and should be like Arabic and Russian.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is because the community made Russian and Arabic (and Swedish, Ukrainian) are not yet standalone languages. They rely on English big files to provide the base audio, that the Patch replaces in part. As long as that is the case, the community made languages use "English".
Perhaps we should document that somewhere. We cannot put comments in Json.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would this work in a way?
"name": "CoreLangArabic",
"_comment": "This Arabic localization is community-made",
"big": true,
"setGameLanguageOnInstall": "English",
"files":
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that would work.
} | ||
], | ||
"params": { | ||
"language": "Swedish", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this cause an error on build?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it depends on where or how an error would occur, i ran the BuildInstallRunWithGui.bat without issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Select the Swedish pack in the gui and click on Build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for the gametextcompiler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it work with old gametextcompiler that does not have Swedish added yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it work with old gametextcompiler that does not have Swedish added yet?
Short answer: No
Long answer:
I have made a pull request over at xezon/thyme this will implement Swedish and many other languages.
We would need to add this new version over at https://github.com/TheSuperHackers/GeneralsTools/tree/main/Tools
so this will be fetched when running the setup
Here is a link for a compiled gametextcompiler.exe https://www.dropbox.com/scl/fi/4jsp12fdb2tzh1y4d1wi1/gametextcompiler.zip?rlkey=dfnlkc5rja8wem32abosoh4rs&st=dt0qyodw&dl=0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. If Swedish breaks build right now then we need to postpone submit until after new gametextcompiler is hooked up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. If Swedish breaks build right now then we need to postpone submit until after new gametextcompiler is hooked up.
Building Swedish and Ukrainian fail, building all the other languages succeed.
Do Post Build ...
Build Job completed in 21.192 s
We need to update the WindowsTools.json with size hash and github link.
The error when building Swedish with the current main branch build progress and compiler:
subprocess.CalledProcessError: Command '['C:\\Users\\Film\\Desktop\\GENERALS\\PATCH1~1\\Scripts\\Windows\\.tools\\gametextcompiler.exe', '-LOAD_STR', 'C:\\Users\\Film\\Desktop\\GENERALS\\PATCH1~1\\.Build\\RawBundleItems\\CoreLangSwedish\\Data\\Swedish\\generals.csf.tmp', '-SAVE_CSF', 'C:\\Users\\Film\\Desktop\\GENERALS\\PATCH1~1\\.Build\\RawBundleItems\\CoreLangSwedish\\Data\\Swedish\\generals.csf', '-LOAD_STR_LANGUAGES', 'Swedish']' returned non-zero exit status 2.
My additions should fix all this when the compiler gets added to the tool repo.
Sure when as soon as we figure out what is needed or not since I'm not a coder i heavily rely on random testing, asking questions and GPT misinformation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks
} | ||
], | ||
"params": { | ||
"language": "Swedish", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it work with old gametextcompiler that does not have Swedish added yet?
}, | ||
{ | ||
"name": "CoreLangSwedish", | ||
"_comment": "This Swedish localization is community-made", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this comment is not necessary here. There already is a wiki page somewhere which could list the original game languages, which then implies which ones are community made.
Here is the wiki page
https://github.com/TheSuperHackers/GeneralsGamePatch/wiki/localization_contribution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
Reason i added the comment in the file was for future people so they could know by just browsing the JSON about what language were community-based!
The other JSON PR #2717 i made has all the comments in all the files but i guess i should remove them there as well.
053bc61
to
6701c8b
Compare
6701c8b
to
8f09413
Compare
Adding various entries for the Swedish language in the .json located in /Patch104pZH/
This will not build with current gametextcompiler.