Ivorforce / RecurrentComplex

Structure mod for Minecraft
MIT License
53 stars 40 forks source link

Tree Generation goes crazy with Realistic Terrain Generator Mod #96

Closed Biochao closed 8 years ago

Biochao commented 8 years ago

When using Recurrent Complex with Realistic Terrain Generation the tree generation in forests becomes really dense causing massive tick lag from the generating. Here's a log file after creating a new world, spawning next to a taiga biome. fml-client-latest.txt

2016-10-05_20 12 37 2016-10-05_20 13 21

I've reported this to the RTG team as well: https://github.com/Team-RTG/Realistic-Terrain-Generation/issues/987

Ivorforce commented 8 years ago

Since forests are acceptable in vanilla, I'll need to check their source code to see why the forests are so much more dense. Can you give one or two specific examples of biomes with this problem?

Biochao commented 8 years ago

Forests, Birch Forests, Taiga, and Jungle are ones I've seen. 2016-10-03_21 46 30 2016-10-06_15 25 08 2016-10-06_15 10 32

Roofed forests seem fine. 2016-10-06_15 26 22

srs-bsns commented 8 years ago

Why are there fences on those trees?

Biochao commented 8 years ago

Temporary fix is to set "S:baseWeight_tree" to 0 in the config.

4rz0 commented 8 years ago

Wow, proper forests in my minecraft? If they only weren't so heavy on generation...

Ivorforce commented 8 years ago

Right. I've looked at the code and in RTG, it's because the mod posts the TREE decoration event multiple times, while ReC assumes it will be posted just once and the biome tree amounts will be set already (they aren't). I have a Forge event redesign in mind that will take care of this (and a few other) problems. Until I get that out though there can be no realistic solution.

Ivorforce commented 8 years ago

Whoops, wrong button. I'll close the issue when I have the event out.

ridjack commented 8 years ago

The tick lag is not from the generation of the trees, it's from the decaying leaves and the drops from them. You can see in the pictures from Biochao that leaves are decaying even right next to the trees. I was using RC in my server with vanilla generation and seeing the same thing.

My educated guess is that RC appears to be using the wrong log blocks (they have the bark texture on all sides) and the leaves aren't recognizing them as valid log blocks, thus beginning decay en-masse, which is a LOT of block updates, and dropping their items on the ground, which is more lag.

Also, why ARE there fence posts on the trees? I don't know if that's factoring in, but it sure is weird as hell.

Ivorforce commented 8 years ago

The leaves are decaying because they only stay if there is a vanilla log block within (I think) 3 blocks of them - and ReC trees are a lot bigger. You can make them undecayable in the config (decayTreeLeaves) to avoid the lag.

ridjack commented 8 years ago

The leaves in the RC trees are decaying right up next to the logs. In both sets of pictures posted by Biochao, you can see that; it's particularly noticeable in this one: https://cloud.githubusercontent.com/assets/6515654/19167971/f64fba50-8bdb-11e6-9931-d028116319f5.png

All those birch tree logs you can see are places where the leaves decayed when they were literally touching the tree.

You can also see it in the oak tree on the right hand side of this image: https://cloud.githubusercontent.com/assets/6515654/19136733/b8828ea4-8b3c-11e6-8c7d-776daf5b48dc.png

That log in the middle would have originally had a leaf block in front of it.

An additional note: leaves spawned with world gen normally don't decay until they're hit with a block update. I dunno what's going on there, but I still suspect that the six-sided-bark logs being spawned by RC are at the core of it.

Ivorforce commented 8 years ago

The birch trees just generally are very hole-y, I'm not convinced they decayed much yet there at all, let alone to the log itself. ReC doesn't change the behavior and as far as I see from vanilla code, that's pretty much impossible. Any leaves block fully connected to a log with a short distance will not decay.

Regarding the additional note: The same is the case with ReC. If you use the Inspector or debug window you can see that all the trees are decayable but won't check for decay - until hit with a block update. The 6 sided logs are a natural vanilla entry that happens to have been used for some of the trees and it has nothing to do with it, really.

I will check myself once again, but for now I'd just say that some block updates happened and caused a chain reaction.

srs-bsns commented 8 years ago

@Ivorforce @whichonespink44 and I have been discussing TREE decoration events to see if there's anything we can improve on RTG's end. Currently we don't see any changes that we can make that will be desirable. Hopefully your solution will work out, and we will be continuing to look into this in the future.

As for the tick lag issue, I've been observing a serious issue for some time now, but it has nothing to do with leaf decay, it has to do with light updates on the server. It is not a mod specific issue as I've observed the same thing happening with just Forge with no mods. Excessive tickable blocks in the area of where a player is located makes it worse (dense forests, lots of leaves), but the issue isn't location specific.

The only remedy I've experienced so far is to set the view distance under 10. For some reason this issue (from my observation) starts when the view distance is set to 10 or higher.

I've not had time lately to look into this further, so hopefully this limited info is of some help.

ridjack commented 8 years ago

@Ivorforce Fair enough. Further testing on my end does seem to confirm that what I was seeing as rampant leaf decay on ReC trees is not actually a thing. Apologies, I was quite certain I was right about that.

@srs-bsns Thanks for that. I'll give that a try and see what it does.

whichonespink44 commented 8 years ago

@Ivorforce If there's anything that RTG can do on its side of things to make compatibility easier, just let us know. Although, as @srs-bsns mentioned, we're kind of in a pickle with regards to when those TREE events get fired, so not sure what else we could do. We could maybe post some custom events which RC could then listen for if it detects RTG... would just need to know where/when to post them.

Ivorforce commented 8 years ago

Linking this up to https://github.com/MinecraftForge/MinecraftForge/pull/3318.

keybounce commented 8 years ago

Let me see if I understand this correctly.

ReCurrent expects one TREE event per chunk. RealT generates one TREE event per group of trees of a type, without regard to chunk boundaries.

Because the number of events generated is higher than the number of chunks generated in these forests, the treegen rate is too high.

And, Lex seems to want you to be able to clearly identify what your events are for, so that mods can actually tell what is going on ... exactly the opposite of the biome dictionary or ore dictionary junk.

Ivorforce commented 8 years ago

You identified the issue correctly, yes. We'll see what Lex says about the PR next. I've adjusted to his feedback, but I'm not entirely sure he understands the use case or why the current implementation is pretty useless.

whichonespink44 commented 8 years ago

@Ivorforce If #3318 doesn't make it into Forge (or lingers there for ages), I'm happy to go down the 'sub event' approach in the meantime if you want? Or maybe RTG could post its own custom event that RC could listen for if it detects RTG? I'm easy, so whatever you think's best.

Ivorforce commented 8 years ago

I have bad experiences with Forge PRs... Who knows when Lex is gonna reply next. Compatibility should definitely be our priority right now. I'll throw some temporary solution that will be easy to implement for both of us together real quick.

Ivorforce commented 8 years ago

Alright, with 7bdcbe732c15748ee5d81fc57f7c5032a8c29baa it is now possible to provide boolean hasAmountData(), int getModifiedAmount() and void setModifiedAmount(int) in the decorate event class. @whichonespink44 if you subclass the event and provide them as in the PR ReC will use the information, fixing our issue for now :)

whichonespink44 commented 8 years ago

Nice one - I'll look at this tonight after the day job and try to get something in place.

whichonespink44 commented 8 years ago

Ok, I think I've added the necessary stuff:

https://github.com/Team-RTG/Realistic-Terrain-Generation/pull/1011

When RTG posts the event, it sets amount to the number of times it's going to try and generate a tree, and then uses getModifiedAmount() before it does its thing.

Here's a compiled build with the changes if you want to try it out with your stuff:

https://goo.gl/cwoCjL

I've tested it on its own, and with WTFExpedition (@WhiskyTangoFawks), which also cancels RTG's tree generation, and it seemed to work fine in both cases, but let me know how it goes - happy to tweak as necessary.