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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TrueBrain
force-pushed
the
saveload-refactor
branch
2 times, most recently
from
June 6, 2021 10:26
a403080
to
4446f55
Compare
TrueBrain
force-pushed
the
saveload-refactor
branch
4 times, most recently
from
June 7, 2021 09:55
19b9a72
to
7f0bcdd
Compare
LordAro
reviewed
Jun 7, 2021
TrueBrain
force-pushed
the
saveload-refactor
branch
2 times, most recently
from
June 7, 2021 13:08
67e7c15
to
c2a4a6e
Compare
rubidium42
reviewed
Jun 7, 2021
TrueBrain
force-pushed
the
saveload-refactor
branch
3 times, most recently
from
June 7, 2021 22:10
46ae39a
to
b669beb
Compare
rubidium42
reviewed
Jun 8, 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
force-pushed
the
saveload-refactor
branch
from
June 9, 2021 07:20
b669beb
to
24756af
Compare
rubidium42
approved these changes
Jun 10, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
SL_DEQEUE
suggested to support signed values, but they were silently cast to unsigned .. which, with conversion code likeSLE_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.SlList
is very similar toSlDeque
, but a copied the code. So, together with the third commit, I changed theSlDequeHelper
into a more general one, where anything supportingbegin()
andend()
will work on.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 toSL_REFLST
to be more obvious of its function.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.