Heretic and Hexen: check for valid, existing characters in font drawing functions #1201
Conversation
|
@fabiangreffrath, how about this approach? There is definitely some potential for readability improvement, like using macros instead of bare ACSII indexes ( |
src/heretic/mn_menu.c
Outdated
| // allow [ (91, cursor) only in small fonts. | ||
| return 32; | ||
| } | ||
| else if (ascii_index >= 97 && ascii_index <= 122) |
There was a problem hiding this comment.
You could use char literals instead of magic numbers, i.e. 'a' instead of 97 and 'z' instead of 122.
src/heretic/mn_menu.c
Outdated
| else if (ascii_index >= 97 && ascii_index <= 122) | ||
| { | ||
| // Force lowercase "a...z" characters to uppercase "A...Z". | ||
| return ascii_index -= 32; |
There was a problem hiding this comment.
return ascii_index + 'A' - 'a';
There was a problem hiding this comment.
Thanks, look much better now! Both corrections are done, but I'd like to polish everything more. This is complicated right now - too much distractions on day job. 🥵 Hexen should be straight forwards well, it have just extra small gold font, it's pretty much same to white one.
src/heretic/mn_menu.c
Outdated
| @@ -224,7 +224,7 @@ static Menu_t MainMenu = { | |||
| }; | |||
|
|
|||
| static MenuItem_t EpisodeItems[] = { | |||
| {ITT_EFUNC, "CITY OF THE DAMNED", SCEpisode, 1, MENU_NONE}, | |||
| {ITT_EFUNC, "a small [\\]^_`{|}~ text z" /*"CITY OF THE DAMNED"*/, SCEpisode, 1, MENU_NONE}, | |||
There was a problem hiding this comment.
Please remove this before merging. 😉
Co-Authored-By: Fabian Greffrath <fabian@greffrath.com>
src/heretic/mn_menu.c
Outdated
|
|
||
| static const char MN_CheckValidChar (char ascii_index, int have_cursor) | ||
| { | ||
| if ((ascii_index >= '[' + have_cursor && ascii_index <= '`') || ascii_index >= '{') |
There was a problem hiding this comment.
Nitpick: Could you write > 'Z' and < 'a' and > 'z' here? This would make it clearer why these restrictions are there?
There was a problem hiding this comment.
Sure, will do. My idea of using >= was to point to exact chararacter, not to next one. Another thing is - we also have a F_TextWrite font drawing function on finale screen, better I double check it too.
There was a problem hiding this comment.
we also have a F_TextWrite font drawing function on finale screen
Well, I think we are fine with this one, since it have toupper() correction, and the texts are in lowercase format right in source code. Characters check is still possible here, but not sure it's really needed - I'm sure that mod authors will never especially use non existing characters for finale texts.
|
Just a moment, just a moment, there is still Hexen to take care. |
|
Allright, it's all done! Hexen is using Thank you very much for recommendations, it's much cleaner and readable now than my prototype I wrote in the morning, almost right after getting from bed. 😀 Ah yes, and of course, initial issue fixed and time is now appearing as it should for lowercase
|
…awing functions See: fabiangreffrath/crispy-doom#1201 Co-Authored-By: Fabian Greffrath <fabian@greffrath.com>
This should fix possible crashes in cases like:
am/pminstead ofAM/PM) in savegame time. This was fixed by not drawing lowercase characters.