Repository navigation
Add scattered Yendorian trees - #539
Numbers633 wants to merge 4 commits into
Conversation
Ralith
left a comment
There was a problem hiding this comment.
Really excited to have this feature!
Made a brief initial skim and didn't notice any major issues. I think the most important review will be @patowen's opinion of the isotropy and composability represented here.
The voxel generation logic introduced here further complicates the already large and ad-hoc terrain gen code, but I think that's an encapsulation problem we need to solve separately. Having two different megastructures, plus the base terrain, will help us do so successfully.
|
Yeah, I'm also excited to see this! I'll give this a thorough review when I have time. |
|
I made a small simplification in the mountain generation code. The previous version used temperature-based propagation when nodes had a Yendorian parent; that was more complicated than necessary and could subtly impact tree distribution in mountain regions. Underground nodes now consistently use precipitation-based probability regardless of whether they inherited a Yendorian branch. This change fixes a potential issue with isotropy at high precipitation and low temperature. |
|
Currently, trees are unable to fill all of the space above them. To do that, they would also need to propagate into 10 edge neighbors and 5 corner neighbors, resulting in trees being able to generate up to 21 child branches. However, state propagation to edge and corner neighbors is significantly more complicated. I'm not sure if it's worthwhile to include it in this PR, but it will be necessary for trees to have a continuous canopy. |
|
@Ralith, I believe the math and general logic in this PR is solid. I don't see any major isotropy or composability issues. Since these trees are node-aligned, a fair amount of the logic needed around "peer nodes" for horospheres isn't needed here, nor is any tricky math needed for probabilities, since it's more of a "yes or no" question for whether a node can seed a tree. I did find one minor isotropy issue (#539 (comment)), but I believe it's easily fixable. |
I think that's fine for now. I think that the "peer node" and "partial node state" logic we have would make logic like this possible, although getting the level of isotropy we desire could be significantly more difficult (but may end up being perfectly doable). If we would like a continuous canopy, I would be happy to leave that to future work. |
|
I like all the ideas for diverse kinds of trees, but let's keep this PR focused. Having literally anything that isn't vaguely wobbly terrain is huge already. |
|
Agreed. If we want, we can open separate issues for these ideas and keep this PR focused on the current megatree implementation. |
patowen
left a comment
There was a problem hiding this comment.
Thank you for addressing the comments. One overarching recommendation I have is to be judicious about the amount of code added in the PR, as well as when it is safe to loosen encapsulation or change the implementation of existing features.
|
@Numbers633, in case you're not aware, most discussion related to Hypermine has been via the |
9ed8843 to
d450fa4
Compare
|
I haven't forgotten about this. I hope to get to reviewing this PR soon. |
|
One thing potentially worth considering: Much of the complexity of this PR comes from trying to handle trees planted on horohills correctly. It might be worth adding that logic in a second PR instead. Then, this PR can be simplified greatly. I can think of at least two possible compromises for this:
|
|
Sounds good. I think option 1 would be simpler for now, as we don't have to care about the terrain. I'll move the horohill-specific megatree logic into That would produce distribution artifacts on the surface of horohills, but that can be fixed later. |
|
SGTM; I'm always in favor of reducing the complexity of individual changes. Though I am curious what "distribution artifacts" means here. Just having no trees at high altitude doesn't seem intrinsically objectionable. |
9e1e07e to
035a9bb
Compare
patowen
left a comment
There was a problem hiding this comment.
I believe this PR now has a good scope. I think it's pretty much good to merge as-is, but there are a few smaller improvements I would recommend first.
035a9bb to
b31b428
Compare
patowen
left a comment
There was a problem hiding this comment.
Looks good to me! I added one more minor comment.
Adds scattered Yendorian trees throughout the world, following the architecture outlined in #75 (comment).

Each Yendorian node can branch into nodes not adjacent to the parent face, resulting in nodes having up to 6 children. This confines the bounding box Yendorian trees can occupy to a pentagonal region. Each node records which of the 6 children were selected.
Trees seed on Land nodes, with the generation chance increasing linearly with precipitation (
precipitation * 0.025; clamped to 0-1). Sky branches continue independently with a probability that increases with temperature. DeepLand trunks continue straight down, anchoring any Yendorian trees in a valley to the surface.Branches generate as a wooden cylinder, and terminal nodes generate a radius-0.5 leaf ball at the center. Yendorian trees use preexisting Wood and Leaves blocks; no new blocks were added. Yendorian trees do not overwrite non-air terrain.