Fix POI update scheduling #419

Open

Dueris wants to merge 1 commit from CraftCanvasMC/fix/poi-scheduling into ver/1.21.11

Dueris commented

2025-12-25 21:01:37 +01:00

(Migrated from github.com)

While POI update scheduling works fine on Folia, it has a major flaw with how it works. In the region threading base patch, Folia replaces the method call BlockableEventLoop#execute with RegionizedTaskQueue#queueChunkTask. While this works fine in general gameplay, it does cause all POI updates to be scheduled to the next tick, which technically breaks vanilla behavior and also breaks a few things(videos and images bellow).

The BlockableEventLoop#execute method is as such:

    public void execute(Runnable task) {
        R runnable = this.wrapRunnable(task);
        if (this.scheduleExecutables()) {
            this.schedule(runnable);
        } else {
            this.doRunTask(runnable);
        }
    }

Essentially, it checks if it is the "main thread" or not(which no longer exists in Folia, but for this case we will assume the "main thread" is the region owning the BlockPos we are updating at), and if it is the "main thread", it will run the task immediately, otherwise, it will schedule for the next tick.

Folia breaks this logic, as it always schedules for the next tick. While in normal gameplay, this is fine, but in some occurrences this does cause issues, like bellow:

https://github.com/user-attachments/assets/0d484092-1798-4e32-be6c-3e6d77a652a7

The image and video were linked to me in DMs, and I did followup testing in a local Folia instance. This is replicatable by creating a nether portal, either in the overworld or nether, with no exit portal currently generated. Then, if you spam entities through the portal, it will spam create portals on the other end, due to the POI update being scheduled for the next tick. While this probably is unlikely to happen in survival or something, other issues may be present that haven't been caught yet, and it is probably best to restore the Vanilla functionality of POI updating.

The fix I propose in this PR just swaps scheduling for the next tick with the Consumer bellow:

   final java.util.function.Consumer<Runnable> poiProcessor = (task) -> {
       if (ca.spottedleaf.moonrise.common.util.TickThread.isTickThreadFor(this, pos.getX() >> 4, pos.getZ() >> 4)) {
           task.run();
       } else io.papermc.paper.threadedregions.RegionizedServer.getInstance().taskQueue.queueChunkTask(this, blockPos.getX() >> 4, blockPos.getZ() >> 4, task);
   };

The consumer provided above fixes the functionality difference between Folia and Vanilla, while also fixing the issue shown above with the nether exit portal.

While POI update scheduling works fine on Folia, it has a major flaw with how it works. In the region threading base patch, Folia replaces the method call `BlockableEventLoop#execute` with `RegionizedTaskQueue#queueChunkTask`. While this works fine in general gameplay, it does cause all POI updates to be scheduled to the next tick, which technically breaks vanilla behavior and also breaks a few things(videos and images bellow). The `BlockableEventLoop#execute` method is as such: ```java public void execute(Runnable task) { R runnable = this.wrapRunnable(task); if (this.scheduleExecutables()) { this.schedule(runnable); } else { this.doRunTask(runnable); } } ``` Essentially, it checks if it is the "main thread" or not(which no longer exists in Folia, but for this case we will assume the "main thread" is the region owning the `BlockPos` we are updating at), and if it is the "main thread", it will run the task immediately, otherwise, it will schedule for the next tick. Folia breaks this logic, as it always schedules for the next tick. While in normal gameplay, this is fine, but in some occurrences this does cause issues, like bellow: <img width="1963" height="1005" alt="image" src="https://github.com/user-attachments/assets/9879a165-69d0-42b9-be63-83e1f201ebb1" /> https://github.com/user-attachments/assets/0d484092-1798-4e32-be6c-3e6d77a652a7 The image and video were linked to me in DMs, and I did followup testing in a local Folia instance. This is replicatable by creating a nether portal, either in the overworld or nether, with no exit portal currently generated. Then, if you spam entities through the portal, it will spam create portals on the other end, due to the POI update being scheduled for the next tick. While this probably is unlikely to happen in survival or something, other issues may be present that haven't been caught yet, and it is probably best to restore the Vanilla functionality of POI updating. The fix I propose in this PR just swaps scheduling for the next tick with the Consumer bellow: ```java final java.util.function.Consumer<Runnable> poiProcessor = (task) -> { if (ca.spottedleaf.moonrise.common.util.TickThread.isTickThreadFor(this, pos.getX() >> 4, pos.getZ() >> 4)) { task.run(); } else io.papermc.paper.threadedregions.RegionizedServer.getInstance().taskQueue.queueChunkTask(this, blockPos.getX() >> 4, blockPos.getZ() >> 4, task); }; ``` The consumer provided above fixes the functionality difference between Folia and Vanilla, while also fixing the issue shown above with the nether exit portal.

This pull request has changes conflicting with the target branch.

folia-server/minecraft-patches/features/0001-Region-Threading-Base.patch

View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Merge commit title

Merge pull request 'Fix POI update scheduling' (#419) from CraftCanvasMC/fix/poi-scheduling into ver/1.21.11

Merge commit body

Reviewed-on: https://git.diath.systems/Minecraft/Folia/pulls/419

Merge pull request 'Fix POI update scheduling' (#419) from CraftCanvasMC/fix/poi-scheduling into ver/1.21.11

Reviewed-on: https://git.diath.systems/Minecraft/Folia/pulls/419

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin CraftCanvasMC/fix/poi-scheduling:CraftCanvasMC/fix/poi-scheduling

git switch CraftCanvasMC/fix/poi-scheduling

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch ver/1.21.11

git merge --no-ff CraftCanvasMC/fix/poi-scheduling

git switch CraftCanvasMC/fix/poi-scheduling

git rebase ver/1.21.11

git switch ver/1.21.11

git merge --ff-only CraftCanvasMC/fix/poi-scheduling

git switch CraftCanvasMC/fix/poi-scheduling

git rebase ver/1.21.11

git switch ver/1.21.11

git merge --no-ff CraftCanvasMC/fix/poi-scheduling

git switch ver/1.21.11

git merge --squash CraftCanvasMC/fix/poi-scheduling