RakambdaOrg / FallingTree

Minecraft mod to make the trees fall in one cut
https://www.curseforge.com/minecraft/mc-mods/falling-tree
GNU Lesser General Public License v3.0
84 stars 32 forks source link

Whitelist for blocks connected to trees(logs) #54

Closed ghost closed 3 years ago

ghost commented 3 years ago

Just something for you to put into your ”idea box” to consider in the future if you want. :) Maybe you’ve already thought of it, if not:

I was thinking of ways to identify/separate trees from other log/wood structures. Not leaves necessarily, but the logs themselves. What I came up with: (A use case for a "connected block" whitelist)

Test for blocks connected to the log that is to be removed (blocks connected to the furthest log from the cutting point). So for example, if the log is connected to wooden planks or glass it won’t be considered part of a tree and won’t be removed. However if for example it’s instead connected to any of the following: (All possible natural blocks connected to a generated tree[log] I could think of)

In the case of nether ”trees:”

Only under these conditions would it be considered part of a tree and be removed like normal. If anything else is in contact, it’s not part of a tree but some other wooden structure and won’t be removed.

So a config whitelist for blocks that are allowed to be in contact with a log for it to be considered part of a tree and be removed. A whitelist would be better than a blacklist as there are far more blocks that aren’t connected to generated trees than there are ones that are connected (naturally that is).

Visualization of blocks connected to log (barriers being connected blocks):

Senario image below: (cutting point being the lowest log in the image) In a case like this where the tree scan finds a "non-tree log"(log in contact with a non-whitelisted block). I'm not sure if the scan should stop when finding this log and ignore any blocks above/after it and only remove the 3 blocks below it (considering it a dead end). Or if the scan should remove the 1 block above skip the non-tree log and then the three logs below it. So if a "non-tree log" should be a dead end for the scan or not. I'm guessing that a dead end would be more useful in most cases.

Rakambda commented 3 years ago

For now there is some kind of check possible, being the number of leaves around the top most (with highest y value) log. What are leaves is defined by everything tagged as leaves + whitelist + steams (if turned on in the config).

The idea is quite interesting though.

Just so it's clear, that'd be something that happens on the detection of the tree and not the breaking part. That means that being in instantaneous mode or shift down doesn't change anything to this.

A few points that maybe need some thinking.

Whitelist/blacklist

Honestly I think this part isn't that trivial. Each have its advantages/disadvantages. Plus I think it's not that easy to explain them x)

In my opinion a blacklist would fit this a bit better. The "default" config would be to cut down all trees, whatever is around them (can be restricted with the number of leaves) and the user that wants to override that can provide the blocks/tags that forbids the cutting. Now not sure if anything about the "it'd do a long list" can be done. Honestly i think that this list will be populated as the user see cases where it shouldn't be cut and that the list won't be that that long. As everyone builds with different styles and such probably not all blocks would be in the list. But same point as below, if the player uses other mods then this can make this list even longer.

Having the whitelist would rule out trees way too easily. If someone uses another mod and isn't really into configs then if the trees generates with something attached to them (like a beehive is) then it'll be ruled out by default. Also imagine a tree growing next to a jungle temple. Cobblestone and such should then be added to the list of whitelisted blocks even if it's quite "rare" and instinctively shouldn't be whitelisted.

What happens on a match

Let's suppose this is a blacklist that is used. If a log matches one of the blocks, what should happen?

Where to match

ghost commented 3 years ago

With the whitelist I was thinking that it would be left empty by default. And when it’s empty it would be ignored and everything would be whitelisted, like it is ATM. That or there would be an option to disable/enable it in the case of mods for example. (I wasn’t sure if fallingtree supported other mods, so i left it unmentioned, my bad)

What I was thinking when it comes to rare cases like jungle temples. This contact would usually be made quite low on the tree. So it would be easy for the player to make another cut attempt to cut the rest of the tree. I thought it would be kind of ”minecrafty” for the tree to be left there beyond the ”non-tree log.” :D If you know what I mean. The tree is never left competely floating in these cases. It would always be attached to the side of something. And when a player would see this, it would ”kind of” make sense why. It’s supported by that structure, even if not from below but from the side.

What happens on a match I was thinking that the branch would be stopped. Again using the logic that the rest of the branch is attached to other terrain, even if from the side. It’s supported by something and thus doesn’t fall like the rest.

Where to match "Every log” is what I thought. Ofc I don’t know if this would be hard on performance, I’m very much a beginner to modding.

I’ll be inactive for a while :P You can point out any flaws in my approach in the meanwhile, I’ll get back to it as soon as I can.

Rakambda commented 3 years ago

Yeah good point. Can leave it "unset" by default and nothing is applied. And only activate it when at least one block is whitelisted. Every whitelisted leaves & whitelisted logs would always be considered whitelisted adjacent blocks though.

I guess in the end the best is just to make it configurable. Either not cut anything or forget the branch.

For the performances I don't really know. For sure as it'd be doing more then it'll have more impact. Especially that it'll be done around every part of the trunk. The bigger the tree, the more impact it'll have.

Don't worry, you don't have to reply under an hour 😆

Rakambda commented 3 years ago

This isn't optimized at all, rather brute force. But for tested at least the ideas that's a start.

Can have a test with that version: https://www.dropbox.com/s/h15hh4goh61axvj/FallingTree-Fabric-1.16.4-2.9.1b1.jar?dl=0

ghost commented 3 years ago

I've been testing this for a while and as far as I can see it seems to be working exactly as I would expect it to. :+1:

ghost commented 3 years ago

I'm not sure if this is intended work like this but I noticed that when I add an item on the list it "overwrites the default list of whitelisted blocks." So if I just add vines to the whitelist, all logs, leaves, air, etc are now not whitelisted like they are normally. Is this how it's supposed to work or should these always be on the whitelist no matter what? EDIT: not sure if all are overwritten but some seem to be atleast.

Rakambda commented 3 years ago

Not all should be overwritten. Just that the air block isn't part of the whitelist (as it isn't a default leaf nor default log). Maybe air should be included by default no matter what.

If something is whitelisted the adjacent whitelist is: isLog? + isLogWhitelisted? (and not blacklisted) + isLeaf? + isLeafWhitelisted? (and not backlisted) + adjacentWhitelistFromConf.

Though I kinda noticed that the nether warts + shroomlight are not part of this, as it isn't a log nor a leaf. Should it be though? I guess they kinda fall in the category of beehives. It's not really leaves or trunk, but is part of the generation and should be added to the whitelist.

Rakambda commented 3 years ago

This version whitelists the air by default, maybe that solves your issue: https://www.dropbox.com/s/gvzxk0a3txye2f2/FallingTree-Fabric-1.16.4-2.9.1b2.jar?dl=0

ghost commented 3 years ago

You were right, it was just the air block causing me confusion it seems. 👍

ghost commented 3 years ago

When it comes to wart blocks and shroomlight, it's hard to say if they should be on the list by default or not. However just in case someone would not want them on the list i guess it's safer to exclude them. In the case of the air block, I couldn't imagine anyone not wanting it on the list at all times.

Rakambda commented 3 years ago

As it seems to behave as wanted, this probably is the closes to what may be released if nothing has to change: https://www.dropbox.com/s/hglgfe8m9ti3m9b/FallingTree-Fabric-1.16.4-2.9.1b3.jar?dl=0

It got the lang file updated (so the config menu should have actual sentences) & the cache for the config (this should improve performances, I don't know by how much). Maybe will have to test a bit on config change to see if it reloads well the white/blacklists, but from the few I did it seemed fine.

If this seems fine, will have to do the forge part and can be released.

ghost commented 3 years ago

Yeah, all seems to work perfectly as far as I can see. Great job once again 👍