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

Use italic for disabled subtab (with ncurses emulation for SDL) #23419

Merged

Conversation

Projects
None yet
5 participants
@PatrikStridvall
Copy link
Contributor

commented Apr 8, 2018

Initially I intended to use italic for the disabled subtabs in #23333, however, it turned out to be more challenging than I originally thought so I submitted it without it.

However, since, I'm not somebody that gives up lightly I have now implemented it for both for the console as well as the ncurses emulation in the SDL tiles version.

I also did some experimentation with bold and underline with SDL tiles, with shrink font size to fit, to avoid making the font size of normal text smaller. This is not turned on for italic, it didn't look good IMHO, but other people might disagree. Anyway tweaking can be done later.

Note everything looks very nice in the console with both italic and underline. It just the SDL tiles version that doesn't look 100% but that is because I didn't want to shrink the font size of normal text like console version does (read: it lowers the cell rows and columns to support it).

Anyway this is best effort given the limitations and time spent on my part, and it shouldn't make things worse for anybody, just giving opportunities to use italic and underline.

@Leland

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2018

screen shot 2018-04-08 at 3 28 29 pm

Looks a bit wonky on the Mac app. Perhaps specify an italic font in fonts.json instead of doing faux-italics?

@illi-kun

This comment has been minimized.

Copy link
Member

commented Apr 8, 2018

What is the benefit of doing it italic?

@PatrikStridvall

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2018

@illi-kun You could ask the question, what is the benefits of using colors? Italics and underline are like colors already supported by ncurses. I just added support for it in the SDL ncurses emulation layer.

Enhancing text with italics and underline have be done for centuries predating use of computers. Enhancing test using colors though was rare before computers. So using color is in some meaning more odd than using italics and underline.

In short, italics, underlne as well as colors enhanches readabillity and in this case using italics for disabled is a quite normal programming practise in my experience.

@PatrikStridvall

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2018

@Leland Well, since the patch, unlike ncurses, doesn't expand the cells to show italics, neither do it shrink to fit since it looked even more odd during my tests. Shrink to fit for bold and underline is mostly OK but not perfekt. But not so for italics, during my limited test, that is.

Another reason it might look odd is the font. I'm not sure what SDL does if a font doesn't have italtics as a face style. Maybe it emulates it... Try another font.

In addition the current version of the SDL layer doesn't do any kerning like a normal terminal emulators does so that might effect things too.

In short, I just tell SDL that is should write in italic and it does so but without kerning since it only get one character at the time.

@Leland

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2018

Another reason it might look odd is the font. I'm not sure what SDL does if a font doesn't have italtics as a face style. Maybe it emulates it... Try another font.

SDL can only specify a single font file (e.g. data/font/unifont.ttf), which intrinsically will lack an italic variant.

Here's how another font looks:

screen shot 2018-04-08 at 4 35 12 pm

Using non-bitmap fonts with fontblending enabled yields better results, although certainly not approaching what you would get from the actual italic variant of the font.

screen shot 2018-04-08 at 4 45 39 pm

Anyway, I don't find that this conveys "disabled" more than just having it be greyed out. It may even convey it less - italics are for emphasis, which is likely the opposite of your goal here.

@PatrikStridvall

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2018

@Leland Whether italics or greyed out or both should be used for disabled in this particular case can out course be discussed. I just used this an example, because this is what made me notice that italic and underline wasn't supported in SDL tiles mode.

The main issue is whether italics and underline are useful for this game in general. Since it works perfectly for me in console mode using just ncurses (Using konsole on Debian (stretch)) , it almost certainly can be made to work somehow.

Perhaps SDL itself is the problem, perhaps it the current implementations use of SDL that is the problem. For example the current code search for a font index. Perhaps this is wrong for italics. I'm far from certain that my current patch is correct. I just experimented until things looked reasonable.

But of course expanding the cell will help, however, in that case this needs to be optional since it would lower either the screen size or shrinking the font size for current users.

Related is this is #23161. Ideally there should be options for use to specify what the user wants to happend as well as dynamic resizing of either the screen size or the font size as well as whether underline and/or italics should be shrunken to fit or whether an expanded cell should be used.

@kevingranade kevingranade merged commit 48599f4 into CleverRaven:master Apr 29, 2018

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
coverage/coveralls Coverage increased (+0.07%) to 23.563%
Details
gorgon-ghprb Build finished.
Details
@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Apr 30, 2018

Regression in all SDL builds since experimental build 7354.

e.g. http://ci.narc.ro/job/Cataclysm-Matrix/7355/Graphics=Tiles,Platform=Linux_x64/console

22:49:09 src/sdltiles.cpp:1988:33:   required from here
22:49:09 /usr/include/c++/4.8/bits/stl_tree.h:140:49: error: no matching function for call to ‘std::pair<const cata_cursesport::font_style, std::unique_ptr<_TTF_Font, TTF_Font_deleter> >::pair(cata_cursesport::font_style&, _TTF_Font*&)’

ZhilkinSerg added a commit that referenced this pull request Oct 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.