//regen Does not function with negative custom seeds

Question

//regen Does not function with negative custom seeds

encode42 opened this issue 3 years ago · 6 comments

encode42 commented 3 years ago

Server Implementation

Purpur, Paper

Server Version

1.17.1

Describe the bug

Changing Spigot or Purpur's custom seeds (seed-igloo, seed-mansion, buried_treasure, etc.) to a negative number will cause an error when using the //regen command.

To Reproduce

Set a custom seed from Spgiot or Purpur to a negative value, I mainly tested with seed-igloo
Start/restart the server
Select a region and run //regen

Expected behaviour

An errorless regeneration of my selection

Screenshots / Videos

No response

Error log (if applicable)

https://mclo.gs/kSSH8BN, https://paste.gg/p/anonymous/b095501b6ac74ae2bed99e7fcec970c0

Fawe Debugpaste

https://athion.net/ISPaster/paste/view/fbabcf12a8d84cb7b4a16ddf31175257

Fawe Version

FastAsyncWorldEdit version 1.17-393;806ea14

Checklist

I have included a Fawe debugpaste.
I am using the newest build from https://ci.athion.net/job/FastAsyncWorldEdit-1.17/ and the issue still persists.

Anything else?

This may apply to Paper's custom seed options as well, though I have not tested such. Purpur uses -1 as the "default" seed value, which makes this a common error for users of Purpur.

SirYwell · Answer 1 · 2021-11-11T17:19:28.000Z

Minecraft itself does not allow negative seeds there, spigot does not care about it and forces it in anyways. But that leads to a configuration state, that can not be read again. I'm not sure if we want to deal with that.

NotMyFault · Answer 2 · 2021-11-11T14:08:16.000Z

Can't replicate on Spigot or Paper.

encode42 · Answer 3 · 2021-11-11T16:05:27.000Z

Was able to reproduce on a fresh Paper server (1.17.1 build 378) with the configuration attached below.

It's a completely fresh Paper instance with the latest FAWE build. Started the server to generate the world and config files, made seed-igloo and seed-mansion negative by simply adding - before the auto-filled seed, restarted the server, and tried to run the regen command.

https://cloud.backlight.games/s/LX4Qs4pdnrFEbS3

BillyGalbreath · Answer 4 · 2021-11-12T00:58:43.000Z

Minecraft itself does not allow negative seeds there, spigot does not care about it and forces it in anyways. But that leads to a configuration state, that can not be read again. I'm not sure if we want to deal with that.

Negative seeds have always worked with Minecraft, and you can clearly see it works fine with regular WorldEdit's //regen command using negative seeds in spigot.yml. This issue only exists with FAWE, and can be replicated by using any negative seed in any configuration file from any Bukkit fork (spigot.yml, paper.yml, or purpur.yml)

SirYwell · Answer 5 · 2021-11-12T07:42:29.000Z

I don't understand what's so difficult about looking at the minecraft code for some people, but the CODEC in StructureFeatureConfiguration clearly expects the salt (which is the "seed" for the structures) to be non-negative. And that's where it currently fails. With this patch:

diff --git a/worldedit-bukkit/adapters/adapter-1_17_1/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_17_R1_2/regen/PaperweightRegen.java b/worldedit-bukkit/adapters/adapter-1_17_1/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_17_R1_2/regen/PaperweightRegen.java
--- a/worldedit-bukkit/adapters/adapter-1_17_1/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_17_R1_2/regen/PaperweightRegen.java	(revision 6f33c5223d206b5fa7b2c68f88762c6d0d0272c9)
+++ b/worldedit-bukkit/adapters/adapter-1_17_1/src/main/java/com/sk89q/worldedit/bukkit/adapter/impl/fawe/v1_17_R1_2/regen/PaperweightRegen.java	(date 1636700185360)
@@ -231,8 +231,13 @@
                         seed,
                         new HashSet<>()
                 )))
-                .result()
-                .orElseThrow(() -> new IllegalStateException("Unable to map GeneratorOptions"));
+                .get()
+                .map(
+                        l -> l,
+                        error -> {
+                            throw new IllegalStateException("unable to map GeneratorOptions: " + error.message());
+                        }
+                );
         LevelSettings newWorldSettings = new LevelSettings(
                 "worldeditregentempworld",
                 originalWorldData.settings.gameType(),

We even get the message unable to map GeneratorOptions: Value must be non-negative: -14357618; Value must be non-negative: -14357618; Value must be non-negative: -14357618 (idk what Mojang was doing there that it appears three times).

That said, I'm not against finding a workaround for this, but I'm not sure if it should be allowed by spigot in the first place - a configuration, that can't be encoded and decoded again feels wrong to me.

NotMyFault · Answer 6 · 2021-11-18T23:25:31.000Z