Video-Hub-App icon indicating copy to clipboard operation
Video-Hub-App copied to clipboard

Settings.json is not saved

Open CrendKing opened this issue 4 years ago • 7 comments
trafficstars

With the latest commits (HEAD @ https://github.com/whyboris/Video-Hub-App/commit/26fb13b6a9387ef271b39b800d10c38b62c0c1c1), if I use the X button to close the window, everything works fine. But if I Alt+F4 the window, settings.json will be reset to 0 byte.

Maybe it has something to do with https://github.com/whyboris/Video-Hub-App/commit/268294b2ca28cbe6359184b35ad6489f2fe4413e?

CrendKing avatar Jul 11 '21 14:07 CrendKing

Thank you for keeping track of this 🙇 -- I removed remote because it's deprecated by Electron - with the commit you point to: https://github.com/whyboris/Video-Hub-App/commit/268294b2ca28cbe6359184b35ad6489f2fe4413e#diff-856b4a4969a5de4eb6bf7ac1f8f156c7063fc7bdf68209192b7b8769e059b788L493

I thought I solved the problem by making the app save everything before it finally quit, but seems like there are some loose ends 😓

I'll keep this Issue open until I get to it to fix it. PR for a fix is also welcome 👍

whyboris avatar Jul 11 '21 20:07 whyboris

If I comment out the settings.json write at main-ipc.ts line 367, no reset happens. If I change the write mode to something non-destructible like r+, Alt+F4 wouldn't reset to empty but sometimes reset to all default values.

So it seems this function behaves differently depending on how "close window" is invoked. Maybe Alt+F4 doesn't leave enough time for the function to execute, and it was OK before because remote was synchronous?

If that's the case, I wonder maybe the ultimate solution is write settings whenever a setting is changed, instead of doing one lump sum write at closing time. Same for the vha2 file I guess?

CrendKing avatar Jul 12 '21 00:07 CrendKing

There's bound to be a way to preventDefault() somewhere 👌 or just properly use an Electron hook somewhere like -beforeunload 🤔

I would rather not write the settings every time the users plays with the buttons, and especially not write a possibly large (5+ MB) .vha2 file 😅 every time the hub gets updated (a video plays, tag is added, etc).

I'll try to find some time this weekend to replicate the bug and then fix it 🤞 though I'm also happy if anyone prefers to fix it instead of me 🤝 🙇 😁

whyboris avatar Jul 12 '21 14:07 whyboris

I spent some time on this issue. It seems if I add event.preventDefault() before the getAngularToShutDown(); in win.on('close'), the problem is solved. So I think the logic you intended was

  1. When the main window is about to close, intercept it, prevent it (which is currently missing), send event to save files.
  2. When files are saved, try to close the window again after setting readyToQuit = true.
  3. Now the event handler would actually close the window.

I guess because the event was not prevented, the closure still happened. So there was a race condition between the app quitting and file being saved. Since file change can't commit before quit, we got 0 byte settings.json.

And based on the observation, I wonder if app.exit() was necessary. app.on('window-all-closed') already handles application termination when all windows closed. And you can't have child windows exist when the main window is closed.

CrendKing avatar Aug 20 '21 05:08 CrendKing

Thank you so much! I'll try all this out on my end. If you happen to have the code already, please consider opening up a PR 🙇 And if it's not 100%, I could clean it up too - please don't feel like you have to do everything 🤝

whyboris avatar Aug 20 '21 13:08 whyboris

PR: https://github.com/whyboris/Video-Hub-App/pull/694

CrendKing avatar Aug 20 '21 20:08 CrendKing

I had this issue and it broke the app in weird ways for me. Settings changes would never save anymore after some point, regardless of the exit method. I was stuck with the settings I had a month ago. I renamed the .json file and it didn't make a difference- I didn't get a new one. I reinstalled and it worked, even after copying my original settings file back. After that, settings save if I exit with the X button, but they don't save if I exist with alt-f4, which is what my mouse gesture uses. However, I don't get a zero length settings.json file. It just doesn't update.

Could you also add a "save settings" button if the bug persists? Then making changes but forgetting to hit X would be less of an issue.

okumam avatar Dec 05 '21 16:12 okumam

Closed with #694 🙇 thank you @CrendKing for the fix!

whyboris avatar Feb 21 '23 00:02 whyboris