adventuregamestudio / ags

AGS editor and engine source code
Other
697 stars 159 forks source link

Support asynchronously saving the game #1618

Open ericoporto opened 2 years ago

ericoporto commented 2 years ago

Describe the problem Saving the game in AGS may produce a stutter, because it saves in a way that blocks the game loop. This is usually fine when the player is manually saving the game, but may produce visible stutter when done in a Autosave like way, where the game is saved after a room fades in or some in-game event happens. This is less perceptible in Windows since the OS is able to alleviate, but may be more perceptible in other systems.

Suggested change There are different ways to go here:

Essentially this means that until the game is saved, the state of the game at that point has to be duplicated in RAM to allow the game to proceed forward without waiting a write to disk.

Whatever the way, we don't want this in the Emscripten port since it will write to ram anyway for now, and also because we don't want threads there (in case this is enabled through threads).

additional context

ivan-mogilko commented 2 years ago

To clarify: is the goal to keep game fully running, or prevent the game window/screen "freezing" because the system events don't poll?

ivan-mogilko commented 2 years ago

EDIT:

Essentially this means that until the game is saved, the state of the game at that point has to be duplicated in RAM to allow the game to proceed forward without waiting a write to disk.

Literally duplicating the game state may be quite complicated with the current state of the engine code.

Writing into the ram is already supported using a memory stream class, so that may be an alternative. That is virtually a matter of providing a temporary buffer and substituting one stream object with another for the "save game" function call. Then test and see how much faster that is.

One extra concern I have, if the game has alot of dynamic objects in use, that might push it closer to user's system limits.

ericoporto commented 2 years ago

is the goal to keep game fully running, or prevent the game window/screen "freezing" because the system events don't poll?

I don't understand what is the difference between the two options. I observe in Linux when an autosave happens in a game, the game tends to lock until the write to disk finishes, usually the screen and everything seems to lock by some time.

Writing into the ram is already supported using a memory stream class, so that may be an alternative.

Yes, that seems fine too, but then something has to pick this resulting memory stream and write to disk, right? If this is what you mean, then it's ok, this does implement what I meant of duplicating the state in RAM, as the game will proceed, but you now have this copy from serialization which you need to write to disk. Essentially, I want to avoid depending on disk IO performance to say if the next game frame will happen perceptually instantly or not.

One extra concern I have, if the game has alot of dynamic objects in use, that might push it closer to user's system limits.

Uhm, I don't think this would be a problem when serializing to RAM, but the alternative would be doing some sort of compression as it serializes. While this compression could be nice, it is something that goes beyond the scope I had in mind originally.

Edit: to clarify, I don't actually have an idea on how to do this right now, it's mostly the issue of the stutter I meant to solve, but I didn't thought it made sense to report it as a bug since it's something that the solution is a new feature. The second part of the thing which would write to disk in a decoupled way I don't know yet how to accomplish.

ivan-mogilko commented 2 years ago

is the goal to keep game fully running, or prevent the game window/screen "freezing" because the system events don't poll?

I don't understand what is the difference between the two options.

Well, 1) the first requires to make a "full copy" of a game state in memory, as you say, to let the game continue functioning as usual while it's writing to disk; 2) while the second could be easier, as it requires to poll system events and keep render, while the game state itself is completely paused.

Yes, that seems fine too, but then something has to pick this resulting memory stream and write to disk, right?

Indeed, as soon as you have a ready buffer filled with data, you need to run disk operation asynchronuously, either in a thread or step by step if threads are forbidden.

Uhm, I don't think this would be a problem when serializing to RAM

It may not be in practice with most of AGS games, but in theory it's possible to create a game that e.g. creates alot of dynamic sprites and huge arrays, and occupies most of the memory range available for 32-bit programs; then trying to save all that might crash it. That's much less likely with 64-bit programs of course.

ericoporto commented 2 years ago

Well, 1) the first requires to make a "full copy" of a game state in memory, as you say, to let the game continue functioning as usual while it's writing to disk

Yeah, that's what I had in mind. For the player, they will be able to keep playing as nothing happened.

On the game developer side, he could then use a GUI/Overlay or something to show the save is being written, and then when an event happen/polling returns a confirmation, hide the indicator. This indicator is something scripted, just to materialize the information of "save write to disk started", "save write to disk finished", or possibly even blocking the save interface for the player until the write has happened - again, if there's some way to get feedback and if the dev decides to script like this.

In the Engine, I guess it needs some way to schedule writes to disk, and some way to get a feedback after it's finished - if a filesystem error occurred, it would be informed asynchronously. This is possibly a thread that watches for a list of jobs of file content (memory streams?) and path, and executes as they happen.

An edge case I think can happen is the player saves the game and the developer did not properly leverage the async save system (blocking the load game ui until previous save has finished), so now the player clicks to load a save before it has been written to disk fully, I am not sure what happens in this case.

Maybe it's useful to look into async IO libraries to see how those work with this. But to close the scope of the issue, the problem is just with Save files, as other types of File write operations can usually be workaround by writing just a little bit per frame if needed.

ivan-mogilko commented 2 years ago

so now the player clicks to load a save before it has been written to disk fully, I am not sure what happens in this case.

It may save to a temp file which is invisible to the game, and then rename/move to the save folder.

ericoporto commented 2 days ago

I don't know if useful or not, but I saw that SDL3 has an async io implementation in the works here: https://github.com/libsdl-org/SDL/pull/10605, I imagine it would be possible to have the file writing in this case run through this somehow - I haven't looked too much in the API changes of SDL3.

ivan-mogilko commented 2 days ago

But the main problem in here is not a lack of async io, it's in the fact that the game cannot continue until it's written, because engine writes parts of the game without cloning data first.

EDIT: I can't tell at this point whether having async io is even a obligatory requirement in this issue. Running the saving on a separate thread is, but afaik you don't have to have a async io api for that.

EDIT2: Previously in this discussion I mentioned an opportunity to save into ram using memory stream synchronously, and then dump the resulting array into the file on a separate thread. I suppose that is not too hard to make this experiment and see whether it makes things any faster.

In addition, there's #2543, that may potentially make saves smaller by cutting out parts that do not have to be in saves, such as dynamic sprites.

nfries88 commented 1 day ago

Came over here because I saw the mention in SDL3. the main motivation for async I/O in SDL3 is reading assets, not savefiles.

@ivan-mogilko looks to be essentially right about why you don't really need async I/O and what the best approach probably is. building the entire savefile into memory before writing it out is what you need to do.

Note that you might not even need the background thread. On most OSes the write of data to the file itself is (subject to memory availability and limits in the system configuration) essentially asynchronous when using normal I/O, but writes that change the size of a file always have to block on updating filesystem metadata. Skimming your code it looks like you make lots of fairly small writes, which means you're blocking on updating this metadata a lot. So building the savefile in memory and sending it to disk as a single write may be enough to make this latency more acceptable.

ericoporto commented 1 day ago

On most OSes the write of data to the file itself is

Honestly I can only actually observe this in Windows and I never could have this actually work with me on Linux - I haven't payed attention on this specifically on macOS, iOS or Android.