`MultiChunkFeature` unsafely casts `Random` to `WorldGenRandom`
alcatrazEscapee opened this issue ยท 1 comments
Describe the bug
MultiChunkFeature
does an unsafe cast here:
To WorldGenRandom
, when the type of FeaturePlaceContext.random()
is only specified as RandomSource
. This is unsafe, and mods which do not use WorldGenRandom
will cause this to crash.
To Reproduce
- Install TerraFirmaCraft 1.18.2-2.2.27 and Occultism 1.18.2-1.81.4
- Try and create a ne world
- Crash
Example crash log (from TerraFirmaCraft/TerraFirmaCraft#2486): crash-report.txt
System:
- Occultism Version: 1.81.4
- OS: Mac OS X (x86_64) version 11.7.7
- Minecraft Version: 1.18.2, although the problematic code is present in 1.20.1 (latest) as well
- Modpack Link and Version, or list of mods: see crash report, although only the aforementioned two mods are necessary to reproduce.
Additional context
TerraFirmaCraft does not use WorldGenRandom
example as it is slowly being replaced throughout Mojang's code, and is the legacy version of the new random implementation (XoroshiroRandomSource
). It also has some terrible seeding in setFeatureSeed
which has caused us (and vanilla) issues with it's low entropy in feature seeding before.
In this instance, there's no need to use WorldGenRandom
in this code. You could accomplish the same thing by simply creating a new Random()
or new XoroshiroRandomSource()
, and calling .setSeed()
with a hash function of your own making (or even copying the hash function in WorldGenRandom.setLargeFeatureSeedWithSalt
, because that one at least, is passible.
There is also another problem in the current implementation: you reset the seed of the random
which is provided to every feature in sequence. This means that any feature that runs after your feature gets affected by the reset seed instead of being purely chunk based. So instead, you should really be creating a new random anyway, if you intend to call .setSeed()
on it directly.