delvr / Farseek

A Scala API for Minecraft mods.
https://minecraft.curseforge.com/projects/farseek
20 stars 4 forks source link

1.7.10 potential StackOverflow issues with a modified GenLayerShore/GenLayerRiverMix? #23

Open ReikaKalseki opened 7 years ago

ReikaKalseki commented 7 years ago

For an upcoming ChromatiCraft biome I modified a few GenLayer classes (with ASM, not replacement) to allow me to modify the placement of biomes, the calculation of shores (and which biome types are used), and the placement of rivers.

In vanilla this works fine, but with Farseek installed there seem to be an increased number of StackOverflow errors related to generating the terrain; does Farseek core (or Streams, though the issue appears to be Farseek and not Streams) modify chunk generation such that either A) the modification of GenLayers causes issues or B) world generation that 'spills' outside its chunk (and thus triggering a few more chunks to generate) is more likely to run out of control?

Here is one such crash report. http://pastebin.com/XPLftWnu

If it helps, the biome does have a significant amount of worldgen that can 'spill' (including liquids), but in vanilla this is of no consequence; there are never any issues.

ReikaKalseki commented 7 years ago

I have confirmed disabling the GenLayer patches fixes the issue, at the cost of breaking my biome's terrain.

delvr commented 7 years ago

It's quite possible that chunkgen "spillover" could cause Streams to generate too much, such as pre-generating a full 16x16-chunk river zone which then spills on its edges causing generation of more river zones. I'd have to look further into this to confirm. Would you happen to have any source code for your new GenLayer patches?

(Edit: to clarify, I mean spillover beyond the chunk at generation time or the 2x2 chunk zone at populating time. Normal spread into the 2x2 populating zone should never cause issues unless there's a bug).

ReikaKalseki commented 7 years ago

The code of my patches can be found here and here. The actual logic of the patches is to replace the assignment operators here and here to allow mods to watch the placement of beaches and rivers and cancel (or in the case of beaches, modulate) them as desired.

delvr commented 7 years ago

Thanks Reika, I'll take a look and let you know if I can fix something on my end.

ReikaKalseki commented 7 years ago

I am willing to tweak on my end as well, provided the patches function both with and without Streams/Farseek.

delvr commented 7 years ago

Hi Reika, sorry I kind of dropped the ball on this one - did you end up implementing the feature in question and if so, did it cause crashes with Streams? If so I'll download your mod and see if I can repro on my end.

ReikaKalseki commented 7 years ago

The feature was implemented, and yes, the ASM it requires causes a crash with streams.

delvr commented 7 years ago

Thanks. Which mod and/or config option triggers the crash? I just tried with the latest ChromatiCraft and DragonAPI, and streams appeared without crashing.

ReikaKalseki commented 7 years ago

The relevant ASM patch is to GenLayerShore (I think; that or to GenLayerRiver). Check your logs for "applying BEACHGENLAYEREVENT" or similar.

delvr commented 7 years ago

Thanks - but does this patch need to be utilized by some config option or other mod in order to bring about this Streams crash?

ReikaKalseki commented 7 years ago

No, it is on by default.

delvr commented 7 years ago

OK, I do see BEACHGENLAYEREVENT in my logs and Streams are working fine. Are you able to reproduce the crash with only Chromaticraft/DragonAPI and Streams/Farseek loaded?

ReikaKalseki commented 7 years ago

I have had multiple people experience it and work around it by disabling that ASM patch. I believe the actual issue was in Farseek; are you using a different version than the release?

delvr commented 7 years ago

I used ChromatiCraft-1.7.10-V17c and DragonAPI-1.7.10-V17c, and tested with both Streams 0.2/Farseek-1.1 and Streams 0.3.3/Farseek-2.0. I saw references to CoFHCore in your original crash log so I tried adding that too, but still no crash.

ReikaKalseki commented 7 years ago

I do not know why I forgot about this, but it is most prevalent in areas with heavy "spilling" worldgen. In our case usually the new CC biome. Find whatever the "Luminous Cliffs" biome ID is for you, and then use DragonAPI's "findbiome" command with that ID as the argument. Teleport to a biome it found and wait for it to generate (or crash). If it generates, move around within the biome.

delvr commented 7 years ago

So I've explored a couple of Luminous Cliffs biomes but haven't found one with a Streams river in it yet; do you have a sample seed and coordinates? I did notice dramatic FPS slowdows around this and certain other Chromaticraft biomes in Streams 0.2 which might indicate some Streams-related recursion even though it didn't overflow the stack in my case. Stream 0.3.x gave me much better performance so perhaps one of the fixes in there addresses the recursion issue. Do you have reports of the crash with Streams 0.3.x? Again if so, a seed/coordinates would be very helpful. Thanks!

ReikaKalseki commented 7 years ago

do you have a sample seed and coordinates?

@Freyskol might, but he is unlikely to be available at this time.

It was reported as being very consistent, every time or close to it.