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: 'font' console command to configure fonts #10278

Merged
merged 3 commits into from Dec 23, 2022
Merged

Conversation

glx22
Copy link
Contributor

@glx22 glx22 commented Dec 22, 2022

Closes #8596.

Motivation / Problem

Configuring fonts requires multiple iterations of

  • editing openttd.cfg
  • start OpenTTD to see the result
  • close OpenTTD

So a console command to do the same from a running OpenTTD will be an improvement.

Description

I implemented font console command.
The command allows to list loaded fonts, with indication of the config as loaded font may differ (fallback mechanism ...).

Usually fonts are initialised before the command is accessible, but FS_MONO has to be different as it's not loaded unless a TextFile is displayed. So I included a mechanism to make sure all non sprite fonts are loaded when listing them.

For "normal" fonts I used CheckForMissingGlyphs() to initialise and check the modified font, but for "monospace" I directly InitFontCache(true) because the validation is only done when loading a file in a TextFile window, but that should at least validate the user selected font could work.

Fallback mechanism is also a huge annoyance as it reinitialises the fonts and may invalidate a perfectly fine font for a given size.
I tried to work around this by temporary storing in the config the fonts currently loaded for other FontSize when changing config for a font. That way if other sizes use fallback font they will still use it while validating the new font.
Of course if the new font is not valid and a fallback is searched, other sizes, which might have been valid while not being the fallback font, will end up using the fallback font.
We really need a per character fallback mechanism instead of the global one we currently have, but that will be another journey.

Limitations

I tested only with non freetype windows, but I think it should work for other platforms as long as FontCache::GetFontName() overrides return a sensible value.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR touches english.txt or translations? Check the guidelines
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@LordAro
Copy link
Member

LordAro commented Dec 23, 2022

Confirmed works well on Linux + SDL2.

Command syntax may need some work though - explicit "name" and "size" arguments seem a bit odd, any particular reason for them? why not just font medium "Liberation Sans" [15] [noaa|aa] ?

Using "" to reset to sprite doesn't seem to work. Helpfully, just using any other string ("sprite", "flibble", etc) works fine as it just falls back to the sprite font for unrecognised fonts

the aa setting for fonts appears to be stateful. noaa seems to be the default (should it?), but whichever it was previously set to appears to be retained, even if you go via the sprite font.

@glx22
Copy link
Contributor Author

glx22 commented Dec 23, 2022

The idea behind explicit "name" and "size" is to allow to modify only part of the config, like changing font size without touching the font name.

Using "" just works fine for me, and it should also work with freetype as the first thing in LoadFreeTypeFont() is if (settings->font.empty()) return; which skips loading and falls back to sprite. It's the same in LoadWin32Font() and LoadCoreTextFont().

If you don't specify aa or noaa, the current value is kept, same for name and size.

@LordAro
Copy link
Member

LordAro commented Dec 23, 2022

ah yeah, looks like i was just forgetting the name prefix previously. Told you it was confusing :p

@glx22
Copy link
Contributor Author

glx22 commented Dec 23, 2022

I understand the confusion, the syntax is intended to work like editing openttd.cfg manually.
I guess I could add a 'font [medium|small|large|mono] <font_name>' shortcut.
Hmm even 'font [medium|small|large|mono] [<font_name>] [<size>] [aa|noaa]' should be doable as it's possible to "check" value types (basic integer/non integer).

@glx22
Copy link
Contributor Author

glx22 commented Dec 23, 2022

Simplified syntax.

LordAro
LordAro previously approved these changes Dec 23, 2022
Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

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

LGTM.

@PeterN
Copy link
Member

PeterN commented Dec 23, 2022

ReInitAllWindows() should be called with true (the parameter naming is a bit wrong, sorry) so that dimensions are recalculated.

Viewport signs need to invalidated if the fonts are changed as the sign sizes change. UpdateAllVirtCoords().

Might need to call LoadStringWidthTable(), though I'm not sure when that data is used anymore.

src/fontcache.cpp Outdated Show resolved Hide resolved
src/fontcache.cpp Outdated Show resolved Hide resolved
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.

Provide GUI for changing of fonts ingame
4 participants