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 dead ongen saplings due to biome_lib secession #35

Merged
merged 8 commits into from
Jan 12, 2024
Merged

Fix dead ongen saplings due to biome_lib secession #35

merged 8 commits into from
Jan 12, 2024

Conversation

debagos
Copy link
Contributor

@debagos debagos commented Jan 11, 2024

I recently updated moretrees and noticed that the ongen_saplings that were already present prior to the update never grow anymore.

This is a big thing for me because over months I slowly let the whole map surface generate, without activating the newly generated areas. So there are dead ongen saplings everywhere on the map...

The dimension of this problem is very unique to my server because of the pre-generated map, but I expect dead saplings on other publicly hosted servers as well, due to how biome_lib works (see: biome_lib_queue_ratio). There are more reasons to why an ongen saplings hasn't grown yet than a pre-generated map (eg. NodeTimer not run yet due to high lag or a server shutdown/crash).

My solution to this issue is adding a unique group to those ongen saplings and let a LBM kick them active.
Furthermore I added a setting to enable the mentioned LBM. The LBM isn't enabled per default, to not create unnecessary overhead for simple and small scale users of this mod, while providing a simple fix for people like me.

@BuckarooBanzay BuckarooBanzay added the bug Something isn't working label Jan 11, 2024
@BuckarooBanzay
Copy link
Member

A few line-too-long errors here: https://github.com/mt-mods/moretrees/actions/runs/7485615804/job/20379754526?pr=35 not sure if they should be fixed or just the min-length increased, i'm fine with both :)

How rare is that use-case? Could it be something that could be done in a standalone mod perhaps (not suggesting this, just figuring out who else would have that problem too)

@debagos
Copy link
Contributor Author

debagos commented Jan 11, 2024

A few line-too-long errors

Oops, that's fixed now.

How rare is that use-case?

I think it's a pretty rare use case, but like I said I expect to see dead saplings not just on my own server.
Let me give you an explaination on what can cause dead saplings:

  • All saplings spawned between e476b81 and d43b79f are non-functional if ethereal, nether or any other mod was installed that incautiously uses/used minetest.clear_registered_decorations()
  • Saplings spawned with moretrees prior to e476b81 who have not had their node timers ran until the mod got updated.

To elaborate a little bit further, here are two reasons why a saplings node timer may never have ran:

  • Area got generated but server hard-crashed (SIGKILL, OOM, LuaJIT anyone?) and the area never getting activated again until the mod got updated (reasons could be: player never rejoined or pre-crash saved player position is outside the range of the sapling).
  • biome_lib itself. Sometimes when a player explores and the lag is high an area gets deactivated before biome_lib is done. The reasons is how biome_lib tries to keep the lag low and delays stuff. If you want to get into how that works, have a look at biome_lib_queue_ratio. I have seen that a lot on my server, because I am using a rather strict biome_lib_queue_ratio.

Could it be something that could be done in a standalone mod perhaps

That would work, yes.
I personally prefer the in-mod fix, but I wouldn't mind rejecting this PR completely and rely on the admins themselves to notice and fix the issue. But that would leave people who aren't able to come up with a simple mod who fixes those broken saplings pretty much on their own.

Oh and I would like to mention that @Niklp09 gave me the idea of the LBM after he learned about me having this issue.

Copy link
Member

@Niklp09 Niklp09 left a comment

Choose a reason for hiding this comment

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

Added some codestyle improvements

init.lua Outdated Show resolved Hide resolved
node_defs.lua Outdated Show resolved Hide resolved
Copy link
Contributor

@wsor4035 wsor4035 left a comment

Choose a reason for hiding this comment

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

seems this issue really isnt our problem because it was caused by non standard creation of terrain, but because its locked behind a setting and not that much future code debt to maintain, whatever.

@wsor4035 wsor4035 merged commit 9a6c64d into mt-mods:master Jan 12, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants