Iron's Spells 'n Spellbooks

Iron's Spells 'n Spellbooks

Mods
18M Downloads

PriestEntity performs unsafe POI lookup in #finalizeSpawn

jpenilla opened this issue ยท 2 comments

commented

POI lookups from threads other than the main server thread are unsafe, however vanilla does not perform integrity checks around this. I am working on a mod that optimizes POI lookups and also adds integrity checks for unsafe access. This means the unsafe lookup manifests in a crash during world generation in combination with my mod.

irons-spells-n-spellbooks/src/main/java/io/redspace/ironsspellbooks/entity/mobs/wizards/priest/PriestEntity.java

Lines 126 to 136 in 50631c4

public SpawnGroupData finalizeSpawn(ServerLevelAccessor pLevel, DifficultyInstance pDifficulty, MobSpawnType pReason, @Nullable SpawnGroupData pSpawnData, @Nullable CompoundTag pDataTag) {
RandomSource randomsource = Utils.random;
this.populateDefaultEquipmentSlots(randomsource, pDifficulty);
if (this.level instanceof ServerLevel serverLevel) {
Optional<BlockPos> optional1 = serverLevel.getPoiManager().find((poiTypeHolder) -> poiTypeHolder.is(PoiTypes.MEETING),
(blockPos) -> true, this.blockPosition(), 100, PoiManager.Occupancy.ANY);
optional1.ifPresent((blockPos -> {
this.setHome(blockPos);
//IronsSpellbooks.LOGGER.debug("Priest new home: {}", this.getHome());
}));
}

During post-processing structure placement (which runs on a world gen thread), this.level can be a ServerLevel while pLevel is a WorldGenLevel.

This can be fixed pretty easily (while maintaining behavior) with a two-pronged approach:

  • Firstly the instanceof ServerLevel check should be performed on pLevel, not this.level.
  • Secondly, add an else branch to the if statement that sets a flag on the entity to search for the POIs during the next AI tick.
    • This is similar to what vanilla does with Villagers: 
          public SpawnGroupData finalizeSpawn(ServerLevelAccessor arg, DifficultyInstance arg2, MobSpawnType arg3, @Nullable SpawnGroupData arg4) {
        // ...
        if (arg3 == MobSpawnType.STRUCTURE) {
            this.assignProfessionWhenSpawned = true;
        }

I haven't checked if any other entities in the mod have this problem, I just noticed it for the Priest as while exploring I ended up locking myself out of my singleplayer world by exploring a location within range of their structure. (MC 1.21)

commented

Our repo is now public: https://github.com/Tuinity/Moonrise
Hopefully this makes testing easier :)

commented

fixed in 3.4.5