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

Localize header strings on ship information screen #5063

Merged
merged 3 commits into from
Dec 15, 2020

Conversation

gunchleoc
Copy link
Contributor

@gunchleoc gunchleoc commented Dec 9, 2020

  • Localize table headers
  • Fix ragged layout by using text instead of textWrapped. The textWrapped function really needs a width parameter, but we have sufficient space on this screen at 800x600 resolution and I didn't want to dig this deep into it.

Tested with Scottish Gaelic and Hungarian - in my experience, if it fits these 2 languages, it fits.

Before:
pioneer-ship-information-alignment

After:
pioneer-ship-information-alignment-after

@bszlrd
Copy link
Contributor

bszlrd commented Dec 9, 2020

"Tested with Scottish Gaelic and Hungarian - in my experience, if it fits these 2 languages, it fits" As a Hungarian, I love this sentence.

@Gliese852
Copy link
Contributor

Gliese852 commented Dec 9, 2020

It seems to fix #5040

@gunchleoc
Copy link
Contributor Author

No, it doesn't - it only circumvents the bug for this particular screen.

As a Hungarian, I love this sentence.

Well, it's true! As a general rule, Gaelic needs more space, but there are some cases where Hungarian is long and Gaelic is short. And Hungarians rock anyway -> hunspell :)

@Web-eWorks
Copy link
Member

The textWrapped function call was added to specifically support the display of mission details on the mission info screen, which rely heavily on tables wrapping text. I definitely agree that the wrapping failure is a problem, but it seems to be endemic to ImGui's text wrapping calculations at our font size and screen resolution.

I would be amenable to a solution that adds a separate table function that draws unwrapped text, but unfortunately the proposed PR does in fact break other display elements in a nontrivial way.

Good catch on those strings though! I occasionally change my language from English and check, but there are so few languages with 100% complete translations it's difficult to tell what's incorrectly hardcoded from what's simply untranslated as of yet.

@gunchleoc
Copy link
Contributor Author

I have undone the textWrapped change and rebased master. I still recommend squashing the commits when merging though.

@gunchleoc gunchleoc changed the title Ship information screen fixes Localize header strings on ship information screen Dec 15, 2020
@Web-eWorks
Copy link
Member

Thank you, merging!

@Web-eWorks Web-eWorks merged commit d3c9915 into pioneerspacesim:master Dec 15, 2020
@gunchleoc gunchleoc deleted the ship-information-screen branch December 15, 2020 16:51
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.

4 participants