Make --window turn fullscreen off again - #3163
Open
tomjn wants to merge 1 commit into
Open
Conversation
SetFullScreen worked out the windowed case and then overwrote it on the next line, so --window did nothing whenever the config said Fullscreen = 1. Introduced in d1eec3d (April 2017), which rewrote a working if/else chain into two assignments. This restores that chain's meaning: --window beats --fullscreen, either beats the config.
Collaborator
Perhaps the CLI tags should instead use the config overlay feature? That prevents it from being saved while still being readable elsewhere as if it was in the config (in case wupgets use that to tell if we're fullscreen): if (cliWindowed)
configHandler->Set("Fullscreen", 0, true);
else if (cliFullscreen)
configHandler->Set("Fullscreen", 1, true);
// ^^^^ overlay
fullScreen = configHandler->GetBool("Fullscreen"); |
This file contains hidden or 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
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.
--windowhas done nothing since April 2017 whenever the config saidFullscreen = 1.SetFullScreenworks out the windowed case and then overwrites it on the next line.d1eec3d rewrote a working if/else chain into two plain assignments and dropped the first result. This restores that chain's meaning:
--windowbeats--fullscreen, either beats the config.Found while working on macOS, where it is hard to miss: exclusive fullscreen forces a display mode change, so the desktop leaves its 2x scale and every other window's text goes tiny. The bug itself is not platform specific.
One thing worth a second opinion on. The result is written back to the config, so
--windownow persistsFullscreen = 0into later runs.--fullscreenhas always persistedFullscreen = 1the same way, so the two are symmetric, but it is a change from nine years of the flag doing nothing at all.Tested by hand on macOS: with
Fullscreen = 1in springsettings.cfg,--windowgives a decorated window and the log readswindowed::decorated.