Skip to content
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

Chinese Version #27

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Chinese Version #27

wants to merge 15 commits into from

Conversation

Haceau-Zoac
Copy link

A little contribution :)

@Haceau-Zoac Haceau-Zoac changed the title Chinese README Chinese Version Nov 15, 2020
@rapiz1
Copy link
Owner

rapiz1 commented Nov 15, 2020

Thanks for your interest!

I took a quick look at your contribution. Good Job.

I've been thinking about adding i18n support for a long time but failed to find time to do that. And you're going in the right direction by replacing hardcoded text with text from loaded files.

And here are some feedbacks:

  • There are some refactoring not related to i18n support. Please place them in a separate PR.

  • It may be better to rename the language option to 'Languages'/'界面语言'

BTW, I've seen some mistakes in your translation. Are you using a web translator? I would say it's not very satisfying. Maybe there are some words misspelled in the original README.

I don't have much time for the review because of the upcoming exams. So my reply maybe delayed. Thanks.

@Haceau-Zoac
Copy link
Author

Thanks for your interest!

I took a quick look at your contribution. Good Job.

I've been thinking about adding i18n support for a long time but failed to find time to do that. And you're going in the right direction by replacing hardcoded text with text from loaded files.

And here are some feedbacks:

  • There are some refactoring not related to i18n support. Please place them in a separate PR.
  • It may be better to rename the language option to 'Languages'/'界面语言'

BTW, I've seen some mistakes in your translation. Are you using a web translator? I would say it's not very satisfying. Maybe there are some words misspelled in the original README.

I don't have much time for the review because of the upcoming exams. So my reply maybe delayed. Thanks.

Yes, I am using a web translator, plus a little bit of my own understanding.

And can the feedbacks be more detailed?

Thanks.

sh.exe.stackdump Outdated Show resolved Hide resolved
@rapiz1
Copy link
Owner

rapiz1 commented Jan 5, 2022

Sorry for the delay! I've forgotten this stuff 🤡🤡🤡 Your PR looks good and need a little improvement as pointed out.

@rapiz1
Copy link
Owner

rapiz1 commented Jan 5, 2022

I see the fix of #28 is still in this PR. Can you cherry-pick that and open a separate PR? If you don't have time, I can do that :)

@Haceau-Zoac
Copy link
Author

OK

src/res.c Outdated Show resolved Hide resolved
@rapiz1
Copy link
Owner

rapiz1 commented Jan 17, 2022

behaved weird on my machine (archlinux)

chooseLanguageUi
image

After set to the second option (simplified chinese maybe?)
image

In gameplay
image

Scoreboard
image

@Haceau-Zoac
Copy link
Author

behaved weird on my machine (archlinux)

chooseLanguageUi image

After set to the second option (simplified chinese maybe?) image

In gameplay image

Scoreboard image

I merged new commits in this repo but I'm not test it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants