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

Change: improved english tooltips #7870

Closed
wants to merge 1 commit into from
Closed

Conversation

sheepo99
Copy link

No description provided.

src/lang/english.txt Outdated Show resolved Hide resolved
@sheepo99 sheepo99 changed the title improved english tooltips draft Doc: improved english tooltips draft Dec 22, 2019
Copy link
Contributor

@nielsmh nielsmh left a comment

Choose a reason for hiding this comment

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

The industry directory strings not matching the changes made in #7809 need to be fixed at least.

src/lang/english.txt Outdated Show resolved Hide resolved
src/lang/english.txt Outdated Show resolved Hide resolved
src/lang/english.txt Outdated Show resolved Hide resolved
src/lang/english.txt Outdated Show resolved Hide resolved
@LordAro
Copy link
Member

LordAro commented Dec 23, 2019

In addition to the above comments about the out-of-sync english.txt, the CI is failing due to the commit messages not matching the correct style - you'll need to rebase to squash the extras out

@sheepo99
Copy link
Author

Fixed the file and commit titles accordingly (thanks glx!)

New formatting follows some capitalization rules inspired by the Microsoft Style Guide, and some current OpenTTD conventions. Inquired people on IRC on preferred style. The results are as follows. Will create a proper Manual of Style after and submit to OpenTTD wiki afterwards.

General text: Sentence capitalization
Button text: Sentence capitalization
Field text (i.e "Income this year:..."): Sentence capitalization
Tooltips: Sentece capitalization
Window titles: Sentence capitalization

Resource names(Coal, Iron, etc): Title Capitalization
Product/Vehicle names: Title Capitalization
Country/Nationality/Coin names: Title Capitalization
App within App names (Jazz Jukebox, Scenario Editor): Title Capitalization (including text in buttons)
Company names: Title Capitalization
Given names/Ranks (i.e. Chief Executive Officer): Title Capitalization
Keyboard shortcuts (i.e. "Ctrl+Click+Drag"): Title Capitalization

@sheepo99 sheepo99 changed the title Doc: improved english tooltips draft Change: improved english tooltips draft Dec 26, 2019
@sheepo99 sheepo99 changed the title Change: improved english tooltips draft Change: improved english tooltips Dec 26, 2019
@sheepo99 sheepo99 marked this pull request as ready for review December 26, 2019 22:57
Copy link
Contributor

@glx22 glx22 left a comment

Choose a reason for hiding this comment

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

Just did a quick read

src/lang/english.txt Show resolved Hide resolved
STR_TOOLTIP_DEMOLISH_BUILDINGS_ETC :{BLACK}Demolish buildings etc. on a square of land. Ctrl selects the area diagonally. Shift toggles building/showing cost estimate
STR_TOOLTIP_VSCROLL_BAR_SCROLLS_LIST :{BLACK}Scroll bar{NBSP}-{NBSP}Scrolls list up/down
STR_TOOLTIP_HSCROLL_BAR_SCROLLS_LIST :{BLACK}Scroll bar{NBSP}-{NBSP}Scrolls list left/right
STR_TOOLTIP_DEMOLISH_BUILDINGS_ETC :{BLACK}Demolish{NBSP}-{NBSP}{}Clear all objects on a selected area. Ctrl+Click+Drag selects the area diagonally. Shift+Click shows estimated cost without purchase
Copy link
Contributor

Choose a reason for hiding this comment

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

newline after the - seems weird to me

Copy link
Author

Choose a reason for hiding this comment

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

This was made on purpose as a standard for most tooltips. It ensures you do not get any weird fragmentation after the hyphen (which happens frequently if the TT name is too long) and it makes the tooltip name nice and centered. I personally find the final result to be pleasing aesthetically.

1
2
3
4

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok I get the idea, but in this case a newline without the hyphen should work too

Copy link
Author

Choose a reason for hiding this comment

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

The aim of the hyphen is to provide an aesthetically pleasing divider. Colons were another option, and although perhaps better suited for newline, they do not look as nice IMO.

Copy link
Author

Choose a reason for hiding this comment

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

Going back to this, aside from preventing bad fragmentation, another issue here is that because all the tooltips text is in bold (tool name + description), there needs to be some form of separation telling tool name from description, and the line break alone does not do it very well if, for example, the tool name has the same length or is wider than the description line(s). Assuming some of you find the hyphen a bit jarring, would you find it more agreeable if it were to be replaced with colons (":")? This would also simplify editing since it would only require 1 linebreak and no non-breaking spaces, greatly reducing the number of tags required.

src/lang/english.txt Outdated Show resolved Hide resolved
src/lang/english.txt Show resolved Hide resolved
src/lang/english.txt Outdated Show resolved Hide resolved
src/lang/english.txt Show resolved Hide resolved
src/lang/english.txt Show resolved Hide resolved
src/lang/english.txt Show resolved Hide resolved
src/lang/english.txt Show resolved Hide resolved
src/lang/english.txt Outdated Show resolved Hide resolved
src/lang/english.txt Outdated Show resolved Hide resolved
Copy link
Contributor

@nielsmh nielsmh left a comment

Choose a reason for hiding this comment

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

I still don't like these changes in general. I will not approve this, but I'm also not in a position to reject this.

@sheepo99
Copy link
Author

sheepo99 commented Jan 11, 2020

I still don't like these changes in general. I will not approve this, but I'm also not in a position to reject this.

Can you elaborate? I've already done a lot of modifications on capitalization and style after discussing it on IRC, improved descriptions as requested, and I'm also willing to compromise on other things.

Moreover, I'm sure we can at least agree it is an improvement over the current state of tooltips in terms of accuracy, grammar, and consistency.

@sheepo99 sheepo99 requested a review from glx22 January 11, 2020 22:47
@LordAro
Copy link
Member

LordAro commented Jan 12, 2020

The issue is that it's very hard to review changes to 400 strings. Really this needs splitting up into individual logical changes, so a single commit for (each) capitalisation change, another for Ctrl+Click changes, another for newlines, etc etc. This will likely require some git wizardry I'm afraid

Incidentally, I agree with @glx22 about the - \n - it's just a bit weird, especially with NBSP on either side of it. Would prefer just a :\n

@sheepo99
Copy link
Author

@LordAro I can replace all hyphens with colons, no prob there. As for reviewing, I don't find it so difficult, since i feel it's more about testing in-game. These are text strings, not practical code, so changes will be purely textual for that matter. Uploading every single capitalisation via individual commits would be very difficult if not impossible and it would bring no advantage to review this I feel. But I'll see what can be arranged.

@LordAro
Copy link
Member

LordAro commented Jan 13, 2020

I don't mean every single change as a single commit, I mean every logical change -

General text: Sentence capitalization
Button text: Sentence capitalization
Field text (i.e "Income this year:..."): Sentence capitalization
Tooltips: Sentece capitalization
Window titles: Sentence capitalization

Would be a commit each. Then we can more easily review which style we want to use

@sheepo99
Copy link
Author

sheepo99 commented Jan 13, 2020

@LordAro, the problem here is that many of these capitalization changes were inserted at the same time as many rewrites were made on a variety of strings and, as such, cannot be separated easily, if at all. When reviewing and rewriting text one does not insert changes in logical steps, because that is highly impractical and leads to less-than-optimal results. It makes little sense to perform a rewrite, submit it, and only then do style changes on top and submit them again, because that would require a lot of time and has a higher chance of inducing into error with multiple, possibly conflicting commits. This is why when reviewing text both style, formatting and rewriting are generally handled simultaneously by proofreaders based on agreed conventions.

The best way to review these changes is to do traditional proofreading over compared docs with a style/formatting guide on your side, and inserting individual comments on lines that require further discussion. Github already allows this, but perhaps assigning someone to do proofreading and handle discussion with me personally would be a better way. What can also be done is to further discuss the style guide beforehand. I've already done so in IRC, and the listed rules above are the preferences from most users gathered from these talks, but we can still change and agree on them officially before review.

Moreover, a certain degree of trust is also required: as of the moment translations produced by volunteers from eints are simply dropped in automatically with little to no scrutiny, because it is assumed translators know how to do their job. This is the right attitude, but the practical reality given the current problems with the source text is a bit different: at least one translator mentioned to me resorting to custom formatting tags for fitting his/her language best, which is a hackish and inconsistent approach that may lead to problems. The point of these proposed changes is precisely to produce textual consistency and rules that will bring uniformity to a large part of translation work. However, it is exceptionally difficult for textual reviewers to make these permanent changes, if it is required of them to follow the same methods of scrutiny programmers are meant to follow, which are simply inadequate to process plain text.

@SpComb
Copy link

SpComb commented Jan 16, 2020

I don't mean every single change as a single commit, I mean every logical change -

...

Would be a commit each. Then we can more easily review which style we want to use

If you have a series of commits all touching parts of the same/nearby lines, then trying to split up those commits will be extremely painful, not to mention attempting to amend them after the fact based on review. Pick some representative examples and agree on the "design" first, then review the end result?

Incidentally, I agree with @glx22 about the - \n - it's just a bit weird, especially with NBSP on either side of it. Would prefer just a :\n

I'm a complete bystander here, but using this kind of formatting in the translation files to achieve some kind of "title" feature for the dialog does smell a little to me. Ideally there would be some kind of distinct "title" vs "description" semantics with the UI code handling the formatting of the two?

@TrueBrain
Copy link
Member

Sorry about the huge delay, as with #8144 .. and sorry I have to ask this, but:

Is this an addition to #8144 or does #8144 supersedes this?

I am trying to figure out what to process first, hence the question :) Tnx!

@TrueBrain TrueBrain added candidate: most likely This Pull Request is probably going to be accepted size: extra large This Pull Request is very large; it properly needs to be split up before it can be properly reviewed labels Dec 14, 2020
@TrueBrain
Copy link
Member

Hi @sheepo99 ; as mentioned earlier, I would love to include this in OpenTTD, but it is currently out-of-date. I am not sure if you would be interested in updating it for us, I would really appreciate it!

I did give it a go myself, but I could not validate if I wouldn't be introducing more inconsistencies than I am fixing .. so I left it at that :D

For now I will close this PR; not because I do not appreciate the work or am no longer interested, so please feel free to re-open the PR if you feel up to it! Thank you!

@TrueBrain TrueBrain closed this Dec 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
candidate: most likely This Pull Request is probably going to be accepted size: extra large This Pull Request is very large; it properly needs to be split up before it can be properly reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants