Cancel blocked natural spawns before the entity is created - #2300
Conversation
The mob-spawning and deny-spawn checks in WorldGuardEntityListener fire on CreatureSpawnEvent, at the very end of the spawn pipeline. By that point the server has picked a spawn position, run the placement checks, constructed the mob and run finalizeSpawn, and the cancelled mob is thrown away. Since a cancelled spawn never counts toward the mob cap, the natural spawner keeps retrying the same area at full rate, so regions that deny mob spawning become permanent spawn attempt hotspots that pay entity construction over and over for nothing. On Paper servers the same checks can run in PreCreatureSpawnEvent, before the entity exists. Cancelling there also ends the remaining attempts for the chunk in that spawn cycle, so the wasted work is gone almost entirely. Measured on a flat test world with a region denying mob-spawning over the whole spawn range and the mob cap kept empty, the attempt rate around a single player collapsed from roughly 75000 attempts per second to roughly 60 per second, with no entities constructed at all. The new listener mirrors the natural spawn conditions from onCreatureSpawn exactly: activity halt, block-creature-spawn, mob-spawning and deny-spawn region flags, and block-ground-slimes. All other spawn reasons keep going through the existing CreatureSpawnEvent checks unchanged, and the listener is only registered when the Paper event class is present.
Review feedback: platform specific code lives in the Paper inner classes of the main listeners, and WorldGuardEntityListener already has one for EntityZapEvent, so the handler moves in there and the separate listener class is gone.
| return; | ||
| } | ||
|
|
||
| Location eventLoc = event.getSpawnLocation(); |
There was a problem hiding this comment.
So from what I can tell here, this line onwards is basically identical to the current CreatureSpawnEvent listener. Would it be possible to please extract this into its own method, similar to how the pig zap event handler does it? As these don't have a shared ancestor, you could just pass the event in as a Cancellable to allow cancelling it, and the necessary data in as parameters.
There was a problem hiding this comment.
done, extracted it into handleCreatureSpawn. the entity-specific checks stayed in onCreatureSpawn since there's no entity yet at pre-spawn time. re-tested on a live server, behaviour is identical.
The pre spawn handler duplicated the tail of onCreatureSpawn. Both now call handleCreatureSpawn, which takes the event as a Cancellable and the location, entity type and spawn reason as parameters, following the same shape as handlePigZap. The entity specific checks (plugin spawning, armor stands, tamed animals) stay in onCreatureSpawn since no entity exists yet at pre spawn time, and the activity halt check stays in each handler so its ordering relative to those checks is unchanged.
| } | ||
|
|
||
| private static void handleCreatureSpawn(Cancellable event, Location location, EntityType entityType, SpawnReason spawnReason) { | ||
| ConfigurationManager cfg = getConfig(); |
There was a problem hiding this comment.
Thanks for making these other fixes. Just a final one IMO, if you'd be able to please make cfg & wcfg passed in here too. Both are already available in the CreatureSpawnEvent handler, and the Pre one already pulls in cfg, so it'd remove the need for a second lookup (performance-wise)
There was a problem hiding this comment.
done, both passed in now
Both handlers already have them, so the shared method no longer does a second lookup.
The comment claimed that cancelling ends the chunk's remaining spawn attempts. It does not. Only setShouldAbortSpawn(true) makes the spawner return early; a plain cancel falls through to the next candidate position exactly like a failed placement check. What the handler does buy is stated accurately instead: the placement checks, the entity construction and finalizeSpawn are skipped for a spawn that was going to be refused. Two consequences a server operator should know are now named. The region query runs for every candidate position rather than every constructed mob, because Paper fires the event before the vanilla suitability checks. And on Paper with per-player-mob-spawns enabled, every cancelled pre spawn is charged to a per player mob backoff counter, so a large denied region can suppress spawning in neighbouring chunks that allow mobs.
|
Sorry is there a reason you've marked this as draft? |
|
Sorry, that wasn't meant to be cryptic. I drafted it because I realised the second paragraph of my javadoc was wrong. A plain cancel doesn't end the chunk's remaining attempts, only Two things I ran into while testing that you should probably know before this merges. Paper fires And with Happy to put the whole thing behind a config gate if you'd rather it be opt-in. |
|
Numbers. Purpur 26.2, stock spawn settings, one player, nether, three 120x120 spawn platforms with the player 35 blocks above. Mobs within 70 blocks of the farm, sampled every 20s. Region covering the platforms:
Then the one I cared about, whether the per-player backoff leaks outside the region. Deny region moved 80 blocks east so the farm sits outside it:
Last two runs match, so denying next door cost the farm nothing. First run is low because the world was cold from the previous test. Couldn't get the backoff to bite at this scale. Charge rate settles around 20/s and it bleeds off at about the same, and cancels only happen during spawn attempts, which the cap already gates. |
|
Old vs new under the same conditions, since that's the more useful comparison. Deny region over the farm, same platforms and sampling:
Same outcome, so the flag behaves identically. The difference is only where the refusal happens. |
|
Two alternatives I looked at and didn't use, in case you were going to ask.
Both come back to the same thing. The events are chunk or player shaped and regions aren't, and there's no API to tell the spawner a specific area is off limits. |
|
Thanks for clarifying, it's all good :) I feel this current event is the most appropriate, however if it is a potential trade-off in performance (worse on some types of servers) it might be worth adding an option that defaults to true, similar for the hopper item events etc. Although I feel at this stage it's likely fine to merge it in and see how it behaves. If we start getting reports of slowdowns here then it might be worth revisiting that (or if this does end up changing behaviour). Generally a large number of region queries scales fairly well, whereas I'm very aware of the kind of load that repeated mob spawn attempts can cause on a server. |
regions.use-pre-creature-spawn-event, default true, matching how regions.use-creature-spawn-event already gates the CreatureSpawnEvent path. Turning it off leaves the existing handler doing all the work, so a server that sees a regression can revert without downgrading.
|
Added the option anyway, it's small enough that it seemed silly not to: My own testing lines up with your read, for what it's worth. I couldn't measure any drop in spawn rates from this, inside or outside denied regions, in any setup I tried. |
|
Ah I did not realise we already had an option for the creature spawn event, that makes a lot of sense, thank you :) – I'll take another quick look then merge if it looks good. |
2 participants
The mob-spawning and deny-spawn checks currently run in CreatureSpawnEvent, which fires at the very end of the spawn pipeline. By that point the server has picked a spawn position, run the placement checks, constructed the mob and run finalizeSpawn, and the cancelled mob is thrown away. Because a cancelled spawn never counts toward the mob cap, the natural spawner keeps retrying the same area at full rate for as long as those conditions hold, so regions that deny mob spawning become permanent spawn attempt hotspots that pay entity construction over and over for nothing. On a production server (61 players) we profiled recently, natural spawn machinery accounted for roughly a quarter of the main thread, and areas denying spawns contribute to that without anything visible happening.
This PR adds a listener for Paper's PreCreatureSpawnEvent that applies the same natural spawn checks before the entity exists: activity halt, block-creature-spawn, the mob-spawning and deny-spawn region flags, and block-ground-slimes. Cancelling at the pre event stage also makes the server end the remaining spawn attempts for that chunk in the current cycle, so the retry pressure disappears as well.
Measured on a flat test world with a region denying mob-spawning across the whole spawn range and the mob cap kept empty: the attempt rate around a single player collapsed from roughly 75000 attempts per second reaching the pre spawn stage to roughly 60 per second, with no entities constructed at all.
Only NATURAL spawns are handled here. Every other spawn reason keeps going through the existing CreatureSpawnEvent checks unchanged, so there is no behavior change for spawners, breeding, plugins spawning entities and so on. The listener is registered only when the Paper event class is present, using the same PaperLib gate style as elsewhere in the plugin.
One thing worth noting for review: plugins that watch CreatureSpawnEvent will no longer see the natural spawns WorldGuard denies, since those never reach entity creation anymore. Anything that wants to observe or override them can use PreCreatureSpawnEvent.
Edit, measurement details after a follow-up test:
The attempt counter counts PreCreatureSpawnEvent dispatches, so positions that already passed the placement checks and are one step from entity construction. It was a LOWEST priority listener on an otherwise idle flat world with a single stationary player, difficulty hard, permanent night.
So the current late cancel does not just waste the construction work, it sustains the maximum spawn attempt pressure under the tested conditions, while the pre event cancel lets the spawn cycle stop early and the whole workload collapses.