Case sensitive game command

[Diordany]

Issue

Sometimes I leave my CAPS on by accident, this leads to an issue with the game command. For example:

I'm trying to load Arcane Dimensions, which lives in a subdirectory called ad in my basedir.

1. I accidentally run the command GAME AD.
2. The engine sets com_gamedir to <path_to>/AD, and doesn't load the ad subdirectory.
3. I try to correct my action with the command game ad.

This leads to the following (the difference between lower- and uppercase is very subtle with the default charset):

This happens because I'm running QuakeSpasm on Linux, and the filesystem utils are case sensitive.

To summarize: the issue is that the engine uses case sensitivity for loading game directories, but doesn't do the same for comparing a requested gamedir with the current com_gamedir.

Potential Fix

I wrote a quick fix that solves the issue. However, I'm also considering the fact that QuakeSpasm aims for portability and that this change may annoy users that run it on an OS for which the filesystem utils are not entirely case sensitive (like Windows if I remember correctly).

To address that, I wrote another potential fix, however I feel like a platform dependent solution is out of place in common.c.

I'd like to hear what the maintainers think about this, if they're interested in fixing the issue.

[sezero]

The issue is problematic. One can hypothetically have both an ad and an AD directory in there on purpose.

[sezero]

Although I'd like to hear what @ericwa has to say about all this.

[Diordany]

The issue is problematic. One can hypothetically have both an ad and an AD directory in there on purpose.

I agree, I'll await further response.

[sezero]

For 'correction' purposes though, like the case you described, we can add a gamereset command to go back to id1 ???

[sezero]

For 'correction' purposes though, like the case you described, we can add a gamereset command to go back to id1 ???

Initial patch for that: gamereset.patch.txt

[Diordany]

For 'correction' purposes though, like the case you described, we can add a gamereset command to go back to id1 ???

Yeah, I'd been thinking about a similar solution earlier. I wasn't aware of this patch. However, I think that solves a different issue.

I think this would work fine in my example, but consider the hypothetical situation you mentioned before: if someone actually has another AD directory, that person would still run into the same problem after attempting to switch to ad.

[Diordany]

For 'correction' purposes though, like the case you described, we can add a gamereset command to go back to id1 ???

Oh I think I misunderstood your solution. I guess this could work for your example as well.

[Diordany]

Oh I think I misunderstood your solution. I guess this could work for your example as well.

If the user runs the gamereset command before attempting to load ad.

[sezero]

I think this would work fine in my example, but consider the hypothetical situation you mentioned before: if someone actually has another AD directory, that person would still run into the same problem after attempting to switch to ad.

Well, yes, that actually is a problem. (It seems I wasn't carefully reading the issue.)

Waiting for additional comments

[Diordany]

Well, yes, that actually is a problem. (It seems I wasn't carefully reading the issue.)

Waiting for additional comments

Yeah, I also think that it would be preferable to not require the user to run the gamereset command before loading another game. But let's wait for additional comments then.

[Shpoike]

I accidentally run the command GAME AD. Stuff like this is why FTE's console command names are still case sensitive. tab completion will correct it - both for the name name and for arguments of commands that support completion.

a greater concern is that of the id1/PAK0.PAK of the steam (non-re)release and the occasional 'ID1' dir from old dos/dosbox partitions/installers etc. :( for the gamedir part, FTE has a FS_FixupFileCase function that changes its case to match to an existing path if the case is wrong. for the paks part, loading *.pak (insensitively) avoids the need to check a whole series of potential camelcase names. for stuff within gamedirs/paks/pk3s fte's fs_cache stuff making gamedir access case-insensitive too (on windows it still offers a speedup). so case shouldn't matter in fte - so long as its ascii. emojii in filenames is an entirely separate kettle of nightmarish fish, with fte treating all filenames as unicode/utf-8 instead of quake's weird charset (though the console won't always know to switch parsing modes, unfortunately).

regarding gamereset... why? just game id1 done. shove a minus infront of the id1 if you insist on QS still being weird. side note: does game "" break stuff? presumably that should also do the same thing (considering that id1 cannot be unloaded). game ./game ..? lots of invalid options that could survive aliases.

[Diordany]

side note: does game "" break stuff? presumably that should also do the same thing (considering that id1 cannot be unloaded). game ./game ..? lots of invalid options that could survive aliases.

It doesn't appear to break anything, see this line.

]game ""
gamedir should be a single directory name, not a path
]game .
gamedir should be a single directory name, not a path
]game ..
gamedir should be a single directory name, not a path

regarding gamereset... why? just game id1 done.

yeah, I also run game id1 to 'reset'. It's redundant in that sense, but if @sezero likes that more, I don't see a reason why it shouldn't be a thing. Alternatively, gamereset could also be used as an alias for game id1.

[Diordany]

for the paks part, loading *.pak (insensitively) avoids the need to check a whole series of potential camelcase names. for stuff within gamedirs/paks/pk3s fte's fs_cache stuff making gamedir access case-insensitive too (on windows it still offers a speedup). so case shouldn't matter in fte - so long as its ascii.

The difference is that the engine checks if the pak files are loaded. But doesn't seem to do the same for the game directories. I think that's the actual underlying issue.

Besides, the engine also seems to have a strict pre-determined format for the pak files, that being "%s/pak%i.pak" where it first inserts the game directory and then an int as the index. So the only variable for the filename is the int index, which is not even relevant to case sensitivity. The game directory however could have many different permutations when it comes to the case sensitivity.

for the gamedir part, FTE has a FS_FixupFileCase function that changes its case to match to an existing path if the case is wrong.

This also sounds nice, I'm assuming you mean this.

[Diordany]

Stuff like this is why FTE's console command names are still case sensitive. tab completion will correct it - both for the name name and for arguments of commands that support completion.

Yeah, that's really nice as well.

[ericwa]

I'm on Windows so don't really run into this, but agree with Shpoike said. To me, the bigger issue is the official game having mixed case (Id1, PAK0.PAK) suggests that source ports should emulate case insensitivity, which QS isn't (probably few source ports do, FTE and any others? Do vkQuake or Ironwail do any work on this area?).

I would tend to avoid adding a new command like gamereset if the functionality is already available with game id1.

The patch https://github.com/Diordany/quakespasm/commit/eae6927b0196816cccda38e1032e48a324df668e seems OK given our current situation of case sensitivity, I don't have a Linux system handy to test it though.

[Diordany]

I'm on Windows so don't really run into this, but agree with Shpoike said. To me, the bigger issue is the official game having mixed case (Id1, PAK0.PAK) suggests that source ports should emulate case insensitivity, which QS isn't (probably few source ports do, FTE and any others? Do vkQuake or Ironwail do any work on this area?).

Oh, thank you for clarifying that, I didn't understand how it was relevant to this issue at first.

[Diordany]

So it seems like a choice must be made between having the annoyance of the id1 dir name inconsistency, or not allowing multiple variants when it comes to case.

Also, regarding the commit you mentioned @ericwa :

however I feel like a platform dependent solution is out of place in common.c.

What do you think about that? I brought this up because common.c seems to be platform agnostic.

[Diordany]

So it seems like a choice must be made between having the annoyance of the id1 dir name inconsistency, or not allowing multiple variants when it comes to case.

So let's say that we won't use any of the patches that I've provided in favor of mitigating the incosistency of the id1 dirs. What if we prevent the engine from setting the game to one that wasn't even found in the first place? Which was what I was thinking about when I said:

Yeah, I'd been thinking about a similar solution earlier. I wasn't aware of this patch. However, I think that solves a different issue.

and:

The difference is that the engine checks if the pak files are loaded. But doesn't seem to do the same for the game directories. I think that's the actual underlying issue.

(seems to me that it was actually more relevant to this issue than I initially thought)

[Shpoike]

hack at COM_AddGameDirectory, change the snprintfs() (and by extension the strlcpy(va())s too) to scan their basedir for entries matching the subdir/file inside. if its an exact match then early out with the exact string, if its a partial match then be willing to use that, if there's no matches then also use the exact string. this will make it autocorrect any "ID1" stuff as well as any PAK0.PAK messes. it'll also deal with the whole AD stuff too of course. (aka FTE's FS_FixupFileCase.. but without the other dependencies...)

(side note: q3's pk3 stuff does case-insensitive searches, so always release mods in proper packages?... q2's paks have evil "/../" segments in them! the horror!)

Regarding windows, windows does have case-sensitive directories too nowadays, thanks to their posix stuff of old and moreso their WSL stuff of late. autocorrecting it there will also ensure there's no new strcmp-related bugs on linux (if your devs do all their testing on windows). plus it'll give slightly more correct output from the 'path' command - not that it really matters. So there is justification for also doing it with windows's FindFirstFile/etc stuff instead of just on systems with posix's opendir/etc. For other systems it can just be an snprintf("base/child") wrapper, because frankly that's all it is right now anyway.

and yeah... for windows swap all the fopens to _wfopen and all the GetModuleNameA/GetCurrentDirectoryA/GetCommandLineA/etc stuff to their W variants, and internally use utf-8 for any filenames and voila, you can now not break if the user shoves emoji or CJK or whatever other glyphs into their basedir or gamedirs or whatever. but that's far more invasive, and kinda a separate issue, though still related. It should be noted that SDL_RWFromFile already does this (taking utf-8 on windows), so if you have ANY accents in the basedir then eg the remaster's localisations will get a non-utf8 string and will fail to load or something resulting in weird unreproducible error reports.

and then there's the MAX_OSPATH constant. 250ish ain't enough any more (especially if there's utf-8/cjk in there). recent versions of windows are no longer crippled like that (for the most part) and linux never was. hurrah for another weird source of unreadable/corrupted files. whoever said filesystems were easy?.. :\

[Diordany]

Thank you for the insight @Shpoike! I'll ~try this out later~ wait for additional comments from @sezero and @ericwa.