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

Configurable Dry and Wet Farmland Tick Rates #9968

Merged
merged 2 commits into from
Dec 3, 2023

Conversation

MrPowerGamerBR
Copy link
Contributor

@MrPowerGamerBR MrPowerGamerBR commented Nov 25, 2023

If your server has a lot of farms, the isNearWater calls gets very expensive.

To workaround this, we can throttle the tick rate of farmlands, because after all, you don't really need to check if the farmland is near a source of water that often, right? It is not like the water will magically vanish if they are in a player made farm.

This helps a lot, on my server farm lands were taking up ~8% of CPU time, so throttling the tick rate of wet farmlands helped to bring it down while not having a negative gameplay impact.

There are two options: dry-farmland and wet-farmland, the reason it is separate is because the performance impact of dry farmlands isn't that big, but has a big gameplay impact where players may be confused about why the farmland isn't wet after placing water near it. Yes, dry farmlands have the same isNearWater checks as wet farmlands, but dry farmlands do end up turning to regular dirt after a while, so it is like they are "naturally" throttled by the server.

The code for the tick rate throttle was copied from the "Configurable Grass Spread Tick Rate" patch.

(The reason the name is "Farmland" and not "Farm Land" is because that's what the game calls the block)

@MrPowerGamerBR MrPowerGamerBR requested a review from a team as a code owner November 25, 2023 04:01
@MrPowerGamerBR MrPowerGamerBR changed the title Configurable Dry and Wet Farmland Nearby Water Tick Rates Configurable Dry and Wet Farmland Tick Rates Nov 25, 2023
Copy link
Member

@Machine-Maker Machine-Maker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's almost worth creating a new wrapper type for tick rates cause now we have like 5 of those big ugly long if statements that are identical (FarmBlock and SpreadingSnowyDirtBlock)

@MrPowerGamerBR
Copy link
Contributor Author

It's almost worth creating a new wrapper type for tick rates cause now we have like 5 of those big ugly long if statements that are identical (FarmBlock and SpreadingSnowyDirtBlock)

If it is easier, maybe creating a simple static function shouldSkip(int tickRate, int hashCode) could also help clean up those pesky if statements without requiring too much work.

@Machine-Maker
Copy link
Member

If it is easier, maybe creating a simple static function shouldSkip(int tickRate, int hashCode) could also help clean up those pesky if statements without requiring too much work.

It's probably fine for now. I will probably end up changing it to something custom wrapper type during one of my periodic config cleanup PRs.

Copy link
Member

@Machine-Maker Machine-Maker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

This pr adds the 2nd and 3rd blockstates which have their own tick rates (along with mob spawners). Depending on how many more are useful to add, might be nice to have some sort of way to set the random tick rate of any blockstate, but that can happen later if needed (and if its a good idea at all)

@Machine-Maker Machine-Maker merged commit ffa4115 into PaperMC:master Dec 3, 2023
1 check passed
lynxplay pushed a commit to lynxplay/paper that referenced this pull request Feb 23, 2024
* Configurable Dry and Wet Farm Land Nearby Water Tick Rates

* Rebase and squash

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

Successfully merging this pull request may close these issues.

3 participants