Inconsistent path handling for save games

[apanteleev]

In src/server/save.c, the save files are accessed through different ways:

• The read_binary_file and adjacent functions are going through the Q2's filesystem layer, which handles multiple search paths.
• The ge->ReadLevel(name) and similar functions take real filesystem paths.

In some cases, that may lead to the game trying to access different componets of a save from different locations. One case that I encountered is this:

• A development directory with a game is shared between two machines, a Linux one and a Windows one.
• The Windows machine runs the game, starts the base1 map, and writes a save into <dev-path>/baseq2/save/.current/base1.{sav,sv2,ssv}
• The Linux machine runs the game, attempts to start the same map and fails with an error saying Couldn't open <user>/.local/share/quake2rtx/baseq2/save/.current/base1.sav. Very confusing.

The reason for that failure is that SV_CheckForSavegame calls read_level_file, which checks whether a save exists using read_binary_file that asks the FS layer and therefore picks up the save from baseq2. That succeds, and later the read_level_file function calls ge->ReadLevel(name) where name is a real path based on fs_gamedir. Which is different from the baseq2 location that the FS layer found. And it fails, taking the game down with it.

Granted, this is an obscure use case, but it points at some real issues with save game handling. How should these be fixed? Obviously, all save file access should be done using the same method. Since the game library functions (ge->...) only take real paths, the other functions should also operate on real paths based on fs_gamedir.

Asking the contributors to https://github.com/NVIDIA/Q2RTX/pull/250 (XDG support) - do you see any potential issues with switching the save code to use real paths? @Connor-GH @res2k @runlevel5 @appetrosyan

Thanks!

[res2k]

It looks like upstream Q2PRO may have addressed this issue already: https://github.com/skullernet/q2pro/commit/17e38fd9a17a69dc45296154f75320d3dc8dd172 So one solution would be to port this commit over.

Otherwise, using real paths consistently sounds sensible.

[runlevel5]

I share the same opinion to that of @res2k

[appetrosyan]

I may have a controversial opinion, but i think that porting 17e38fd is not a bad idea, but could be just as error prone as the current behaviour.

Real saves are only stored in the host filesystem. The solution that honours that would work better.

[Connor-GH]

This is not a part of the code I worked on, so please help me understand:

A development directory with a game is shared between two machines, a Linux one and a Windows one. The Windows machine runs the game, starts the base1 map, and writes a save into /baseq2/save/.current/base1.{sav,sv2,ssv} The Linux machine runs the game, attempts to start the same map and fails with an error saying Couldn't open /.local/share/quake2rtx/baseq2/save/.current/base1.sav. Very confusing.

So basically, you have directory A where Windows saved the game and then directory B where a Linux system tried to pull the save from, but failed? I imagine the inverse also holds true?

If this is the case, +gamedir on the command line should get around this.

[apanteleev]

Thanks all for the comments. I think that Q2PRO commit looks more like a hacky solution than a proper one. I guess I'll try to convert all save game logic to use real paths.

So basically, you have directory A where Windows saved the game and then directory B where a Linux system tried to pull the save from, but failed? I imagine the inverse also holds true? If this is the case, +gamedir on the command line should get around this.

The problem is not that the Linux system fails to pull the save, it's that there exists a file system state where a new game cannot be started because of broken logic.