Current master build crashes when loading savefiles created with current release

[mwerle]

Observed behaviour

Attempt to load a savegame created with the current AppImage release (https://github.com/pioneerspacesim/pioneer/releases/download/20240710/Pioneer-x86_64.AppImage) with the current master build crashes.

pioneer: /home/micha/Source/Pioneer/contrib/json/json.hpp:14412: const nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::value_type& nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::operator[](T*) const [with T = const char; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long int; NumberUnsignedType = long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; JSONSerializer = nlohmann::adl_serializer; const_reference = const nlohmann::basic_json<>&]: Assertion `m_value.object->find(key) != m_value.object->end()' failed.
Aborted (core dumped)

This appears to have been broken by #5840

Despite the description stating that there would be a bump in the savefile version, both the old and the new savefile (attached below) are version 90.

A new save created with the current master build ("Micha_test_new.json") shows that the "Shields" entry of the player ship ("MW72") has moved location from the "model_body" to the "ship". There is no fall-back if the "shields" entry under "ship" does not exist and the above assertion in the JSON parser is triggered and the game crashes.

Expected behaviour

Game successfully loads older savegame.

Steps to reproduce

• Create a savefile using the current AppImage release
• Compile current "master" from GitHub
• Attempt to load savefile -> assertion failure and game crashes.

My pioneer version (and OS):

OS: Debian GNU/Linux 12

Pioneer Version: as described above

My output.txt (required) and game save (optional, but recommended)

Save 1: "Micha_test.json" : created using AppImage version
Save 2: "Micha_test_new.json" : created using current "master" build

Micha_test_new.json
Micha_test.json
output.txt

[impaktor]

Yeah, no surprise here. I think we've had other PRs merged (the ones touching name-gen & culture) that aught to have bumped it. Either way, if the new equipment branch is merged soon-ish.

[sturnclaw]

We've typically (in recent memory) bumped savegame versions shortly before release, after all of the PRs that break compatibility are merged. That being said, I'm planning on merging a bump "early" after #5734 is merged sometime in the next two weeks.

[mwerle]

So "master" does not always contain a build which can be reasonably expected to work? Ie, it is a dev branch (which occasionally gets tagged as a release), but not a release branch where any commit on it could potentially be turned into a release? (Not a criticism, just wrapping my head around this project.. :) )

[sturnclaw]

So "master" does not always contain a build which can be reasonably expected to work?

Speaking generally, the master branch is definitionally an unstable branch. There is no guarantee that checking out a random commit on it can be built successfully or will run, though we try to ensure that master can both build and run after each PR is merged.

Speaking to your specific concern - master is only ever "expected" (and sometimes we break that expectation by accident) to be able to directly load a savefile created by a build from the same commit as the one loading it. We also sometimes extend that expectation to also being able to load a save from the most recent release - as long as no PR marked "savegame bump" has been merged since said release.

At current, we're in the middle of a savebump cycle, with multiple PRs in the pipeline and at least one already merged which break compatibility with previous versions of the game. We tend to only increase save versions by a single version between tagged releases, as being able to migrate / restore saves from each incremental change during the development cycle are generally not a concern worth the limited developer time available.

Ie, it is a dev branch (which occasionally gets tagged as a release), but not a release branch where any commit on it could potentially be turned into a release?

TL;DR: this is correct.

Pioneer operates quite a bit differently than any software company you might have experience with - we have a very loose organizational structure, very limited manpower, and a small audience for the game that mostly plays tagged release builds.

As a result, there's little desire to have a truly "stable" branch, and the relative stability of the master branch is mostly a result of an intentional effort towards catching bugs during the PR process rather than discovering them after merge.

Unfortunately, what we're lacking the most is dedicated user QA when it comes to proposed pull requests - often only the PR author or maybe one other developer has the time and ability to fully test a branch, and some bugs (like this one!) can go unnoticed if they don't show themselves within that developer's testing setup.

Hopefully that helps you to wrap your head around the way Pioneer works 😄

[impaktor]

and a small audience for the game that mostly plays tagged release builds.

Indeed. That prompts me to have a look at our stats:

[mwerle]

Hopefully that helps you to wrap your head around the way Pioneer works 😄

Thank you, yes it does. Again, no right or wrong, nor a criticism, just trying to figure it all out :)

[impaktor]

....and since I might be doing violence to our analytics tracker, I'll just post this for posterity in case I mess things up

and github stats (excluding git clones / build from master):

    "name": "Pioneer 2024-07-10",
        "name": "pioneer-20240710-win.exe",
        "download_count": 1894,
        "name": "pioneer-linux-x64-20240710.tar.gz",
        "download_count": 902,
        "name": "Pioneer-x86_64.AppImage",
        "download_count": 816,
    "name": "Pioneer 2024-03-14",
        "name": "pioneer-20240314-win.exe",
        "download_count": 1842,
        "name": "pioneer-linux-x64-20240314.tar.gz",
        "download_count": 681,
        "name": "Pioneer-x86_64.AppImage",
        "download_count": 400,
    "name": "Pioneer 2024-02-03",
        "name": "pioneer-20240203-win.exe",
        "download_count": 885,
        "name": "pioneer-linux-x64-20240203.tar.gz",
        "download_count": 425,
        "name": "Pioneer-x86_64.AppImage",

[mwerle]

Closing as it is "working as intended".