PaperMC / Paper

The most widely used, high performance Minecraft server that aims to fix gameplay and mechanics inconsistencies
https://papermc.io/
Other
9.9k stars 2.29k forks source link

Datapacks modifying nether/end generation don't apply properly #6880

Closed Virenbar closed 2 years ago

Virenbar commented 2 years ago

Expected behavior

Flat terrain to be generated. 2021-11-10_23 55 27 Version: Paper-325

Observed/Actual behavior

Normal terrain generated. 2021-11-10_23 57 16 Version: Paper-377

Steps/models to reproduce

  1. Add datapack to \<world>\datapack and enable it.
  2. Restart server.
  3. Generate new terrain.
  4. Normal terrain generated instead of flat.

Datapack: FlatOverworld.zip This datapack makes terrain in overworld flat.

Plugin and Datapack List

> plugins [23:58:58 INFO]: Plugins (0): > datapack list [23:59:18 INFO]: There are 3 data packs enabled: [vanilla (built-in)], [file/bukkit (world)], [file/FlatOverworld (world)] [23:59:18 INFO]: There are no more data packs available

Paper version

> version [23:57:24 INFO]: This server is running Paper version git-Paper-377 (MC: 1.17.1) (Implementing API version 1.17.1-R0.1-SNAPSHOT) (Git: 3f17694) You are running the latest version Previous version: git-Paper-325 (MC: 1.17.1)

Other

Server was updated from paper-325 to paper-377 and datapack stopped working. Datapack works in vanilla server.

Virenbar commented 2 years ago

Tested other versions. Paper-335 is latest version in which datapack works. In version 336 datapack has stopped working. It seems PR #6789 broke something.

jpenilla commented 2 years ago

Tested other versions. Paper-335 is latest version in which datapack works. In version 336 datapack has stopped working. It seems PR #6789 broke something.

Just re-tested, and

It seems like this issue is unrelated to custom dimensions and #6789, but is instead an issue with the vanilla server relating to loading datapacks which modify overworld generation.

Virenbar commented 2 years ago

Also re-tested. All servers were started with datapack enabled and empty ".\world\region" folder.

Screenshots **Vanilla server** - **Works** ![2021-11-12_22 17 54](https://user-images.githubusercontent.com/10138491/141509978-344132fe-cb86-48c9-b584-48da006e1702.png) **Paper-335** - **Works** ![2021-11-12_22 20 16](https://user-images.githubusercontent.com/10138491/141509989-835aa407-d328-47ae-aff9-f5028219adfe.png) **Paper-336** - **Not working** ![2021-11-12_22 37 23](https://user-images.githubusercontent.com/10138491/141510528-75118a7b-72e8-4b99-a33c-94e656f6d8e7.png) **Paper-master** without 0826-Use-correct-LevelStem-registry-when-loading-default-.patch - **Works** ![2021-11-12_22 31 56](https://user-images.githubusercontent.com/10138491/141509993-ef9f3804-51a3-45c7-98b7-a2fe5a48a270.png)
jpenilla commented 2 years ago

Something is definitely wrong with your testing setup. Creating a new directory with just server.jar from mojang's site, world/datapacks/FlatOverworld.zip, and eula.txt with eula=true results in the datapack not working. I was unable to get the datapack to successfully modify overworld gen anywhere except singleplayer by adding the datapack from the world creation menu.

In my testing, the vanilla server, Paper 335, Paper 336, Paper master, and paper master with the aforementioned patch reverted all resulted in the same behavior in regards to the datapack not modifying overworld gen from a clean server. I understand you claim otherwise, but before anyone on the team spends any more time on this I think we need confirmation from someone else that this ever worked, as like I already said my testing strongly implies this is a vanilla issue.

Virenbar commented 2 years ago

Here a short video of testing on vanilla server (server.jar, eula.txt, ops.json, startmc.bat with Aikar's Flags)

Machine-Maker commented 2 years ago

maybe the disconnect here is that you are only deleting the region file. when I tested, I deleted everything in the world except the datapacks folder, so the first generation would be with the datapack enabled, but maybe that doesnt work

Virenbar commented 2 years ago

Enabled datapacks saved in level.dat, deleting it disables all datapacks.

Machine-Maker commented 2 years ago

yeah, but all not-recognized datapacks are autoenabled upon server start

jpenilla commented 2 years ago

Enabled datapacks saved in level.dat, deleting it disables all datapacks.

sure, but datapacks are enabled by default when added before starting the server, as opposed to while it's running like in your video

jpenilla commented 2 years ago

Well that's nice, apparently datapacks will show as loaded and enabled when put in the correct location before generating a world, but won't actually work until loading the world a second time, which means you need to delete the region files like in your video...

Now that I'm actually able to get the datapack working on a server, the fix for this was quite simple, but I suspect the nether and end will have a similar problem even after applying my fix.

Given the nature of the bug fixed by the levelstem patch, we basically have to accept that trade off for now, until we can apply a different fix, which will likely need a much bigger diff. The root issue there is that upstream made changes which will effectively cause some worlds to have their seed changed when updating, which is really bad for obvious reasons, so although we would like datapacks to work just as in vanilla in all cases, we need to prioritize not damaging existing worlds.

jpenilla commented 2 years ago

Leaving this open to track the nether/end issue, like I mentioned that will need more work to fix without causing damage to existing worlds due to upstreams amazing code

Machine-Maker commented 2 years ago

So Amaranth mentioned that he thinks datapacks should be/should've been per-world, like almost everything else craftbukkit-related. I feel like its too late to make that change, but thought I'd bring it up cause if we did make them per-world, that might make it easier to fix this issue.

Athesiel commented 2 years ago

So Amaranth mentioned that he thinks datapacks should be/should've been per-world, like almost everything else craftbukkit-related. I feel like its too late to make that change, but thought I'd bring it up cause if we did make them per-world, that might make it easier to fix this issue.

Between having commands run from functions via "execute in [dimension]", the ability to have everything in loot tables specify dimension, and worldgen options already being dimension-driven, i feel like this would probably needlessly complicate things for datapack creators & users alike, like as of 1.18 almost any use in having Anything run per-world can be done with datapacks Or plugins pretty easily save for like, gamerules

Hwiggy commented 2 years ago

This server is running Paper version git-Paper-32 (MC: 1.19) (Implementing API version 1.19-R0.1-SNAPSHOT) (Git: 4c9317c) You are running the latest version Previous version: git-Purpur-1670 (MC: 1.19)

Datapacks in other dimensions are not processed unless you delete region files after level.dat is created