Configuration Phase API

[charred hull]

With the configuration API being introduced in 1.21.7, it would be huge if feedback could be introduced as this requires rather large changes in order to get the ball rolling on this API.

[frigid summit]

Can you elaborate on what has been introduced thus far, and what is planned to be introduced? Your comment makes it sounds like we're simultaneously wanting to start on this API ("in order to get the ball rolling") but also have already pushed it ("with the configuration api being introduced in 1.21.7")

[charred hull]

Configuration Phase API

[mental obsidian]

err what? 😅 First time I'm hearing of this

[charred hull]

This is kind of the case. We have at least started "initial" api but there is still some work that is needed.

I'll write something up

(As it seems there is less knowledge of the config phase than I thought.)

[frigid summit]

I think like ten people know what it is and not all of them are in this discord.

[subtle timber]

Ah the new thing that replaces the PlayerLoginEvent?

[frigid summit]

Also if it's not actually ready, with only some initial stuff, can you address why PlayerLoginEvent gets the nag already?

[civic pelican]

I would assume it's for future proofing

so devs dont get a shock therapy once that api drops

[modern meadow]

even if this api get reworked that event can't exists in it's current shape

[civic tangle]

Yay, another Owen API

[stark urchin]

im excited for it, makes lots of things more seamless and allows changing certain things easier, for example skipping a lobby step

as long as it has the ability to put a player in config phase and take them out without kicking them ill be happy

[cinder flower]

By a look in #paper-dev, I think more javadoc in the PlayerConnectionValidateLoginEvent on hwo to get the player profile would be neat.

And the ability to set/get permissions for the connection.

Also in the PlayerConfigurationConnection, if that is an Audience internally anyway why not expose that directly?

[sleek jungle]

https://jd.papermc.io/paper/1.21.7/io/papermc/paper/connection/PlayerConfigurationConnection.html#getAudience() ?

[charred hull]

I am mostly hesitent because 99% of methods do not work

[sleek jungle]

aah

[cinder flower]

then shouldn't adventure's audience be split or some custom interface be used instead of audience?

[charred hull]

They arent willing really

[frigid summit]

Isn't Audience default noop on like all methods

[cinder flower]

yea, but it is just shit to have methods / api that does nothing

[paper nacelle]

Currently my only thing im working is migrate how check permissions for the full event, luckperms can work but if need make a generic way need find how replace the old Player#hasPermission

[charred hull]

Working on this with luckperms. This will take some time

[frigid summit]

My point is, the expectation by audience is that stuff may noop. Clear javadocs on the audience getter would be fine IMO.

[cinder flower]

like everything is clearly abstracted in this pr, but audience is just a random bundle of everything in between

[charred hull]

its mostly the large amount of method completion spam

[paper nacelle]

Working in what way?

[charred hull]

getting a permissible from a uuid for example

[paper nacelle]

From the luckperms api you mean? Its not like try to fetch user and later check or you mean the generic way in paper for check permissions..

Sorry to ask

[charred hull]

No from the bukkit api

[paper nacelle]

Ahhh okay

Maybe can has another comments when touch my custom whitelist plugin because pretty sure i touch the events for result...

[charred hull]

yeah dont use that

[vital swift]

Truly wonderful isn't it

[analog moth]

Probably not the sort of feedback you're after, but having the current event marked as deprecated but its replacement event(s) marked as experimental is awkward - either way I need to SuppressWarnings to keep linting happy.

[charred hull]

This is a mistake, sorry!

[analog moth]

Ahhhh, oops - ty 🙂

[thick echo]

When sending links to a PlayerConnection during config phase, what is the intended way of getting ServerLinks instance? Is there a way to get it currently?

[minor basalt]

Bukkit.getServerLinks().copy()

[thick echo]

ah, thank you!

[charred hull]

My suggestion is to put this logic in the player join event instead, for now.

[frigid summit]

Am I correctly reading that the replacements for playerlogin have no way to perm check? How should a plugin that has a perm node to bypass player limit handle it?

[charred hull]

Correct at this moment. In general I’m working with luckperms to hopefully have some way to get permissions from a player that is offline/not loaded yet.

[barren cobalt]

I'm assuming blocking operations in AsyncPlayerConnectionConfigureEvent are fine to do? Like waiting for player to load all resource-packs before letting them join the server.

(This is how it was done in test plugin too: https://github.com/PaperMC/Paper/blob/e5cc6bf3f20d63492a8ec44dd31a10f1db335d0d/test-plugin/src/main/java/io/papermc/testplugin/TestPlugin.java#L82)

[charred hull]

Just make sure you properly time out/close if connection is closed

[barren cobalt]

Yeah makes sense to also check for that, thank you.

I really like the new API so far, though I'll probably use it only for resource-pack stuff. At least for now 😛

[barren cobalt]

Is that correct that PlayerConnectionValidateLoginEvent don't provide a way to check player's UUID? What should I use to check whether player is banned or not? 🤔

[thick echo]

Been there... See #paper-dev message 😁

[barren cobalt]

Thanks!

I assume that's because event is called twice, and PlayerConnection is PlayerLoginConnection only on the second call?

[charred hull]

Its login connection in first call, config connection on second

[barren cobalt]

Thanks for clarification 👍 Might be good to include that in javadocs with links to corresponding classes.

Also I'm not sure if PlayerConnectionValidateLoginEvent is the right name for this method. According to javadocs it also disallows the player from joining. There's allow(), so wouldn't disallow(Component) make more sense?

Similar to how it used to be in (now deprecated) PlayerLoginEvent

[charred hull]

kickMessage is more what disallow is. But yeah, that could possibly be renamed.

[barren cobalt]

Yes, that's the method I meant. Forgot to put it in my message lol

PlayerLoginEvent#kickMessage(Component) only modified the kick message IIRC.

[charred hull]

Well, if the kick message is present, they wont be allowed in.

[barren cobalt]

I don't know if that's intended, but calling PlayerConnectionValidateLoginEvent#kickMessage(Component) for the first event call (so with PlayerLoginConnection) still lets the player to log-in. Doing so in the second call however, (so withPlayerConfigurationConnection), which happens after all resource-packs are loaded, works correctly and disconnects the player with provided message.

If that's intended then it should probably be included in javadocs as well.

[charred hull]

For the first condition do you have ant plugins that are listening to player login event

[barren cobalt]

Ah, might be it. I do have LuckPerms installed and I saw a warning regarding that.

[charred hull]

Yeah that causes issues with this api due to legacy comparability

**compat

Although. that event should take precedent. I’ll take a look

[still tundra]

Is it supposed to unload the resource pack after a transfer during configuration phase? Loading the pack and calling a transfer on AsyncPlayerConnectionConfigureEvent seems to produce this but it doesn't happen when transfer is called after player joins the world

[glad dune]

but this does not imply that only luckperms is supported right?

[fresh harness]

no, the idea is to work with them, as someone who has a strong idea for an environment agnostic permission system, for designing an appropriate api in general

[sleek jungle]

will that add onto the existing permission api or replace it from the ground up? or is that still up in the air?

[fresh harness]

I mean, stuff like hasPermission is not really going anywhere

we shall see

the idea is to design an api for plugins that provide a permission implementation, what would such a plugin need to work at a stage where a Player is not available, and less so for consumers, plugins asking for permission results

[jade kelp]

If there is a way to modify the registry of the client, it should be possible to do so with PaperAPI

[pseudo gust]

what even is this api? waht is config-phase

[frigid summit]

The client has some different phases of communication it can be in.
Status, where it's just asking a server for motd and such.
Login, where it's negotiating auth and being allowed to join the server.
Configuration, where registries are sent (like what enchantments are present, and in theory in the future maybe stuff like custom blocks?)
Play, where you're actually playing in the server.
-# Do we count handshake as one? I don't know and probably don't need to know.

[pseudo gust]

Do we count handshake as one? I don't know and probably don't need to know.
I guess we should. Shaking hands is a polite gesture

(for tcp it is required)

thx for telling me

[mental obsidian]

https://forums.papermc.io/threads/1-21-7.1635/

This was really helpful to me at least

[rotund peak]

trying to use the new configuration phase api to send a RP
running into an issue where my "fallback" solution which runs when Player#hasResourcePack returns false
tried debugging it and it seems to send the rp in config phase but it returns false in PlayerJoinEvent here?

i can likely remove my fallback since it mainly applied for the old fucky logic i had to do for config phase rp but

or is this new api still not sending the pack "in config phase" only queueing it up for it?
and thuys sending it when the player has joined?

seems it might still just be sending a packet and not using the queue task stuff?
might make sense with why it seems to only ever send post-config phase for me maybe?

first image is in PaperPlayerConfigurationConnection second is ServerConfigurationPacketListenerImpl

[thick echo]

You need to wait for the resource pack to actually download... See the reply, this should work...

[rotund peak]

yeah that works thanks

[paper nacelle]

yeah i try something like

User user = LuckPermsUtils.getLuckPermsAPI().getUserManager().loadUser(uuid).getNow(null);
boolean hasPerm = (user != null && user.getCachedData().getPermissionData().checkPermission("ignore-max-players").asBoolean());

but just ends with null user but maybe this is another thing

[fresh harness]

more of a luckperms question, loadUser will always load the user from storage, that's an asynchronous operation, that's why it returns a future, getNow(null) will return null because the future is not completed, luckperms preemptively loads users in the async pre login event so you can simply call getUser(uuid)

[paper nacelle]

ohh make sense...

okay with that change now works.. its not "generic" because now depends on LuckPerms but for what need currently works thanks.

[civic tangle]

Is there any sane way to check permissions during the AsyncPlayerConnectionConfigureEvent?

[rough spear]

I think that's what like 90% of the messages in here are about

Also no

[civic tangle]

Oh lol my bad

Keep forgetting about the search feature

[rough spear]

You basically just have to ask LuckPerms for the permission information directly

[civic tangle]

I suppose using LuckPerms API directly is the play?

Yeah figures

[barren cobalt]

That's a bit of a late reply but... how do I know when connection got closed? I can't figure this out. When resource-packs are marked as "required", and player clicks "Disconnect" instead of accepting them, no resource-pack status will be sent and player will just disconnect. Can I somehow check if PlayerConfigurationConnection returned from AsyncPlayerConnectionConfigureEvent is still "valid", as if player is still being held in the config phase?

[fresh harness]

you can listen to the PlayerConnectionCloseEvent

[compact stag]

Why does need to be changed?

[paper nacelle]

Ah?

[compact stag]

Not using Player#hasPermission

[paper nacelle]

That event not has a player then cannot use that

[minor basalt]

Was there any reason why AsyncPlayerConnectionConfigureEvent's threads, which are mostly meant to be blocked most of the time, use platform threads instead of virtual threads?

[fresh harness]

there were talks recently about moving the events that use that thread pool to VTs, but there are greater prios rn

[minor basalt]

Well that event has his own dedicated platform thread pool just for this single event, so I could open a pr to move it to VTs

[fresh harness]

there are two events that use that pool

the async spawn location one too i believe

async pre login could also probably be moved to VTs tbf

[minor basalt]

It is called on the thread processing the login logic so it doesn't make sense to block that thread just to put async pre login in a VT as the sync login event does

And async chat event does lots of processing for messages per receiver, so not a good load for VT. But async spawn location and async config do indeed seem like good candidates for VTs