search

PaperMC / Waterfall

BungeeCord fork that aims to improve performance and stability.
https://papermc.io
MIT License
743 stars 298 forks source link

Added ProxyInitializeEvent #836

Closed Gandalf1783 closed 3 months ago

Gandalf1783 commented 10 months ago

Added a new Event for knowing when a Proxy is ready to accept users.

Plugins can use this to register the Proxy. In our case, we add the new Proxy with the IP and Port in our HAProxy server to loadbalance and maintain HA (High-Availability) of our Network.

Gandalf1783 commented 10 months ago

I just noticed I missed a semicolon :/

Im trying to edit the patch, would be great if someone could help me out if this doesnt work out...

Gandalf1783 commented 10 months ago

Jup, semicolon added and the code compiles :)

Janmm14 commented 10 months ago

Naming is not good. A proxy is not ready when server socket fails to get started. isListening is also not great.

Maybe something like ProxyInitializeEvent and isSuccess.

In any case if the event shall have a success/listening/whatever yes/no, there might be more places during initialization where we would have to catch exceptions just to trigger some proxy ready event.

Therefore I think maybe, the event can still be named ProxyReadyEvent but shall have no state and shall only be called when initialization was a success.

Gandalf1783 commented 9 months ago

Alright, I see your point and will update the PR today or in 2-3 days.

Gandalf1783 commented 9 months ago

I renamed the class. I personally think that "isListening" is more meaningful for knowing if the port is open. Maybe I am missing something? I still renamed it, I guess it is now possible to let any possible error be reflected in the value of "isSuccess".

I was wondering if there should be any other important values that should be added. Similar to other events, where players join, I think there should be additional copies of values passed along. Such as the InetAddress along with the port or the queryPort, or if the proxyProtocol is enabled, which can be found anyways in the "operationComplete(ChannelFuture future)" method in the BungeeCord class.

Especially the InetAddress and proxyProtocol are intresting (at least to me), since my plugin could just grab them from the event. This isn't a huge concern for me though, since most values can be access via the BungeeCord-API anyways.

Freel free to comment your thoughts if anything could be improved before merging :)

Gandalf1783 commented 8 months ago

Is anything else a concern for merging? If yes, let me know so I can apply fixes :)

Gandalf1783 commented 7 months ago

@Janmm14 Bump?

Janmm14 commented 7 months ago

@Gandalf1783 I am no Waterfall maintainer.

Gandalf1783 commented 7 months ago

I will say that I these commit should not be merged if my commits already got accepted into Bungeecord: https://github.com/SpigotMC/BungeeCord/pull/3620

Gandalf1783 commented 3 months ago

Closing in favor of Bungeecord PR.