[Linux] Order of savegames.

[maxz]

The savegames are currently not ordered by their creation time or alphabetically on GNU/Linux. Both would be somewhat sensible. I guess they might be ordered by inode order and I did not look at the other platforms to see how they appear there.

Ordering them by their creation time would probably be the best thing to do.

[luciusDXL]

TFE doesn't do any special ordering at the moment, just reads the directory and uses that order. Sorting by date probably makes sense and can be added at some point.

[maxz]

Yeah, then it would be inode order.

I already took a look at the relevant source code sections before creating the issue, but the readDirectory function seemed to be reused in many places and I refrained from changing anything because I have no idea how that might interfere with Windows. Sorting it alphabetically would still be easy, since strings are returned, sorting by date would most likely require an extra stat call for each file, but would be the most expected behaviour for savegames.

Windows should also support the stat system call through its POSIX compliance, but it's probably not the idiomatic solution on that platform.

And after all the date is already present in the savegame header, so that should most likely be used for sorting. But at that point it then got hairy to change.

[mlauss2]

Try this patch, it will sort savegames based on filesystem last modification date (linux only):

Although I think in the case of the savegames, its easier to do the sorting based on the savegame headers, since they are all read anyway and contain save date.

diff --git a/TheForceEngine/TFE_FileSystem/fileutil-posix.cpp b/TheForceEngine/TFE_FileSystem/fileutil-posix.cpp
index 91d789d7..877b53bb 100644
--- a/TheForceEngine/TFE_FileSystem/fileutil-posix.cpp
+++ b/TheForceEngine/TFE_FileSystem/fileutil-posix.cpp
@@ -1,3 +1,5 @@
+#include <algorithm>
+#include <deque>
 #include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -7,6 +9,7 @@
 #include <string.h>
 #include <strings.h>
 #include <sys/stat.h>
+#include <time.h>
 #include <unistd.h>
 #include <TFE_System/system.h>
 #include "fileutil.h"
@@ -18,7 +21,7 @@ namespace FileUtil
    bool existsNoCase(const char *filename);
    static char *findFileObjectNoCase(const char *filename, bool objisdir);

-   void readDirectory(const char *dir, const char *ext, FileList& fileList)
+   void readDirectory(const char *dir, const char *ext, FileList& fileList, enum TFE_FileSortMode sfm)
    {
        char buf[PATH_MAX];
        struct dirent *de;
@@ -27,6 +30,12 @@ namespace FileUtil
        char *dn;
        DIR *d;

+       struct temp_file_list {
+           std::string name;
+           time_t mtime;
+       };
+       std::vector<struct temp_file_list *> tfl;
+
        d = opendir(dir);
        if (!d) {
            TFE_System::logWrite(LOG_ERROR, "readDirectory", "opendir(%s) failed with %d\n", dir, errno);
@@ -52,9 +61,35 @@ namespace FileUtil
            if (0 != strncasecmp(dn + dl - el, ext, el))
                continue;
            // we have a winner
-           fileList.push_back(string(dn));
+           struct temp_file_list *tflx = new struct temp_file_list;
+           if (!tflx)
+               return;
+           tflx->name = dn;
+           tflx->mtime = st.st_mtime;
+           tfl.push_back(tflx);
        }
        closedir(d);
+
+       switch (sfm) {
+       case SORT_NONE:
+           break;
+       case SORT_NAME_ASC:
+           std::sort(tfl.begin(), tfl.end(), [](auto& a, auto& b){ return a->name < b->name; });
+           break;
+       case SORT_NAME_DESC:
+           std::sort(tfl.begin(), tfl.end(), [](auto& a, auto& b){ return a->name > b->name; });
+           break;
+       case SORT_TIME_ASC:
+           std::sort(tfl.begin(), tfl.end(), [](auto& a, auto& b){ return a->mtime < b->mtime; });
+           break;
+       case SORT_TIME_DESC:
+           std::sort(tfl.begin(), tfl.end(), [](auto& a, auto& b){ return a->mtime > b->mtime; });
+           break;
+       }
+       for (auto it = tfl.begin(); it != tfl.end(); it++) {
+           fileList.push_back((*it)->name);
+           delete *it;
+       }
    }

    void readSubdirectories(const char *dir, FileList& dirList)
diff --git a/TheForceEngine/TFE_FileSystem/fileutil.h b/TheForceEngine/TFE_FileSystem/fileutil.h
index ef548c78..d8863d75 100644
--- a/TheForceEngine/TFE_FileSystem/fileutil.h
+++ b/TheForceEngine/TFE_FileSystem/fileutil.h
@@ -6,9 +6,18 @@
 using namespace std;
 typedef vector<string> FileList;

+enum TFE_FileSortMode
+{
+   SORT_NONE = 0,
+   SORT_NAME_ASC,
+   SORT_NAME_DESC,
+   SORT_TIME_ASC,
+   SORT_TIME_DESC
+};
+
 namespace FileUtil
 {
-   void readDirectory(const char* dir, const char* ext, FileList& fileList);
+   void readDirectory(const char* dir, const char* ext, FileList& fileList, enum TFE_FileSortMode fsm = SORT_NONE);
    bool makeDirectory(const char* dir);
    void getCurrentDirectory(char* dir);
    void getExecutionDirectory(char* dir);
diff --git a/TheForceEngine/TFE_Game/saveSystem.cpp b/TheForceEngine/TFE_Game/saveSystem.cpp
index e9a65d94..da06010a 100644
--- a/TheForceEngine/TFE_Game/saveSystem.cpp
+++ b/TheForceEngine/TFE_Game/saveSystem.cpp
@@ -186,7 +186,7 @@ namespace TFE_SaveSystem
    {
        dir.clear();
        FileList fileList;
-       FileUtil::readDirectory(s_gameSavePath, "tfe", fileList);
+       FileUtil::readDirectory(s_gameSavePath, "tfe", fileList, SORT_TIME_DESC);
        size_t saveCount = fileList.size();
        dir.resize(saveCount);

[maxz]

I tested the patch and it seems to work as expected. Thanks.

It's debatable whether SORT_TIME_DESC or SORT_TIME_ASC is preferable, but I can live with both.