matzman666 / PyPipboyApp

A platform independent and extensible unofficial Fallout 4 Pipboy Companion App
GNU General Public License v3.0
85 stars 20 forks source link

Moving settings from Windows registry to .ini-file #36

Closed matzman666 closed 8 years ago

matzman666 commented 8 years ago

I am thinking about moving the QSettings backend storage from the windows registry to an ini-file. Would be more user-friendly as it is easier for inexperienced users to modify or delete existing settings or move them to another computer. Since we now store notes I think this could be an important feature.

Changing the backend-storage is a simple line of code (Linux version already uses ini-files, so we know that it is working).

What we also need to do is to migrate the old settings into an ini-file, and we need to think about where to store the ini-file.

akamal commented 8 years ago

Are you sure it's worth the effort? If you're intending to support normal people hand editing the ini files, then I think we need to wrap every settings call in a try\except. The one (possibly only!) advantage of having the settings in the registry is that people who are comfortable with editing them are usually also capable of reading the crash log to see what has gone wrong and fixing it themselves. If the settings are in plain text people will put strings where ints are expected, and then complain when the app crashes!

If you do decide to go down this route, the 'correct' location for the settings file(s) would be %APPDATA%\pypipboyapp (which will expand to the appdata directory of the current users profile, on every version of windows from XP onwards). But, if the aim is to make them accessible, it may be more appropriate to store them in the same directory as pypipboyapp.py (or perhaps the parent dir if started from the launcher?). This is 'wrong' and will often cause permissions problems (if people have installed pypipboyapp under Program Files) but we're already doing that for the log file and I haven't seen anyone report any issues.

On the migration front, is there any chance of QT making it simple? On startup, check if the config file(s) exist, if they don't then set the backend storage to the registry, read out any existing settings, switch to file backed storage and write them all back out?

akamal commented 8 years ago

Ha! I've just noticed you've already done it! : )

akamal commented 8 years ago

And having looked at it, while it certainly looks like it's working without any issues, I'm not sure the format of that ini file is any easier to edit then having the settings in the registry. For structures with multiple levels (the per user POIs for instance) it might actually be harder to edit than with the gui treeview the registry editor provides.

matzman666 commented 8 years ago

It was very easy to implement and can also be easily reverted, so I choose to commit it without further discussions.

Being able to modify the settings played only a minor part in my decision to store the settings in an ini-file on Windows. Since we now also have note taking features I wanted an easy way to backup my notes and transfer them to another computer. As long as the settings are in the registry both cannot be easily achieved (especially moving them from/to a non-Windows OS). When the settings are in a file I can just sync the file into the cloud with dropbox/owncloud/... and copy it to another computer.

Also I added a command line argument that allows us to specify an arbitrary config file. We can use this to make the application portable if we want.

bgrimm commented 8 years ago

+1 to .ini file. Portability (maintaining consistent settings) is very useful. And keeping in app directory preferred, I run/test on PC and Mac, so following Windows %APPDATA% storage guidelines is less useful. -my 2cents

bgrimm commented 8 years ago

Appears to em when using local INI file, all widget/window positions are not saved. on restart (with same --inifile) some widgets are not restored (example had upper left docked map widgets, lower left docked inventory widgets, on second run with same ini file lower left widgets were no longer displayed (and unchecked in widget dropdown) ... can open an issue if preferred.

matzman666 commented 8 years ago

1) I am afraid that user will accidentally delete their settings when I keep them in the app dir by default. I think the best solution would be to stick to windows guidelines for the default launcher, and provide a 'portable' launcher (that keeps settings in app dir) as optional download.

2) Cannot reproduce. On which OS? Was this a one-time thing or does this also happen after you have re-arranged the widgets and restarted the application? I suspect that this is caused by using settings from an old version with a version that has additional widgets. This case may lead to lost settings. Nothing we can do about this, it is a Qt internal thing.

bgrimm commented 8 years ago

That was 1st run, no connection to game, Win 7(64) Python 3.4, PyQt 5.5.1 This issue disappeared after I merged your most recent commit addressing issue #42. Appears to work normally now. (did not check ini file before/after). (could have been my bad on merging pypipboyapp.py originally and missed a diff, since merging your latest commit fixed issue)

bgrimm commented 8 years ago

As far as INI location, I would retain command line option to specify location at least. (would work fine for me)

akamal commented 8 years ago

%APPDATA% by default and command line param to override seems a like a reasonable compromise.

The only issue I've seen is that the ini file is much more finicky about the type of data being stored\retrieved. Seems like we have to assume every value read back is an str and cast explicitly to int\float\etc as required. Not necessarily a bad thing, but definitely different to the registry based settings, where if you saved an int, an int was what you got back.

matzman666 commented 8 years ago

Ini-file store primitive data types without type information. That's why you always read a string. It's a fatal flaw in how the ini-files are implemented, but the only other choice would be to write my own QSettings backend. You can mitigate it by wrapping the value() function call with an appropriate type constructor call (Only bool requires special attention, I usually cast from bool to int when saving, and when reading from str to int and then to bool).

On Linux/Mac ini-files are the only choice. When you rely on the registry backend to return the correct type, you are prone to writing code that only works on Windows ;)

matzman666 commented 8 years ago

I close this issue because the migration to .ini-files is finished.

The Windows launcher has now to additional settings:

The portable launcher is basically the normal launcher called with --portable.