Add manual save backup management

[xuvatilavv]

This adds a Backups tab that allows manually creating, restoring, and deleting copies of the save directory for each game. Should be a decent foundation for automated backups but wanted to see what you thought before changing too much.

[qrrk]

I like this a lot. Great job! I also appreciate you sticking to the established structure of the project.

I found only one small issue in your code: in backup_current() and restore() you first copy the savegames into a temporary directory and then move them to either current/save or save_backups. Any reason not to copy them directly to the final destination?

I've "play-tested" your branch with both DDA and BN and took notes along the way. Everything works, but I've drafted some ideas for improvements, if you feel like working on them. If not, I can do it myself later.

• Restore and Delete buttons should be enabled only when a backup is selected in the list.

• Pre-popualte the name field with a suggested default name (maybe "Manual_" + date).

• Verify the backup name to avoid potentially illegal folder names, like in the screenshot below. I got away with it, but I bet this would cause problems on Windows. Use String.is_valid_filename().
  
  

• On that note, maybe verify the name continuously as it is being edited and disable the Create button for bad names (and also color the text red, using either Modulate property or Theme Overrides).

• Start backup creation when the user hits Enter in the name field.

• Make it clear that the Backup tab is about save backups, and not, for example, full game backups. Maybe change the label from "Available:" to "Save directory backups:".

• Show backup location on selection and allow opening it in one click (similarly to how it is done for game installs on the game tab).

• Maybe also list what worlds are there in the selected backup (should just be able to use subfolder names for this). It may not be very useful, but at least it will make it clear to new users that all saves get backed up and, more importantly, restored together.

[xuvatilavv]

Glad you like it! I like your suggestions as well, I'll look into adding at least some of them in the next few days.

As for your question: I chose to copy to a temporary directory first to avoid potential side effects that could arise from the source dir's name, since the "copy-into" behaviour of copy_dir() doesn't provide renaming in the same operation. Copying directly to the destination then renaming would have problems like:

• If an existing backup is named "save", creating any new backup of current/save would encounter a name collision and fail before being renamed.
• If an existing backup is named "data", restoring it would copy the save into the game's data dir and the subsequent move_dir() breaks the install.

Of course, it only now occurs to me that something like this would solve the same problem while being cleaner and less I/O:

	Directory.new().make_dir(dest_dir)
	var worlds = _fshelper.list_dir(current_save_dir)
	for world in worlds:
		copy_dir(source_dir.plus_path(world), dest_dir)

I can change it to that.

I had considered having this "copy-to" behavior in FilesystemHelper instead, but wasn't sure how best to add it into/alongside the existing "copy-into" method. What would you suggest?

[qrrk]

Of course, it only now occurs to me that something like this would solve the same problem while being cleaner and less I/O:

	Directory.new().make_dir(dest_dir)
	var worlds = _fshelper.list_dir(current_save_dir)
	for world in worlds:
		copy_dir(source_dir.plus_path(world), dest_dir)

I can change it to that.

Yes, that's what I was thinking, too. It would certainly be more elegant.

I had considered having this "copy-to" behavior in FilesystemHelper instead, but wasn't sure how best to add it into/alongside the existing "copy-into" method. What would you suggest?

I think the approach of copying worlds one by one is better. In the future, we may want to let the user restore only specific worlds from a backup.

[xuvatilavv]

Ended up having a bunch of time today. That's everything except your last two suggestions, I put a method to get the backup details for them but I'm not sure my UI design skills are up to fitting the info in nicely.

[qrrk]

This looks good! I've added a details pane and ability to open backup location by click.

Not entirely happy with how it turned out, but it will do for now.
Do you think this is ready to merge, or were you planning to do something else?

[xuvatilavv]

I think it's ready. I had a couple other ideas but I dunno if I'll get to them soon:

• Confirm delete/overwrite (via popup?)
• Show current saves summary before backing up

If I get to it, next thing I'd do is probably tie it into automatic backups though. For posterity, this is the list I made of what it would take to reach feature parity with the Remy launcher, in case you or someone else gets to it first:

• Setting: Auto backup before restoring
• Setting: Auto backup before updating
• Setting: Auto backup before game launch
• Setting: Auto backup after game end
• Setting: Limit maximum automatic backups count
• Cycle auto backups
• Show more backup details (modified date, character count, disk size)
• Compress backups

[qrrk]

Thanks for laying it out. Some of these points crossed my mind as well, but all this together sounds like quite a bit of work, so let's do any further improvements as a new iteration. For the time being, I'll merge this and likely make a new release soon.