Jump to content

Stop currentWorld check in decorateChunk


Recommended Posts

In the decorateChunk method of BiomeDecorator, there's this little check.


        if (this.currentWorld != null)
            throw new RuntimeException("Already decorating!!");


This check isn't usually correct, but it causes a lot of trouble in complex world generation code. It's very natural for decoration of one chunk to set off decoration of another chunk, and this goes off even though everything is working just fine. The variable should really be passed to the method rather than being an instanceVar, and maybe that's why this check is in (to deal with re-entrant code).


I can manage this in my own code, but I have chronic problems with bugs from other mods which aren't really bugs because if this code wasn't there everything would work. For a long time my solution was simply to erase the currentWorld variable of ALL the biome decorators whenever I ran my procedure, which runs in a post-decoration event, after all the uses of currentWorld are done. This worked nearly perfectly, just to indicate how useless that check is. But, now I'm doing some things in ore generation, and erasing the variables isn't an option, because now some things are using it.


So, can Forge code edit out this check? If there's enough of a need for a check against improper re-entrance into the generate chunk routine, then maybe add in a check that's correct.

Link to comment
Share on other sites

No, this is recursion protection, it is there for a very good reason.

And you're saying.. that when you use a specific subset of events that force the state into a small structured method you don't get errors... And when you expand to a larger decoration system you get errors... you dont say...


Basically, the point is, if you decorate the same chunk twice you're doing something wrong.


No, we're not going to remove this. I know for a fact that it has actually prevented quite a few mods from making it to the market that would break everything.

I do Forge for free, however the servers to run it arn't free, so anything is appreciated.
Consider supporting the team on Patreon

Link to comment
Share on other sites

The problem is that it's not protecting against recursion, it's "protecting" against decorating two *different* chunks at the same time. Sure, that occasionally catches a recursion, but from managing all the bug reports that came up, it's less than 10% of the time. Plus, if you get a problematic recursion, it leads to infinite recursion and you get a stack overflow, so it's caught anyway. Recursion without infinite recursion doesn't cause problems - I expect performance is an issue but the chunks come out right, for my mods anyway.


If you really want to protect against recursion, you need a completely different procedure where you keep a HashSet of chunks being processed and skip calls to redo ones that are being done (or something similar). I actually ended up doing this, to catch the genuine infinite recursions that came up.


I don't think the current check is catching much actually, because any mod with complicated decoration is basically forced to shut the check down. As I said, I was clearing the vars. Highlands has apparently wrapped its calls in a try-catch because I see the stack trace for the call in my console logs. I suspect a lot of other mods are also try-catching their calls but not logging the stack traces.

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Unfortunately, your content contains terms that we do not allow. Please edit your content to remove the highlighted words below.
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.


  • Create New...

Important Information

By using this site, you agree to our Terms of Use.