Game saves vs scripts

[jpab]

I made an attempt at putting Lua in charge of what sounds the hyperdrive makes, because Lua is currently in charge of other aspects of the hyperdrive and I wanted to get WKFO's military drive sound effects into the game. In my first attempt at this (https://github.com/pioneerspacesim/pioneer/pull/4202) I hit a problem because our game saves embed a bunch of state from Lua.

We don't have good separation between static game data (stuff like the tables of equipment information in data/libs/Equipment.lua) and dynamic game state (the stuff that actually needs to be included in the save like what pieces of equipment you've got installed in your ship. They're mixed together so instead of the save recording "player has 'hyperdrive_mil1' installed" it instead records "player has equipment { l10n_key="DRIVE_MIL1", fuel=cargo.military_fuel, byproduct=cargo.radioactives, slots="engine", price=23000, capabilities={mass=3, hyperclass=1}, purchasable=true, tech_level=10 }.

I'm not sure if that explanation is totally correct, but that's what it looks like so far.

This seems quite broken to me, because it means changes to the scripts won't take effect correctly when loading existing save games. Simple things like adjusting the price of a piece of equipment won't even work.

Should we do something about this? Anyone have opinions on what we should do about it?

[jpab]

Btw, here is a dumb program to decompress a save file. It would be good (IMO) if we changed the save format to write the file out with gzip headers or something so that it can be decompressed with widely available command line tools.

Code: Select all

#include <iostream>
#include <fstream>
#include <string>

#define MINIZ_STATIC_FUNCTIONS
#include "miniz.h"

int main(int argc, char **argv) {
	const char *path = argv[1];

    // read entire file into string
    if(std::ifstream is{path, std::ios::binary | std::ios::ate}) {
        auto size = is.tellg();
        std::cerr << "reading " << size << " data bytes.\n";
        std::string str(size, '\0'); // construct string to stream size
        is.seekg(0);
        if (is.read(&str[0], size)) {
        	std::cerr << "decompressing " << size << " data bytes.\n";
        	size_t out_len = 0u;
			char *uncomp = static_cast<char*>(tinfl_decompress_mem_to_heap(str.c_str(), str.size(), &out_len, 0));
        	std::cerr << "decompressed size " << out_len << " bytes.\n";
			std::cout.write(uncomp, out_len);
        } else {
        	std::cerr << "failed to read data\n";
        }
    } else {
    	std::cerr << "failed to open file\n";
    }

	return 0;
}

[jpab]

Also, here is an incomplete change to serialise Lua objects to JSON instead of serialising them to a custom serialisation format which is then compressed with miniz and dumped into the JSON file as an array of numbers. Serialising to JSON is almost certainly less efficient, but makes it possible to read most of the saved Lua state directly from the save file (after decompressing the file and reformatting it) so that you can see approximately what's going on.

https://github.com/johnbartholomew/pion ... ua-to-json

[impaktor]

I know the guy who re-did our save format last, had limited understanding of save system, so he did the best he could, and this is what we have. What I'm hinting at, is that I'm not surprised there be strangeness in the format, nor that there is a lot of things to improve. That's as much I can say about it.

I suspect you, robn, laarmen and fluffy are the ones who have the best understanding in this.

[laarmen]

First off, yay for the Lua2JSON conversion.

For the equipment, the problem is somewhat linked to the fact that we don't really differentiate equipment types and instances, mostly because when I wrote it I didn't have the courage to discuss it with the grown ups so I stuck with the statu quo.

But hey, I'm one of the grown ups now, so let's assume we have instances.
Types should not get serialized, instances should. The base price of an equipment is a type property (your mil1 drive is basically worth as much as mine), which means it shouldn't be serialized. And voilà, problem solved (in the vapourware version of Pioneer, at least).

Instanciation would also neatly solve the cabin problem, as we wouldn't have this hack of adding and removing equipment, it'd just be a boolean to flip in the instance (or even better, simply add a reference to the occupying character).

IMO the first step would be to have all classes subscribe to a registry, and make the de/serialization of the classes themselves simply use the registry key. Then, transform all the Equipment lua class instances into equipment subclasses with type variables. This shouldn't break existing "client" code, so that's a good thing (it would break the saves though). This should take care of the actual issue jpab is raising (at least for the equipment). If we don't mind doing two savebumps, I'd really like having this first part in its own PR as it doesn't fundamentally change anything related to the way we simulate the universe itself.

Afterwards, the fun starts. Theoretically we could get away with simply patching EquipSet so that when one tries to add a type instead of an instance, it takes care of the instantiation (and emits a proper deprecation warning), since most code shouldn't check for equipment but rather for specific capabilities brought by said equipment. Sadly, I'm fairly sure it isn't always so. And in any case the UI code deals with equipment for obvious reasons.
I don't have the code before my eyes, so I'm assuming the EquipSet don't store type-amount couples (as in ((hydrogen,3),(slaves,2)) but rather duplicate references (hydrogen, hydrogen, hydrogen, slaves, slaves). If not so, that would need to be patched.

(I tried to structure my thoughts but in the end it was a bit of brain dump...)

[impaktor]

(as in ((hydrogen,3),(slaves,2))

Wait, are you hinting replacing Lua with Lisp? :P

[laarmen]

I knew either you or ecraven would pick up on that...

[joonicks]

I am (or used to be) maintainer of an IRC bot and I handled savefiles like this;

instead of writing out the data in a weird format that needed to be interpreted by new code, I wrote the savefile pretty much as a program, using existing commands that would re-create the current session perfectly.

Its a crazy idea, but Pioneer has a Lua interpreter. Instead of writing the save in some custom format requiring a custom interpreter, its possible to write the save out as a lua script to recreate the gamestate. Yes there will be things missing at first, but it will be much better to include extra lua commands to alter the gamestate than to write a whole interpreter just for the savefile.

I used this method to save on code in my bot and boy did it work well. Lots of redundancy was removed.

[jpab]

[...]
Its a crazy idea, but Pioneer has a Lua interpreter. Instead of writing the save in some custom format requiring a custom interpreter, its possible to write the save out as a lua script to recreate the gamestate. Yes there will be things missing at first, but it will be much better to include extra lua commands to alter the gamestate than to write a whole interpreter just for the savefile.
[...]

This is possible, but I don't think it's a good idea for Pioneer, for various reasons:

I don't agree that all internal simulation state should be exposed to Lua scripts. There's a lot of internal simulation state that I don't think Lua needs access to at all. For example, animation state, hyperjump countdown, in-progress sounds, etc. I'm ok with Lua controlling these things, e.g., starting a hyperjump or aborting an in-progress jump, but I don't think it needs access to all the internal state. But at least some of those details do need to be included in the save file.

It would be a lot of work to expose every internal detail to Lua, not just a ton of work initially but ongoing work as it would mean most C++ side changes would need to update the Lua bindings. Even if we use some more sophisticated way of binding stuff to Lua to reduce the amount of code needed it's a pretty big burden to add for a lot of the people contributing to Pioneer. Maintaining the existing type of save code takes effort too, but less overall.

I think save/load bugs with a script/imperative type of save file would be even harder to diagnose than the existing system (and we have plenty of bugs with the existing system!)

We might hit performance problems (I think it is likely, but it's difficult to predict) with this approach; it's got a lot of overhead. More for loading than for saving probably, but quite a lot either way. We already had problems with performance of saving and loading the game when we first switched to JSON and had to make a bunch of changes to make it faster. If we're saving to Lua scripts I think fixing performance problems will be much harder because we'd be constrained by what makes sense for the Lua bindings to do.

Saving to scripts makes it extremely difficult to write separate tools to doing anything useful with save files. Saving to a structured format with good library support in several programming languages makes it much easier for people to write tools outside the game itself.