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

Add translations #272

Merged
merged 5 commits into from
Jan 2, 2023
Merged

Conversation

Eraden
Copy link
Contributor

@Eraden Eraden commented Dec 27, 2022

No description provided.

@Flippchen Flippchen changed the base branch from main to dev December 28, 2022 13:42
@Nereuxofficial
Copy link
Contributor

Hey, thank you so much for implementing this! We really appreciate it. It looks good except for the holes being in a broken state and their indicator being present because our check for them is really primitive. I will review it in the following days :)

@Nereuxofficial Nereuxofficial self-requested a review December 28, 2022 14:24
Copy link
Contributor

@Nereuxofficial Nereuxofficial left a comment

Choose a reason for hiding this comment

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

The Pull Request looks great apart from a few minor issues and you can merge it once those are fixed:

  • The English IntroScreen has Text that overruns to the right
  • Some Internal Code sadly depends on the names of the Machines(due to time constraints in the project). So in event.rs Line 164 and in generate_machines.rs Line 34 we use kanguage-dependent variables so changing the language changes the behaviour.

Again thank you for taking the time to implement this

game/src/languages/german.rs Outdated Show resolved Hide resolved
Preallocated hash map to prevent rehashing.
Remove allocations for performance
Clippy fixes.
@Eraden
Copy link
Contributor Author

Eraden commented Jan 1, 2023

Hi, I think I solved all issues here but unfortunately this ends up being huge PR

Here is what I did:

  • Strings where replaces with enum for Machine and Trade, additionally this should prevent some problems with unmatched name and is a little bit faster
  • Hash maps have capacity since overflowing them is really slow and requires rehashing every key
  • Into screen was fixed
  • Typos were removed
  • Some float operations were fixed

Note:

Some inline attributes where removed. Be careful with them and note they are not c++ inline, they are only useful in libraries since they are instruction for compiler this part should be inline in user application not in current library

@Nereuxofficial
Copy link
Contributor

Note:

Some inline attributes where removed. Be careful with them and note they are not c++ inline, they are only useful in libraries since they are instruction for compiler this part should be inline in user application not in current library

Ah so inlining is only useful for usage across crate boundaries. Thanks for pointing that out

Copy link
Contributor

@Nereuxofficial Nereuxofficial left a comment

Choose a reason for hiding this comment

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

LGTM

@Eraden
Copy link
Contributor Author

Eraden commented Jan 2, 2023

Hi, I can't merge or sign reviewer.

Additionally I created couple issues

@Nereuxofficial Nereuxofficial merged commit 38db1c2 into red-life-project:dev Jan 2, 2023
@Eraden Eraden deleted the translations branch January 3, 2023 07:14
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.

3 participants