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

Codechange: merge guiflags and flags in settings .ini files #9333

Merged

Conversation

TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Jun 3, 2021

Motivation / Problem

While working on SaveLoad I ran into the issue that it has two flags which have nothing to do with SaveLoad. Additionally, the Settings were settings SaveLoad settings directly, which gave for some confusing ini file .. flags vs guiflags.

Description

Simplify the situation by only having flags.

This now means that SF_NOT_IN_CONFIG, SF_NOT_IN_SAVE and SF_NO_NETWORK_SYNC remain within the Setting, and don't pollute SaveLoad for no good reason.

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/saveload.h Outdated Show resolved Hide resolved
src/table/settings/gameopt_settings.ini Outdated Show resolved Hide resolved
@TrueBrain TrueBrain changed the title Codechange: move SLF_NOT_IN_SAVE and SLF_NOT_IN_CONFIG to SettingFlag Codechange: only set SettingFlag for settings Jun 3, 2021
@TrueBrain TrueBrain changed the title Codechange: only set SettingFlag for settings Codechange: merge guiflags and flags for settings Jun 3, 2021
@TrueBrain TrueBrain force-pushed the settings-flags-not-saveload-flags branch 2 times, most recently from 6b51368 to 8563e4c Compare June 3, 2021 19:10
src/saveload/saveload.h Outdated Show resolved Hide resolved
@@ -578,7 +578,7 @@ static inline uint SlGetArrayLength(size_t length)
* @param conv VarType type of variable that is used for calculating the size
* @return Return the size of this type in bytes
*/
static inline uint SlCalcConvMemLen(VarType conv)
uint SlCalcConvMemLen(VarType conv)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #9335 I revert this back to a static inline, but for my sanity (and yours) in this PR I first make it available for settings.cpp.

@TrueBrain TrueBrain changed the title Codechange: merge guiflags and flags for settings Codechange: merge guiflags and flags in settings .ini files Jun 4, 2021
@TrueBrain TrueBrain force-pushed the settings-flags-not-saveload-flags branch from 4faaceb to 2904dc4 Compare June 4, 2021 07:38
It is a settings-only flag, so don't pollute SaveLoad code with it.
It is a settings-only flag, so don't pollute SaveLoad code with it.
It was rather confusing which one was for what, especially as some
SaveLoad flags were settings-only. Clean up this mess a bit by
having only Setting flags.
@TrueBrain TrueBrain force-pushed the settings-flags-not-saveload-flags branch from 2904dc4 to 6f7dcf1 Compare June 6, 2021 07:45
@TrueBrain TrueBrain merged commit 648ee88 into OpenTTD:master Jun 6, 2021
@TrueBrain TrueBrain deleted the settings-flags-not-saveload-flags branch June 6, 2021 19:45
glx22 added a commit to glx22/OpenTTD that referenced this pull request Apr 15, 2022
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

3 participants