Reimplement: Savannah Brush about voxelsniper

[itsjoekent]

Think erosion needs to take priority over this though, which I think mike is doing?

[nristock]

Reimplemented brush back into master. Base code from b370f0d + minor refactoring to make it compile and work.

[MikeMatrix]

Please remove it again, since it essentially does the same as the Splatter overlay.
I would recommend adding a random height feature for sover (and even over in this case).

[GavJenks]

No, this should not be removed until and if another brush is given a more general capability to do the EXACT same thing. Splatter overlay as it stands right now does NOT do the same thing at all, and if you delete savannah brush, we will no longer be able to expand or maintain our savannahs in Zah, which take up a huge portion of the continent.

I made the brush for a reason... and that is because what it does is NOT possible with other existing brushes.

So don't get rid of it (unless sover is upgraded, as mike suggested, to allow multiple heights of contingent and gradually decreasing density, thus allowing it to completely replace this one.)

Also, savannah brush has a gunpowder feature that replaces sand on the ocean floor, without touching anything above water (it is very specific, yes, but I still use it almost as much as any other brush, because it gets rid of the horrible looking notch sand blotches in the new worldgen). There is also no brush that can do this currently! Thus, that would also need to be fully replaced before this brush is removed.

[MikeMatrix]

As far as I know @Monofraps got almost all code finished for that.

[nristock]

Code is completely done. It just needs some testing. I'll commit this afternoon (in about 9 hours).

[GavJenks]

As far as I can tell from looking at the changes in that code you just committed, this does not at all sufficiently replace the savannah brush:

1. The savannah brush had lower density the higher up you went. It was something like 50% coverage for the first Y-level above where you clicked, and then only 25% coverage for the second layer above where you clicked. This would mean that people would have to sit there and do an entire layer first, then type in new parameters for offset and seed %, and then try to exactly snipe in the same locations they did before. Pretty ridiculous thing to ask people to do when you consider that a savannah might involve THOUSANDS of snipes. Do you want to be in charge of sniping a second layer of grass directly on top of the positions of your same snipes from the first layer of grass, thousands of times in a row? Neither does anybody else in charge of terraforming these areas.

2. You did not even attempt to replace the powder function of the savannah brush...

[MikeMatrix]

It is completely replaceable through the overlay brush.

[GavJenks]

Um, ignoring both of my points and pretending that a brush can do something that it cannot did not "solve" the issue, so please do not close it.

If you care to offer an explanation of how somebody would REASONABLY (i.e. not taking 20 extra hours of unnecessary work) use the overlay brush to replace the two functions of the savannah brush, then I would be happy to listen and consider how viable it is.

Like I said, the arrow function is not replaceable, without having to do it in two passes, and precisely hit the same snipe points you hit before, which is ridiculous.

And the powder function is only replaced by overlay if you fly underwater and click a block at Y=64. Otherwise, it will also overlay the first block of the COAST as well. But this is also ridiculously more difficult, because you have to fly back up again to see what you're doing, which is super annoying. If you would like to add a function to the overlay brush that allows it the option to ignore water (when calculating the sniped block), THEN we might have a viable replacement. But that hasn't been done yet... so it has not been replaced. You shouldn't remove functions before they are fully replaced.

[nristock]

So it seems like you want to do it yourself. Fine. I have removed the commit - have fun.

[GavJenks]

I ALREADY did it myself. I was the one that coded the savannah brush, and it worked just fine the way it was. YOU guys are the ones messing with it. Somebody removed it, and somebody else decided it should be rolled into other brushes for some unknown reason. None of that was my idea.

If you want to replace something, it is YOUR responsibility to fully replace the functionality. If you can't be bothered to do that correctly, then leave it like it was originally, and the problem will be solved. But you can't come in, break something somebody else made, do a half-assed job of replacing it or not replace it, and then leave.

[MikeMatrix]

We are not leaving. And we certainly replaced it. It's just a stupid minor feature of it, we didn't include yet. Everything else works. And for your information: Me and @Monofraps are not even thinking about leaving. You on the other hand haven't coded a single line in quite a while. We are cleaning up massive amount of code just so we can include better functionality and stability. AND YOU WANT TO TELL US HOW TO DO STUFF?

Either you code it yourself, or you let us decide when we do the minor feature of this brush.

[GavJenks]

No I haven't coded in a long time. And like I said quite awhile ago, multiple times, I do not INTEND to code VoxelSniper or other projects that are team-centered anymore on the VoxelBox in the future.

However, I am still very much a user of the VoxelBox, and a curator of an area that heavily relies on this specific brush with this specific functionality. And we rely on it working EFFICIENTLY and quickly to help terraform massive landmasses, and to match the huge amounts of land that are already in this exact style.

It's not a "minor" feature or a "minor" brush - There have probably been more blocks changed with the savannah brush than 80-90% of the other brushes in VoxelSniper on our server. It has been used on tens of millions of blocks, and almost all of them relied on the specific, minor variations in this brush that are not possible with the existing overlay brushes.

So far, you have still not explained how those specific (and heavily relied upon) needs can be adequately replaced by any other overlay brushes.

So either:
A) Explain how it has been replaced. As in, give me step by step instructions, because I can't think of any way of using the other overlay brushes that doesnt add tens of hours more work.
B) Put the brush back like it was, which will solve the problem and not require any effort on your part, if you don't care about the issue, or
C) Clean up your own mess, and replace the code that YOU got rid of with code that actually does the same job with the same level of convenience. It's not my responsibility to fix something that I made correctly the first time, and that somebody else broke.

The goal of VoxelSniper is not to make coders' jobs as easy as possible. The job of VoxelSniper is to make builders' jobs as easy as possible. This change has hurt several builders, and not helped any builders. Thus, the change is an error, and thus, there remains a ticket open to fix it.

[MikeMatrix]

Ok first of: I doubt it has changed more blocks than 80-90% of the brushes. Why? Because I know it for a fact. How? https://mcstats.org/plugin/VoxelSniper (Brush usage) I measure it.

Then to B)
This will not work, since this method was deprecated so long ago, that it would break undo's.
Meaning we would have to change quite a bit anyway.

C) Actually this would do more than just the old brush. Yes it would not ignore Tree's and water, because we haven't implemented that yet. (NOTICE THE YET!) This is not a priority in any way.

A) The savannah brush does the same thing as gunpowder sover does, except that it also varies the height (by giving the second layer a chance to be existent or not).
The new gunpowder allows people to choose varying height. Not only 1 or 2 high blocks (of any type, in comparison to savannah). This means it also allows for creating stuff that goes downwards and varies in height there.
So in the current version of VS on the VB server it will not be possible for the moment because @Monofraps was working on it. (And yes I am aware of the fact that it's used in Zah. That's why I made him do it. Otherwise this would have been redone later.) The exact commands for it would be the randh flag for a randomized height and the yoff parameter for a height offset.

And yes. I am aware of the fact that VoxelSniper is supposed to make the builders' jobs easier. But in order to do this it must be maintainable, which it currently is not. I am proud that even after the amount we changed only a few things popped up. We are working hard to make it both maintainable and easy to use. Oh and did I mention I am trying to make TVB more desirable to go to with awesome tools and an open approach to our plugins? Yes VoxelGunsmith is one part of it. It is the VoxelSniper API that will equip a sniper. This will be used to allow us and others to just hook their brushes into our system without a hassle.
Oh and did I also mention this means our stuff will be divided as well? Easier to maintain and clearer to handle.

[itsjoekent]

Not looking to get involved in Drama but...

According to plugin metrics- this brush has not been used once this entire week. Nor is it one of the most used brushes at all. Originally, Gav, I thought you were correct, but plugin metrics doesn't lie.

I understand this is an issue, however, I haven't heard a single person complain about it nor are you on to utilize it. We wiped player files over a week ago (maybe 2?) and you have not been on since then. If this is a huge deal, you can just copy the code from the older version, and made a preset for it. Copying & pasting code, along with a simple switch/if wouldn't take too long.

EDIT: Dammit Mike beat me too it.

[GavJenks]

Okay fine whatever, you win on stamina, I don't care enough anymore.

FYI though: I said the brush HAS done more block changes than 80-90% of brushes. As in, since the brush was made. I didn't say that it does so "on a regular day to day basis", or "since October 2nd when you implemented brush metrics." Terraforming is not a very gradual thing. It tends to happen infrequently and in large bursts, but when it does happen, the right tools matter a lot.