dpaulat / supercell-wx

Supercell Wx is a free, open source advanced weather radar viewer.
https://supercell-wx.rtfd.io
MIT License
85 stars 14 forks source link

settings.json and everything else overwritten without permission mysteriously #208

Open acidhorse1972 opened 1 month ago

acidhorse1972 commented 1 month ago

So here's what happened: I click on the icon shortcut for supercell-wx on my taskbar. It loads up but the popup Introduction dialog for getting started shows up. Nothing has been touched altered or anything. Screenshot 2024-05-17 213055

My placefile definitions are gone and settings.json was overwritten by supercell-wx for some mysterious reason. The MapTiler API key is gone too.

dpaulat commented 1 month ago

Did anything precede this occurring (manual edit, changing settings to a specific value, deleting or renaming a file)? The files are stored in %AppData%\Supercell Wx.

acidhorse1972 commented 1 month ago

Did anything precede this occurring (manual edit, changing settings to a specific value, deleting or renaming a file)? The files are stored in %AppData%\Supercell Wx.

Nope nothing.

acidhorse1972 commented 1 month ago

Is the program set to overwrite anything if any kind of conditions are met? It should prompt to overwrite anything. Popup Dialog "Do you want to overwrite blah blah blah" "OK" "Cancel" "Yes" "No" "Abort" "Plan D" A database file would probably work better. SQLite or .DB files "Select * from Placefiles where etc etc" "UPDATE etc"

dpaulat commented 1 month ago

I chose JSON over a binary format like a SQLite database, because it's human readable.

The only time the application should overwrite the JSON files is if the files have syntax errors (e.g., missing/extra commas). Invalid or missing values are corrected, and should not impact the rest of the file. I do not expect outside of hand-editing that this should occur. It seems I may have missed a case. I'm thinking it'd be good that if there is a syntax error, I at least create a backup of the file prior to saving updated settings to give the opportunity for a correction.

If you open an older version of Supercell Wx, newer settings values could be removed/reset.

I'm trying to think of other scenarios, some hypothetical:

Multiple instances of Supercell Wx being open should not impact this significantly, as they both use the same set of configuration files. They only write on settings change or exit (or startup if there is something invalid), not without user interaction. Ultimately though, one of these instances could overwrite settings made by another instance.

It's strange that both settings and placefiles were nuked, as these are stored in separate files.

acidhorse1972 commented 1 month ago

I chose JSON over a binary format like a SQLite database, because it's human readable.

The only time the application should overwrite the JSON files is if the files have syntax errors (e.g., missing/extra commas). Invalid or missing values are corrected, and should not impact the rest of the file. I do not expect outside of hand-editing that this should occur. It seems I may have missed a case. I'm thinking it'd be good that if there is a syntax error, I at least create a backup of the file prior to saving updated settings to give the opportunity for a correction.

If you open an older version of Supercell Wx, newer settings values could be removed/reset.

I'm trying to think of other scenarios, all hypothetical:

* File open in a text editor that prevents reading by another program

* File system error

* Out of disk space

Multiple instances of Supercell Wx being open should not impact this significantly, as they both use the same set of configuration files. They only write on settings change or exit (or startup if there is something invalid), not without user interaction. Ultimately though, one of these instances could overwrite settings made by another instance.

It's strange that both settings and placefiles were nuked, as these are stored in separate files.

One way to make sure that a user knows that something fishy is going on is to ask whether they want to allow it to overwrite the existing file or notify the user that the file doesn't exist and then ask if the user wants to create a new settings file. An empty placefile list index file should pre-exist before any placefile references are ever created so that if the settings and the placefile list file doesn't exist when it should then flags can be thrown and errors declared and logs checked.

dpaulat commented 1 month ago

I edited and added two more potential items with the previous post after you started to quote:

With the settings and placefile list being stored in the user directory rather than the application directory, there has to be an initial "create if it doesn't exist", if this is the first time a user is running, or upgrades to a version that supports the JSON file. That's not an error condition.

I did start to add the setup dialog if certain conditions were not met in the settings file.

Did you happen to note if there was anything in the terminal that displayed? I have a handful of warnings and errors with the following prefixes:

acidhorse1972 commented 1 month ago

I edited and added two more potential items with the previous post after you started to quote:

* Running as a different user

* User folder changed (%AppData%)

With the settings and placefile list being stored in the user directory rather than the application directory, there has to be an initial "create if it doesn't exist", if this is the first time a user is running, or upgrades to a version that supports the JSON file. That's not an error condition.

I did start to add the setup dialog if certain conditions were not met in the settings file.

Did you happen to note if there was anything in the terminal that displayed? I have a handful of warnings and errors with the following prefixes:

* [scwx::qt::util::json]

* [scwx::qt::manager::settings_manager]

Nope I didn't realize that I needed to check that. There needs to be a flag somewhere that determines when the "First Run" occurred. So that it can compare time and dates. If the settings is missing and there is something that has the original first run date and time and dates don't match then it should log everything to an error.log file. and run Notepad.exe on it.

acidhorse1972 commented 1 month ago

might be a good idea to set read-only attributes on those files.