External generator settings (new implementation) #10

Foghrye4 · 2018-12-22T11:07:27Z

No description provided.

Barteks2x · 2018-12-22T16:32:23Z

...st/java/io/github/opencubicchunks/cubicchunks/cubicgen/TestCustomGeneratorSettingsFixer.java

+public class TestCustomGeneratorSettingsFixer {
+    String cc655ExamplePreset = "{\n" + 
+            "   \"waterLevel\":63,\n" + 
+            "   \"caves\":true,\n" + 


Use all settings non-defaults in tests

Barteks2x · 2018-12-22T16:32:57Z

...st/java/io/github/opencubicchunks/cubicchunks/cubicgen/TestCustomGeneratorSettingsFixer.java

+    @Test
+    public void testCustomGeneratorSettingsFixer() {
+        CustomGeneratorSettings settings = CustomGeneratorSettings.fromJson(cc655ExamplePreset);
+        assertEquals(13,settings.standardOres.size());


It doesn't verify that the preset actually has all the right data

So what I should assert?

First, test conversion between specific versions, and second, just put the expected json and assert that they make equal JsonObject

…. Use GSON nice formatting.

Barteks2x · 2019-01-19T22:37:39Z

...java/io/github/opencubicchunks/cubicchunks/cubicgen/customcubic/CustomGeneratorSettings.java

+        CustomGeneratorSettings settings = null;
+        if (externalGeneratorPresetFile.exists()) {
+            try (FileReader reader = new FileReader(externalGeneratorPresetFile)){
+                CharBuffer sb = CharBuffer.allocate((int) externalGeneratorPresetFile.length() * 2);


What... why twice the length?

CharBuffer.allocate(int) consume char buffer capacity in chars, File.length() return size in bytes.

Well, I'm derping a little. I should divide by 2.

Actually no. Files and java chars use completely different character encoding. Files are UTF-8 and java uses UTF-16. For the ASCII subset, 1 byte in file corresponds to 1 java character. There can be multiple bytes in file that also correspond to a single java char. But not the other way around.

Barteks2x · 2019-01-19T22:41:47Z

...io/github/opencubicchunks/cubicchunks/cubicgen/customcubic/CustomGeneratorSettingsFixer.java

+            for (JsonElement entry : array) {
+                JsonArray mapEntry = entry.getAsJsonArray();
+                JsonElement key = mapEntry.get(0);
+                JsonObject value = stringToJson(fixGeneratorOptions(mapEntry.get(1).getAsJsonObject(), true));


This needs the "parent" one as context, and use parent preset as defaults (this also means getCubeAreas needs to be called last, so that the parent preset is otherwise complete)

This means that every cube area will be as huge as full list of settings. And manual editing of preset would be slightly harder. Are you sure it should be that way?

Yes, this is the way it should be done. No magic inheritance.
The goal is that layers as they are now are eventually made irrelevant and I know what I need to do to make that happen.

To explain in more detail the reasoning:

This mechanism is just confusing and extremely hard to explain. So far there has been one person who used it explicitly, and that's in the exact place where it doesn't actually work the way it's intended to...

Which bring the "it doesn't work" part. For big presets, the single biggest part is ores. And this is also the exact place where it just doesn't work and can't work.

It's a pain to handle in code, especially as preset format evolves. If each layer doesn't depend on ANYTHING else, then you can put a valid preset (or layer from that preset) from ANY format version (with the exception of compressed v3 and earlier, so anything without "version" specified is not going to work) and put it as a layer in preset of any other version.
With inheritance, mixing versions just can't work without either special handling for different version combinations, or some kind of intermediate representation, where old fixers would have to be kept up to date with. Fixers are supposed to be write once and never touch it again things, so this is obviously not going to work that way. For v3 and newer, you would just plug a layer into the whole fixer system starting from the beginning, it would figure out the versions and convert as needed, and you wouldn't need any kind of special context. It would also be increasingly difficult to handle and document all the possible version interactions

The layer system will get more or less phased out. It will remain there as a supported use case, but for what it's used right now, there will be a far superior system. So preset size and editing convenience is going to be less of a concern. I'm just waiting on this to actually start implementing these things because I really want this before I start messing around with presets again, to avoid making handling the "legacy" preset format even more of a pain.

Barteks2x · 2019-01-19T22:46:11Z

...st/java/io/github/opencubicchunks/cubicchunks/cubicgen/TestCustomGeneratorSettingsFixer.java

+            "   \"highNoiseOctaves\":3\n" + 
+            "}";
+
+    String expectedConversionResult = "{\"waterLevel\":100,\"caves\":false,\"strongholds\":false,\"alternateStrongholdsPositions\":false,\"villages\":false,\"mineshafts\":false,\"temples\":false,\"oceanMonuments\":false,\"woodlandMansions\":false,\"ravines\":false,\"dungeons\":false,\"dungeonCount\":101,\"waterLakes\":false,\"waterLakeRarity\":102,\"lavaLakes\":false,\"lavaLakeRarity\":103,\"aboveSeaLavaLakeRarity\":104,\"lavaOceans\":true,\"biome\":1,\"biomeSize\":104,\"riverSize\":105,\"standardOres\":[{\"blockstate\":{\"Properties\":{\"variant\":\"dirt\",\"snowy\":\"false\"},\"Name\":\"minecraft:dirt\"},\"spawnSize\":134,\"spawnTries\":135,\"spawnProbability\":136.0,\"minHeight\":-Infinity,\"maxHeight\":Infinity},{\"blockstate\":{\"Name\":\"minecraft:gravel\"},\"spawnSize\":137,\"spawnTries\":138,\"spawnProbability\":139.0,\"minHeight\":-Infinity,\"maxHeight\":Infinity},{\"blockstate\":{\"Properties\":{\"variant\":\"granite\"},\"Name\":\"minecraft:stone\"},\"spawnSize\":140,\"spawnTries\":141,\"spawnProbability\":142.0,\"minHeight\":-Infinity,\"maxHeight\":143.0},{\"blockstate\":{\"Properties\":{\"variant\":\"diorite\"},\"Name\":\"minecraft:stone\"},\"spawnSize\":144,\"spawnTries\":145,\"spawnProbability\":146.0,\"minHeight\":-Infinity,\"maxHeight\":147.0},{\"blockstate\":{\"Properties\":{\"variant\":\"andesite\"},\"Name\":\"minecraft:stone\"},\"spawnSize\":148,\"spawnTries\":149,\"spawnProbability\":150.0,\"minHeight\":-Infinity,\"maxHeight\":151.0},{\"blockstate\":{\"Name\":\"minecraft:coal_ore\"},\"spawnSize\":152,\"spawnTries\":153,\"spawnProbability\":154.0,\"minHeight\":-Infinity,\"maxHeight\":155.0},{\"blockstate\":{\"Name\":\"minecraft:iron_ore\"},\"spawnSize\":156,\"spawnTries\":157,\"spawnProbability\":158.0,\"minHeight\":-Infinity,\"maxHeight\":159.0},{\"blockstate\":{\"Name\":\"minecraft:gold_ore\"},\"spawnSize\":160,\"spawnTries\":161,\"spawnProbability\":162.0,\"minHeight\":-Infinity,\"maxHeight\":-163.0},{\"blockstate\":{\"Name\":\"minecraft:redstone_ore\"},\"spawnSize\":164,\"spawnTries\":165,\"spawnProbability\":166.0,\"minHeight\":-Infinity,\"maxHeight\":-167.0},{\"blockstate\":{\"Name\":\"minecraft:diamond_ore\"},\"spawnSize\":168,\"spawnTries\":169,\"spawnProbability\":170.0,\"minHeight\":-Infinity,\"maxHeight\":-178.0},{\"blockstate\":{\"Name\":\"minecraft:emerald_ore\"},\"biomes\":[\"minecraft:extreme_hills\",\"minecraft:smaller_extreme_hills\",\"minecraft:extreme_hills_with_trees\",\"minecraft:mutated_extreme_hills\",\"minecraft:mutated_extreme_hills_with_trees\"],\"spawnSize\":3,\"spawnTries\":184,\"spawnProbability\":185.0,\"minHeight\":-Infinity,\"maxHeight\":-186.0},{\"blockstate\":{\"Properties\":{\"variant\":\"stone\"},\"Name\":\"minecraft:monster_egg\"},\"biomes\":[\"minecraft:extreme_hills\",\"minecraft:smaller_extreme_hills\",\"minecraft:extreme_hills_with_trees\",\"minecraft:mutated_extreme_hills\",\"minecraft:mutated_extreme_hills_with_trees\"],\"spawnSize\":187,\"spawnTries\":188,\"spawnProbability\":189.0,\"minHeight\":-Infinity,\"maxHeight\":190.0},{\"blockstate\":{\"Name\":\"minecraft:gold_ore\"},\"biomes\":[\"minecraft:mesa\",\"minecraft:mesa_clear_rock\",\"minecraft:mesa_rock\",\"minecraft:mutated_mesa\",\"minecraft:mutated_mesa_clear_rock\",\"minecraft:mutated_mesa_rock\"],\"spawnSize\":191,\"spawnTries\":192,\"spawnProbability\":193.0,\"minHeight\":-194.0,\"maxHeight\":196.0}],\"periodicGaussianOres\":[{\"blockstate\":{\"Name\":\"minecraft:lapis_ore\"},\"spawnSize\":179,\"spawnTries\":180,\"spawnProbability\":181.0,\"heightMean\":182.0,\"heightStdDeviation\":183.0,\"heightSpacing\":0.0,\"minHeight\":0.0,\"maxHeight\":0.0}],\"expectedBaseHeight\":110.0,\"expectedHeightVariation\":109.0,\"actualHeight\":544.0,\"heightVariationFactor\":106.0,\"specialHeightVariationFactorBelowAverageY\":107.0,\"heightVariationOffset\":108.0,\"heightFactor\":109.0,\"heightOffset\":110.0,\"depthNoiseFactor\":111.0,\"depthNoiseOffset\":112.0,\"depthNoiseFrequencyX\":113.0,\"depthNoiseFrequencyZ\":114.0,\"depthNoiseOctaves\":5,\"selectorNoiseFactor\":116.0,\"selectorNoiseOffset\":117.0,\"selectorNoiseFrequencyX\":118.0,\"selectorNoiseFrequencyY\":119.0,\"selectorNoiseFrequencyZ\":120.0,\"selectorNoiseOctaves\":1,\"lowNoiseFactor\":122.0,\"lowNoiseOffset\":123.0,\"lowNoiseFrequencyX\":124.0,\"lowNoiseFrequencyY\":125.0,\"lowNoiseFrequencyZ\":126.0,\"lowNoiseOctaves\":7,\"highNoiseFactor\":128.0,\"highNoiseOffset\":129.0,\"highNoiseFrequencyX\":130.0,\"highNoiseFrequencyY\":131.0,\"highNoiseFrequencyZ\":132.0,\"highNoiseOctaves\":3,\"cubeAreas\":[],\"replacerConfig\":{\"defaults\":{\"cubicgen:biome_fill_noise_octaves\":4.0,\"cubicgen:ocean_block\":{\"Properties\":{\"level\":\"0\"},\"Name\":\"minecraft:water\"},\"cubicgen:height_scale\":64.0,\"cubicgen:biome_fill_noise_freq\":0.0078125,\"cubicgen:water_level\":63.0,\"cubicgen:biome_fill_depth_factor\":2.3333333333333335,\"cubicgen:terrain_fill_block\":{\"Properties\":{\"variant\":\"stone\"},\"Name\":\"minecraft:stone\"},\"cubicgen:mesa_depth\":16.0,\"cubicgen:biome_fill_depth_offset\":3.0,\"cubicgen:horizontal_gradient_depth_decrease_weight\":1.0,\"cubicgen:height_offset\":64.0},\"overrides\":{}},\"version\":3}";


Split into multiple lines (without \n)

Barteks2x · 2019-01-19T22:49:51Z

I also still don't think it actually works with presets bigger than 64kB correctly.

Just try such preset. Last time I tested it (and I didn't see any relevant changes since then), MC would throw an exception when writing level.dat (the exception is then ignored, but it means level.dat never gets updated)

Foghrye4 · 2019-01-20T14:20:03Z

Tested with 380 Kb preset - level.dat saved fine.

… new world creation

Barteks2x · 2019-01-20T16:14:23Z

...java/io/github/opencubicchunks/cubicchunks/cubicgen/customcubic/CustomGeneratorSettings.java

            settings = CustomGeneratorSettings.fromJson(CustomCubicWorldType.pendingCustomCubicSettingsJsonString);
            CustomCubicWorldType.pendingCustomCubicSettingsJsonString = "";
+            CustomCubicWorldType.createNewWorld = false;


There should be a better way to determine if it's a new world that is used in common event handler.

I can suggest to do a same thing but in a AttachCapabilitiesEvent<World> handler with high priority. Another option is to retrieve saveDirName from GuiCreateWorld and create generator settings file before World instance creation on CustomCubicWorldType.onGUICreateWorldPress().

Barteks2x · 2019-01-22T10:06:48Z

src/main/java/io/github/opencubicchunks/cubicchunks/cubicgen/proxy/ClientProxy.java

+
+    @Override
+    public void preInit() {
+        MinecraftForge.EVENT_BUS.register(new GuiEventHandler());


Proxy not needed. use @Mod.EventBusSubscriber(modid = modid, value = Side.CLIENT)

Barteks2x · 2019-01-22T10:08:36Z

src/main/java/io/github/opencubicchunks/cubicchunks/cubicgen/common/gui/GuiEventHandler.java

+            if (!action.getButton().enabled)
+                return;
+            // "Recreate world" has button id = 5
+            if (action.getButton().id != 5)


Why a comment instead of making a private static final int field?

Barteks2x · 2019-01-22T10:11:45Z

src/main/java/io/github/opencubicchunks/cubicchunks/cubicgen/common/gui/GuiEventHandler.java

+            String json = CustomGeneratorSettings.loadJsonStringFromSaveFolder(isavehandler);
+            isavehandler.flush();
+            if (json != null) {
+                CustomCubicWorldType.pendingCustomCubicSettingsJsonString = json;


I really don't like that static field here. Verifying that this field has the right value at the right time is nearly impossible without actually trying every possibility.

Instead I would suggest just mixing into WorldSettings and WorldInfo (same as what I did in CC itself, with MixinWorldSettings and MixinWorldInfo).

This also should remove the need for createNewWorld variable.

This would also allow to actually store the loaded preset text somewhere instead of loading it from disk each time it's needed.

If you actually inject in the right place, you can almost completely hide the fact that the preset is not saved in level.dat, by just not saving that part into level.dat for CustomCubic worlds, and instead have code somewhere else that saves it.

Barteks2x · 2019-01-22T10:13:29Z

...java/io/github/opencubicchunks/cubicchunks/cubicgen/customcubic/CustomGeneratorSettings.java

-import java.util.Map;
-import java.util.Set;
-
-import javax.annotation.Nullable;


Why change import order?

Barteks2x · 2019-01-22T10:14:35Z

...java/io/github/opencubicchunks/cubicchunks/cubicgen/customcubic/CustomGeneratorSettings.java

+
+    public static boolean isOutdated(String settingsJsonString) {
+        JsonObject root = CustomGeneratorSettingsFixer.stringToJson(settingsJsonString);
+        return !root.has("version") || root.get("version").getAsInt() != CustomGeneratorSettingsFixer.VERSION;


< instead of !=. Don't touch the settings if the stored version is newer. Ideally, CWG wouldn't even let you open such world without a warning, but I'm not sure how to do that.

Barteks2x · 2019-01-22T10:29:41Z

...io/github/opencubicchunks/cubicchunks/cubicgen/customcubic/CustomGeneratorSettingsFixer.java

+    };
+
+    private static boolean isActual(JsonObject root) {
+        return root.has("version") && root.get("version").getAsInt() == 3;


Actual is most likely the wrong word here.

Barteks2x · 2019-01-22T10:29:54Z

...io/github/opencubicchunks/cubicchunks/cubicgen/customcubic/CustomGeneratorSettingsFixer.java

+        Gson gson = CustomGeneratorSettings.gson();
+        JsonObject oldRoot = CustomGeneratorSettingsFixer.stringToJson(json);
+        boolean rootIsActual = isActual(oldRoot);
+        if (rootIsActual) {


Barteks2x · 2019-01-22T10:31:41Z

...io/github/opencubicchunks/cubicchunks/cubicgen/customcubic/CustomGeneratorSettingsFixer.java

+        }
+        JsonObject newRoot = stringToJson("{}");
+        newRoot.add("version", new JsonPrimitive(3));
+        newRoot.add("waterLevel", getWaterLevel(oldRoot,parent));


Barteks2x · 2019-01-22T10:32:16Z

...io/github/opencubicchunks/cubicchunks/cubicgen/customcubic/CustomGeneratorSettingsFixer.java

+        newRoot.add("highNoiseFrequencyZ", getHighNoiseFrequencyZ(oldRoot,parent));
+        newRoot.add("highNoiseOctaves", getHighNoiseOctaves(oldRoot,parent));
+        newRoot.add("replacerConfig", getReplacerConfig(oldRoot,parent));
+        if (parent == null) // cube areas allowed only for root


Actually no. Any sub-layer can contain more CubeAreas. Any valid preset can be used as a cube area.

Barteks2x · 2019-01-26T09:52:44Z

Closing, implemented in #11

Foghrye4 added 12 commits December 16, 2018 12:32

External generator settings

55f7093

No mixin

4d76206

Data fixes

e7a596a

Simplify fixes

ffdac8d

Remove saving on done

ded565f

Saving settings only using provided World instance path

4bd1b8f

Expanded transversion json fixer

da1e965

Half-assed fixer for JSON preset

9d35601

False

bfeaf97

Full-assed generator settings fixer

6a572b7

Fixing Unit Test

23b470c

Removing minimization code

aeeded6

Barteks2x requested changes Dec 22, 2018

View reviewed changes

Foghrye4 added 3 commits December 22, 2018 21:18

Expanding test to expected conversion case

a1e01bf

Fix creating file writer before creating directories

0f9f5ff

Extend hardcoded limit of char buffer to accept presets with any size…

a7c572a

…. Use GSON nice formatting.

Barteks2x changed the title ~~External generator settings (Full-assed with unit test)~~ External generator settings (new implementation) Jan 19, 2019

Barteks2x requested changes Jan 19, 2019

View reviewed changes

Barteks2x mentioned this pull request Jan 19, 2019

External generator settings #9

Closed

Creating copy of a world will copy generator settings

b819ef2

Remove unnecessary Malisiscore dependency, fix missing ore entries on…

c79512f

… new world creation

Barteks2x reviewed Jan 20, 2019

View reviewed changes

Barteks2x reviewed Jan 22, 2019

View reviewed changes

Barteks2x closed this Jan 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

External generator settings (new implementation) #10

External generator settings (new implementation) #10

Foghrye4 commented Dec 22, 2018 •

edited

Barteks2x Dec 22, 2018

Barteks2x Dec 22, 2018

Foghrye4 Dec 22, 2018

Barteks2x Dec 22, 2018

Foghrye4 Dec 22, 2018

Barteks2x Jan 19, 2019

Foghrye4 Jan 20, 2019

Foghrye4 Jan 20, 2019

Barteks2x Jan 20, 2019

Barteks2x Jan 19, 2019

Foghrye4 Jan 20, 2019

Barteks2x Jan 20, 2019

Barteks2x Jan 20, 2019 •

edited

Barteks2x Jan 19, 2019

Barteks2x commented Jan 19, 2019

Foghrye4 commented Jan 20, 2019

Barteks2x Jan 20, 2019

Foghrye4 Jan 20, 2019 •

edited

Barteks2x Jan 22, 2019

Barteks2x Jan 22, 2019

Barteks2x Jan 22, 2019

Barteks2x Jan 22, 2019

Barteks2x Jan 22, 2019

Barteks2x Jan 22, 2019

Barteks2x Jan 22, 2019

Barteks2x Jan 22, 2019

Barteks2x Jan 22, 2019

Barteks2x commented Jan 26, 2019

External generator settings (new implementation) #10

External generator settings (new implementation) #10

Conversation

Foghrye4 commented Dec 22, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Barteks2x Jan 20, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Barteks2x commented Jan 19, 2019

Foghrye4 commented Jan 20, 2019

Choose a reason for hiding this comment

Foghrye4 Jan 20, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Barteks2x commented Jan 26, 2019

Foghrye4 commented Dec 22, 2018 •

edited

Barteks2x Jan 20, 2019 •

edited

Foghrye4 Jan 20, 2019 •

edited