Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ReikaKalseki opened this issue Feb 12, 2017 · 17 comments
Labels

Comments

@ReikaKalseki
Copy link

ReikaKalseki commented Feb 12, 2017

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
Copy link
Author

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

@delvr
Copy link
Owner

delvr commented Feb 12, 2017

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
Copy link
Author

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
Copy link
Owner

delvr commented Feb 17, 2017

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

@ReikaKalseki
Copy link
Author

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

@delvr
Copy link
Owner

delvr commented Jul 17, 2017

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
Copy link
Author

ReikaKalseki commented Jul 17, 2017

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

@delvr
Copy link
Owner

delvr commented Jul 17, 2017

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
Copy link
Author

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

@delvr
Copy link
Owner

delvr commented Jul 17, 2017

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
Copy link
Author

No, it is on by default.

@delvr
Copy link
Owner

delvr commented Jul 18, 2017

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
Copy link
Author

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
Copy link
Owner

delvr commented Jul 18, 2017

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
Copy link
Author

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
Copy link
Owner

delvr commented Jul 21, 2017

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants