Skip to content

Properly grow plants with growth modifier#8564

Closed
lynxplay wants to merge 1 commit into
PaperMC:masterfrom
lynxplay:bugfix/growing-block-growth-modifier
Closed

Properly grow plants with growth modifier#8564
lynxplay wants to merge 1 commit into
PaperMC:masterfrom
lynxplay:bugfix/growing-block-growth-modifier

Conversation

@lynxplay
Copy link
Copy Markdown
Contributor

Spigots implementation of the growth modifier boils down to a plain chance for a plant to either grow immediately or to grow a single age value towards its fully grown age.

This behaviour is specifically troubling for plants like the cactus or the sugar cane, as they change an unrelated block when growing up (the free air block above their tip). This blocks validity is checked during neighbour updated which are triggered by resetting the previous tip of the plant back to [age=0].

With spigots modifier implementation, blocks at the age of zero are able to grow a new block above them however, causing the block change of the previous tip to change a block of age zero to a block of age zero, which does not cause a neighbour update as nothing actually changed. This means that the about block is not updated and hence may remain in an otherwise invalid state. (E.g. a cactus block next to a solid block).

This commit rewrites the existing spigot growth modifier implementation specifically for the plants that grow new blocks like the cactus or the sugar can block.

The modifier provided for the plants is separated into two distinct values. One being the remains after a floored division by 100, e.g. the original modifier without its 10^0 and 10^1 digits, the other the modifier modulo 100.
The modifier 475 would, for example, be split into a base growth of 4 and a percentage chance of an additional age progression of 75%. This way, on average, the plant would grow 4.75 times as fast as a normal plant, which would only grow a single age per random tick.

Resolves: #8544

@lynxplay
Copy link
Copy Markdown
Contributor Author

I am opening this as a draft pull request as I'd like more input here before I finalise the system (and hence also ship it to the sugar cane implementation).

As @Lulu13022002 pointed out in #8555, while this system works, it will always grow the plant by the base growth at least.
A plant with a modifier of 350 would hence always at least grow 3 age steps per random tick and in 50% of the random ticks it would grow 4 age steps per random tick.

This change however means that certain age values are never met in the current implementation. E.g. age 1 and 2 are always completely skipped. So is age 5.
We could change this implementation hence to potentially instead use a gaussian or normal distribution, however that also comes with some caveats:

  1. We need to decide on the limits for such a random distribution. E.g. a modifier of 300 should imply an average growth of 3 age levels per random tick. What are the limits of the Gaussian ? At best obviously the average itself, the plant cannot grown a negative age, but it is still a question to consider.
  2. This would most certainly require a specific exemption for the default modifier of 100, as it (at least in my opinion) should act as a form of NOOP, always leading to one (and only one) age grown per tick.

Feedback would be greatly appreciated :)

@Lulu13022002
Copy link
Copy Markdown
Contributor

Lulu13022002 commented Apr 6, 2023

Sorry for the late reply
I don't think we need to overthink this and imo this impl should be straightforward it's just spigot that has made a unwise decision in the first place. But i really like your solution and think that just a note on the paperdoc about what we change for theses config is enough and say that even if we fixed spigot it can still break niche plugin/command/datapack/resource pack for map maker purpose i guess when the percent overflow 100 percent since it skip some natural step.
There potentially some similar issue for the beetroot and crops where the percent change the moisture rate and so is not independent like the other. The beetroot/torchflower has even a bigger issue since its class override the parent method with a 1/3 chance.

@Lulu13022002
Copy link
Copy Markdown
Contributor

There's also another issue when the cactus reach its maximum age (15) it normally creates a new block only in the next random tick however with this pr the new block is created instantly without waiting a new randomtick. That means the cactus grow faster than normal even with the default growth modifier. Another issue is that the last stage (15) is always skipped since it is almost immediately reset to age=0 when a new block grow and so this stage will be unseen in the server.

@lynxplay lynxplay force-pushed the bugfix/growing-block-growth-modifier branch from a0c182a to af34a4b Compare July 14, 2023 12:59
@lynxplay lynxplay force-pushed the bugfix/growing-block-growth-modifier branch from af34a4b to 68f1f71 Compare September 5, 2023 16:46
@thiagogebrimm
Copy link
Copy Markdown

This fix would be great, I'm waiting

Spigots implementation of the growth modifier boils down to a plain
chance for a plant to either grow immediately or to grow a single age
value towards its fully grown age.

This behaviour is specifically troubling for plants like the cactus or
the sugar cane, as they change an unrelated block when growing up (the
free air block above their tip). This blocks validity is checked during
neighbour updated which are triggered by resetting the previous tip of
the plant back to [age=0].

With spigots modifier implementation, blocks at the age of zero are able
to grow a new block above them however, causing the block change of the
previous tip to change a block of age zero to a block of age zero, which
does **not** cause a neighbour update as nothing actually changed.
This means that the about block is not updated and hence may remain in
an otherwise invalid state. (E.g. a cactus block next to a solid block).

This commit rewrites the existing spigot growth modifier implementation
specifically for the plants that grow new blocks like the cactus or the
sugar can block.

The modifier provided for the plants is separated into two distinct
values. One being the remains after a floored division by 100, e.g. the
original modifier without its 10^0 and 10^1 digits, the other the
modifier modulo 100.
The modifier 475 would, for example, be split into a base growth of
4 and a percentage chance of an additional age progression of 75%.
This way, on average, the plant would grow 4.75 times as fast as a
normal plant, which would only grow a single age per random tick.
@lynxplay lynxplay force-pushed the bugfix/growing-block-growth-modifier branch from 68f1f71 to 9f5bd90 Compare November 20, 2023 11:22
@lynxplay lynxplay marked this pull request as ready for review November 20, 2023 11:25
@lynxplay lynxplay requested a review from a team as a code owner November 20, 2023 11:25
@SwartZCoding
Copy link
Copy Markdown

Maybe up this and PR we waiting since 2 years lol

@DR4GONSTEAR
Copy link
Copy Markdown

I'm still waiting for this. If it's not going to become master, is there a plugin that can achieve this functionality? Is there a way to hack this into your own private server? I really don't want to move everything over to fabric.

@thiagogebrimm
Copy link
Copy Markdown

Any eta to add this fix?

@Warriorrrr Warriorrrr moved this from Awaiting review to Waiting For Author in Paper PR Queue Mar 5, 2025
@lynxplay
Copy link
Copy Markdown
Contributor Author

Closed in favour of #12264

@lynxplay lynxplay closed this Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Modifying cactus growth modifier causes it to not break after growing

7 participants