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

Add basic save backup system #2096

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

NinjaPedroX
Copy link
Contributor

@NinjaPedroX NinjaPedroX commented Dec 1, 2023

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:

  • Backups are kept within the save directory, in a subfolder called "Backups"
  • Each backup is a .zip file with the title "Backup - yyyyMMddhhmmss". When the system detects a number of backup files higher than the max value defined in the config, it deletes the oldest one(s), unless there is no limit defined.
  • There are two new server commands: BackupCommand and AutoBackupCommand. The former performs a backup when it's run and the latter automatically backs up the save when the server autosaves.
  • I have added two new config settings: DisableAutoBackup and MaxBackups. The former is there for a similar reason as the DisableAutoSave setting, while the latter defines the max number of backups the system keeps. The default is set to 10.
  • Having save files that are corrupted no longer makes the server back them up and start anew; instead, it simply tells the user to restore one of their backups.

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.

image

@Drakonkinst
Copy link

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?

@NinjaPedroX
Copy link
Contributor Author

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.

NitroxServer/Serialization/World/WorldPersistence.cs Outdated Show resolved Hide resolved
{
FileInfo fileInfo = new(file);

if (fileInfo.Extension != ".zip" || !fileInfo.Name.Contains($"Backup - "))

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.

Copy link
Contributor Author

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.

NitroxServer/Server.cs Show resolved Hide resolved
NinjaPedroX and others added 2 commits December 1, 2023 16:01
Co-Authored-By: Wesley Ho <11655960+drakonkinst@users.noreply.github.com>
@tornac1234 tornac1234 added the Area: user interface (launcher) Related to UI/UX in the launcher label Dec 23, 2023
NitroxServer/ConsoleCommands/AutoBackupCommand.cs Outdated Show resolved Hide resolved
{
internal class BackupCommand : Command
{
public BackupCommand() : base("backup", Perms.ADMIN, "Creates a backup of the save")
Copy link
Member

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

Copy link
Contributor Author

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).

NitroxServer/Serialization/ServerConfig.cs Outdated Show resolved Hide resolved
NitroxServer/Serialization/World/WorldPersistence.cs Outdated Show resolved Hide resolved
- 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: user interface (launcher) Related to UI/UX in the launcher
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants