Skip to content

Conversation

@Warriorrrr
Copy link
Contributor

@Warriorrrr Warriorrrr commented Aug 9, 2025

This PR fixes wrapping on folia by using getElevation non-blockingly, and as a free bonus, prevents doing a sync chunk load on paper servers.

@pop4959
Copy link
Owner

pop4959 commented Aug 10, 2025

Some initial comments & requests:

  • Can you break this up into separate PRs for fixing wrap, and breaking this up into per-player tasks (logically, they're separate changes, and it'll make things slightly easier to review). I think both are reasonable changes to make.
  • I haven't checked again in detail, but I do not believe there is any advantage to schedule the tasks differently between Paper and Folia. If going task per player, it's probably fine to just do that on both platforms. I think running a single global task was more of a micro-optimization on my part originally. The task is making a loop over all players anyway.
  • I know there was some back and forth about getElevation in chunky, and whether that should return CompletableFuture. Originally the idea was to squeeze in this Folia-specific managed block code to avoid a whole refactor, but seeing as we're doing it anyway here, I'd rather not have ChunkyBorder wrap around all of this just to create a CompletableFuture out of it when that's the only thing getElevation is being used for in the first place. So we might as well just make that change in Chunky at this point to avoid all of this extra wrapper code (including that in BukkitBorderCheckTask which should not be necessary then).

Aside from that, the changes here look generally good to me at a glance. Thanks for cooking this draft up.

@Warriorrrr Warriorrrr changed the title Use a task per player for border checking & fix wrap on folia Fix border wrapping on folia Aug 11, 2025
playerData.setLastLocation(location);
} else if (!player.hasPermission("chunkyborder.bypass.move") && !playerData.isBypassing()) {
final Location redirect;
} else if (!playerData.isBypassing() && !player.hasPermission("chunkyborder.bypass.move")) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch here

@pop4959
Copy link
Owner

pop4959 commented Aug 12, 2025

Thanks for addressing my comments, this is more in-line with what I was expecting for a refactor in this part of the code. Pending the merging of pop4959/Chunky#424 this looks good.

@Warriorrrr Warriorrrr marked this pull request as ready for review August 12, 2025 07:52
@pop4959 pop4959 merged commit 14e012c into pop4959:master Aug 12, 2025
1 check passed
@Warriorrrr Warriorrrr deleted the fix/folia-wrap branch August 28, 2025 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants