mrkite / minutor

Mapping for Minecraft
http://seancode.com/minutor
BSD 2-Clause "Simplified" License
278 stars 52 forks source link

return -1 for invalid biome offset in any case #328

Closed Kolcha closed 2 years ago

Kolcha commented 2 years ago

in case of invalid biome offset previously -1 was returned only for Debug builds, causing undefined behavior for Release builds.

this is relevant only for Minecraft < 1.18, changed this to the same as for 1.18+ code - return -1 in invalid case, and print warning only for Debug builds.

EtlamGit commented 2 years ago

I think we have to extend this fix: When this function does return a -1 this value will get into one of these methods:

const BiomeInfo &BiomeIdentifier::getBiomeByChunk(qint32 id) const
const BiomeInfo &BiomeIdentifier::getBiomeBySection(qint32 id) const 

And these are not expecting negative numbers jet (at least the second one).

Kolcha commented 2 years ago
const BiomeInfo &BiomeIdentifier::getBiomeByChunk(qint32 id) const

is safe - it will return unknownBiome object, which can be used (and is used these days)

const BiomeInfo &BiomeIdentifier::getBiomeBySection(qint32 id) const 

is updated to be prepared to invalid indexes (more correctly slightly extended to handle negative values)

Kolcha commented 2 years ago

you can test these changes using this world. it has "holes" (created in Creative mode) at (0, 0) in Overworld and Nether dimensions, hovering over them triggers "Section not found for Biome lookup!" message and as result -1 is passed to getBiomeBySection(). world is created by Minecraft 1.19.1

this is maybe unusual case, but initial issue was also detected using some "very strange" worlds. it worth fixing as part of error handling and general reliability improvement.