Skip to content
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

Fix Monster natural spawning problem due to light #11359

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DavidTs93
Copy link

This fixes the natural spawning problem of monsters due to light, which causes any custom natural spawning light levels (from a datapack altering the dimension type or by changing paper-world-defaults#monster-spawn-max-light-level) to be completely ignored.

The natural spawning code (in NaturalSpawner) checks the mob's custom spawn rules (Mob#checkSpawnRules), which, for sentient creatures always checks if PathfinderMob#getWalkTargetValue is 0 or positive. This logic, specifically for monsters, is flawed (except for: Giant, Guardian, Pillager, Silverfish, and Warden), because the value of this function for monsters depends on the light level - in the overworld this means that light levels 7 or below are ok and above 7 are not (which is the default behavior).

This PR changes checkSpawnRules (Monster#checkSpawnRules) to ignore natural spawning (including chunk population) because all monsters already have a custom check for spawning rules in SpawnPlacements. Also fixes #10265.

@DavidTs93 DavidTs93 requested a review from a team as a code owner September 5, 2024 09:47
@lynxplay
Copy link
Contributor

lynxplay commented Sep 6, 2024

While this PR fixes the issue, I think it is best for us to wait with merging this until we have some mojang input on the linked ticket.
Messing with spawn logic like this could break contraptions and until we know why mojang is even checking this in the first place when they are already checking brighness values in SpawnPlacements, this may cause more harm to normal players than good.

@DavidTs93
Copy link
Author

DavidTs93 commented Sep 6, 2024

While this PR fixes the issue, I think it is best for us to wait with merging this until we have some mojang input on the linked ticket.
Messing with spawn logic like this could break contraptions and until we know why mojang is even checking this in the first place when they are already checking brighness values in SpawnPlacements, this may cause more harm to normal players than good.

I opened that ticket today, so doubt it'll be soon...
As I see it, they did the check to add some extra checks for each entity type, which I believe is just a leftover (why not just add that check to the default checker in SpawnPlacements?...).
Anyway, my solution is just one of the many possible solutions - maybe Mojang will say it's an intended behavior (even though that makes no sense - why add custom light levels for monster spawning if you then ignore it?...), or will fix it in another way.

I, for one, will just compile this patch into my own server.

@lynxplay
Copy link
Contributor

lynxplay commented Sep 7, 2024

Yea, I agree that it seems to be a leftover.
But on the other hand, we really have 0 idea why that entire method even exists and diff around there was moved back in 22w12a, so not that long ago.

We'll try to get some info on the ticket, your patch certainly works yea, and for most users this change is probably 100% fine, but messing with spawning like this without a way to disable the early out on the Mob instance spawn check might break contraptions we would not want to break.

Thank you anyway for the PR, we'll poke around with the linked mojira 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Awaiting review
Development

Successfully merging this pull request may close these issues.

Max Light Level monster spawn
2 participants