Paper 26: async spawn rescue, Paper API build, DummyChunk, and WorldEdit-safe command init#373
Paper 26: async spawn rescue, Paper API build, DummyChunk, and WorldEdit-safe command init#373rkfsociety wants to merge 3 commits intoJikoo:masterfrom
Conversation
Replace deprecated PlayerSpawnLocationEvent; run world access on the main thread via callSyncMethod. Compile against paper-api 26.1.2.build.60-stable. Update DummyChunk for current Chunk API (load level, structures, tile entities, four-arg chunk snapshot).
Defer FlagHandler construction until first flag/unflag command so WorldEdit classes (e.g. SessionOwner) are on the classpath. Add WorldEdit to plugin softdepend for ordering when Bukkit resolves dependencies.
Jikoo
left a comment
Jikoo
left a comment
There was a problem hiding this comment.
Re: WorldEdit - WG depends on WE, so this should not be an issue unless you have dependency loops entirely breaking load order. You should probably check your other plugins if you're having problems.
If you are interested in fixing the problems with it, please submit it as a separate PR.
Re: AsyncPlayerSpawnLocationEvent: I'm not interested in dropping Spigot compatibility. I would be fine with a Spigot and Paper module, but I am not interested in an AI PR doing the required modularization because I would probably go insane wasting the time reviewing that much weird code.
In general, please try to actually look at what your AI did.
| FlagHandler local = this.flagHandler; | ||
| if (local == null) { | ||
| synchronized (this) { | ||
| local = this.flagHandler; | ||
| if (local == null) { | ||
| local = new FlagHandler(this.plugin); | ||
| this.flagHandler = local; | ||
| } | ||
| } | ||
| } | ||
| return local; |
There was a problem hiding this comment.
Locking is not really necessary here. Commands are processed on the main thread. Even if they weren't (i.e. maybe Folia, which also isn't supported because we use the regular Bukkit scheduler), the odds of two players just so happening to execute commands both requiring a flag handler at the same exact time are incredibly small.
I'm way more concerned with the fact that AI thinks using a publicly-accessible instance for a lock is a good idea. Good lord.
| /** | ||
| * Lazily created: {@link FlagHandler} touches WorldEdit types. If we construct it from | ||
| * {@link Regionerator#onEnable()} before WorldEdit has enabled, {@code SessionOwner} etc. are not | ||
| * visible yet and the plugin fails with {@link NoClassDefFoundError}. | ||
| */ |
There was a problem hiding this comment.
I'd prefer docs on the method to the field.
| * {@link Regionerator#onEnable()} before WorldEdit has enabled, {@code SessionOwner} etc. are not | ||
| * visible yet and the plugin fails with {@link NoClassDefFoundError}. | ||
| */ | ||
| private volatile @Nullable FlagHandler flagHandler; |
There was a problem hiding this comment.
Volatile and a lock? Unnecessary, I would drop the keyword.
/e: Ah, it is necessary due to how the AI set it up. Eugh.
| Player player = event.getPlayer(); | ||
| // Check if the player has logged in since this feature was added. | ||
| PersistentDataContainer playerPdc = player.getPersistentDataContainer(); | ||
| if (!playerPdc.has(loggedInSinceFeature, PersistentDataType.BYTE)) { | ||
| playerPdc.set(loggedInSinceFeature, PersistentDataType.BYTE, (byte) 1); |
There was a problem hiding this comment.
Quick returns before incurring a scheduler hit should be re-added. PDC logic does not need to wait for main thread.
| Chunk chunk = spawnLoc.getChunk(); | ||
| PersistentDataContainer chunkPdc = chunk.getPersistentDataContainer(); | ||
| NamespacedKey logoutKey = getLogoutKey(player.getUniqueId()); | ||
| // If the key is set, the chunk has not been deleted since the player last logged out. |
…aper-only API Restore spigot-api build, revert Chunk API changes, and remove WorldEdit load-order workaround from this PR. Register Paper AsyncPlayerSpawnLocationEvent handler via reflection when available while keeping PlayerSpawnLocationEvent for Spigot.
2 participants
This branch contains two logical changes (two commits).
1) Paper 26 spawn / rescue + build +
DummyChunkPlayerSpawnLocationEventwithAsyncPlayerSpawnLocationEventinRescueListener.BukkitScheduler#callSyncMethod.runTaskafter thePlayerexists (same intent as before).OfflinePlayer#getRespawnLocation(true)for respawn-based rescue.io.papermc.paper:paper-api26.1.2.build.60-stable(the26.1.2-R0.1-SNAPSHOTcoordinate is not on Paper’s Maven).DummyChunkfor the currentChunkAPI (getLoadLevel,isGenerated,getStructures,getPlayersSeeingChunk, four-arggetChunkSnapshot,getTileEntitiesoverloads) and fixisLoaded()(chunkX/chunkZ).2) WorldEdit classpath / load order
FlagHandlerpulls in WorldEdit types. Creating it fromRegioneratorExecutorduringonEnablecould run before WorldEdit finished enabling and triggeredNoClassDefFoundErrorforcom.sk89q.worldedit.session.SessionOwner.FlagHandlercreation on first/regionerator flagorunflag, and addWorldEdittoplugin.ymlsoftdepend.