https://paste.ee/p/c1t8O

Seems to be a very common issue based on my research

Hey, chunky author here (common pre-gen plugin).

We have definitely seen a few people with issues using Lootr over the last month or so.

My suspicion is that Lootr is doing a blocking chunk load here on server tick, which indirectly causes crashes.

Similar issue over here along with several solutions from myself, and the c2me author ishland:
Luke100000/minecraft-comes-alive#842

I hope this helps!

@pop4959 Ahah! Due to the chunk loading changes with regards to setBlock in 1.17 (loading a 5x5 around the relevant chunk), I changed Lootr to check for the chunk load events firing for the complete chunks in a radius around the area before beginning to do any modification, as simply checking to see if a chunk was loaded was generally insufficient and would cause this issue.

In Vanilla and most modded environments, Lootr's behaviour is correct and doesn't cause issues. Looking at it now, it seems as though I'm not actually checking for whether or not the chunk is currently loaded, just whether it has been generated in the past. This would be the main first part of the issue.

Multiple times in the past I've been stung by relevant "is this chunk loaded" checks actually causing chunk load and blocking.

The correct solution would be to check whether the chunk is currently loaded or not, although I'm not sure the best methodology for that post-1.17 (I'll check through the linked issue).

My other inclination is to access the ChunkyProvider to get a copy of the API and then determine whether chunk generation is taking place, and if so, defer all Lootr functionality while generation is occurring. This has actually been my recommendation and the reason why Lootr's most recent versions have a disable feature that can be changed through the configuration (and while the game is running, in theory).

That would fix it for Chunky specifically but not necessarily any other scenario, so the sane solution would be to go with both.

That said, do you have a specific method for accessing the ChunkyAPI on Fabric/Forge/etc? I see your recommendation is to access it via the plug-in manager; the ChunkyProvider method would seem to work but that isn't part of the API package, implying that it wouldn't be the correct method for accessing it.

@pop4959 I've discovered the other "is chunk generated" check, however I'm unsure how this would interact with your code due to the reliance on the chunk future:

          ServerLevel world = (ServerLevel) entity.level();
          ServerChunkCache provider = world.getChunkSource();
          if (provider.getChunkFuture(Mth.floor(entity.getX() / 16.0D), Mth.floor(entity.getZ() / 16.0D), ChunkStatus.FULL, false).isDone()) {
          ...

Hi Jon, this sounds mostly good.

If you are looking to process a chunk after it has been generated, there are a couple different solutions I would suggest. Firstly, if the chunk needs to be processed immediately following generation, I would recommend placing a ticket on the chunk. This will prevent it from unloading, thus avoiding the issue here with the chunk not being loaded when accessing.

Alternatively, if it is possible to process Lootr's logic as part of world gen, it would make most sense there instead (so there is no race condition here).

If none of that works, you can also check if the chunk is loaded and skip logic, or perform an async load on the chunk and await it before performing your logic.

I would highly discourage attempting to detect, or integrate with chunky in any way for this. There is nothing particularly special in the way that the mod loads and generates chunks. The primary issue is likely that it loads and generates a lot of chunks rapidly, which increases the probability of there being issues, especially in what seems to be a race condition (chunk unloading and Lootr attempting to access the unloaded chunk). It should happen the same with any other mod given the right circumstances.

The logic primarily triggers when a block entity is created; if the chunk is unloaded shortly after, then there's no point me doing anything with that block entity entry. Hence, just checking to see if the chunk is still loaded should be sufficient; I'm just uncertain if the check I'm doing (code mentioned above) would be compatible with Chunky.

That said, I'll begin testing it myself locally.

@pop4959 I've discovered the other "is chunk generated" check, however I'm unsure how this would interact with your code due to the reliance on the chunk future:

          ServerLevel world = (ServerLevel) entity.level();
          ServerChunkCache provider = world.getChunkSource();
          if (provider.getChunkFuture(Mth.floor(entity.getX() / 16.0D), Mth.floor(entity.getZ() / 16.0D), ChunkStatus.FULL, false).isDone()) {
          ...

I don't really like this method since it does look like it does a managed block to get the status.

The logic primarily triggers when a block entity is created; if the chunk is unloaded shortly after, then there's no point me doing anything with that block entity entry. Hence, just checking to see if the chunk is still loaded should be sufficient; I'm just uncertain if the check I'm doing (code mentioned above) would be compatible with Chunky.

That said, I'll begin testing it myself locally.

If you don't need the chunk if it isn't loaded, I would perhaps recommend checking ServerChunkCache#hasChunk or something like that instead, since this appears to be non-blocking and checks the existing chunk map for full chunks.

Or really any other alternative, including loading the chunk (if need by) asynchronously, and performing logic on the result of the future.

Feel free to add me on Discord if it's more convenient - pop4959

Let me know what your tag is as well if you decide to do this, since I normally don't accept unsolicited friend requests

So this is now implemented for Forge for 1.18.2 and 1.19.0/1.19.1/1.19.2. I'm continuing to forward port it, and will port it to Fabric shortly.

What's available on CurseForge is unfortunately the limit of what has been done so far.

Great news! I took a peek at the code and it looks very promising. Hopefully all issues are resolved now. I'll be sure to let you know if we continue to see any reports after this is released.

For the time being, is there anywhere I can direct users for in-development builds with this change?

Noted, no worries and don't rush it. Just wanted to ask in case it was a thing.

Hi, I've just seen Luke100000/minecraft-comes-alive#842 and Idk where to report this but I think Chunky caused some villages to be inhabited...

At first I thought the villagers just died but after finding 6 villages I realized something was wrong. Only one has spawned more than 1 villager, two spawned just 1 villager and the rest were completely abandoned (but with golems, cats, camels and farm animals, they're not real abandoned villages)

I created a new world and started locating a bunch of villages, but couldn't find any abandoned ones. That's when I thought it could be because of Chunky so here I am.

Is there anything I can do? Maybe undo the pre-gen? It's just singleplayer but I don't want to start all over again :(

[EDIT] Just wanted to reply to your comment, but we should redirect this to somewhere else, should I open an issue at Chunky's page

Hi, I've just seen Luke100000/minecraft-comes-alive#842 and Idk where to report this but I think Chunky caused some villages to be inhabited...

At first I thought the villagers just died but after finding 6 villages I realized something was wrong. Only one has spawned more than 1 villager, two spawned just 1 villager and the rest were completely abandoned (but with golems, cats, camels and farm animals, they're not real abandoned villages)

I created a new world and started locating a bunch of villages, but couldn't find any abandoned ones. That's when I thought it could be because of Chunky so here I am.

Is there anything I can do? Maybe undo the pre-gen? It's just singleplayer but I don't want to start all over again :(

[EDIT] Just wanted to reply to your comment, but we should redirect this to somewhere else, should I open an issue at Chunky's page

Please don't de-rail this issue - proper place to ask for support for chunky is our Discord server.

That said, chunky does not make any changes to world gen, so either an issue with another mod or you're unlucky.

@SkightMeow New releases for 1.20.0/1.20.1 (Forge) and 1.20.4 (NeoForge) are now up on CurseForge and should resolve this issue.

Fabric releases will be forthcoming.

@SkightMeow New releases for 1.20.0/1.20.1 (Forge) and 1.20.4 (NeoForge) are now up on CurseForge and should resolve this issue.

Fabric releases will be forthcoming.

Waiting patiently for fabric, thank you for all the work to fix this! <3

Slight setback, I believe #312 might be related to recent changes. I'm hopeful that it's just the redundant level.destroyBlock calling updateNeighbors, but I'll need to do some testing.

EDIT: So the call isn't redundant as the updateNeighbourForOutputSignal is called regardless and ends up at chunksource::getChunk which is blocking. It's possible that the chunk cache I was using previously isn't equivaent to chunkprovider::hasChunk...

EDIT 2: Bizarrely, this seems to be an issue with Vanilla in general. It iterates over Direction.values() and uses the relative positions from the input blockstate, then checks if there is a chunk at that location -- but then it creates a new blockpos, if the blockstate is a redstone conductor, that is relative to the direction again.

In this instance, I guess it can potentially cross the border into another chunk...

This is now live for 1.18.2, 1.19.0/1.19.1/1.19.2 and 1.20.0/1.20.1 for Forge.

Side-ported for Fabric 1.20, although I will not be backporting for 1.19 on Fabric.