Update FileClass code to build and work on Unix. by daliborfox · Pull Request #145 · OpenTS-Developers/OpenTS

daliborfox · 2026-09-07T21:16:04Z

This pull request introduces updates from Vanilla Conquer to the FileClass classes to make them work on Unix / POSIX systems.

It introduces a file.cpp module from Vanilla Conquer that allows for searching/identifying files in the filesystem. This replaces the file finding functions in CDFileClass, and it should be possible to replace the other file finding code in the project with them.

The utf.h header from Vanilla Conquer is also introduced. Perhaps it could be merged with the newly-introduced utf8.h header?

daliborfox · 2026-09-08T14:29:50Z

I've removed the utf.h file that I've originally copied over from Vanilla Conquer. Its purpose was to allow the binary to be compiled in UTF-16 mode, though it appears that the project targets UTF-8 mode instead.

I've also cleaned up 2 accidental changes, the removal of DOS date parsing macros (they're restored), and changing permissions to a script file (they're back to normal now).

I think the PR is ready for review now. I've tested the code, the RawFileClass behaves outwardly the same to the old version, including the READ|WRITE mode, for which I had to introduce fseek() calls when switching between reads and writes as per the following documentation page: https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fopen-wfopen?view=msvc-170

The DOS timestamp setting/getting also works on both Windows and Linux now.

With these changes, I'm able to locally build the CCFileClass / MixFileClass on Linux and load assets from MIX files, and the game on Windows appears to still work correctly too (cutscenes load, levels load, settings work, saving and loading games works).

daliborfox · 2026-09-08T16:42:54Z

I'm attaching the test programs I've been using to test these changes:

test_main.cpp - Loads a single file out of a mixfile
test_raw.cpp - Performs file reads, writes, and date getting/setting

I've used the first program to verify that the MixFileClass / CCFileClass is able to correctly load assets on Linux, including correctly dealing with differing case sensitivity, and I've used the second program to verify that reading/writing and setting/getting the modification timestamp works as expected, matching the behavior of the old code.

MahBoiDeveloper · 2026-09-08T17:28:27Z


 		default:
-			return(path + '\\');
+			return(path + (char)std::filesystem::path::preferred_separator);


std::filesystem::path::preferred_separator for Win32 build is L'\\'?

That is correct, here is the reference page that I've followed: https://en.cppreference.com/cpp/filesystem/path/make_preferred

MahBoiDeveloper · 2026-09-08T17:33:01Z

+bool Find_File_Data_Posix::FindNextWithFilter()
+{
+    while (true) {
+        DirEntry = readdir(Directory);
+        if (DirEntry == nullptr) {
+            return false;
+        }
+        if (fnmatch(FileFilter, DirEntry->d_name, FNM_PATHNAME | FNM_CASEFOLD) == 0) {
+            strcpy(FullName, DirName);
+            strcat(FullName, DirEntry->d_name);
+            break;
+        }
+    }
+    return true;
+}


Why not use methods from std::filesystem and less dangrous functions like strcpy?

Good point! The file* module is copied as-is from Vanilla Conquer, but I can certainly spend some time updating it to be more idiomatic.

Good point! The file* module is copied as-is from Vanilla Conquer, but I can certainly spend some time updating it to be more idiomatic.

I might be wrong, but Vanilla Conquer better to comment with permalink to the github repo.

MahBoiDeveloper · 2026-09-08T17:34:33Z

+    ULARGE_INTEGER ull;
+    ull.LowPart = FindData.ftLastWriteTime.dwLowDateTime;
+    ull.HighPart = FindData.ftLastWriteTime.dwHighDateTime;
+    return (unsigned int)(ull.QuadPart / 10000000ULL - 11644473600ULL);


Magic numbers.

MahBoiDeveloper · 2026-09-08T17:35:23Z

+#include <utime.h>
+#include <ctime>
+#define _unlink         unlink
+#define fopen(x, y) fopen(x, y)


define fopen as fopen?

Good point! Thanks for catching this leftover, it's from when there was a raw_fopen() wrapper call for dealing with a potential UTF-16 build, that's been removed though with the removal of the utf.h file. I'll clean this up.

MahBoiDeveloper · 2026-09-08T17:37:14Z

+		if (fclose(Handle) != 0) {
+			Error(errno, false, Filename);


Suggested change

if (fclose(Handle) != 0) {

Error(errno, false, Filename);

if (!fclose(Handle)) {

Error(errno, false, Filename);

I prefer having != 0 here to emphasize that the success condition is zero. This is because not including that comparison operator makes it look like a typical boolean operation where zero is failure and non-zero is success, but it's the other way around for this call - and your suggestion introduces a bug :)

MahBoiDeveloper · 2026-09-08T17:38:06Z

+			Date = (((parsed_time->tm_year - 80) & ((1 << 7) - 1)) << 9) |
+			       (((parsed_time->tm_mon + 1)   & ((1 << 4) - 1)) << 5) |
+			       (parsed_time->tm_mday         & ((1 << 5) - 1));


Explain magic pls

The data format is defined here: https://learn.microsoft.com/en-us/windows/win32/api/oleauto/nf-oleauto-dosdatetimetovarianttime

Would you like me to introduce macros to make this clearer, or perhaps explanation comments?

The header file rawfile.h also contains comments that describe the dos date/time bitfields

But yeah, the way the bitshifts work is that ((1 << n) - 1) creates an n-bit mask of all 1s, used for ANDing (picking out the relevant bits from the input), and the subsequent << n shifts the selected bit range into the correct position as defined in the spec.

MahBoiDeveloper · 2026-09-08T17:38:12Z

+			Time = ((parsed_time->tm_hour        & ((1 << 5) - 1)) << 11) |
+			       ((parsed_time->tm_min         & ((1 << 6) - 1)) << 5)  |
+			       ((parsed_time->tm_sec >> 1)   & ((1 << 5) - 1));
+
+			return(Date << 16 | Time);


Explain magic pls

MahBoiDeveloper · 2026-09-08T17:38:31Z

+	Date = (datetime >> 16) & 0xFFFF;
+	Time =  datetime        & 0xFFFF;
+
+	input_time.tm_year = ((Date >> 9) & ((1 << 7) - 1)) + 80;
+	input_time.tm_mon  = ((Date >> 5) & ((1 << 4) - 1)) - 1;
+	input_time.tm_mday = Date         & ((1 << 5) - 1);
+
+	input_time.tm_hour = (Time >> 11) & ((1 << 5) - 1);
+	input_time.tm_min  = (Time >> 5)  & ((1 << 6) - 1);
+	input_time.tm_sec  = (Time        & ((1 << 5) - 1)) << 1;
+
+	input_time.tm_isdst = -1;


Another black magic

MahBoiDeveloper · 2026-09-08T17:39:51Z

 	**	the staging buffer.
 	*/
-	int add_count = std::min((int)length, SRC_BLOCK_SIZE - PartialCount);
+	int add_count = std::min((int)length, (int)SRC_BLOCK_SIZE - PartialCount);


Maybe better static_cast at least with std::min?

Okay, I'll play around with it. I added the cast because I got an overloading resolution failure when trying to build the original line with g++

Okay, I'll play around with it. I added the cast because I got an overloading resolution failure when trying to build the original line with g++

I'm not the project maintainer or contributor, but I see the danger in using C-casts in C++ code base instead of CPP-casts. The maintainers' opinions may differ from mine.

MahBoiDeveloper · 2026-09-08T17:44:49Z

+#define _MAX_FNAME 255
+#define _MAX_EXT   8
+#define _MAX_PATH  512


I have a lot of questions about this block because Win 10 support much longer paths

https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry

Yeah, that's true, but like the linked document page suggests, in order to make use of the extended path lengths, you need to prefix them with "\?".

I think that enabling long paths is going to be a long-term refactoring effort outside the scope of this particular pull request. I can certainly start investigating it though

Yeah, that's true, but like the linked document page suggests, in order to make use of the extended path lengths, you need to prefix them with "?".

I think that enabling long paths is going to be a long-term refactoring effort outside the scope of this particular pull request. I can certainly start investigating it though

Sorry, but I worried if the next OTS build would be worse than official game.exe because game can crash or if there are non-ascii chars in path.

Only on < Windows 10 Version 1903 (May 2019 Update

MahBoiDeveloper · 2026-09-08T17:46:14Z


 		default:
-			UserPath += '\\';
+			UserPath += std::filesystem::path::preferred_separator;


What type is UserPath?

It's std::string, it's defined at the start of the cdfile.c file

It's std::string, it's defined at the start of the cdfile.c file

Would you kindly check, is UserPath are wchat_t with win32 build? Because std::filesystem::path::preferred_separator is a wchat_t for win32 and char for linux.

No, UserPath is not wchar_t, it's a regular 8-bit character string. The OpenTS project uses UTF-8 internally, and it has it configured in the manifest, so it should be fine to use the "narrow" APIs instead of the wide APIs, passing them 8-bit character strings.

The preferred_separator is realistically only going to have values in the ASCII range, either forward-slash or backslash, perhaps a colon on Classic MacOS but I don't see there being a C++20 compiler for that, so it should be fine to down-cast it from wchar_t to char.

MahBoiDeveloper · 2026-09-08T17:47:38Z

+#ifndef _WIN32
+    // step through each sub-directory before going for the win
+    char* next = fname;
+    while (next = strchr(next, '/')) {
+        *next = '\0';
+        Resolve_File_Single(fname);
+        *next++ = '/';
+    }
+
+    Resolve_File_Single(fname);
+#endif


Are you sure that type of magic is a good one?

It's been tested under Vanilla Conquer, that's where I got it from. But it is true that since we have C++20 enabled, I should probably investigate how to rewrite this cleaner with std::filesystem

It's been tested under Vanilla Conquer, that's where I got it from. But it is true that since we have C++20 enabled, I should probably investigate how to rewrite this cleaner with std::filesystem

With std::filesystem it would be much cleaner and CPPish

MahBoiDeveloper · 2026-09-08T19:22:22Z

+#define _MAX_EXT   8
+#define _MAX_PATH  512
+#define MAX_PATH   _MAX_PATH
+#define _CONTROL   0x20  // space, first non-control character in ASCII


Why space called _CONTROL?

It delimits the range of ASCII control characters vs printable characters. It's an internal Microsoft ctype.h thing that Westwood ended up using in their code. Should be factored out, not in the scope of this PR

MahBoiDeveloper · 2026-09-08T19:23:20Z

+#undef _stricmp
+#define stricmp  strcasecmp
+#define _stricmp strcasecmp
+#define strnicmp strncasecmp
+#define memicmp  strncasecmp
+#define __cdecl


What magic is this? Also why #define __cdecl

__cdecl doesn't exist on some compilers.

MahBoiDeveloper · 2026-09-08T19:24:55Z

+    }
+    *ffblk = nullptr;
+
+    *ffblk = Find_File_Data::CreateFindData();


Could Find_File_Data::CreateFindData() return nullptr?

MahBoiDeveloper · 2026-09-08T19:25:58Z

+    delete *ffblk;
+    *ffblk = nullptr;


God bless smart pointers. I wish OTS would use them.

It does, where appropriate. Smart pointers are not a panacea

MahBoiDeveloper · 2026-09-08T19:29:36Z

+#ifndef _WIN32
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif


Why #endif in the start of thee file if this file are *nix only?

Because we have cmake glob files so this will also get built on other platforms

MahBoiDeveloper · 2026-09-08T19:38:32Z

+		int position = ftell(Handle);
+		if (position < 0) {
+			Error(errno, false, Filename);
+			return(0);


Why not return 0;?

MahBoiDeveloper · 2026-09-08T19:38:36Z


+		if (fseek(Handle, 0, SEEK_END) < 0) {
+			Error(errno, false, Filename);
+			return(0);


Why not return 0;?

Because Westwood devs apparently liked using parenthesis around the return values.

I'm trying to keep the style of the code the same as the remaining fine, there's nothing more jarring than having multiple different coding styles within a single file :P

MahBoiDeveloper · 2026-09-08T19:39:46Z

+			WORD dosdate;
+			WORD dostime;
+			FileTimeToDosDateTime(&info.ftLastWriteTime, &dosdate, &dostime);
+			return((dosdate << 16) | dostime);


Need explanation.

The DOS date/time format is described in the header files, there are macros provided at the top of wwfile.h to parse out the fields, and the bit ranges are described in the class declaration in rawfile.h.

I've added links to Microsoft and Single UNIX Specification docs for the corresponding date/time formats into the functions.

MahBoiDeveloper · 2026-09-08T19:40:40Z

+		buf.actime  = unix_time;
+		buf.modtime = unix_time;
+
+		return(utime(Filename, &buf) == 0);


tbf this syntax of return feels like C99, not C++20.

The thing with return having parenthesis is a Westwoodism, I'm trying to match the code to its surroundings.

MahBoiDeveloper · 2026-09-08T19:40:58Z

+			if (fseek(Handle, pos, dir) < 0) {
+				Error(errno, false, Filename);
+			}
+			LastAccessType = 0;


Magic number

It's not a magic number, it's the same data type as the read/write permission arguments to Open(), the used values are READ, WRITE, and 0 for there being no previous I/O operation.

ZivDero · 2026-09-09T23:38:45Z

This is gonna need a bit of an update once the save file format rework lands, because it replaces CLSID with a local ClassID copy, and thus the INI methods will need to stay.

daliborfox · 2026-09-10T17:43:38Z

Thanks for the heads-up, I'll rebase the changes tomorrow, and hopefully also add a change description.

In the meantime, I've updated the copyright notices, added comments with links to docs about the date/time format, and updated the failing test to perform case-insensitive string matching.

I've used _stricmp() for this, which appears to be used throughout the game for this purpose, but if you'd prefer a different way to do this, I'll happily update the code again!

daliborfox force-pushed the file-class-unix branch 2 times, most recently from 0f549bc to a1c357d Compare September 8, 2026 14:22

daliborfox force-pushed the file-class-unix branch 2 times, most recently from 52b97e5 to 3b01d74 Compare September 8, 2026 15:37

MahBoiDeveloper reviewed Sep 8, 2026

View reviewed changes

MahBoiDeveloper suggested changes Sep 8, 2026

View reviewed changes

daliborfox added 2 commits September 10, 2026 19:37

Update FileClass code to build and work on Unix.

ac0da88

Update filename checks in gamedirscontract test to be case insensitive.

8c5a34c

daliborfox force-pushed the file-class-unix branch from 3b01d74 to 8c5a34c Compare September 10, 2026 17:39

Uh oh!

Conversation

daliborfox commented Sep 7, 2026

Uh oh!

daliborfox commented Sep 8, 2026

Uh oh!

daliborfox commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MahBoiDeveloper Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daliborfox Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

daliborfox commented Sep 8, 2026 •

edited

Loading

MahBoiDeveloper Sep 8, 2026 •

edited

Loading

daliborfox Sep 10, 2026 •

edited

Loading

daliborfox Sep 10, 2026 •

edited

Loading