Opened 20 years ago

Closed 23 months ago

#7389 closed defect (fixed)

GUI does not reflect command line config overrides

Reported by: SF/mbauer0xff Owned by: sev-
Priority: normal Component: GUI
Version: Keywords:
Cc: Game:

Description

Versions:

Win32

ScummVM 0.6.1b (Aug 4 2004 10:07:42) Features compiled in: Vorbis MP3 zLib MPEG2

Details: ScummVM allows setting of the savepath through the SCUMMVM_SAVEPATH environment variable. The path displayed in the GUI under "Options/Misc/Savegame path" is not the value of SCUMMVM_SAVEPATH as expected, but the location of the ScummVM.exe.

Reproducing:

- go to the Command Prompt - set SCUMMVM_SAVEPATH=%USERPROFILE%\ScummVMSaves\ - mkdir "%SCUMMVM_SAVEPATH%" - run ScummVM.exe from wherever it is installed. - In the GUI check "Options/Misc/Savegame path" to be different from SCUMMVM_SAVEPATH

Ticket imported from: #1045171. Ticket imported from: feature-requests/205.

Attachments (1)

config-manager.diff (142 bytes ) - added by SF/mbauer0xff 20 years ago.
diff of the changes to config-manager.cpp

Download all attachments as: .zip

Change History (28)

comment:1 by fingolfin, 20 years ago

My prefered solution to this "bug" is to just remove SCUMMVM_SAVEPATH, which IMO is an aberration that should never have been added in the first place.

comment:2 by fingolfin, 20 years ago

Priority: normallow

comment:3 by eriktorbjorn, 20 years ago

I'm in favor of removing SCUMMVM_SAVEPATH, but I'm usually running ScummVM on a UNIX-ish system where each user has his own ~/.scummvmrc file. On Windows I believe the scummvm.ini file is always stored in C:\WINDOWS, and I don't know where it's stored on a Mac.

Though I guess you could use the --config command-line option to get around that...

comment:4 by SF/mbauer0xff, 20 years ago

I resolved for using SCUMMVM_SAVEPATH when creating a CD with ScummVM, scummvm.ini and some games on it. I wanted it to completely run from CD, but store the savegames in the current user's profile.

So i created a scummvm.ini with relative paths for the CD, passing it to ScummVM using the --config argument.

As the save directory depends on the current user, i use a batchfile to set SCUMMVM_SAVEPATH = %USERPROFILE%\ScummVM-Savegames before starting ScummVM.

However if you decide to remove SCUMMVM_SAVEPATH i'd really appreciate if you add a command line argument for the savepath instead.

Granted that i'm new to ScummVM, there might be other solutions for my problem, so i'd appreciate any suggestions for better solutions.

comment:5 by sev-, 20 years ago

scummvm --savepath=/blah/blah/ will do the trick

comment:6 by SF/mbauer0xff, 20 years ago

Thanks, that works. But it works just the same as the SCUMMVM_SAVEPATH: The given path is used by ScummVM, but is still not displayed correctly in the GUI.

I guess i'll have to start reading the sources instead of the readme.txt to find the latest tweaks... ;-)

comment:7 by eriktorbjorn, 20 years ago

I've documented --savepath now. Thanks for pointing it out. ;-)

comment:8 by sev-, 20 years ago

For me it seems a correct behaviour. I.e. GUI reflect what could be changed, that is config file value. There is no way to change command line option or environment variable.

comment:9 by SF/mbauer0xff, 20 years ago

Actually you *can* change the savepath in the GUI. After the change the new path correctly displayed and also used to store the savegames. This works even if the .ini cannot be saved. However, if someone just goes to Options/Misc to check on the location of his savegames, he'll get prompted the wrong path. So to me the behavior is correct, only the presentation is flawed.

comment:10 by fingolfin, 20 years ago

Well, I'll look at the UI issue eventually. For now, ender, please comment whether you have objections to removing SCUMMVM_SAVEPATH.

comment:11 by fingolfin, 20 years ago

Owner: set to SF/ender

comment:12 by SF/mbauer0xff, 20 years ago

I fixed the problem with my own build of ScummVM based on the sources of the 0.6.1b release. It's working for me so i'll share my changes.

comment:13 by fingolfin, 20 years ago

Assuming you wanted to attach a file, make sure you enable the "Check to Upload and Attach a File" checkbox.

by SF/mbauer0xff, 20 years ago

Attachment: config-manager.diff added

diff of the changes to config-manager.cpp

comment:14 by SF/mbauer0xff, 20 years ago

Sorry, i overlooked the checkbox.

comment:15 by fingolfin, 20 years ago

That patch doesn't apply over here on latest CVS. You should do patches using the -u option, so they carry some context, and apply in multiple versions of the source...

Anyway, that change isn't a proper solution to the problem ; it changes ConfigManager::get() to work incorrectly. A proper fix would almost definitiely be done in the GUI code.

comment:16 by SF/mbauer0xff, 20 years ago

I already thought so. My idea was just to make ConfigManager::get() work like you would guess it does from looking at ConfigManager::hasKey(). At first i had changed GlobalOptionsDialog::open() to look for the transient savepath, but the second patch looked better... ;-)

Anyway, thanks for your comment. I'm looking forward to seeing the "real fix".

comment:17 by SF/ender, 20 years ago

Hmm, SCUMMVM_SAVEPATH is an interesting one. Really, we should depreciate it since there is now a commandline and config/gui option.

However some users may prefer adding a SCUMMVM_SAVEPATH=... override in their .bashrc/.profile/whatever. This is also useful for system-wide installs.

I think it should probably be left in, since the codepath should handle it the same as the cmdline switch - which has the same issue at the moment. Updating summary to suit :)

comment:18 by SF/ender, 20 years ago

Summary: SCUMMVM_SAVEPATH is not reflected in GUIGUI does not reflect savepath overrides

comment:19 by sev-, 19 years ago

What is the status of this item?

comment:20 by SF/ender, 19 years ago

The situation is unchanged, the GUI still ignores
any path override via environment variable or commandline
option. Perhaps this should be moved to RFE?

There are two reasonable behaviours for this particular
situation:

A) Only display the editable savepath, ignoring the static
override, as we currently do. I think this is correct and could be
considered 'behaviour by design', but perhaps non-intuitive to
some users.

B) If an override path is specified and we do decide to display it
in the GUI, the field should simply be disabled to ensure the
path cannot be changed.... thus accidently saved to the config
file.

Personally I'm quite happy with it, but it would only be a few lines
of code to implement the alternate behaviour.

comment:21 by sev-, 19 years ago

Moved it to RFE. Same lowest priority.

Yes, logic is flawed here, and I think to prevent it best would be to implement behaviour (B) and describe it in documentation.

comment:22 by sev-, 19 years ago

Component: --Unset--

comment:23 by fingolfin, 17 years ago

Owner: changed from SF/ender to Kirben
Summary: GUI does not reflect savepath overridesGUI does not reflect command line config overrides

comment:24 by fingolfin, 17 years ago

This problem does not just affect the savepath, mind you, but *any* command line config. Be it the music volume or the subtitles settings.

This nicely demonstrates one of the pathologies that occur when you mix command line options and GUI options; the former being completely transient, the latter usually being persistent. I am not sure if there is a proper way to resolve those in a really satisfactory way, ever.. :-/

Let's think about it: The user fires up ScummVM via the command line with some special (transient!) settings, but *without* specifying a game. Why would he do that, normally? The only plausible thing I can come up with is: To start a game for which he doesn't recall the precise target name. Fine. Would he open the options dialog in that case? Probably not, *unless* he wanted to change some additional settings before running his beloved game, some settings for which he didn't recall the command line options and/or was too lazy to type them all in.

So, I imagine in that case he would want: 1) the additional changes (made via the GUI) to be transient, too, not persistent (as usual) 2) his command line options to have precedence over the game/target specific settings Hence, one might consider simply turning of persistence of the global options dialog in the launcher when any command line options are present. That should work fine for most users: * nothing changes if you never use special command line options * nothing changes if you use command line options + a target * it would be similar to Enders' "plan B" from below, only that the user could still use the options dialog, for temporary settings

Of course, this approach is far from perfect, too. But in the end, the user could *always* come up with some odd combination of effects should he want to. For example, specify a custom "--savepath", then run ScummVM, open the options dialog, and set yet another savepath. Which would probably be a rather silly thing to do. Unless you consider the "CD with ScummVM" scenario mbauer0xff described below. Then again, in his scenario, the .ini is on a CD anyway, so it'd be read-only (and hence all settings transient) anyway.

A final suggestion: Ignore all (well, almost all, --config being the exception) config settings when running the launcher, possibly printing a message that we just did so. It would resolve the issue presented here completely and in a very simple fashion. Of course it would also make it impossible to use the "user specific savepath" trick mbauer0xff wanted to use. But then we could resolve *that* by using a standard savepath inside the user home dir under Windows, too (like we now do with the default config file location on windows, too). Kirben, would that be feasible/sensible (independent of the other thoughts mentioned here)?

Gah. I like none of these particularly. :-(

comment:25 by fingolfin, 15 years ago

Owner: Kirben removed

comment:26 by digitall, 5 years ago

Component: GUI
Priority: lownormal
Type: enhancementdefect

comment:27 by sev-, 23 months ago

Owner: set to sev-
Resolution: fixed
Status: newclosed

Fixed. Now the overrides are designated by color.

Note: See TracTickets for help on using tickets.