mojira.dev

Cubitect

Assigned

No issues.

Reported

MC-241546 The biome generation can have inconsistent results for the same seed Plausible MC-131536 Some ocean-based buffet worlds don't generate ocean monuments Cannot Reproduce MC-124577 Missing Latin translation for some items. Invalid MC-94025 A world with seed 0 cannot be copied using the "Re-Create" option Fixed MC-87565 Conversion to integers in packets causes loss of information in entities and results in desychronisation of the server and client Fixed

Comments

Added a warning that this has a major affect on stronghold positions and should preferably be fixed before the main release.

This is probably caused by the change that the noise sources now get initialized with an MD5 hash of the noise generator name, which are different for large biomes (e.g. "minecraft:temperature" and "minecraft:temperature_large"). In previous versions the layered biome generation was (almost) just an up-scaling of the normal generation.

No, this just causes the game thinks that a location has a different biome, depending on which part of the code is looking. Also it affects mostly just the location of biome borders, not the generation as a whole.

Regarding the large biomes in 1.18-pre1, the random number generators now get initialized with an MD5 hash of the generator name. This noise generator name is "minecraft:temperature" for normal generation and "minecraft:temperature_large" with large biomes and similar for the other noise parameters. This means the noise generation is completely different for large biomes.

I agree. The seed text should be left empty for a random seed. If I specify that I want my seed to be '0' then it should be zero. I found it a little irritating that I couldn't even copy the world once I actually had one with seed zero.

This bug is caused from dealing with the Hell and Sky biomes in a bad manner. The intended behavior was apparently to remove those biomes from the list in the biome selector. However, this offset in biomeIDs is dealt with in the wrong bit of code: in the preset string interpretation, which is often executed multiple times (e.g. when the "Customize World Settings" are opened). Instead, the offset calculation should be done immediately where the value of the slider is evaluated.
There are two easy fixes that could be done here. The first option is the simplest and my favorite, which is simply to keep the Hell and Sky in the biome selector. They don't really harm anyone. The second option would be to do the offset management right at the slider read out, just as I mentioned.

I will also try to explain this bug and fixes in terms of code. I got the original code through MCP, but I will only show the key bits here, with a bunch of variables renamed, to help convey the information (also so I'm not posting actual source code).

Current code:

ChunkProviderSettings.java

// Line ~526
fixedBiomeID = JsonUtils.getJsonObjectIntegerFieldValueOrDefault(preset, "fixedBiome", fixedBiomeID);

if (fixedBiomeID < 38 && fixedBiomeID >= -1)
{
    if (fixedBiomeID >= BiomeGenBase.hell.biomeID) fixedBiomeID += 2;
}
else
{
    fixedBiomeID = -1;
}

As I mentioned, this bit of code is in totally the wrong place! Nothing in this class should deal with any sort of offset, as it may be called multiple times.

GuiCustomizeWorldScreen.java

// Line ~220
String getSliderLabel(int sliderID, float sliderValue)
{
    switch(sliderID)
    // ...
    case BIOME_SELECTOR: // = 162
        if (sliderValue < 0.0F)
        {
            // return "All"
        }
        else
        {
            if ((int)sliderValue >= BiomeGenBase.hell.biomeID)
            {
                // return name for biomeID = (int)sliderValue + 2
            }
            else
            {
                // return name for biomeID = (int)sliderValue
            }
        }
    // ...
}

// Line ~370
void evaluateSlider(int sliderID, float sliderValue)
{
    switch(sliderID)
    // ...
    case BIOME_SELECTOR:
        chunkProviderSettings.fixedBiomeID = (int)sliderValue;
        break;
    // ...
}

An implementation of the the first fix would be:

ChunkProviderSettings.java

// Line ~526: Remove  all the unnecessary code
fixedBiomeID = JsonUtils.getJsonObjectIntegerFieldValueOrDefault(preset, "fixedBiome", fixedBiomeID);
// ...

GuiCustomizeWorldScreen.java

// Line ~220
String getSliderLabel(int sliderID, float sliderValue)
{
    switch(sliderID)
    case BIOME_SELECTOR:
        if ((int)sliderValue < 0)
        {
            // return "All"
        }
        else
        {
            // return name for biomeID = (int)sliderValue
        }
    // ...
}

// Line ~370: Unchanged
void evaluateSlider(int sliderID, float sliderValue)
{
    switch(sliderID)
    // ...
    case BIOME_SELECTOR:
        chunkProviderSettings.fixedBiomeID = (int)sliderValue;
        break;
    // ...
}

An implementation of the the second fix would be:

ChunkProviderSettings.java

// Line ~526: Remove  all the unnessesary code
fixedBiomeID = JsonUtils.getJsonObjectIntegerFieldValueOrDefault(preset, "fixedBiome", fixedBiomeID);
// ...

GuiCustomizeWorldScreen.java

// Line ~220: Unchanged, although one could play around with (int) casts or ceiling/floor values.
String getSliderLabel(int sliderID, float sliderValue)
{
    switch(sliderID)
    case BIOME_SELECTOR:
        if (sliderValue < 0.0F)
        {
            // return "All"
        }
        else
        {
            if ((int)sliderValue >= BiomeGenBase.hell.biomeID)
            {
                // return name for biomeID = (int)sliderValue + 2
            }
            else
            {
                // return name for biomeID = (int)sliderValue
            }
        }
    // ...
}

// Line ~370: Do the offset management here
void evaluateSlider(int sliderID, float sliderValue)
{
    switch(sliderID)
    // ...
    case BIOME_SELECTOR:
        biomeID = (int)sliderValue;
        if (biomeID >= BiomeGenBase.hell.biomeID) 
        {
            biomeID -= 2;
        }
        chunkProviderSettings.fixedBiomeID = biomeID;
        break;
    // ...
}

Also, in order to get the right position of the slider upon reload, one has to change the default value of the slider, to revert the offset.
After "createWorld.customize.custom.fixedBiome" in GuiCustomizeWorldScreen.java, change

(float)(this.field_175336_F.field_177869_G)

to

(float)((this.field_175336_F.field_177869_G>=BiomeGenBase.hell.biomeID) ? this.field_175336_F.field_177869_G-2 : this.field_175336_F.field_177869_G

After this overkill of a comment I should mention that I can confirm this bug (not the crash, but the offsets) in versions 1.8, 1.8.8 and 15w49b 😛
P.S.
I tested the all the fixes in MCP for 1.8.

Using the internal representation would be preferable to making it harder to reproduce.
Bundling the packets to the chunks seems like a good option to me. The grid-snapping might cause problems with the motion (assuming ints are used for the motion as well). This would become especially noticeable at slow speed and small angles.
Also I just did a few tests and it seems that about three quarters of the S15PacketEntityRelMove packets store a Y-change of zero. This is probably due to all the mobs that are moving while staying on the ground. It would be unnecessary to store the full 8 bytes of a double for this very common case. So separating ground motion and other motion could be an option to consider.

I can confirm this for both java 7 and 8 under Ubuntu 15.04.

This line of code causes the server to sleep for a minimum of 1 ms.

I just did some more testing and code digging and as far as I can tell there are two types of packets that are called rather often:

  • S14PacketEntity, which exists in 3 variants:

    • S15PacketEntityRelMove

    • S16PacketEntityLook

    • S17PacketEntityLookMove

  • S18PacketEntityTeleport

The S14PacketEntity packets do indeed contain size optimised data. Single bytes represent an offset to the current position in 1/32 blocks (which allows for changes of about +-4 blocks).

Assuming a boolean is one byte, then a S14PacketEntity packet would (probably) have a size of 11 bytes.
The S18PacketEntityTeleport seems to be called rather often as well, in particular it seems to be called when an entity moves fairly quickly, e.g. whenever it jumps. This packet contains the absolute positions in 1/32 blocks in the form of ints. Again assuming a boolean is one byte, then a S18PacketEntityTeleport packet would (probably) have a size of 19 bytes.

The ints in the S18PacketEntityTeleport class could easily be replaced with floats, which for the most part would already fix the issues I described, without increasing the packet sizes at all. Furthermore one could use a form of half-precision floats (2 bytes) or other minifloats instead of the one-byte integers, which would increase the resolution considerably, without increasing the packet sizes too much.

All the other packets concerned (S0FPacketSpawnMob, S0EPacketSpawnObject etc.) are called less often and use ints, just like S18PacketEntityTeleport. So they should probably also use doubles or at least floats instead.