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

WIN32: Add Portable Mode #3499

Merged
merged 5 commits into from Nov 13, 2021
Merged

WIN32: Add Portable Mode #3499

merged 5 commits into from Nov 13, 2021

Conversation

@sluicebox
Copy link
Member

@sluicebox sluicebox commented Nov 7, 2021

This PR adds a "portable mode" to Windows builds. This has been requested several times and produced some long threads in the forums. The idea is that if someone manually puts a scummvm.ini file in a directory along with the exe, ScummVM will operate in a self-contained portable mode and use that config file and directory for logs/saves/screenshots. In other words: in portable mode, ScummVM won't touch the user profile directory.

I'm not the one asking for this feature but I see why it's wanted, it opens up interesting possibilities for people to play with. I can think of some experiments of mine it would help with. Being able to easily have a self-contained copy of highly configurable software is still in fashion! More importantly: it's pretty easy to implement given the recent cleanups to the Windows backend.

Notepad++ is a good Windows citizen and has this feature, so I modeled this off of what it does. It also uses the presence of a local config file in the exe directory as a trigger. To ensure UAC compatibility, it does not do this if it's running out of Program Files on Vista+. I've included that same check. That's also a good way to make sure that if any normal users have a scummvm.ini laying around in their Program Files\ScummVM\ directory (for some weird reason) that the next upgrade doesn't immediately cause the real config file in their user profile to stop being used. I don't think that's a realistic scenario but it's a nice failsafe for the one thing that could go wrong on upgrade.

I see that this has been proposed before in #1504 and was closed, but that proposal was just a start. This PR adds portable mode as a formal full feature that takes all paths and UAC into account.

All that said, I don't feel too strongly about this, I just think it's kinda neat.

@aquadran
Copy link
Member

@aquadran aquadran commented Nov 7, 2021

I think it's nice option.

Loading

@bluegr
Copy link
Member

@bluegr bluegr commented Nov 7, 2021

Cool! I like the logic that toggles the portability mode - simple and elegant. +1 from me :) Nice feature

Loading

@lotharsm
Copy link
Member

@lotharsm lotharsm commented Nov 7, 2021

+1 from me as well. Taking care of UAC really was the bit missing at the last time we had this request.

Loading

Adds support for a self-contained portable mode in which the
executable's directory is used for application files instead
of directories in the user's profile.

Portable mode is activated by placing a scummvm.ini file in
the executable's directory. The directory must be outside of
the system's Program Files directory to comply with UAC.
sev-
sev- approved these changes Nov 12, 2021
Copy link
Member

@sev- sev- left a comment

+1 from me as well. This is a long-anticipated feature.

I have only one small request: could you please also add a note to NEWS.md right away?

@lotharsm will you merge it?

Loading

@lotharsm
Copy link
Member

@lotharsm lotharsm commented Nov 12, 2021

Mentioning the new feature in NEWS.md would be a really nice touch indeed. I wonder if we should backport it to the 2.5.1 branch as well?

I'll merge this as soon as the NEWS file is updated :-)

Loading

@Macleykun
Copy link

@Macleykun Macleykun commented Nov 12, 2021

If this gets implemented, what will happen if an update is released and scummvm is being updated? Will it install in the portable location or install in the default installation location?

Loading

@sluicebox
Copy link
Member Author

@sluicebox sluicebox commented Nov 13, 2021

I've added a news item to the 2.6.0 section. Please feel free to move that around or improve the wording. I think this is a good candidate for 2.5.1 because it's a straightforward change and easy to confirm that it doesn't alter anything when not activated.

@Macleykun that's a good question. This doesn't change check-for-update behavior, and all check-for-update does (if enabled) is notify you and offer to download and run the installer, at which point you still get to choose to install to any directory as before. I believe the installer defaults its directory text box to the one you selected last time you ran the installer, which is stored in the registry. So the update mechanism is loose and portable mode doesn't interfere with it. (And you can disable the check in your portable copy if you like.)

Loading

@bluegr
Copy link
Member

@bluegr bluegr commented Nov 13, 2021

The news item looks good to me! I've done some minor fixes to whitespace.

Loading

@lotharsm
Copy link
Member

@lotharsm lotharsm commented Nov 13, 2021

@Macleykun: This is a really interesting question indeed. As sluicebox mentioned, the downloaded installer will pre-fill the directory selection with the last installation destination target.

@sluicebox: I wonder if we should disable the update check when running in portable mode? I think the main way of distributing a portable version will be in a seperate ZIP file or SFX archive, so downloading the "main" installer doesn't make much sense here.

Loading

@sluicebox
Copy link
Member Author

@sluicebox sluicebox commented Nov 13, 2021

@lotharsm Yes, I think that makes a lot of sense.

I've also updated the GUI to no longer display update controls when there's not an UpdateManager, which was probably the intent anyway since this check makes it consistent with the other GUI update code.

Loading

@lotharsm
Copy link
Member

@lotharsm lotharsm commented Nov 13, 2021

Thank you - merging 👍

Loading

@lotharsm lotharsm merged commit e82f6c6 into scummvm:master Nov 13, 2021
8 checks passed
Loading
@sluicebox sluicebox deleted the win-portable branch Nov 16, 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
6 participants