fabiangreffrath / crispy-doom

Crispy Doom is a limit-removing enhanced-resolution Doom source port based on Chocolate Doom.
https://fabiangreffrath.github.io/crispy-homepage
GNU General Public License v2.0
807 stars 132 forks source link

Strife: save timestamp seems to be broken #1218

Closed maxmanium closed 3 months ago

maxmanium commented 3 months ago

Background

Version of Crispy Doom: 7.0.0

Operating System and version: Windows 11 23H2

Game: Strife

Any loaded WADs and mods (please include full command line): N/A

Bug description

Observed behavior: The timestamp for a saved game seems to always read 12/31/1969, 6:00:00pm.

Expected behavior: Should have the actual time.

fabiangreffrath commented 3 months ago

I see, thanks. Why doesn't the code in Strife check the return value of stat()?

ceski-1 commented 3 months ago

It's trying to read the file date of doomsav.dsg which doesn't exist for Strife. The actual save files are in the strfsav#.ssg subfolders. Related commit? https://github.com/fabiangreffrath/crispy-doom/commit/4c658f28658ba6781fbda96290bf68cfa42e5d73

JNechaevsky commented 3 months ago

Oh my, turns out, P_SaveGameFile() was reading file times of Doom saves, which were made shortly before this change was added to Strife and I didn't noticed a difference of few minutes. I have to investigate then.

JNechaevsky commented 3 months ago
crispy-doom/src/strife/p_saveg.c
@@ -72,7 +72,7 @@
         filename = malloc(filename_size);
     }

-    DEH_snprintf(basename, 32, SAVEGAMENAME "%d.dsg", slot);
+    DEH_snprintf(basename, 32, "strfsav%d.ssg/name", slot); // [crispy] changed to Strife savegame file name

     M_snprintf(filename, filename_size, "%s%s", savegamedir, basename);

In case you wondering why name, Strife saves a bunch of files, but seems to overwriting them every time saving is performed. Probably no difference, if another file will be chosen.

crispy-doom\build\src\strfsav1.ssg

26.08.2024  14:16    <DIR>          .
26.08.2024  14:16    <DIR>          ..
26.08.2024  14:14           109я998 2
26.08.2024  14:14                 4 current
26.08.2024  14:14           109я998 here
26.08.2024  14:14               300 mis_obj
26.08.2024  14:14                32 name
fabiangreffrath commented 3 months ago

Alright, but please take care to also check the return value of stat().

JNechaevsky commented 3 months ago

But how exactly? It should show either proper date or nothing? Or something else?

fabiangreffrath commented 3 months ago

You pass it a pointer to a struct stat and if the function cannot read the stats for the file for whatever reason, that struct will not get modified at all.

https://github.com/fabiangreffrath/woof/blob/387c0deb03d138632dde763ba21fe53ce9682704/src/mn_snapshot.c#L94-L110

JNechaevsky commented 3 months ago

Something like this? We don't need an empty name '\0' anyway, since we are already checking for file existence via LoadMenu[itemOn].status:

crispy-doom/src/strife/m_menu.c
@@ -864,8 +864,8 @@
         struct stat st;
         char filedate[32];

-        stat(P_SaveGameFile(itemOn), &st);
-
+        if (stat(P_SaveGameFile(itemOn), &st) != -1)
+        {
 // [FG] suppress the most useless compiler warning ever
 #if defined(__GNUC__)
 #pragma GCC diagnostic push
@@ -876,6 +876,7 @@
 #pragma GCC diagnostic pop
 #endif
         M_WriteText(ORIGWIDTH/2-M_StringWidth(filedate)/2, y, filedate);
+        }
     }
 }
rfomin commented 3 months ago

stat(P_SaveGameFile(itemOn), &st)

Please use M_stat, otherwise it will not work with Unicode names in Windows (e.g. from a folder on the desktop). Also we need this patch:

diff --git a/src/m_misc.c b/src/m_misc.c
index ace206d4..14c509e4 100644
--- a/src/m_misc.c
+++ b/src/m_misc.c
@@ -302,6 +302,7 @@ int M_stat(const char *path, struct stat *buf)
     // incompatible with struct stat*. We copy only the required compatible
     // field.
     buf->st_mode = wbuf.st_mode;
+    buf->st_mtime = wbuf.st_mtime;

     free(wpath);