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

Added multi language support. The language can now be switched (Need translation) #216

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

Conversation

HumansAreWeak
Copy link

…is not

yet saved in the settings storage. I also started the translation process,
by removing hardcoded strings and replacing them with keys, so they are
found in the JSON files.
And lastly I added a new menu entry, which looks really messy, but changing
the language works without restart.

…is not

yet saved in the settings storage. I also started the translation process,
by removing hardcoded strings and replacing them with keys, so they are
found in the JSON files.
And lastly added a new menu entry, which looks really messy, but changing
the language works.
@lxgr-linux lxgr-linux requested a review from MaFeLP July 11, 2022 22:42
@lxgr-linux
Copy link
Owner

Hey, first of all this looks really nice, me and @MaFeLP will look into this. Funny how you chose German as the second language xD.

@lxgr-linux
Copy link
Owner

I would suggest removing the pokete_classes/ui folder and placing it's content in pokete_classes since most of pokete_classes content is related to ui anyways, so a distinction there does not make sense.

@HumansAreWeak
Copy link
Author

Yeah German is my main language so I thought I would go for it :)
I've put them in a separate folder so we have a distinction between actual game objects and UI related objects, which might come in handy for future changes.

@lxgr-linux
Copy link
Owner

Hey,

All this also begs the question of what else should be translated, besides dialogues (some of which just work in the englisch language), headings etc. also Pokete and attack infos have to be translated, the Pokete names them selfs should stay though.

Another issue I see with the translations is that things like the minimum supported terminal resolution of 70*20 has to be maintained in other languages, as well as the texts it self have to be maintained in different languages, english and german are no challenge to me, but when other languages are added those have to be maintained too, especially when new features or parts of the map are added.

As far as I see you outsourced the menu from pokete.py to pokete_classes/ui/menu, but which is not yet in use?

And as of the newly added setting isn't yet savable I will look into how incorporate this into the already present settings system, this may also obsolete the language overlay menu, since VisSettings support multi choice settings.
BTW I like your use of BetterChooseBox.

Greetings

Comment on lines +12 to +16
try:
import scrap_engine as se
except ImportError:
print("You seem to not have the 'scrap-engine' package installed")

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this is really necessary, since python itself will throw an error about this and this isn't even the first file to import scrap_engine.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this was kind of a test I forgot to remove. I thought it would be nice to fetch all the required packages on startup first and see if all are available, otherwise a message with all the missing packages will be printed. But I wasn't able to implement this behavior yet :o

@lxgr-linux
Copy link
Owner

I now added the integration with the settings and therefore changed some stuff in the language class.

@lxgr-linux
Copy link
Owner

I would also like you to take a look at that code.

…tance

directly in the class, it is passed in as parameter (DI), so I could test
the class properly.
Furthermore I have added a new method in the Settings class to get the
setting via a method call, instead of relying on magic functions.
I have also added a fallback language (en_US), which can be modified in the
constants.py file, to load these language strings if no translation can be
found in the current language file.
@HumansAreWeak
Copy link
Author

Yes everything looks correct, except that part where you commented out the language file existence check. As everything is dynamic, it would be good to check for existence first. I also added a fallback language which can be changed in the constants.py file. Look at b9eead8 for more information.
I also added some more string key pairs in the language json file (en_US).

@lxgr-linux
Copy link
Owner

Then I think it would also be great to make the setting in the menu to select the language it self dynamic, so that when new languages are added, the keys are determined by what files are present.

To also get the languages name, like hardcoded right now, the translation files it self could contain such a name.

@lxgr-linux
Copy link
Owner

I will also look into the extending the German translation.

Oh, and I also think translating logging messages isn't a good idea, since those should be reproducable and the same no matter the language.

@HumansAreWeak
Copy link
Author

Just made a new push with more translations. Yeah I also translated the logs, since I believe it is kind of interesting to play with translated logs xd, but it can be reverted if important to do so. I think I got most of the language strings in the classes folder, if there are any left, then let me know! Cheers.

@lxgr-linux
Copy link
Owner

Hey,
Sorry for replying so late, I was very busy with another project. I will look into pokete_classes at saturday.
But what I think, what is then also really important to translate, are the pokete and attack description in pokete_data. But this then also begs the question if the from this data auto generated wiki should be then generated for every of the awaylable languages.

Cheers

@HumansAreWeak
Copy link
Author

Hey, this is a future problem, for now we need the translation for the different languages. The wiki can be modified later on :)

@HumansAreWeak
Copy link
Author

Hey, Sorry for replying so late, I was very busy with another project. I will look into pokete_classes at saturday. But what I think, what is then also really important to translate, are the pokete and attack description in pokete_data. But this then also begs the question if the from this data auto generated wiki should be then generated for every of the awaylable languages.

Cheers

Also sorry for my late response, I'm currently prepping for exams, have the last one in one week. Then I can make more translation stuff

@lxgr-linux
Copy link
Owner

Good luck!

I may have added some additional whitespaces in the used data structures.
But I think that is not such a huge problem.
@HumansAreWeak
Copy link
Author

Alright new commitments are done. Please check them out. I separated the commits that might be problematic with the current idea of the game, so it can be easily reverted.

@lxgr-linux
Copy link
Owner

Oh, that's very nice. I hope your exam went good. But I think the translatable Pokete names are not a good idea, because some Pokete name like steini, vogli etc. already have a deep connection to in this case the german language and therefore don't really have a german translation. It was also a hassle to make up the names in the first place and now having to imagine most of the names is a lot of work.

@HumansAreWeak
Copy link
Author

Oh, that's very nice. I hope your exam went good. But I think the translatable Pokete names are not a good idea, because some Pokete name like steini, vogli etc. already have a deep connection to in this case the german language and therefore don't really have a german translation. It was also a hassle to make up the names in the first place and now having to imagine most of the names is a lot of work.

My idea behind translating the names was, that some names have a pun intended. But you may not get it when playing the game in english, so I thought it would be nice for the translators to make the pun work in other languages, besides german. But this is just an idea and I can revert that change if this feature is not planned.

Aaaand yes my exams went absolutely perfect, thank you for asking! :D ^^

@lxgr-linux
Copy link
Owner

Hey,

Aaaand yes my exams went absolutely perfect, thank you for asking! :D ^^

That's very nice to hear.

My idea behind translating the names was, that some names have a pun intended. But you may not get it when playing the game in english, so I thought it would be nice for the translators to make the pun work in other languages, besides german. But this is just an idea and I can revert that change if this feature is not planned.

Hmm, that's a very good idea, but I don't know whether or not I like it.
How about you @MaFeLP, what do you think?

Cheers

lxgr-linux added a commit to ondrejmyska/pokete that referenced this pull request Aug 21, 2022
Made audio volume use `Settings` and backported additions to settings
made in lxgr-linux#216
@MaFeLP
Copy link
Collaborator

MaFeLP commented Aug 27, 2022

Sorry for the late reply.

I'm really split on this topic. On one hand, I'm liking the idea of having puns, not everyone can understand, on the other hand, the names of Pokémon (in the original) are also (partially) translated.

Therefore, changing pokete's names should depend on the pokete itself and the language. For example having a Schmetterling in the English version is pretty funny, but having a butterfly in the German version wouldn't be as funny as the pun would partially go away with "German is very harsh".

@HumansAreWeak
Copy link
Author

@MaFeLP I agree with you, just partially translate Pokete names is better than translating all pokete names. But for Poketes like Horny I would prefer a translation to get even crazier names in other languages.

And are the overall commits I have done so far acceptable or do I need to change something?

Cheers

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.

None yet

3 participants