Why does shift down not check for leaves?

[Fourmisain]

We previously used breakMode: INSTANTANEOUS on our server, which worked fine, except modded trees can be huge and increasing maxSize to accomody them (especially when they merge together) quickly becomes ridiculous because of the amount of logs you get just by chopping 1 single block (it defeats the purpose of Efficiency too).

So we tried switching to breakMode: SHIFT_DOWN, but noticed that this would apply not only to trees but for example villager houses.

The first thing I found was this description about the algos: #53 (comment), which clearly states

Shift down or instantaneous works the same on the tree detection.

so at this point I thought this is a bug.

But I did notice the description of minimumLeavesAroundRequired states it only works for instanteneous, so I looked into it further and found this: 0451784

So this is something which was changed intentionally, but it doesn't state any reason.

I'm wondering why the leaf check was disabled in the first place?
Since the user can always turn it off by setting minimumLeavesAroundRequired to 0, I assume it had to be some kind of buggy interaction or whatnot?

[Rakambda]

This is simply because to me it doesn't really make any sense.

Let's suppose you have a normal tree, like an oak one and you set the minimum leaves around to 1.
What will happen is :

• Tree will be detected (regardless what mode you're in) => That basically gives an object that describe what leaves are part of the tree, what logs, what warts, ...
• Then we use the shift down method
  • It looks for the furthest log
  • <hypothetically checks if there's 1 leaf around it>
  • Breaks it

Now on the first log that works fine. But then you'd have (in most cases) the top log of the tree that is broken. So when you repeat the steps, then no more leaf will be around the log and the mod won't apply anymore.
In shift down, every time you break 1 log, you consider it as a new tree.

I don't know if my explanation makes sense or not.

(#53 is more about tree detection modes rather than breaking mode)

[Fourmisain]

Ah, I get it!

Hm... yeah, that's a pickle indeed.

I think there's at least some workaround, e.g. you could remember if the log (position) you first attacked fulfilled the minimum leaves requirement and if you attack it again, then keep applying shift down.
(Maybe reset/forget this when attacking a different log.)

This would prevent non-trees being cut and would mostly solve the issue when you are cutting a tree in one go.

[Rakambda]

This could be done that way. But would require quite some work and I'm not really sure it's the best as somehow each time you cut it is basically a new tree. Who knows what other event could happen at the same time and change the tree detection itself (like a tree growing next to the one you're cutting). IMO that'd lead to more weird situations.

Regarding the general problem, another approach could be to set the leaves required to 0 and use the adjacent blocks configuration (as you seem fine with tweaking it). This was basically #54 and is described a bit here (at the bottom).
What this does is basically a whitelist of blocks that are allowed to be touching the log. If anything else does touch the log, then tree detection will be aborted (depending on Adjacent stop mode).

This way, you'll explicitly set what blocks can be around, and things usually found in a house (like planks, cobble, or other stuff) shouldn't be in this list and will therefore not cut the tree if it "looks like" a house.
You just need to enter one entry in the list and it'll apply. By default some values are automatically added by the mod (not visible in conf, so no need to add them manually):

• + logs
• + logWhitelist
• - logBlacklist
• + leaves
• + leavesWhitelist
• - leavesBlacklist
• + adjacentWhitelist

As an example you can add minecraft:air to the list, and only logs + leaves will be allowed next to the tree. If for example there's a glass, or a beehive, or cobblestone, it'll stop.

[Fourmisain]

Who knows what other event could happen at the same time and change the tree detection itself (like a tree growing next to the one you're cutting). IMO that'd lead to more weird situations.

There will always be weird situations because there's no standardization for trees and random stuff can happen at any time.
I don't really see how this proposed feature would change that though, since without it, it'll always try cutting the "tree" no matter what.

I understand that you're not motivated to try implementing it because of the amount of work it takes though. I myself thought about building my own version months ago and haven't even started it.

I still wish there'd be some kind of tree API, where e.g. biome tree generation would tag each tree so that everyone knew exactly which block is part of a tree and which is not, while also being able to distinguish between different trees.

another approach could be to set the leaves required to 0 and use the adjacent blocks configuration

I'll looked into this a bit and can already tell that this will require tons of tweaking, e.g. if there's a (tall) flower next to the tree, shift down won't work on the bottom log - not a huge issue, but still.
Quite a few modded trees have decoration blocks like pollen, glowshroom, etc, which would all need to be whitelisted and it's not easy looking up every tree in a big mod pack.
There are also some structures, e.g. houses with diagonal roofs made out of logs (having only air besides them) which will be cut regardless of setting.

So maybe we'll end up using reverseSneaking to make tree cutting intentional.

---

On a kinda related side note:

Is there a way to tell FallingTree to still cut (part of) the tree when maxSize is reached?
That was one of the biggest annoyances of using the instantaneous break mode (though I think it also applies to shift down, right?) - either the tree would be fully cut, or it would not be cut at all, which usually meant tons of manual labor (modded trees are huge), effectively defeating the purpose of the mod.

What I mean is: When it finds the tree has more than maxSize many logs, cut the ones you already discovered.
For shift down mode, this could have some unintentional side effects when e.g. cutting a branch, it can then reach further down another branch the next time, though to me this doesn't sound too bad. Since you're cutting them one-by-one, if something is going on that you don't want, you have time to notice it.

[Rakambda]

I understand that you're not motivated to try implementing it

Honestly it's not really the motivation that is missing. It's just that right now I feel like the way I have been doing the mod is reaching its limits. It's hard to not make a change that doesn't feel like a weird patch somewhere or that doesn't introduce another thing in the configuration (and the way it's organized right now it can be confusing).

The TreeAPI and more in general the GenerationAPI would help for sure 😆

Quite a few modded trees have decoration blocks like pollen

Ah yeah, I rarely play with lots of mods that modify the generation so I tend to forget those. But ofc in that case it wouldn't really nice to do.

Is there a way to tell FallingTree to still cut ...

I can do that yeah. Like adding another mode in the config, something like "Tree detection aborting":

• NEVER => We search while there's things to search (like in the early versions of the mod), though if trees are close together then welcome lag
• TOO BIG DO NOT CUT => The current behavior, if the limit size is reached, nothing is done
• TOO BIG CUT => What I think you refer to, when the limit size is reached then we consider what we scanned as being a tree

Though: As the tree is scanned from the hit point towards the "branches", that'd mean that if the tree is too big, most of the time the bottom part of the tree will be cut and the top will be left flying. I guess in vanilla that doesn't cause problems as trees aren't that big and in modded you probably have elytras or even better to move around.
BUT, that means that the minimumLeavesAround will probably fail to apply as the top of the tree won't be scanned.

Maybe you can share a screenshot of when the original issue happened. Like how are the trees touching each other ?
If it's by the leaves there could be a TOO BIG CUT IF WE STOPPED IN LEAVES 😆 .

[Fourmisain]

• NEVER => We search while there's things to search (like in the early versions of the mod), though if trees are close together then welcome lag
• TOO BIG DO NOT CUT => The current behavior, if the limit size is reached, nothing is done
• TOO BIG CUT => What I think you refer to, when the limit size is reached then we consider what we scanned as being a tree

Yeah that's what I mean! NEVER will surely cause mayhem though, so maybe a simple cutIfTooBig boolean is enough?

As the tree is scanned from the hit point towards the "branches", that'd mean that if the tree is too big, most of the time the bottom part of the tree will be cut and the top will be left flying.
(...)
BUT, that means that the minimumLeavesAround will probably fail to apply as the top of the tree won't be scanned.

That's indeed an issue and it doesn't look like there's much one could do about that, so maybe reverseSneaking is the way to go afterall 😅

The only thing I could imagine is to split maxSize into maxScanSize and maxBreakSize, so the whole tree up to size maxScanSize is detected but only the first maxBreakSize logs are cut.
That'd help with the minimum leaves thing, but it's probably not worth the effort (and how it complicates the mod even more).

Maybe you can share a screenshot of when the original issue happened. Like how are the trees touching each other ?

I quickly tried scouting a few:

BYG's Mahogany trees are very thick and twisted so they easily collide with one another, it happens mostly at the top, but not always (like the tree on the left is leaning into the tree in the middle).
There's very dense forests where this kind of thing happens regularly, I also remember that some swamp biomes have trees that "climb" up mountains, which easily broke our size limit of 256 logs too.

BetterEnd's Shadow Forest has gigantic trees (consisting out of logs and bark), which almost always collide (this screenshot seems relatively tame), no size limit is big enough for them.

I recently ran into this "interesting" world generation in the 1.17 preview version of the modpack I'm playing:

Yes, those are clouds, yes the tree had a 5x6 cross section and yes, this is not the only one...

I also remember straight 2x2x100 trees being very common in... some Terrestria biome maybe? I think it was redwood trees.

Maybe you can now see why it's so hard to deal with modded trees 😆

[Rakambda]

maxScanSize and maxBreakSize

That's actually a rather great idea I think.
I also just thought about another thing but what can be also configure is a stop search after x leaves in a row to avoid scanning a connected foliage. The search would only be stopped on the current path it tries to take. (actually leaves aren't scanned)

I'll make a quick shot at this, and give you the modified mod so you can play with it and see if it somehow fits your needs.

[Rakambda]

I made the changes in #133

[Fourmisain]

Just gave that a try and it seems to work just fine!

breakMode: INSTANTANEOUS + breakOrder: FURTHEST_FIRST (+ maxSizeAction: CUT) is a nice combination, since it'll get rid of the worst bit, aka the tree top first.

One issue I found is that this combination has the same issue shift down had with minimumLeavesAroundRequired: since you're cutting away the tree top first, the left over bottom part of the tree will fail the leaf check.
So you might wanna make it default to breakOrder: CLOSEST_FIRST.
Though the player can always crouch to cut the rest (I've kind of accepted this at this point 😄).
(edit: Just realized that this is not true, crouching doesn't override the leaves check, the only way is to disable it.)

I feel like there's some big potential for simplification here, because the above combination is basically like shift down, except breaking multiple blocks at once.

I think the break modes could actually be removed by using shift down as the default and applying break size to it e.g. by breaking X-1 blocks at the top and moving 1 block to the bottom.
That way you can still get the old instantaneous behavior by simply cranking up the break size.

The only effective difference between instantaneous and shift down seems to come down to minimumLeavesAroundRequired again and I still feel this should apply to shift down, possibly with the mentioned workaround.

Dunno, food for thought. 🙃

[Rakambda]

There's a lot of things to improve and make it overall simpler.

breakOrder, i'll set it to FURTHEST_FIRST as this is the current behavior.

If this is already acceptable for you this way, I'll probably release this first, and one day when I feel like basically cleaning all the mod in its internals, your ideas can be brought to life 😆

[Fourmisain]

If this is already acceptable for you this way, I'll probably release this first, and one day when I feel like basically cleaning all the mod in its internals, your ideas can be brought to life

Yeah, this version is fine for me!
I'd suggest making it a bit more clear what the "break order" refers to, since shift down is unaffected by it (right?) and it only plays a role if break size < scan size.
edit: Maybe calling it "Max size break order" or something?