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

Support second line for "AUDIO / ZOOM OPTIONS" message #345

Merged
merged 2 commits into from Jun 11, 2019

Conversation

@Forgon2100
Copy link
Contributor

commented Apr 29, 2019

The translation of the sidetext "AUDIO / ZOOM OPTIONS" is too long in
some foreign languages so that it cannot fit in the 640x480 screen.
Therefore, an optional delimiter "\n" allows to split it into two lines.
Any additional instances of that delimiter are ignored.

Note that the message's translation must not begin or end with "\n",
because otherwise msgfmt will terminate with a fatal error. Example:

PO file:

msgid "AUDIO / ZOOM OPTIONS"
msgstr "AUDIO- UND ZOOM-\nEINSTELLUNGEN\n"

Partial output of make -C po update-po:

de.po:10475: 'msgid' and 'msgstr' entries do not both end with '\n'
/usr/bin/msgfmt: found 1 fatal error
2882 translated messages, 3 fuzzy translations, 1 untranslated message.
make[1]: *** [Makefile:467: de.gmo] Error 1
make[1]: Leaving directory '/home/x/the_project/warzone2100/po'
make: *** [Makefile:764: update-po] Error 2

The attached ZIP file contains

  • screenshots of the translated message with both one and two delimiters
  • a shell script to generate them

As a paragraph separator for campaign mission briefings, the string " "
should not be translatable. That same message remains commented out in
the following lines of data/base/messages/strings/resstrings.txt:

19://RES_CYW_LG1_MSG1			_(" ")
20://RES_CYW_LG1_MSG2			_(" ")
21://RES_CYW_LG1_MSG3			_(" ")
429://RES_SY_SP1MK1_MSG1		_(" ")
430://RES_SY_SP1MK1_MSG2		_(" ")
431://RES_SY_SP1MK1_MSG3		_(" ")
432://RES_SY_SP1MK1_MSG4		_(" ")

translation_placeholder_documentation.zip

// initialize its msgstr to a copy of header contents
// TRANSLATORS: This placeholder message provides an extra line
// for the translation of the sidetext "AUDIO / ZOOM OPTIONS"
_(" ");

This comment has been minimized.

Copy link
@past-due

past-due Apr 29, 2019

Contributor

Since this is a placeholder, why don't we change it to something obvious like "<"AUDIO / ZOOM OPTIONS" - OPTIONAL SECOND LINE - SET TO EMPTY IF NOT REQUIRED>", and then just ignore it if it's empty or that string.

This comment has been minimized.

Copy link
@Forgon2100

Forgon2100 Apr 30, 2019

Author Contributor

Since this is a placeholder, why don't we change it to something obvious like
"<AUDIO / ZOOM OPTIONS - OPTIONAL SECOND LINE - SET TO EMPTY IF NOT REQUIRED>" [...]

This is too obscure.
My translator comment should sufficiently explain the message's meaning.
Alternatively, a msgctxt keyword could be used, but it is unnecessary.

[...] and then just ignore it if it's empty or that string.

I have implemented this.
In addition, removing the message " " from all PO files ensures that
update-po.sh; make update-po will recreate it as untranslated message
directly below the message which it complements:

#. TRANSLATORS: "AUDIO" options determine the volume of game sounds.
#. "OPTIONS" means "SETTINGS".
#: src/frontend.cpp:926
msgid "AUDIO / ZOOM OPTIONS"
msgstr "AUDIO- UND ZOOMEINSTELLUNGEN"

#. TRANSLATORS: This placeholder message provides an extra line
#. for the translation of the sidetext "AUDIO / ZOOM OPTIONS"
#: src/frontend.cpp:932
msgid " "
msgstr ""

This comment has been minimized.

Copy link
@Cyp

Cyp May 2, 2019

Member

Wouldn't it make more sense to either allow a \n in the msgstr or implement word wrapping than to have a separate one for the second line?

This comment has been minimized.

Copy link
@Forgon2100

Forgon2100 May 2, 2019

Author Contributor

Wouldn't it make more sense to either allow a \n in the msgstr [...]

I briefly considered this, but there were too many problems.
You'd have to parse the msgstr to ensure all escape sequences except for
the first newline are ignored, warn about any syntax errors and explain
the format to translators, who may be unfamiliar with format specifiers.

or implement word wrapping [...]

Line breaking rules differ across languages (example).
Supporting non-breaking (hard) and optional (soft) hyphens is required
to support merely the conventions of English. I prefer not to.

Of course, you can prove me wrong with a patch implementing your ideas.

@Forgon2100 Forgon2100 force-pushed the Forgon2100:translation_placeholder branch from aeea06d to bd83316 Apr 30, 2019
Forgon2100 added a commit to Forgon2100/warzone2100 that referenced this pull request Apr 30, 2019
As a paragraph separator for campaign mission briefings, this text
should not be translatable.

Fixes Warzone2100#345
Forgon2100 added a commit to Forgon2100/warzone2100 that referenced this pull request Apr 30, 2019
* allows correct translation of this sidetext into foreign languages
  within a window size of 640x480 pixels (the minium screen resolution)
* untranslated, the placeholder text consists of a single space

Refs ticket:4629
Fixes Warzone2100#345
Forgon2100 added a commit to Forgon2100/warzone2100 that referenced this pull request Apr 30, 2019
* do not break minimum screen resolution of 640x480 pixels
* show sidetext on two lines, using a placeholder message added in Warzone2100#345

Refs Warzone2100#345
Fixes Warzone2100#328
@past-due

This comment has been minimized.

Copy link
Contributor

commented May 5, 2019

Thinking about this a bit more, adding an "empty" translation string to permit a second line for this particular entry in the translation file just doesn't strike me as the right approach.

  • It's a layout problem (which should ideally be handled by the layout / text / drawing engine).
  • It depends on the dimensions of the window + UI elements + current font used. (i.e. A string might be too long at the minimum window size, but could fit on a single line at a larger window size.)
  • This handles a specific instance, but there are plenty of other potential cases (depending on source string, UI location, and target language).
  • Hard-coding it in the translations file is inflexible (in the face of possible future UI element sizing changes, font changes, etc).
  • I still don't like using an empty string / relying on the translation comments to provide context.
  • It likely won't play well with future integration with a web-based service to help with translations.

A solution for this problem is definitely warranted, but I'm not sure heading down this path is the right one.

@Forgon2100

This comment has been minimized.

Copy link
Contributor Author

commented May 11, 2019

Thinking about this a bit more, adding an "empty" translation string to permit a second line for this particular entry in the translation file just doesn't strike me as the right approach.

* It's a layout problem (which should ideally be handled by the layout / text / drawing engine).

Correct.
But unless someone wants to implement (and test) such an engine, we are
stuck with what we currently have.
Since line-breaking rules differ among languages and include countless
special cases, I prefer not trying to automate this.

* It depends on the dimensions of the window + UI elements + current font used. (i.e. A string might be too long at the minimum window size, but could fit on a single line at a larger window size.)

* This handles a specific instance, but there are plenty of other potential cases (depending on source string, UI location, and target language).

* Hard-coding it in the translations file is inflexible (in the face of possible future UI element sizing changes, font changes, etc).

While this is true, we probably don't have the resources to ensure every
possible case is handled. Fitting the GUI layout into 640x480 pixels
makes it substantially easier to test it.

Even far better-run open source games like 0 A.D. do not manage to avoid
breakage with common screen resolutions (presumably because developers
just don't happen to use them).

If users want to set a custom font, that's up to them.
I consider that feature to be irrelevant.
Crappy mods sometimes use different fonts, but they are not popular.

I think that the way forward is to improve the GUI layout gradually.
For example, an optional second line might be made available for all
sidetexts if they cannot be translated concisely.

Of course, an alternative is to rewrite the GUI layout from scratch, and
make it far more flexible. Yet I'm not aware of anyone working on this.

* I still don't like using an empty string / _relying_ on the translation comments to provide context.

In gettext, translator comments are supposed to provide context.
They should not identify messages.

* It likely won't play well with future integration with a web-based service to help with translations.

I think that any good translation platform displays translator comments,
not unlike any regular dedicated PO editor.

A solution for this problem is definitely warranted, but I'm not sure heading down this path is the right one.

I would appreciate a better solution, yet your complaint seems like
yak shaving to me. The problem I'm trying to solve is that the German
translation needs an extra line to print a long sidetext.

@past-due

This comment has been minimized.

Copy link
Contributor

commented May 11, 2019

* It's a layout problem (which should ideally be handled by the layout / text / drawing engine).

Correct.
But unless someone wants to implement (and test) such an engine, we are
stuck with what we currently have.
Since line-breaking rules differ among languages and include countless
special cases, I prefer not trying to automate this.

I have been working off-and-on on proper script detection and multi-line / paragraph shaping support for the text engine. No idea when it'll be in shape to review, but it's at least (tentatively) being worked on. (No guarantees, though, of course.)

Of course, an alternative is to rewrite the GUI layout from scratch, and
make it far more flexible. Yet I'm not aware of anyone working on this.

This is also ultimately the goal. Nothing to review yet.

* I still don't like using an empty string / _relying_ on the translation comments to provide context.

In gettext, translator comments are supposed to provide context.
They should not identify messages.

* It likely won't play well with future integration with a web-based service to help with translations.

I think that any good translation platform displays translator comments,
not unlike any regular dedicated PO editor.

The issue is proximity. I am not aware of a way to guarantee that the second line is tied to the first line in those various online translation platforms, as the way they display and sort translation strings is different from that of a flat .po file (and also allows filtering / sorting / etc).

This, in my opinion, makes the way this PR is currently structured very problematic. Ultimately, the benefits of a translation platform outweigh this specific downside.

A solution for this problem is definitely warranted, but I'm not sure heading down this path is the right one.

I would appreciate a better solution, yet your complaint seems like
yak shaving to me. The problem I'm trying to solve is that the German
translation needs an extra line to print a long sidetext.

Since the ultimate, exhaustive fix (an improved text layout / shaping / splitting engine) is not likely to be ready soon, here are some alternatives:

  1. It's a bit odd that Audio and Zoom options are combined in a single menu IMHO, so we could split them up. That would have the side effect of shortening the side-text string. (But we'd need to handle an additional menu label.)

  2. Picking up on @Cyp's suggestion: Adding support for handling "\n" in the side-text translation strings would enable us to support this second line for side-text without having a secondary empty entry in the .po file with all the disadvantages I listed above. The implementation does not have to be complex: literally treat the first "\n" as a line break. I am not aware of any reason that a literal "\n" should ever be in any of the side-text strings, so I'm not sure this has to be any more complicated than that - we probably do not need to handle other escape sequences at all - just treat the first "\n" as a special case.

If we want a stop-gap patch for now, that enables a second line for all side-text, I'm leaning towards 2. Yes it requires a translation comment to inform translators, but so does this current PR. And sure, it isn't exhaustive escape sequence parsing, but the specific problem you identified is "how do we split side-text into a second line", and it solves that without the disadvantages of the currently-proposed .po solution.

Forgon2100 added 2 commits Nov 1, 2018
As a paragraph separator for campaign mission briefings, this text
should not be translatable.

Fixes #345
* add support for optional delimiter "\n" to show message on two lines
  so that it can be translated correctly within a 640x480 pixel window
  (that is, the minium screen resolution)
* only process the first delimiter, ignoring all others that may follow
* note that the messages's translation must not begin or end with "\n"
  (otherwise msgfmt will terminate with a fatal error)

Refs ticket:4629
Fixes #345
@Forgon2100 Forgon2100 force-pushed the Forgon2100:translation_placeholder branch from bd83316 to bd0514f Jun 10, 2019
@Forgon2100 Forgon2100 changed the title Add placeholder message for "AUDIO / ZOOM OPTIONS" Add placeholder for "AUDIO / ZOOM OPTIONS" message Jun 10, 2019
Forgon2100 added a commit to Forgon2100/warzone2100 that referenced this pull request Jun 10, 2019
* do not break minimum screen resolution of 640x480 pixels
* show sidetext on two lines, using a message delimiter added in Warzone2100#345

Refs Warzone2100#345
Fixes Warzone2100#328
@past-due past-due added this to the 3.3.0_beta2 milestone Jun 10, 2019
@past-due past-due changed the title Add placeholder for "AUDIO / ZOOM OPTIONS" message Support second line for "AUDIO / ZOOM OPTIONS" message Jun 10, 2019
@past-due

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2019

Thanks, @Forgon2100 - this resolves the concerns re: the translation platform. 👍

@past-due past-due merged commit 7073993 into Warzone2100:master Jun 11, 2019
6 of 8 checks passed
6 of 8 checks passed
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python No code changes detected
Details
LGTM analysis: C/C++ No new or fixed alerts
Details
WIP Ready for review
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
freebsd_build FreeBSD:freebsd-11-2-release-amd64 Task Summary
Details
freebsd_build FreeBSD:freebsd-12-0-release-amd64 Task Summary
Details
past-due added a commit that referenced this pull request Jun 11, 2019
As a paragraph separator for campaign mission briefings, this text
should not be translatable.

Fixes #345
past-due added a commit that referenced this pull request Jun 13, 2019
* do not break minimum screen resolution of 640x480 pixels
* show sidetext on two lines, using a message delimiter added in #345

Refs #345
Fixes #328
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.