AbyssalCraft

AbyssalCraft

Mods
22M Downloads

Darklands Mountains stone replacer checks biomes at incorrect positions

Closed this issue · 1 comments

commented

Description:

The event hook to replace stone with darkstone in Darklands Mountain biomes uses the wrong BlockPos to retrieve the biome. On this line, when calling Chunk#getBiome() you pass in a BlockPos in the localized chunk space (where x/z are 0-16). This is technically incorrect usage of the method; if this method is ever called where the biome array is not yet initialized (k == 255), it will query the provider using the passed pos with BiomeProvider#getBiome, which expects a BlockPos in world space. As such, a position in chunk space will always end up retrieving the biomes from the chunk at ChunkPos(0, 0).

To fix this, the position passed into Chunk#getBiome() should convert the coordinates from chunk space to world space, like this:

BlockPos biomePos = new BlockPos((event.getChunkX() << 4) + x, y, (event.getChunkZ() << 4) + z);
Biome biome = chunk.getBiome(biomePos, event.getWorld().getBiomeProvider());

(MutableBlockPos would be nice too)
The existing code only ends up working out because usually in vanilla, the biome array is guaranteed to have been initialized before population. Using RoughlyEnoughIDs (REID) and Chunk Pregenerator with Abyssalcraft happens to fulfilll the following conditions necessary to exhibit the flawed logic:

  1. REID extends biome ids, and (at the moment) does not support any chunk providers besides ChunkProviderServer.
  2. Chunk Pregenerator implements its own chunk generation different from ChunkProviderServer, so REID does not initialize its extended biome array between chunk generation and population.
  3. Abyssalcraft, on calling Chunk#getBiome(), requests the BiomeProvider for the biome which REID populates the chunk's extended biome array with. However, due to the mentioned bug, the biome retrieved ends up being from the chunk at (0, 0).
    You can see the bug manifest very obviously visually when using Chunk Pregenerator's preview system:
    Image

Notice each chunk in the +x/+z quadrant has strikingly similar biomes to the top left chunk (which is chunk (0, 0)). I cannot explain why it's only in the +x/+z quadrant, but I think this is enough to demonstrate the issue.
This bug effectively makes it unfeasible to pregenerate chunks using Chunk Pregenerator with Abyssalcraft and REID.

Crash report:

N/A

Affected versions ("latest" is NOT a version):

  • Minecraft: 1.12.2
  • Forge: 14.23.5.2860
  • AbyssalCraft: 1.11.0 and 2.0.0-BETA-2 (and presumably many versions back)
  • RoughlyEnoughIDs: 2.2.2
  • Chunk Pregenerator 4.4.9
  • Carbon Config Library 1.2.4 (Chunk Pregenerator dependency)

Latest log file for when the issue was present:

Nothing relevant is logged, but I can provide a log if necessary.

commented

That's a nice catch! Now it does make me wonder if this even was correct before BlockPos was a thing, and the biome (along with subscribing to that event) was added back in 1.7.2. TBH, I haven't even touched that code since I (in the past) changed it to use a BlockPos (which I'm also doing wrong as well, as you've already specified besides just looking at the code).

I think this might've been brought up in a slightly different context in the past, without finding the actual culprit for it, but now the culprit has been found.

And finally, yes, that should be done with a MutableBlockPos.

  • 👍1