Add Cookie API

[Luccboy]

I tried adding the missing cookie API. I tested it and it seems to work fine. I would appreciate any feedback.

[pop4959]

This looks overall pretty good, but my biggest concern here is that there is no handling over cookie packets sent or retrieved by backend servers. I think I would like this better if it moved away from CompletableFuture and instead fired events similar to what is done for PluginMessageEvent so you can identify the source, destination, whether to forward it, and have the ability to modify the data before sending to/from the backend.

[electronicboy]

sunday, bad eye; Using a map like that would also create dozens of issues if multiple things tried to request that data

[Luccboy]

Agreed, I will try to implement your feedback. Thanks!

[Luccboy]

Hi, I'm currently working on this and I'm unsure how to proceed. Do you want to have an event for each cookie packet or is one event for the CookieResponse packet sufficient? Also, to determine the destination of a CookieResponse packet, I would need to somehow keep track of the cookies requested by the proxy, right? - what would be the best way to do this, maybe a queue?

[pop4959]

Maybe it helps to think about the data flow here, we have:
StoreCookiePacket which is server -> client, and in-between the proxy may want to block or modify the result.
CookieRequestPacket which is also server -> client, and in-between the proxy may want to block.
CookieResponsePacket which is client -> server, and again in-between the proxy may want to block or modify.

If you look at PluginMessageEvent you basically have the same idea of keyable custom payloads between the client and server, although there the message interaction is two way, server <-> client. It would not be necessary to distinguish between the source and destination if there are separate events here since the client -> server and server -> client flow is implied. You'd be able to figure everything out based on the player the events are being handled for.

Correct me if I'm wrong, but I don't think there is any reason for the proxy to store anything here, since proxy plugins will be able to listen to the events fired here and store any intermediate results they need before it gets forwarded, or cancel the event if it doesn't want them forwarded.

[Luccboy]

Thanks for the clarification, I agree that if we let plugins decide whether to forward the packet or not, we don't need to store anything on the proxy.

[Luccboy]

Short update: The only thing missing in my opinion is how to forward the CookieResponsePacket to the backend server in the login phase, because the backend server to which the client should be connected has not yet been determined. But I also noticed, that when a backend server requests a cookie in login phase, the client is already in config phase. So it should not be possible to receive a CookieResponsePacket from a client in login phase caused by a cookie request from a backend server in login phase.

[Heliumdioxid]

That sounds good to me: If the server can only send a CookieRequestPacket once the client is in config phase, the CookieResponsePacket doesn't have to be forwarded if it is sent in login phase (as it was necessarily requested by Velocity).

[GrayR0ot]

Amazing PR :)

[electronicboy]

build failure, if we're also caring about names, I'd much rather that new packets just use their mojmap'd counterparts, I believe we'd be okay doing that and it would generally make our lives much easier

[sunmisc]

perhaps we should abandon CompletableFuture and other callbacks altogether
and use a regular synchronous API instead.
Fiber - Virtual Threads (jdk 21+) will help to solve performance problems

Of course, Virtual Threads has its own small overhead at the moment, but in general it's a new solution that can possibly make asynchronousness behind us

At the moment you can use it like this

Thread.ofVirtual(() -> {
   CompletableFuture.get();
)