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: Help and manuals window #7786

Merged
merged 3 commits into from Sep 13, 2023
Merged

Add: Help and manuals window #7786

merged 3 commits into from Sep 13, 2023

Conversation

nielsmh
Copy link
Contributor

@nielsmh nielsmh commented Oct 20, 2019

image

This adds another button to the main menu, letting you access both included documentation in the package, as well as links to various official online resources. The Markdown format files are parsed, offering a Table of Contents dropdown and working hyperlinks.
It also adds this choice to the in-game help/about menu.

Several issues:

  • The changelog file is too long for the text file viewer. Currently scrollbars are limited to 65535 items, so this should be expanded. Probably best handled by a separate change/fix.
  • Markdown does not look very nice in the viewer. Can be solved by making the viewer process markdown to make it look better, or by shipping processed files with the game.
  • The location of documentation files is not entirely reliable, especially on Unix and Linux systems, the packaging has several places it could put them (though typically under /usr/share somewhere.) The file searching needs to be improved.
  • For some reason the COPYING file (with no extension) does not get displayed, even though my debugger says it is being read into memory.
  • It seems to be an open question whether TT-Forums is sufficiently official to warrant being linked from inside the game.
  • The Markdown parsing should be moved out of the Help window so it can be used for user content as well.

@James103
Copy link
Contributor

Currently scrollbars are limited to 65535 items

This is probably why the game crashes when you generate too many news messages at once (#7613).

@nielsmh
Copy link
Contributor Author

nielsmh commented Dec 28, 2019

Some fancyness: Internal anchor links are green, and web links are blue. Both work, though at most one link per (source file) line.

image

Possibly also to do: Make links to other files work, and add a back button.

@nielsmh
Copy link
Contributor Author

nielsmh commented Dec 28, 2019

Does anyone have suggestions for how to best handle the "finding the right path for the docs files" problem? In particular on Unix-y systems where the docs may be in /usr/share/openttd while the binary may be in /usr/bin. Are any of the existing search paths useful for this, or is a new one necessary?

@James103
Copy link
Contributor

Commit checker: Expected — Waiting for status to be reported (Required)

a) Can you please re-run the checks, preferably by rebasing this PR?

@nielsmh
Copy link
Contributor Author

nielsmh commented Feb 16, 2020

Can maybe make more of the changelog viewable with the scrollbar fix in #8006 too. Although the text file viewer is already slow with many lines.

@nielsmh
Copy link
Contributor Author

nielsmh commented Feb 24, 2020

@TrueBrain mentioned that he'd like if the Markdown parsing was moved around, so it could also be used for user content and basesets.

There's also a question of where to put the feature on the main menu. Is it really the right thing to do, to remove the highscore table button?

@LordAro
Copy link
Member

LordAro commented Feb 24, 2020

I know I've never (intentionally) clicked it :)
TBH, I'm not sure anyone would much mind if the highscore table was removed entirely...

@sheepo99
Copy link

Linking to the manual is nice, but I do not see the purpose for all the other buttons. Why providing players with links to resources mostly aimed at developers who would end up consulting the documentation with the same info either way? This makes no sense from a user experience POV, much less as a replacement for the highscore board, which is an actual user experience feature.

@nielsmh
Copy link
Contributor Author

nielsmh commented Apr 27, 2020

Which of my proposed buttons are mostly aimed at developers?

  • The readme file is intended for end users, it contains various installation information.
  • The changelog is intended for end users, to know what changed between versions.
  • The known bugs file is intended for end users, to learn about standing (and possibly unfixable) issues.
  • The license is actually required to be viewable from within the program according to the terms of the license. And it is intended for end users to know their rights regarding copying the program and obtaining the source code.
  • Official website, official wiki, community forum, are intended for end users, to know where the actual developers reside and what the official source for the game is. (If gaming magazine cover CDs were still a thing, they would have even more raison d´etrê.)
  • Report a bug is there so end users can know where the actual place to report bugs is, and protect themselves and us from discussing issues in places none of us would learn about it.
  • Community forums is there to point to the traditional home.

As for replacing the high score screen, nobody ever said that was a final decision. However the high score game mechanic makes little sense today, since the scoring is rather arbitrary, and does not take difficulty, game settings, or mods into account. The entire main menu experience is kind of weird considering all the extensions and changes to gameplay, e.g. the four big landscape selection buttons pertain to new games only but are front and center. A completely redesigned main menu could offer a much better location for a help and manuals button.

@sheepo99
Copy link

sheepo99 commented Apr 27, 2020

I completely agree that the main menu requires a redesign, and if modding does indeed make the scoreboard irrelevant, then it should go. My proposition for the time being, however, would be to add all the relevant info to the About window (which already contains the link to the website), and add a new button for it either from the Options window, or the main menu. This is for example what OpenRCT2 does:

image

@TrueBrain TrueBrain added candidate: yes This Pull Request is a candidate for being merged 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
@nielsmh
Copy link
Contributor Author

nielsmh commented Apr 2, 2021

Well here we go, I did manage to make multiple hyperlinks in one line work. Sort of.
There is something weird with how I count characters versus how the text layouter counts characters (for the purpose of getting "character at X pixel position") that makes it miscount in some places.

@TrueBrain TrueBrain added the preview This PR is receiving preview builds label Apr 3, 2021
@DorpsGek DorpsGek temporarily deployed to preview-pr-7786 April 3, 2021 09:14 Inactive
@DorpsGek DorpsGek temporarily deployed to preview-pr-7786 April 3, 2021 10:01 Inactive
@DorpsGek DorpsGek temporarily deployed to preview-pr-7786 April 3, 2021 10:30 Inactive
@DorpsGek DorpsGek temporarily deployed to preview-pr-7786 April 3, 2021 15:34 Inactive
@DorpsGek DorpsGek temporarily deployed to preview-pr-7786 April 3, 2021 16:18 Inactive
@nielsmh
Copy link
Contributor Author

nielsmh commented Apr 3, 2021

Crash and not-working-at-all (with sprite font) issues with links fixed, but still have an outstanding issue with how the click position on the line is measured, causing links to extend the click position for all following text. Right now it's "kind of acceptable", but it might become a problem in the future and probably better to fix now.

Still need to make a plan for squashing.

@DorpsGek DorpsGek temporarily deployed to preview-pr-7786 April 3, 2021 18:00 Inactive
@DorpsGek DorpsGek temporarily deployed to preview-pr-7786 April 4, 2021 11:07 Inactive
src/widgets/help_widget.h Outdated Show resolved Hide resolved
@TrueBrain
Copy link
Member

TrueBrain commented Sep 11, 2023

Right, after asking @nielsmh decided to pick this PR up again. It really was nearly done. I moved the requested commit to #11288 , and did a bunch of cleanup / update to our latest state of code (read: removing C-strings and using the C++ variants we have now). I kept that in a separate commit, so it is more visible what I changed. We will squash this PR when it is done I assumed anyway.

The quality of this Pull Request is REALLY high. There is so much stuff in here .. working links, history, table-of-content .. you did an amazing job @nielsmh , it is really lovely to work with a Pull Request of this quality. So my job was also really easy :)

Next up, is some testing. I will build some release binaries based on this work, and test in various of OSes if it is actually working. To keep a list for myself:

  • Test Linux Generic
  • Test Windows
  • Test MacOS app-bundle

Release build running in https://github.com/OpenTTD/OpenTTD/actions/runs/6144602745 .

src/textfile_gui.cpp Fixed Show fixed Hide fixed
@TrueBrain
Copy link
Member

TrueBrain commented Sep 11, 2023

All files can be found on all OSes we support via our distribution mechanisms. So we should be good there. Found a few bugs that I am addressing (this to keep track of them):

  • Clicking on a directory crashes. fopen succeeds on a directory, with a very very large filesize.
  • Table of Content isn't really visible.
  • Make more clear that the left are links and the right are viewers.
  • Prevent external markdowns from opening local files (so readme.md cannot open other files).

@TrueBrain TrueBrain merged commit 41de0d4 into OpenTTD:master Sep 13, 2023
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
candidate: yes This Pull Request is a candidate for being merged preview This PR is receiving preview builds 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

8 participants