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

Configurable Region Compression Format #8505

Merged
merged 1 commit into from
Jul 23, 2023
Merged

Configurable Region Compression Format #8505

merged 1 commit into from
Jul 23, 2023

Conversation

Astralchroma
Copy link
Contributor

Adds a configuration option to save region files without any compression, this is useful for any servers which wish to use compression at the file system level.

Minecraft already has the ability to save and load region files without compression, however the ability to save isn't used, and this adds a config option to enable that. As Minecraft supports regions saved this way, the worlds saved using this option remain compatible with an unmodified client / server.

@Astralchroma Astralchroma requested a review from a team as a code owner October 25, 2022 16:43
Copy link
Contributor

@lynxplay lynxplay left a comment

Choose a reason for hiding this comment

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

Just as a general thought, wouldn't it be a bit more future proof to have this more of an enum value, rather than a boolean ?

E.g. right now this option lets you switch between deflate and none, however potentially the server offers different IO wrappers in the future here.
Instead of a boolean, this could simply default to "deflate", with the possible alternative format of "plain" or something along those lines.

@Astralchroma
Copy link
Contributor Author

Just as a general thought, wouldn't it be a bit more future proof to have this more of an enum value, rather than a boolean ?

E.g. right now this option lets you switch between deflate and none, however potentially the server offers different IO wrappers in the future here. Instead of a boolean, this could simply default to "deflate", with the possible alternative format of "plain" or something along those lines.

Minecraft already has support for 3 compression formats (gzip, zlib (default), and none), although I don't see why anyone would want to use gzip over zlib or none, but I suppose if we're adding a option for default or none, we may as well just use an enum and allow all 3, so I'll do that. Although I don't think this enum should be used by Paper to add alternative formats in the future as formats added by Paper would be incompatible with Vanilla.

@olijeffers0n
Copy link
Member

As formats added by Paper would be incompatible with Vanilla.

I know that this is something that is technically possible with some folder moving, but moving worlds from Paper to vanilla is something that I hope people can understand may cause issues.

If enough of a warning notice is placed on it when the user selects it, I have no problem with this.

There is potential for a converter tool to be developed to change it back anyways, so I +1 an enum for (the possibility of) new formats as ultimately Paper is here to improve the server experience.

@Astralchroma
Copy link
Contributor Author

Updated to rebase on master (apparently I forgot to do that before), as well as use a supplier like @Machine-Maker suggested, as well as changing it to use an enum.

@Astralchroma Astralchroma changed the title Uncompressed Regions Configurable compression format Oct 27, 2022
@Astralchroma
Copy link
Contributor Author

This has been rebased onto master now, any chance this could be merged soon?

@Owen1212055
Copy link
Member

This was discussed somewhere, but I think that this config option should be moved to the unsupported category. It's not officially supported by mojang and I am not sure this is something we should officially support too.

@Astralchroma
Copy link
Contributor Author

This was discussed somewhere, but I think that this config option should be moved to the unsupported category. It's not officially supported by mojang and I am not sure this is something we should officially support too.

That makes sense, I've gone ahead and done that

@Astralchroma
Copy link
Contributor Author

Rebased

@Astralchroma Astralchroma changed the title Configurable compression format Configurable Region Compression Format Mar 30, 2023
@Astralchroma
Copy link
Contributor Author

the rebase required label can removed btw

@Owen1212055 Owen1212055 merged commit d8b8f61 into PaperMC:master Jul 23, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

None yet

7 participants