Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change: make as many savegame chunks a CH_ARRAY as possible #9375

Merged
merged 4 commits into from Jun 15, 2021

Conversation

TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Jun 14, 2021

Motivation / Problem

Weird goal, I know right? But hear me out.

#9322 sets out to make to make our savegame self-descriptive (see motivation for that there). But in order to do that, we need to make all chunks a table.

The first step in making anything a table, is to first make it an array. And so we come to the motivation of this PR: to make that possible.

Description

Most CH_RIFFs in fact are just an array of length 1. So it might seem silly at first, but that is exactly what this PR does: make those chunks into an array of length 1.

The exception was the GLOG chunk. That required some more love and attention.

After this PR, only the map-chunks are CH_RIFF. Those could be made into CH_ARRAY, but that is not really useful, as they cannot become self-describing for now anyway.

Lastly, the chunk table tells how chunks are stored on disk. Chunks that are no longer stored were still marked by their old type. This was often confusing. So I sneaked that change in here, to mark those as CH_READONLY, making it more clear that it is not by accident.

Limitations

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

src/saveload/animated_tile_sl.cpp Show resolved Hide resolved
src/saveload/animated_tile_sl.cpp Outdated Show resolved Hide resolved
src/saveload/saveload.cpp Show resolved Hide resolved
@TrueBrain TrueBrain force-pushed the saveload-riff branch 2 times, most recently from 026e95b to 8f1acd8 Compare June 15, 2021 11:33
std::vector<bool> is not possible, as .. that is a nice special
case in C++.

This new type will be used in next commit.
This adds two byte extra to those chunks, and might feel a bit
silly at first. But in later changes we will prefix CH_ARRAY with
a table header, and then this change shines.

Without this, we could still add headers to these chunks, but any
external reader wouldn't know if the CH_RIFF has them or not. This
way is much more practical, as they are now more like any other
chunk.
Basically it is very similar to Vehicles, where there first is
a type field, followed by data of that type. So this commit makes
it looks like how Vehicles solved that.
This removes a lot of custom "keeping track of length" stuff.
This makes it easier to spot chunks that have a save_proc that
is a nullptr, but also prevents confusion, where it looks like
the CH_ type of a chunk has influence on how it is being read.
It is not, it is only used for saving.
src/saveload/gamelog_sl.cpp Show resolved Hide resolved
@TrueBrain TrueBrain merged commit 8e91527 into OpenTTD:master Jun 15, 2021
@TrueBrain TrueBrain deleted the saveload-riff branch June 15, 2021 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants