BG-Software-LLC / SuperiorSkyblock2

Optimized, feature packed Skyblock core.
https://bg-software.com/superiorskyblock/
GNU General Public License v3.0
167 stars 141 forks source link

Improvements on the crop-growth system #1757

Open Distolfix opened 1 year ago

Distolfix commented 1 year ago

Minecraft's Version

1.19.4 pufferfish

Plugin's Version

32

Describe the bug

I have carefully reviewed your page: https://wiki.bg-software.com/superiorskyblock/overview/upgrades/island-multipliers and conducted several tests on the crop growth multiplier's functioning.

On the site, it's explained that the crop growth multiplier operates by multiplying the number of blocks selected for "ticking" (i.e., growth) within each interval. The number of blocks selected for ticking is given by the product of randomTickSpeed crop growth multiplier crop interval.

I carried out several tests based on this formula. For instance, if randomTickSpeed is 3 (default for Java servers), the crop growth multiplier is 4 (i.e., the default maximum you've provided in your plugin), and the crop interval is 100, the plugin will attempt to tick 1200 blocks in each interval.

However, I've noted that increasing the crop interval does not improve performance; instead, it worsens it. This is because it appears that many more blocks are grown with each tick. For example, with a crop interval of 20, the situation was laggy, but blocks broke in a uniform manner. But when I increased this value to 100, the blocks all broke at once, significantly worsening the situation.

This is in contrast with your advice, which recommended increasing the crop interval to improve performance. Based on my tests, it seems this solution actually worsens the situation, not improves it.

Continuing the tests, I set the crop interval to 50 and noticed that the situation is even worse compared to when it was set to 20.

Using the formula randomTickSpeed crop growth multiplier crop interval, with randomTickSpeed at 3, crop growth multiplier at 4, and crop interval at 50, the plugin will try to tick 600 blocks in each interval.

The higher the crop interval, the more blocks are grown simultaneously, rather than uniformly. This means that, with a higher interval, you'll have a large number of blocks breaking all at once, which seems to significantly worsen the situation, contrary to your suggestion.

In my opinion, this behavior seems to be anomalous. It's unclear whether it's a plugin issue or an intentional behavior, but having all blocks break at once certainly seems to worsen the performance, not improve it. I wonder if there's room for further optimization or if there's another way to handle crop growth that could lead to a better outcome.

To Reproduce

Reproducing the anomaly is straightforward: take the default .yml file, set the crop growth multiplier to the maximum, and you'll notice that as the crop interval exceeds 20, performance degrades instead of improving, especially when this value is increased.

I understand you suggested disabling the crop growth multiplier, but this isn't feasible considering the existing islands and the users who purchased the multiplier with their own money. Therefore, we must discard this option.

I believe it's necessary to review how the crop growth multiplier is managed in the plugin. As I pointed out, contrary to what you advised in the most recent beta, increasing the crop interval actually leads to worse performance.

Additionally, setting too high values for the crop interval causes the server to crash. I think it's critical to further investigate this problem and find a solution that allows using the crop growth multiplier without compromising server performance.

Additional Information

the continuation of

https://github.com/BG-Software-LLC/SuperiorSkyblock2/issues/1736

OmerBenGera commented 1 year ago

This can be improved, I agree. Especially when I already have an algorithm just for this scenario - instead of doing tasks in specific intervals, it splits the large task into small ones and spreads them across multiple ticks.

I am keeping this opened and will definitely implement that.

Distolfix commented 1 year ago

Approximately how long could this process take?

Distolfix commented 1 year ago

@OmerBenGera any news about this?

OmerBenGera commented 1 year ago

There's no need to keep pinging me on this. I will update you once I have a solution.

I started working on one but my solution doesn't seem to have a big improvement, so I am looking into a better solution, if any even exists.

Distolfix commented 1 year ago

Hello @OmerBenGera any news regarding this?

homerek111 commented 1 month ago

@OmerBenGera this feature should definitely be changed.