Show date and time of savegame in Save/Load menu #1200
Conversation
Co-Authored-By: Fabian Greffrath <fabian@greffrath.com>
Better do not remove it, so the purpose of "-15" will be clear.
Working, but needs pixel perfection.
Strife using spaces, not tabs. ;)
|
Gorgeous! Not sure what to do about save date coloring though, traditionally is should have
|
Since everything is moved slightly up and no extra file slot is being drawed over status bar, "inhelpscreens" is now redundant. Save date should not appear on status bar, since it's using same Y-coords as KIS stats. Also, this making Save/Load menu slightly less expensive in terms of CPU usage, but fortunately, we are still on capped framerate while active menu.
| # pragma GCC diagnostic push | ||
| # pragma GCC diagnostic ignored "-Wformat-y2k" | ||
| #endif | ||
| strftime(filedate, sizeof(filedate), "%x %X", localtime(&st.st_mtime)); |
There was a problem hiding this comment.
Woke up with a bad feeling, that it can be unsafe since Raven's font set doesn't have a backslash symbol, and attempt to draw non existing symbol is a direct path to program crash. But looks likes it is safe all around, none of existing date formats are using \.
There was a problem hiding this comment.
We could just fix font drawing to skip non-existent character patches.
There was a problem hiding this comment.
Neat idea, even outside of this PR, this can potentially improve compatibility with custom texts. For small font drawing, it should be something like this:
+++ crispy-doom/src/heretic/mn_menu.c Tue Apr 23 10:24:01 2024
@@ -631,6 +631,10 @@ void MN_DrTextA(const char *text, int x, int y)
{
x += 5;
}
+ else if (c > 91)
+ {
+ continue;
+ }
else
{
p = W_CacheLumpNum(FontABaseLump + c - 33, PU_CACHE);
Why exactly > 91: no font characters available starting from 92, acording to ASCII table. Raven fonts starting from FONTA01, while in ASCII characters it's a 33. Last available lump for small font is FONTA59, so 59 + 33 equals 92.
Hopefully. 🙂 At least this way I no longer have crashes by using \ symbol in text drawing. But probably worth to leave it for separated PR?
There was a problem hiding this comment.
Yes, Heretic's A-font does only have 59 characters, so everything after 33+59 should be skipped. I would extend the earlier check, though, and just print a blank space.
There was a problem hiding this comment.
💡! True, patch drawing is guarded by following else condition, so it can be simplified to just
if (c < 33 || c > 91)
{
x += 5;
}
Worth to do same trick with big font FONTB**, except it have 58 characters, so check should be if (c < 33 || c > 90)
There was a problem hiding this comment.
Yes, please. And I think it belongs into this bug report, because the time and date string is not under our control.
Co-Authored-By: Fabian Greffrath <fabian@greffrath.com>
|
Glad to be back, happy to make good things good. 🤗 I have one more suggestion, yet it's very small: JNechaevsky/CRL@e68bc1b. Pretty self-explanatory, but I found it useful when I need to restart game level from tally screen, i.e. not only while So, this small correction could be useful for handling such cases, and if I'm not mistaking, Woof behaves same way. |
In Woof, |
|
I'll investigate it then. Correction itself is very straight-forward, but how it should work with possible |
Woof can restart level if |
|
The idea behind |
Maybe we should be more conservative and just add a restart on the tally screen? Other options are not very practical. |
|
I'd agree with |
| @@ -627,7 +627,7 @@ void MN_DrTextA(const char *text, int x, int y) | |||
|
|
|||
| while ((c = *text++) != 0) | |||
| { | |||
| if (c < 33) | |||
| if (c < 33 || c > 91) // [crispy] fail-safe: draw patches above FONTA59 as spaces | |||
There was a problem hiding this comment.
This was a Right Thing to do in terms of fail-safe, but there is still some potential for improvement. Small example, I'm using British Antigua and Barbuda locale, which is using lowercase for am/pm designation. Unlike Doom, Heretic can't handle lowercase characters, that's why all game strings are always uppercased. In this case, designation can't appear:
Possible solution is to forcefully convert lower case chars to uppercase, this will do the trick:
while ((c = *text++) != 0)
{
+++ if (c > 96 && c < 123) // means, all ASCII chars "a" ... "z"
+++ c -= 32; // ... will be reindexed to "A" ... "Z"But how to properly make this condition friendly with newly introduced || c > 91) and avoid messy maze of conditions, that's a good question. 🤔
There was a problem hiding this comment.
Something like this, but needs support for big font (it doesn't have [ symbol (№59) which is a cursor) and probably smarter conditions for indexes checking.
+++ international-doom/src/heretic/mn_menu.c Wed Apr 24 08:41:20 2024
@@ -4148,6 +4148,24 @@
//
//---------------------------------------------------------------------------
+static const char MN_CheckValidChar (char ascii_index, int small_font)
+{
+ // Following characters not exising in font graphics, replace with spaces:
+ // \ ] ^ _ { | } ~
+ if ((ascii_index >= 92 && ascii_index <= 96) || ascii_index >= 123)
+ {
+ return 32;
+ }
+ // Force lowercase a...z characters to uppercase A...Z.
+ if (ascii_index >= 97 && ascii_index <= 122)
+ {
+ return ascii_index -= 32;
+ }
+
+ // Valid char, do not modify ASCII index.
+ return ascii_index;
+}
+
void MN_DrTextA (const char *text, int x, int y, byte *table)
{
char c;
@@ -4157,6 +4175,8 void MN_DrTextA (const char *text, int x, int y, byte *table) @@
while ((c = *text++) != 0)
{
+ c = MN_CheckValidChar(c, 1);
+
if (c < 33)
{
x += 5;
This way, even such messy string will work: E1M1: THE \\ {|} ~ dOcKs
TODO's: