-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add basic save backup system #2096
Add basic save backup system #2096
Conversation
After this feature is merged, would we want to wait until world manager is implemented to do further backup functionality (allowing players to load backups without having to just drag & drop files, maybe naming backups, etc.?) Is there a PR for the world manager or is this just something being discussed? |
I did work on that a while back in this PR, but it was closed because we wanted to focus on improving the world manager design and implementing a cross-platform launcher (see this PR). The purpose of this PR is simply to implement the backend backup functionality to solve the issue I mentioned in the PR summary, but implementing the UI for it will come after the new launcher is finished. |
{ | ||
FileInfo fileInfo = new(file); | ||
|
||
if (fileInfo.Extension != ".zip" || !fileInfo.Name.Contains($"Backup - ")) |
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.
Do we want to allow Backup files that are named differently, such as if users want to rename them as a note to themselves? Currently, a ZIP file would not be considered a backup if named differently, which would exclude it from this automatic pruning process (might be a good thing).
Another possible approach is how Minecraft does resource packs / data packs, where a specific file in the root folder of the backup file (pack.mcmeta
) signifies it is a resource/data pack, not just a random ZIP file. This functions independently of how the files are named.
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.
We can definitely discuss that with the other devs in the developer channels of the Discord server (once you get the Jr-Alterra role).
Personally, I don't think allowing users to rename their backup files is necessary. Ideally, all they would have to do when their world corrupts is to backup up the most recent save file, and the extra backups are there in case some of the recently-made ones are also corrupted.
Co-Authored-By: Wesley Ho <[email protected]>
{ | ||
internal class BackupCommand : Command | ||
{ | ||
public BackupCommand() : base("backup", Perms.ADMIN, "Creates a backup of the save") |
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.
Same for that command, I don't find it useful
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.
The autobackup command could indeed be left out. However, I think that a "Backup" command will prove useful to anyone that wants to force the server to create a backup for peace of mind, such as before they perform an action that might mess up their world (for example, spawning 1000+ seamoths).
- Removed the AutoBackup command - Removed the idea of letting an empty value of "MaxBackups" allow the server to make infinite backups - Fixed and improved functionality regarding having "MaxBackups" have a value of 0 (i.e. no backups are made) - Other small improvements.
Uses the same logic from my Backup System PR: SubnauticaNitrox#2096 TODO: Need to fix UI not updating after restoring a backup (mentioned as a comment within the code). Will try working on this issue tomorrow. The backups restore dialog could maybe do with a style redesign but I think it looks fine enough for now and will leave that decision for others to make.
This PR focuses on adding a basic backup system to the Nitrox server, and that it would ideally be able to function without the world manager so that it can be included in v1.8.0.0 for use right away.
The first two commits were made on my copy of the v1.7.0.0 branch of the main repository, then cherry-picked them into this up-to-date branch. The third commit fixes the code for use with the world manager. I have done this in an attempt to make it easier for you guys to cherry pick the first two commits of this PR for use in v1.8.0.0 of Nitrox. Not sure if I did it effectively though.
The backup system functions as follows:
BackupCommand
andAutoBackupCommand
. The former performs a backup when it's run and the latter automatically backs up the save when the server autosaves.DisableAutoBackup
andMaxBackups
. The former is there for a similar reason as theDisableAutoSave
setting, while the latter defines the max number of backups the system keeps. The default is set to 10.I think having this in the next release is essential since we get a lot of users that talk about their worlds being corrupted in the Discord server's help channel. This system will help them recover most of their progress and improve their overall experience as Nitrox continues to be more stable.