-
Notifications
You must be signed in to change notification settings - Fork 116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Auto save backup mappings #446
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it should be thread safe (or at least if it isn't, the normal way of saving is also violating thread safety lmao)
public GuiController(Gui gui, EnigmaProfile profile) { | ||
this.gui = gui; | ||
this.enigma = Enigma.builder() | ||
.setProfile(profile) | ||
.build(); | ||
|
||
autoSaveExecutor.scheduleAtFixedRate(this::autoSave, 0, 60, TimeUnit.SECONDS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about using a swing timer here but not sure which is better.
final Path autoSavePath = autoSaveDir.resolve(DateTimeFormatter.ofPattern("yyyy-MM-dd-HH-mm-ss").format(LocalDateTime.now()) + ".mappings"); | ||
|
||
if (Files.notExists(autoSaveDir)) { | ||
Files.createDirectories(autoSaveDir); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd just create the directory unconditionally. Doesn't really make a difference though tbh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, what if that name is used by a regular file instead of a directory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd just let it fail. Other option would be deleting the file, which is probably a bad idea.
I would probably show a message in the status bar about the autosave status, like "Auto-saved mappings to path /foo/bar" or "Failed to auto-save mappings to /foo/bar", would be a good use of this instead of just showing it in the log
.forEach(path -> { | ||
try { | ||
System.out.println("Removing previous auto save: " + path); | ||
Files.delete(path); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe delete after saving has finished to make sure it was successful?
final Path autoSaveDir = ConfigPaths.getConfigPathRoot().resolve("enigma").resolve("autosave"); | ||
|
||
try { | ||
final Path autoSavePath = autoSaveDir.resolve(DateTimeFormatter.ofPattern("yyyy-MM-dd-HH-mm-ss").format(LocalDateTime.now()) + ".mappings"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add name of current project (jar file name, I guess?) to this to make finding which project each file belongs to easier?
@@ -52,6 +52,14 @@ public static String getLanguage() { | |||
return ui.data().section("General").setIfAbsentString("Language", I18n.DEFAULT_LANGUAGE); | |||
} | |||
|
|||
public static boolean autoSave() { | |||
return ui.data().section("General").setIfAbsentBool("AutoSave", true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Auto Save"
} | ||
|
||
public static int autoSaveCount() { | ||
return ui.data().section("General").setIfAbsentInt("AutoSaveAmount", 5); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Auto Save Amount"
Adds an auto save feature that saves a backup of the mappings into the enigma config dir. This can be used to recover lost mappings. Keeps a rotation of auto saved mappings.
Not really tested, is this thread safe?