ChunkLoadEvent and ChunkUnloadEvent never fire after upstream changes

joshuaprince commented 2 months ago

Spark link

Not Relevant

Expected behavior

ChunkLoadEvent and ChunkUnloadEvent should fire regularly as chunks load and unload.

Observed/Actual behavior

These events do not fire at all as of today's Purpur build 2266.

Steps/models to reproduce

Added to test plugin:

    @EventHandler
    public void onChunkLoad(ChunkLoadEvent event) {
        getLogger().info("Chunk load " + event.getChunk().getX() + " " + event.getChunk().getZ());
    }

    @EventHandler
    public void onChunkUnLoad(ChunkUnloadEvent event) {
        getLogger().info("Chunk UNload " + event.getChunk().getX() + " " + event.getChunk().getZ());
    }

    @EventHandler
    public void onPlayerMove(PlayerMoveEvent event) {
        // Control test to verify that events do fire
        getLogger().info(event.getTo().getX() + " " + event.getTo().getY() + " " + event.getTo().getZ());
    }

Join the server and confirm that the move event correctly logs positions as a control test, but the other two events never fire.

Purpur version

Self-built Purpur 2266, 86a65436

Agreements

[X] I am running the latest version of Purpur available from https://purpurmc.org/downloads.
[X] I have searched for and ensured there isn't already an open issue regarding this.
[ ] I ticked all the boxes without actually reading them
[X] My version of Minecraft is supported by Purpur.

Other

The same test plugin was tested on the following:

Paper 1.21 build 89 ("fix exact choice shapeless recipes", 44c3dd0) ~~(working)~~ Actually NOT working
Purpur 1.21 build 2265 (8f0f20d) (working)
Purpur 1.21 build 2266 (86a6543) (NOT working)

joshuaprince commented 2 months ago

I would also add that this was discovered in part because WorldGuard depends on these events, and regions do not appear to load at all when the events are not fired. That means that servers updating to this version will likely lose world protection entirely.

granny commented 2 months ago

@joshuaprince Since these are paper upstream commits you'll want to test this on Paper builds too. Purpur build 2266 merges in changes from Paper builds 82-89.

granny commented 2 months ago

Oh I completely missed that you already tested on latest Paper. Will look into this.

joshuaprince commented 2 months ago

@granny It looks like ChunkSystem.java isn't patched the same way Paper's version is. I'm trying to track down why, but you'll probably beat me to it.

Paper ca/spottedleaf/moonrise/patches/chunk_system/ChunkSystem.java:

    public static void onChunkBorder(final LevelChunk chunk, final ChunkHolder holder) {
        // TODO move hook
        io.papermc.paper.chunk.system.ChunkSystem.onChunkBorder(chunk, holder);
        chunk.loadCallback(); // Paper
    }

    public static void onChunkNotBorder(final LevelChunk chunk, final ChunkHolder holder) {
        // TODO move hook
        io.papermc.paper.chunk.system.ChunkSystem.onChunkNotBorder(chunk, holder);
        chunk.unloadCallback(); // Paper
    }

Purpur ca/spottedleaf/moonrise/common/util/ChunkSystem.java:

    public static void onChunkBorder(final LevelChunk chunk, final ChunkHolder holder) {
        ((ChunkSystemServerLevel)((ServerLevel)chunk.getLevel())).moonrise$getLoadedChunks().add(
                ((ChunkSystemLevelChunk)chunk).moonrise$getChunkAndHolder()
        );
    }

    public static void onChunkNotBorder(final LevelChunk chunk, final ChunkHolder holder) {
        ((ChunkSystemServerLevel)((ServerLevel)chunk.getLevel())).moonrise$getLoadedChunks().remove(
                ((ChunkSystemLevelChunk)chunk).moonrise$getChunkAndHolder()
        );
    }

loadCallback and unloadCallback are responsible for calling these events, and the definitions for both show gray in my IDE, indicating that they're unused.

edited to add: They're actually 2 different ChunkSystem.java paths

granny commented 2 months ago

The copy of Paper that you're traversing is outdated by 4 commits. Leaf moves ChunkSystem under a different package in the following commit: https://github.com/PaperMC/Paper/commit/00b949f1bbbf444e2b5e7b8de7c9b14fbd2133c6

joshuaprince commented 2 months ago

You're right, trying it again on a freshly-cleaned Paper repo, I can reproduce it there too. I'll open an issue on their tracker.

joshuaprince commented 2 months ago

https://github.com/PaperMC/Paper/issues/11103

granny commented 2 months ago

I've created a PR to Paper fixing this issue here: https://github.com/PaperMC/Paper/pull/11104. I've also gone ahead and applied it to Purpur ahead of time since I'm getting ready for bed (and won't be able to upstream until I wake up!).

PurpurMC / Purpur