thexaero / Better-PVP-Translations

Translations to different languages for the Better PVP Minecraft mod.
Creative Commons Zero v1.0 Universal
8 stars 22 forks source link

Cleanup of most translation files #7

Closed Greg-21 closed 3 years ago

Greg-21 commented 3 years ago

In this pull request I cleaned up all translation files besides the source U.S. English one, which has already been improved in a separate PR.

The cleanup includes:

First important note: The most boring things weren't done by hand. I simply wrote a program that compared files and then rearranged lines so that they are in the same order as in the source file, removed the ones that didn't match, and also pointed out the ones that may have needed further review by me.

~~Second important note: Both Russian and Brazilian Portuguese (which are being worked on in PR #6 and PR #8 respectively) are within the scope of this pull request as well. It means that if you merge this PR, conflicts will arise in the other two, and you (as I don't think the other two folks have a Git client) will have to deal with them e.g. by rebasing. So if you don't think it is a good idea, please let me know, as I can always exclude these two translations from this PR, for example.~~ EDIT: Both excluded from this pull request. EDIT2: Russian translation is now included back. EDIT3: Brazilian Portuguese is now included back as well.

Greg-21 commented 3 years ago

A few lesser changes in response to acf1d3004e3f413684b86d031358ff1a34eec069.

thexaero commented 3 years ago

This is great, I appreciate it! Sorry for not responding sooner. We can exclude the WIP translations for now. Do you want me to review the changes?

Greg-21 commented 3 years ago

Do you want me to review the changes?

That's entirely up to you. I don't (and didn't) plan to modify anything else here, unless you suggest something to change, or remove lines from the U.S. English translation file but forget to remove them from others as well.

We can exclude the WIP translations for now

OK, give me a while, I'm going to exclude them.

Sorry for not responding sooner.

No worries. You would have to try very hard to exceed e.g. the Forge review process that often takes much longer than it should ;p

Greg-21 commented 3 years ago

Another batch of changes, but this time in response to commit de19f0960453edaf3acf1bb32d849079a5405df2.

Greg-21 commented 3 years ago

Since PR #9 has been closed, the Chinese translation is now included back here.

thexaero commented 3 years ago

Alright, thanks. I might delete the current Chinese translation though. It's very bad according to native speakers.

Greg-21 commented 3 years ago

If you delete the Chinese translation, I will remove it from this pull request as well. Or you can review and merge this PR first and then delete the Chinese translation, not much difference to me.

Greg-21 commented 3 years ago

I've seen that in commit a2944c4318d22853855ba5b13aa40c719af204e2 you slightly changed the ID of line gui.xaero_box_compass_over_wp, repositioned it, and added a second sentence to the existing one. This pull request is not based on that yet, so I'm marking it as a draft for now.


Anyway, what do you think that should be done now in all the other translation files to keep consistency?:

  1. Change the ID and move the line without adding the second new sentence.
  2. Change the ID, move the line, and add the new second sentence in English to all translation files where it is missing.
  3. Remove the old translated line, don't add anything back, and instead let the Minecraft fallback mechanism handle it.

To me, 1 & 3 are the best options here.

thexaero commented 3 years ago

I think 3 is the best option because, with how different languages can get, it's better to retranslate the whole line. Luckily it is also the simplest option.

thexaero commented 3 years ago

Hey, Do you think you can send me the source code for the script/program that you made the changes with? It is very difficult to review the changes themselves.

EDIT: I'll check the code, execute it and then compare the results to yours.

Greg-21 commented 3 years ago

I'm not sure if I understand you correctly, but my program is responsible for only about 1/5 (I guess) of the changes in this pull request. The rest has been done by hand, otherwise I could not vouch for the correctness and accuracy of the translations. I also used regex to search for redundant whitespace characters and incorrect letter cases.

If the summary in the description of this PR is not enough, I can willingly provide more information, context, motivation, and so on.

thexaero commented 3 years ago

Really? I knew you did some stuff manually but you made it seem like most of the work was automated. I'm surprised that it is only about 1/5. Either way, I'll be able to compare the automatic results I get against your fork, and review the differences manually.

Greg-21 commented 3 years ago

Much depends on the interpretation. If I had compared all files line by line by hand, it would have taken 99% of the time spent, but looking at the diff, my program is responsible for only about 1/5 of the changes.

Anyway, https://pastebin.com/fNtkci1P

thexaero commented 3 years ago

Thank you very much! Didn't expect it to be written in C! I'll check it out as soon as I can.

Greg-21 commented 3 years ago

As the current Brazilian Portuguese translation would benefit from this cleanup as well, and the author of the new translation proposal doesn't seem to be very interested in pushing their work forward (it's been almost 5 months since their last documented input here), I'm considering including back the Brazilian Portuguese translation. What do you think? Good or bad?

thexaero commented 3 years ago

Yeah, that seems like a good idea. I'll probably close their PR soon.

Greg-21 commented 3 years ago

Rebased on top of the current master to resolve merge conflicts.

thexaero commented 3 years ago

I really appreciate it!

Greg-21 commented 3 years ago

As the Simplified Chinese has been removed, I'm excluding it from this pull request to resolve merge conflicts.

thexaero commented 3 years ago

Hey, Can you please resend me the source code? I can't seem to find where I put it and the link expired.

thexaero commented 3 years ago

Nevermind, wrote something myself already to compare the files. I'll make a review very soon.

thexaero commented 3 years ago

Awesome! Thank you once again for your contribution! Going to merge it now.

Greg-21 commented 3 years ago

Thank you. I hope everyone will benefit from it.

Greg-21 commented 3 years ago

Just a nitpick, it seems that you've missed the line ending unification to always use LF (so Croatian and Russian still use CRLF). This problem can be permanently resolved by adding a .gitattributes file, but I don't know if you would like to have it.

thexaero commented 3 years ago

Every other file was using CRLF on my end, which is why I changed these 2 back. Everything uses LF if I manually download it though. Weird. I'll fix it.