SpongePowered / Sponge

The SpongeAPI implementation targeting vanilla Minecraft and 3rd party platforms.
MIT License
393 stars 211 forks source link

Some regressions with WorldTemplate and Offline worlds #3252

Open thibaulthenry opened 3 years ago

thibaulthenry commented 3 years ago
SpongeVanilla version: 1.16.4-8.0.0 (commit 81f8ff9aab292daec57472cad8bb08b194d65997)
Java version: 8
Operating System: Windows 10
Plugins: WIP plugin for api-8

Hey, yesterday many modifications were added to the world api and the WorldTemplate was introduced. I'm having some issues as WorldTemplate is now the only representation of offline worlds.

There are several things that I could do with offline worlds that are not possible anymore with WorldTemplate:

There are also things that I managed to do but not in an ideal way :

Having a world template template on which I try to modify the generateFeature with a value boolean

final WorldGenerationConfig worldGenerationConfig = MutableWorldGenerationConfig.builder()
        .seed(template.generationConfig().seed())
        .generateFeatures(value)
        .generateBonusChest(template.generationConfig().generateBonusChest())
        .build();

final WorldTemplate modifiedTemplate = template.asBuilder().generationConfig(worldGenerationConfig).build();

Sponge.getServer().getWorldManager().saveTemplate(modifiedTemplate);

Two problem here: 1) When using asBuilder on the template, the WorldGenerationConfig is still not mutable 2) There is no way to use the MutableWorldGenerationConfig.builder().from(...) as it only accepts MutableWorldGenerationConfig and not WorldGenerationConfig. So I have to copy the value of each field of the WorldGenerationConfig, and modify the generateFeatures to the desired value

value = RegistryReference.referenced(Sponge.getGame().registries(), RegistryTypes.DIFFICULTY, difficultyUserInput);
final WorldTemplate modifiedTemplate =template.asBuilder().difficulty(value).build()
Sponge.getServer().getWorldManager().saveTemplate(modifiedTemplate);

Here I don't even know which RegistryHolder I have to use. I just assumed that difficulties were game scoped.. I either don't know if there is a better way to retrieve the RegistryReference<Difficulty> from a Difficulty object.

Thanks for reading this issue.

Zidane commented 3 years ago

For your final point (the others are fair), that is the correct way to generate a reference. Though I am looking into variety of ways of using the Registry API in as easy to use as possible.

thibaulthenry commented 3 years ago

How can I be sure that I'm using the correct RegistryHolder to generate a reference ?

Zidane commented 3 years ago

If you aren't sure and you don't use the Optional return methods, you'll be greeted with an exception.

Vanilla Registry system is very dynamic, no assumptions are allowed.

API wise, we try to help with this by adding easy forwarders, a future commit will make this easier. For DefaultedRegistryReferences, their <type> will have some <types> class that dictates the RegistryScope

See Weathers, BlockTypes, ItemTypes, etc.

thibaulthenry commented 3 years ago

Okay thanks! I forget to add this in the issue :

Zidane commented 3 years ago

Added some sugaring for registry API, will help you get references much easier now.

Zidane commented 3 years ago

So I'll add the stuff that deals with the WorldBorder but I admit that the day time and weather are odd ones. Mostly cause they don't really stand out as a "template".

For example, weather and day time are constantly changing. I could have it always save with the save tick in Sponge for ServerWorld but using the template to create additional worlds will always end up as those values to start off with.

There is also something be said for the fact that...are those static values or values to start off as? If you consider them static values, putting them in the template makes 100% sense.

thibaulthenry commented 3 years ago

I understand that these have no sense in the context of "template" for world creation. But now template are also the only way to manipulate offline worlds properties.

For example, in my plugin, maybe someone will try to save a backup of a world at a specified time of the day. So he will first unload the world, then change the time, then make the backup. (same for weather)

There is also something be said for the fact that...are those static values or values to start off as? If you consider them static values, putting them in the template makes 100% sense.

I don't get the difference to be honest. Actually this depends of what the template represents. For a non-created world, this would be the values to start off For offline world, this would be the time/weather before being offline, and then the values to start off when going online again, so static values as you said.


Off topic

I also forget to add this in the issue :

In Minecraft 1.16.4 we can modify the game rules in the world generation menu so I think we have to add them in template

Zidane commented 3 years ago

Spawn position, WorldGenerationConfig concerns now handled.

Zidane commented 3 years ago

Reading/Saving of offline world data returns. Needs some testing, let me know if it explodes.

Zidane commented 3 years ago

View distance is now being respected in the template.