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

Several code refactors of the SaveLoad code #9338

Merged
merged 6 commits into from Jun 10, 2021

Conversation

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Jun 6, 2021

Depends on #9335

Motivation / Problem

When you start fiddling with the SaveLoad code, you find many odd things. And sometimes they annoy me enough that you get this collection of commits.

Description

First commit is #9335, after that:

  • First patch is some code hygiene
  • Second patch too
  • Third: SL_DEQEUE suggested to support signed values, but they were silently cast to unsigned .. which, with conversion code like SLE_FILE_I8 | SLE_VAR_I32 might not work as you expect. Luckily, nobody was using this code for signed values, but let's make sure we are ahead of this issue.
  • Forth: SlList is very similar to SlDeque, but a copied the code. So, together with the third commit, I changed the SlDequeHelper into a more general one, where anything supporting begin() and end() will work on.
  • Fifth: SL_LST is a list of references, which the name did not tell you. The way reference are done is already very error-prone, and this wasn't helping. So I renamed it to SL_REFLST to be more obvious of its function.
  • Sixth: cheat-chunk was weird. Just that. Weird. Made it slightly less weird.

I could have made 4 PRs out of this too, so if this is too hard to review, let me know, and I will split it up :)

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')
@TrueBrain TrueBrain force-pushed the TrueBrain:saveload-refactor branch 2 times, most recently from a403080 to 4446f55 Jun 6, 2021
@TrueBrain TrueBrain force-pushed the TrueBrain:saveload-refactor branch 4 times, most recently from 19b9a72 to 7f0bcdd Jun 6, 2021
src/saveload/saveload.cpp Outdated Show resolved Hide resolved
src/saveload/saveload.h Outdated Show resolved Hide resolved
@TrueBrain TrueBrain force-pushed the TrueBrain:saveload-refactor branch 2 times, most recently from 67e7c15 to c2a4a6e Jun 7, 2021
src/saveload/saveload.cpp Outdated Show resolved Hide resolved
src/saveload/saveload.cpp Outdated Show resolved Hide resolved
src/saveload/saveload.cpp Outdated Show resolved Hide resolved
src/saveload/saveload.cpp Show resolved Hide resolved
src/saveload/saveload.h Outdated Show resolved Hide resolved
@TrueBrain TrueBrain force-pushed the TrueBrain:saveload-refactor branch 3 times, most recently from 46ae39a to b669beb Jun 7, 2021
src/saveload/saveload.cpp Outdated Show resolved Hide resolved
src/saveload/saveload.cpp Outdated Show resolved Hide resolved
TrueBrain added 6 commits May 31, 2021
Also move it to static, as nobody else is using it.
…e generic

Future additions will start using it for std::list too.
…part

You can easily mistake SlList / SL_LST to be a list of SL_VAR, but
it is a list of SL_REF. With this rename, it hopefully saves a few
people from "wtf?" moments.
@TrueBrain TrueBrain force-pushed the TrueBrain:saveload-refactor branch from b669beb to 24756af Jun 9, 2021
@TrueBrain TrueBrain merged commit 1749524 into OpenTTD:master Jun 10, 2021
13 checks passed
13 checks passed
@github-actions
Emscripten
Details
@github-actions
Commit checker
Details
@github-actions
Check preview needs update Check preview needs update
Details
@github-actions
Linux (clang, clang++, libsdl2-dev)
Details
@github-actions
Linux (gcc, g++, libsdl2-dev)
Details
@github-actions
Linux (gcc, g++, libsdl1.2-dev)
Details
@github-actions
Linux (gcc, g++, -DOPTION_DEDICATED=ON)
Details
@github-actions
Mac OS (x64, x86_64)
Details
@github-actions
Windows (windows-latest, x86)
Details
@github-actions
Windows (windows-latest, x64)
Details
@github-actions
Windows (windows-2016, x86)
Details
@github-actions
Windows (windows-2016, x64)
Details
@github-actions
Check Annotations
Details
@TrueBrain TrueBrain deleted the TrueBrain:saveload-refactor branch Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants