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

[Linux] Order of savegames. #324

Open
maxz opened this issue Sep 14, 2023 · 4 comments
Open

[Linux] Order of savegames. #324

maxz opened this issue Sep 14, 2023 · 4 comments

Comments

@maxz
Copy link
Contributor

maxz commented Sep 14, 2023

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
Copy link
Owner

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
Copy link
Contributor Author

maxz commented Sep 25, 2023

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
Copy link
Contributor

mlauss2 commented Sep 26, 2023

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
Copy link
Contributor Author

maxz commented Sep 26, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants