Page MenuHomeWildfire Games

more gui/manual/intro.txt improvements
Needs ReviewPublic

Authored by Nescio on Sep 19 2019, 2:53 PM.

Details

Reviewers
Gallaecio
elexis
Summary

This patch further improves the gui/manual/intro.txt file:

  • "building" → "structure"
  • "Hide/show" and "Show/hide" → "Toggle"
  • "panel" (something that is fixed) → "window" (something that can open and close)
  • put window names inside quotation marks
  • added a few missing entries
  • reordered some items
  • several minor corrections

It is a follow-up to D2110/rP22904 and earlier.
https://trac.wildfiregames.com/wiki/HotKeys is already updated in advance.

D2307 will update comment strings in the default.cfg file.

Test Plan

Check for mistakes and omissions.

Unit TestsFailed

TimeTest
0 msJenkins > TestGuiManager::test_hotkeysState
Error: Expected (hotkey_pressed_value == true), found (false != true) Error: Expected (hotkey_pressed_value == true), found (false != true)
0 msJenkins > TestAllocators::test_da
0 msJenkins > TestAllocators::test_da
0 msJenkins > TestAllocators::test_da
0 msJenkins > TestAtlasObjectXML::test_parse_attributes1
View Full Test Results (1 Failed · 968 Passed)

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Nescio added inline comments.Sep 19 2019, 6:36 PM
binaries/data/mods/public/gui/manual/intro.txt
66–67

"Toggle pause overlay" would be more correct, but also confusing.

87–88

Sorry, what do you mean exactly?

157–158

Any suggestions?

elexis requested changes to this revision.Sep 19 2019, 6:51 PM
In D2305#96247, @Nescio wrote:

The same changes should be applied to default.cfg in the same patch I suppose?

  • it includes many other settings not in the manual
  • it doesn't list many of the manual entries (esp. those that involve clicking or specify multiple cases)
  • completely different order
  • different format ("code = key ; comment" instead of "key: function")

These points are irrelevant to the argument that the sentences are the exact same in both places, targetted at the same reader, expressing the same information, the patch proposing to correct or improve that information based on an argument that applies to the instances of the sentence alike; fixing one instance of the sentences because its better but then leaving the other copies of the sentences with the less correct or less accurate information.

  • file is for developers, does not really contain user-facing text strings

False, HotKeys informs the player to read default.cfg online and copy it into his user.cfg, also they share the same exact sentences, except for people who didn't care to keep them in sync and wrote mutually contradicting information in the three locations (wiki page, txt, default.cfg).
If the argument for changing the sentences in this file is that the sentences are wrong or should be improved, then that argument applies to that sentence regardless of location.

  • already out of sync prior to D1719

So this patch makes it go out of sync further, worse.
It's bad, so it still matters if it gets worse, becaues it should actually get better.

Ideally the duplication would be avoided completely to avoid specifically this issue:

  • the common code in the wiki page and intro.txt (the part about hotkeys certainly) could be generated from a shared manifest
  • the default.cfg comments could be removed completely and readers directed elsewhere for example)

Also there will be a new JSON file with every hotkey, i.e. a fourth file to be kept in sync (#2604).
That would be compatible to generate the wiki page and text file from this (or perhaps delete the hotkey section of the introduction with a reference to the hotkey page, while presenting the hotkey list introduction in the hotkey editor page similar to the current introduction, whatever).

Since there is no duplication feasible for now, at least introduce the intro.txt changes in default.cfg too as possible.

Taking everything together, I concluded it's too much work and not really necessarily a good idea to update the default.cfg to match the intro.txt file.

So if those arguments were only debunking the idea to make the files 100% in sync, then I agree that this plan is to be refuted.
But that wasn't the plan, the plan was to apply changes to one instance of the sentence to the other instance of the same sentence equally.

This revision now requires changes to proceed.Sep 19 2019, 6:51 PM
Nescio updated this revision to Diff 9864.Sep 19 2019, 7:11 PM
Nescio edited the summary of this revision. (Show Details)

One example, default.cfg has:

screenshot = F2                          ; Take PNG screenshot
bigscreenshot = "Shift+F2"               ; Take large BMP screenshot

and intro.txt has:

F2 – Take screenshot (in .png format, location is displayed in the top left of the GUI after the file has been saved, and can also be seen in the console/logs if you miss it there)
Shift + F2 – Take huge screenshot (6400×4800 pixels, in .bmp format, location is displayed in the top left of the GUI after the file has been saved, and can also be seen in the console/logs if you miss it there)

In my honest opinion neither replacing the concise former with the verbose former nor vice versa would be an improvement.

Another example, default.cfg has:

1 = "Z"            ; add first unit type to queue
2 = "X"            ; add second unit type to queue
3 = "C"            ; add third unit type to queue
4 = "V"            ; add fourth unit type to queue
5 = "B"            ; add fivth unit type to queue
6 = "N"            ; add sixth unit type to queue
7 = "M"            ; add seventh unit type to queue
8 = Comma          ; add eighth unit type to queue

and intro.txt has:

Z, X, C, V, B, N, M, , (Comma) – With training structures selected. Add the 1st, 2nd, … unit shown to the training queue for all the selected structures

Undoubtedly both files share a number of sentences (the result of copy-pasting), but I still don't see why that should matter; those files have different purposes.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/250/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/762/display/redirect

Nescio updated this revision to Diff 9865.Sep 19 2019, 7:20 PM
Nescio edited the summary of this revision. (Show Details)
  • put window names inside quotation marks

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/763/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/251/display/redirect

Why change anticlockwise to counterclockwise in one file but not in the other?
There actually was no argument provided why it was right to change the string in D1744, but I suppose it's because the word is more common, regardless whether you're a developer or player.
It was an exact copy of the same sentence, so I don't see why one of the files would just not be updated when we already can identify the location quickly inside the known file.
Why changing building to structure in one file but not the other?
Why describe different behavior for the same hotkey in different places?
The argument at least applies to all of the sentences changed in this diff.

binaries/data/mods/public/gui/manual/intro.txt
41–45

Alt + F4 is wrong in fact. Some OS assign Alt+F4, others don't.
Alt+f4 however was set as a 0ad hotkey prior to rP18830.
So it was an actual mistake not keeping the information in the files in sync resulting in false information to be advertized to the user.
There are many more incidences as there are 65 commits to intro.txt and 259 to default.cfg.

79

From default.cfg:

stop = "H" ; Stop the current action
backtowork = "Y" ; The unit will go back to work
unload = "U" ; Unload garrisoned units when a building/mechanical unit is selected

I guess the difference is negligible, so I can pass on this if you want. Still doesn't make me unsee all the issues arising from the inconsistencies which arose from commits where people just neglected the other file. Also I dont know why one polishes the one file but neglects the other file if those are legitimate phrasing improrvements, especially considering that users are directed at working with default.cfg on the Hotkeys page (see bottom).

You can check out the ​default configuration (https://trac.wildfiregames.com/browser/ps/trunk/binaries/data/config/default.cfg) to see how to configure HotKeys. Then copy the relevant section you want to change into ​your personal configuration file and adjusted the associated keys.

For example if the user works with this file and reads Stop the current action, one could argue that its not so obvious that it relates only to units but not to things like replays if one simultaneously justifies improving little string changes for this files to be sufficiently worth the time.

It's a weaker argument compared to the other arguments were inconsistent descriptions led to actually wrong information being presented.

86

Left Click on a group icon – Select the members of the group
Double Left Click on a group icon – Focus the members of the group
Right Click on a group icon – Disband the group

Those are not hotkeys but an UI button tooltip while the other ones above and below are hotkeys.

102

There are three buttons that open a new page on top of the curent page in the ingame menu: Objectives, Diplomacy, Barter&Trade. So I suggest to keep them grouped instead of inserting tutorial panel (that isn't a window? but a panel, or overlay) in between.

Why change anticlockwise to counterclockwise in one file but not in the other?
There actually was no argument provided why it was right to change the string in D1744, but I suppose it's because the word is more common, regardless whether you're a developer or player.

The reason is “anticlockwise” is English and “counterclockwise” is American. Spelling and vocabulary differences matter for translations, but not really for comments in code.

Why changing building to structure in one file but not the other?

Well, I suppose I could change that and similar things in the default.cfg too, but I still think it would be better to do that in a separate patch.

Why describe different behavior for the same hotkey in different places?

?

binaries/data/mods/public/gui/manual/intro.txt
41–45

Alt+F4 may not be a hotkey defined by 0 A.D., but it does close the game immediately, at least on my end (Fedora 30).

86

True. But where should those three be listed then?

102

Point taken, I'll change that.

By the way, shouldn't there be a hotkey to open the Manual?

Nescio updated this revision to Diff 9867.Sep 19 2019, 8:25 PM
Nescio edited the summary of this revision. (Show Details)

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/765/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/253/display/redirect

Nescio added inline comments.Sep 19 2019, 8:27 PM
binaries/data/mods/public/gui/manual/intro.txt
102

It now follows the order their corresponding buttons are displayed on the session top panel.

Why describe different behavior for the same hotkey in different places?

Well I described the problem already multiple times. Some sentence mentions that the page is "shown", other places mention that the page is "toggled". I didn't know that the civ dialog should be able to toggle and even committed a bug changing the behavior that way without noticing (D2273), and default.cfg still says

structree = "Alt+Shift+T" ; Show structure tree
civinfo = "Alt+Shift+H" ; Show civilization info

while you propose to change it in Intro.txt to:

Alt + Shift + T – Toggle the “Structure Tree” window
Alt + Shift + H – Toggle the “Civilizations” (“History”) window

(and the lobby and summary screen descriptions story)

So to be precise about the requested change, the change is to keep the files in sync where possible instead of not caring it at all, first and foremost because the differences lead to bugs, incomplete or wrong information presented unless ruled out by a careful review while performing changes.

The reason is “anticlockwise” is English and “counterclockwise” is American. Spelling and vocabulary differences matter for translations, but not really for comments in code.

We use en-us throughout the codebase. Just because its not a translation mistake doesnt mean there arent other consequences. Consider how much of the codebase contained "colour" instead of "color" and people made multiple commits throughout the years to change it to become consistent (Armour too just that people didnt rename it, only wished for it throughout years).
It takes 10 seconds to apply the anticlockwise counterclockwise change in the intro.txt file and maybe less than a minute to change building to structure with ctrl+F.
If one had carefully compared intro.txt against default.cfg, for the lines changed in this patch, one would have also found the alt+f4 instance for example.

The changes in this diff arent so worthy for concern if it wasnt a pattern to ignore default.cfg that had led to many mistakes before that would have been found if carefully compared and always cosidered.

binaries/data/mods/public/gui/manual/intro.txt
41–45

Which is an operating system key and other operating systems have other keys. For example on my end I only have it because I manually inserted that into i3wm. This file should not list operating system defined hotkeys that inherently are different depending on the operating system.

cat .config/i3/config
bindsym $mod+F4 kill

It was my mistake for not removing it in rP18830 so I can correct that if needed.

Nescio added inline comments.Sep 19 2019, 8:36 PM
binaries/data/mods/public/gui/manual/intro.txt
100

Wouldn't Ctrl+L be more consistent than Alt+L?

The changes in this diff arent so worthy for concern if it wasnt a pattern to ignore default.cfg that had led to many mistakes before that would have been found if carefully compared and always cosidered.

Isn't it actually more of a problem that people change keys in the default.cfg file but forget to update the intro.txt file?

binaries/data/mods/public/gui/manual/intro.txt
41–45

So should it be listed in the manual or not?

Freagarach added inline comments.Sep 19 2019, 9:02 PM
binaries/data/mods/public/gui/manual/intro.txt
87–88

Ctrl + F5 (and so on up to F8) vs. F5, F6, F7, and F8.

123

I'm not sure what you mean here?

127

How come you want to rephrase?

157–158

When in the load game window
When selecting a saved game to load

Nescio updated this revision to Diff 9870.Sep 19 2019, 9:16 PM
Nescio added inline comments.
binaries/data/mods/public/gui/manual/intro.txt
123

Are we talking about 121 or another line?
I believe you can set a rally point on an ungarrisonable structure; whether the units trained can actually garrison it doesn't really matter.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/767/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/255/display/redirect

Nescio added inline comments.Sep 19 2019, 9:22 PM
binaries/data/mods/public/gui/manual/intro.txt
86

See also 52 to 55 above.

Freagarach added inline comments.Sep 19 2019, 9:23 PM
binaries/data/mods/public/gui/manual/intro.txt
123

Ah, I was talking about L123.
Just checked: no one cannot.

Nescio added inline comments.Sep 19 2019, 9:40 PM
binaries/data/mods/public/gui/manual/intro.txt
123

123 seems clear enough to me.

elexis added inline comments.Sep 20 2019, 1:28 PM
binaries/data/mods/public/gui/manual/intro.txt
51–52

During Building Placement was changed to lowercase except for the first word. That seems grammatically correct. Should "game" be changed to lowercase here for the same reasoning? People might feel invited to use capitals for headings if they see some similar precedent.
(See also "Game setup", "Random map" etc.)

elexis added inline comments.Sep 20 2019, 1:45 PM
binaries/data/mods/public/gui/manual/intro.txt
157–158

Also in the "Save Game" window, also in the "Replay Menu" window. Don't need quotes.
I came up with In the savegame and replay menu:, but I'm open to suggestions in D2309.

What about the colon, its used when introducing a list, so it would be grammatically incorrect to not use it here.
"Hotkeys:" has it too, the other items also look like they would benefit from having it?

Two very good points, thanks! Those section captions should have neither title case nor colons; I'll check and correct them all.

binaries/data/mods/public/gui/manual/intro.txt
157–158

You're right the “Load Game” and “Save Game” windows are similar; the “Replay Games” page is a bit different, though; Esc doesn't work there, nor should (cf. “Match Setup” and “Modifications” pages).

Nescio updated this revision to Diff 9885.Sep 20 2019, 6:07 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/266/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/778/display/redirect

elexis requested changes to this revision.Sep 21 2019, 5:31 AM

Why no colons? It introduces a list, f.e. https://www.thepunctuationguide.com/colon.html

binaries/data/mods/public/gui/manual/intro.txt
157–158

With escape you probably mean the "cancel" hotkey (which might or might not be assigned to escape depending on user config).
There are still other hotkeys that are bound to "cancel", for example tabs close upon pressing escape in the gamesetup (with default config).

As mentioned the cancel hotkey says dialog boxes should be closed:

cancel = Escape ; Close or cancel the current dialog box/popup

Esc – Close all dialogs (chat, menu) or clear selected units

And the gamesetup page is not a dialog.

Not sure what this has to do with the "savegame" hotkeys though.

159

This change should be performed in default.cfg as well, since duplicated sentence over there is just as wrong as it is here.

This revision now requires changes to proceed.Sep 21 2019, 5:31 AM

Why no colons? It introduces a list, f.e. https://www.thepunctuationguide.com/colon.html

No colon because these are section headers. Cf. the following:

The United Kingdom consists of four countries: England, Scotland, Wales, and Northern Ireland.

The United Kingdom consists of four countries:

  • England
  • Scotland
  • Wales
  • Northern Ireland.

The United Kingdom

  • England
  • Scotland
  • Wales
  • Northern Ireland

All three examples are correct. However, colon usage in this one isn't:

The United Kingdom:

  • England
  • Scotland
  • Wales
  • Northern Ireland
binaries/data/mods/public/gui/manual/intro.txt
157–158

You think the Esc line is superfluous here and it's better to remove it?

159

Right now I'm still at a loss whether these last four lines are supposed to refer only to the session (hence only save game window), or also outside (i.e. load and replay).
Once that's clear here I'll update D2307 accordingly.

Nescio edited the summary of this revision. (Show Details)Sep 21 2019, 9:41 AM

Another question, which of the following do you think is best?

  1. PgUp and PgDn
  2. PageUp and PageDown
  3. Page Up and Page Down
  4. PgUp (Page Up) and PgDn (Page Down)
Nescio updated this revision to Diff 9903.Sep 22 2019, 9:53 AM
Nescio added inline comments.
binaries/data/mods/public/gui/manual/intro.txt
163

Still needs a better name.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/278/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/790/display/redirect

Freagarach added inline comments.Sep 22 2019, 12:09 PM
binaries/data/mods/public/gui/manual/intro.txt
163

(...) menu?

I’m worried that discussing so many different changes may take some time. @Nescio what would you think about creating separate patches instead? Some of the changes should be pretty straightforward to apply that way.

It's been a while since last time I looked at this. Whilst I agree this has become a large patch, it's not obvious to me which changes are straightforward and which are potentially controversial.
Feel free to commandeer and split or update as you see fit.

it's not obvious to me which changes are straightforward and which are potentially controversial.

I doesn’t really matter. You can split in changesets that are tightly related, conceptually. The bullet points of your patch description make for a good starting point, I think. For example, you could create a patch that only contains the ‘building → structure’ change. Once we’ve merged that, or before that if the review takes too long, you can create a new patch with the next change.

“building” → “structure” incorporated in D2429.