feat: introduce gui app state persistence #71
Conversation
There was a problem hiding this comment.
Thanks @cleydyr for this implementation.
I think it's a nice addition and can be extended in the future also for things like default crop rect sizes etc.
As stated in the comments I see a human readable file format more fitting for the state but we are on a good way towards that!
| if (osName.contains("Mac OS")) { | ||
| _IS_WINDOWS = false; |
There was a problem hiding this comment.
This check is redundant as it's covered in the else case.
I'm wondering if it makes more sense to have a class which returns the default storage location depending on the OS instead.
In case of unix it would return System.getProperty("user.home" and on Windows System.getenv("LOCALAPPDATA").
| return (AppState) readObject; | ||
| } | ||
|
|
||
| throwUnknownAppState(); |
There was a problem hiding this comment.
This will be thrown whenever we add/change the attributes on the AppState class. So whenever someone already has a stored state and uses a newer Briss version which has these additional attributes, it will immediately crash on start.
Instead we should be defensive and just return no app state (i.e. null).
|
|
||
| try (FileInputStream fileInputStream = new FileInputStream(stateFile); | ||
| ObjectInputStream objectInputStream = new ObjectInputStream(fileInputStream)) { | ||
| final Object readObject = objectInputStream.readObject(); |
There was a problem hiding this comment.
Java serialized objects have the benefit of being straight forward and not needing additional dependencies.
However they also lack flexibility and a human readable format.
- Missing flexibility: Whenever the content of the AppState changes, the previously serialized objects can no longer be deserialized, therefore resetting the state
- Non human readable format: Serialized files are not plain text and cannot be read or changed by a human by just opening the file
Therefore I suggest we use a more versatile approach which uses a human readable format like Json, Yaml or Property files.
I like Jackson for that, as it's easy to map Json properties to field on a class.
| File appStateDestinationFile = new File(System.getProperty("user.home"), ".briss"); | ||
|
|
||
| if (OSDetector.isWindows()) { | ||
| appStateDestinationFile = new File(System.getenv("LOCALAPPDATA"), ".briss"); | ||
| } | ||
|
|
There was a problem hiding this comment.
File creation is duplicated (code and execution) here
| File appStateDestinationFile = new File(System.getProperty("user.home"), ".briss"); | |
| if (OSDetector.isWindows()) { | |
| appStateDestinationFile = new File(System.getenv("LOCALAPPDATA"), ".briss"); | |
| } | |
| String userHome; | |
| if (OSDetector.isWindows()) { | |
| userHome = System.getenv("LOCALAPPDATA"); | |
| } else { | |
| userHome = System.getProperty("user.home"); | |
| } | |
| File appStateDestinationFile = new File(userHome, ".briss"); |
Closes #70