Crashes with Terralith

[gniftygnome]

Installing Simply Improved Terrain and Terralith 1.3.2 on Minecraft 1.17.1 causes crashes. I believe that in MixinDiskFeature.java on line 39, diskFeatureConfig.radius.get(random) returns a negative number, which is later divided by 2 and fed to random.nextInt(radiusRange) on line 44. However, I am not sufficiently knowledgeable to say if this is properly a bug in Simply Improved Terrain or in Terralith. I'm filing here because Discord is apparently the only way to "file" a bug on Terralith. :-/

To reproduce the crash reliably (I think) you just need to:

• Install the mods listed below and start the game (sometimes it crashes during initial chunk generation)
• /locatebiome terralith:yellowstone
• /tp [coordinates]

Mods installed:
fabric-api-0.37.2+1.17.jar
notenoughcrashes-3.4.0+1.17-fabric.jar
SimplyImprovedTerrain-0.4.0.3-Fabric-1.17.jar
Terralith+v1.3.2.jar

Top of stack:
java.lang.IllegalArgumentException: bound must be positive
at Not Enough Crashes deobfuscated stack trace.(1.17.1+build.39)
at java.util.Random.nextInt(Random.java:388)
at net.minecraft.world.gen.feature.DiskFeature.handler$zgf000$injectGenerate(DiskFeature:544)
at net.minecraft.world.gen.feature.DiskFeature.generate(DiskFeature)
at net.minecraft.world.gen.feature.UnderwaterDiskFeature.generate(UnderwaterDiskFeature:19)
at net.minecraft.world.gen.feature.ConfiguredFeature.generate(ConfiguredFeature:58)
at net.minecraft.world.gen.feature.DecoratedFeature.method_30384(DecoratedFeature:29)

I am attaching the full crash dump.
crash-2021-08-17_19.05.33-server.txt

[galacticwarrior9]

As a disclaimer, I am not familiar with modding or world generation. But after some investigation using Blame, it seems the issue lay with the fact that Terralith has a number of configured features with a radius of 1 (such as "acid_yellow" that generates in Yellowstone biomes, which OP mentioned). Thus the radiusRange here could be a decimal number and not an integer - as the comment states, radiusRange can be "roughly 0.75x [...] the defined radius". This leads to an IllegalArgumentException when attempting to calculate the radiusBase here as the nextInt function expects an integer bound. I was able to fix the crashes locally by forcing the minimum value of radiusRange to be 1.

[gniftygnome]

Those are int variables so it is not possible for radiusRange to be anything but an integer. If it is set to 0.75 then its value will be 0. I had not specifically considered the case of a configured radius of 1 but here is how that would play out:

        int configuredRadius = diskFeatureConfig.radius.get(random);  // 1
        int radiusMin = (configuredRadius * 3 + 2) >> 2;              // 1
        int radiusRange = configuredRadius >> 1;                      // 0

        int radiusBase = random.nextInt(radiusRange) + radiusMin;     // IllegalArgumentException (see below)

Here is the bottom of the documentation for Java's Random.nextInt(int):

Returns:
the next pseudorandom, uniformly distributed int value between zero (inclusive) and bound (exclusive) from this random number generator's sequence
Throws:
IllegalArgumentException - if bound is not positive

Zero is not considered a positive integer in mathematics so it makes sense this may return an IllegalArgumentException. Obviously, that is not what Minecraft's vanilla implementation does.

Negative numbers will also cause a crash so a more robust implementation would probably address both issues. F.e. by requiring configuredRadius to be at least 0 as well as guarding the call to Random.nextInt(int):

        // Choose a radius range roughly 0.75x to 1.25x the defined radius
        int configuredRadius = diskFeatureConfig.radius.get(random);
        if (configuredRadius < 0) configuredRadius = 0;
        int radiusMin = (configuredRadius * 3 + 2) >> 2;
        int radiusRange = configuredRadius >> 1;

        // Choose the radius
        int radiusBase = (radiusRange > 0) ? random.nextInt(radiusRange) + radiusMin : radiusMin;

However, I'm not sure about possible interactions or desired behavior in Simply Improved Terrain so I figured I would leave the nuts and bolts up to KdotJPG.

[KdotJPG]

Good find (and work finding where the problem is)! I've pushed a quick fix to the Forge-1.16 branch, which I'm currently working on some broader changes in. I am planning to release a new version in the not-too-distant future so it will include this.

[KdotJPG]

Porting Forge 1.16 to Fabric 1.16, then porting that forward to Fabric 1.17. Aiming for this week.

EDIT: Focusing on the also-much-requested configurability so I can include that in the port-forward. So, slight delay, but making progress!