Skip to content

Prevent zombie reinforcements from loading chunks #11175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Prevent zombie reinforcements from loading chunks #11175

wants to merge 2 commits into from

Conversation

NewwindServer
Copy link
Contributor

@NewwindServer NewwindServer commented Jul 27, 2024

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

@NewwindServer NewwindServer requested a review from a team as a code owner July 27, 2024 10:17
@lynxplay
Copy link
Contributor

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
Copy link
Contributor Author

NewwindServer commented Jul 27, 2024

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 NewwindServer marked this pull request as draft July 28, 2024 21:04
@NewwindServer NewwindServer changed the title Prevent spawn placement loading chunks Prevent zombie reinforcements from loading chunks Jul 29, 2024
@NewwindServer NewwindServer marked this pull request as ready for review July 29, 2024 14:34
@NewwindServer
Copy link
Contributor Author

NewwindServer commented Jul 29, 2024

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
Copy link
Member

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.

@NewwindServer NewwindServer closed this by deleting the head repository Sep 20, 2024
@Warriorrrr Warriorrrr moved this from Awaiting review to Closed in Paper PR Queue Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

Zombie reinforcement checking crashing server by triggering sync chunk loads
3 participants