Skip to content
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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

modmuss50
Copy link
Member

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?

Copy link
Collaborator

@2xsaiko 2xsaiko left a 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);
Copy link
Collaborator

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);
Copy link
Collaborator

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

Copy link
Contributor

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?

Copy link
Collaborator

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);
Copy link
Collaborator

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");
Copy link
Collaborator

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);
Copy link
Collaborator

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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Auto Save Amount"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants