Prevent zombie reinforcements from loading chunks

[NewwindServer]

When a zombie calls reinforcements it tries to spawn them in a random location within a 40 block radius of the zombie, before spawning, it checks isSpawnPositionOk() for the position which loads the block to check if a mob can spawn on said block. This patch ensures the chunk at the random location is loaded before trying to spawn the reinforcement zombie in it.

Fixes #11174

[lynxplay]

How certain are you that such a change does not break existing game mechanics?

Unless I am missing something, the current logic, while being costly due to the sync chunk load, would still load the respective chunk and perform the spawn. The chunk spawned in may be unloaded and the entity may be discarded then, but that isn't really a given.

Beyond that, a lot of other spawn placement types perform such checks. Should a patch like this (if we determine this change to not affect gameplay) also fix other spawn placement types? Or does the fact that really only zombie reinforcements being the issue maybe mean we could just move this check into the zombie logic instead.

[NewwindServer]

How certain are you that such a change does not break existing game mechanics?

Unless I am missing something, the current logic, while being costly due to the sync chunk load, would still load the respective chunk and perform the spawn. The chunk spawned in may be unloaded and the entity may be discarded then, but that isn't really a given.

Beyond that, a lot of other spawn placement types perform such checks. Should a patch like this (if we determine this change to not affect gameplay) also fix other spawn placement types? Or does the fact that really only zombie reinforcements being the issue maybe mean we could just move this check into the zombie logic instead.

You could go either way, just fixing the zombie reinforcements would be simpler but there is a similar scenario that is possible in the WanderingTraderSpawner:

 @Nullable
    private BlockPos findSpawnPositionNear(LevelReader world, BlockPos pos, int range) { // range = 48
        BlockPos blockposition1 = null;
        SpawnPlacementType spawnplacementtype = SpawnPlacements.getPlacementType(EntityType.WANDERING_TRADER);

        for (int j = 0; j < 10; ++j) {
            int k = pos.getX() + this.random.nextInt(range * 2) - range;
            int l = pos.getZ() + this.random.nextInt(range * 2) - range;
            int i1 = world.getHeight(Heightmap.Types.WORLD_SURFACE, k, l);
            BlockPos blockposition2 = new BlockPos(k, i1, l);

            if (spawnplacementtype.isSpawnPositionOk(world, blockposition2, EntityType.WANDERING_TRADER)) {
                blockposition1 = blockposition2;
                break;
            }
        }

        return blockposition1;
    }

I am leaning towards preventing all spawn placement types from loading chunks, here is where spawn placements are used in the game:

Spawning wandering traders randomly in radius of the player Could it cause a sync chunk load?: yes, the spawning radius is 48 blocks around the player Consequences of it skipping an unloaded chunk: it will simply pick another chunk near the player to spawn the trader

Spawning reinforcements for zombies Could it cause a sync chunk load?: yes, the spawning radius is 40 blocks around the zombie Consequences of it skipping an unloaded chunk: it will simply pick another chunk near the zombie to spawn the reinforcements

Spawning cats around the player Could it cause a sync chunk load?: no, minecraft already ensures the chunks are loaded before checking isSpawnPositionOk Consequences of it skipping an unloaded chunk: N/A

Spawning waves during a raid Could it cause a sync chunk load?: no, minecraft already ensures the chunks are loaded before checking isSpawnPositionOk Consequences of it skipping an unloaded chunk: N/A

Placing mobs in chunks during generation This needs investigated, getBlockStateIfLoaded() calls getChunkIfLoadedImmediately() which requests a chunk with at least status FULL, but the spawnMobsForChunkGeneration() method is called during the ChunkStatus.SPAWN step of worldgen, which is the step before the FULL chunkstatus

However, even if we are at a lower chunk status, calling getChunk in WorldGenRegion, it will still try get access to the chunk using getChunkIfPresentUnchecked(), so this may not be an issue.

[NewwindServer]

How certain are you that such a change does not break existing game mechanics?

Unless I am missing something, the current logic, while being costly due to the sync chunk load, would still load the respective chunk and perform the spawn. The chunk spawned in may be unloaded and the entity may be discarded then, but that isn't really a given.

Beyond that, a lot of other spawn placement types perform such checks. Should a patch like this (if we determine this change to not affect gameplay) also fix other spawn placement types? Or does the fact that really only zombie reinforcements being the issue maybe mean we could just move this check into the zombie logic instead.

I have decided just to do the simpler thing of preventing the reinforcements from loading chunks, I will make a separate PR in future to address the wandering trader spawning also being able to potentially sync load chunks.

[Machine-Maker]

I think, perhaps, this warrants a configuration option. Defaulting to the behavior being implemented here (don't allow chunk loading), but there's probably some functionality somewhere that someone expects this to load chunks.