Brettflan / WorldBorder

Bukkit plugin for maintaining borders for your worlds to limit their sizes, as well as generating missing chunks or trimming excess chunks.
https://www.spigotmc.org/resources/worldborder.60905/
BSD 2-Clause "Simplified" License
139 stars 210 forks source link

Incorrect usage of Paper's async chunk API #147

Closed Spottedleaf closed 5 years ago

Spottedleaf commented 5 years ago

WorldBorder is loading chunks through the forceLoad api, and not the paper api. For example: https://github.com/Brettflan/WorldBorder/blob/master/src/main/java/com/wimbli/WorldBorder/WorldFillTask.java#L326

This is because, internally, force load api will load the target chunk. A fix is to delay adding the force loaded tag until after the chunk is loaded, which can be done via the thenAccept method on the future returned by paper api.

There's no rush to resolve this issue given over at paper we actually have not implemented asynchronous chunk loading for 1.14, so currently it makes no difference.

Brettflan commented 5 years ago

Ah, so World.setChunkForceLoaded actually loads the chunk rather than just setting a flag to prevent it from being unloaded? I didn't seen an indication of that in the documentation. https://hub.spigotmc.org/javadocs/spigot/org/bukkit/World.html#setChunkForceLoaded-int-int-boolean-

Do you know of another way to set that flag without also loading the chunk?

Spottedleaf commented 5 years ago

No, the flag must be set after loading the chunk

Spottedleaf commented 5 years ago

Since our API returns a completablefuture, you can use the thenAccept method to set the force loaded flag - alternatively the plugin ticket api I added to spigot recently.

Brettflan commented 5 years ago

I don't have a whole lot of time recently, and I'm also not the one who implemented the Paper API into WorldBorder. I took a look briefly and couldn't see how to do that; could you possibly provide a brief example?

Spottedleaf commented 5 years ago

Yes, here's an example:

      PaperLib.getChunkAtAsync(world, x, z, false).thenAccept((Chunk chunk) -> {
          if (chunk != null) {
              // if generated
              world.setChunkForceLoaded(x, z, true);

              // alternatively for 1.14.4+
              //world.addPluginChunkTicket(x, z, pluginInstance);
          }
      });
Brettflan commented 5 years ago

Great, thanks.

Based on that code, with my limited knowledge of CompletableFuture, I'd appreciate if you could check this to be sure it looks correct: https://github.com/Brettflan/WorldBorder/commit/f88ede397c6625993a6d7412ea74e0ce233875c3

Spottedleaf commented 5 years ago

Looks good to me

Brettflan commented 5 years ago

Cool, thanks again.