SpigotMC / BungeeCord

BungeeCord, the 6th in a generation of server portal suites. Efficiently proxies and maintains connections and transport between multiple Minecraft servers.
https://www.spigotmc.org/go/bungeecord
Other
1.53k stars 1.09k forks source link

ServerKickEvent is not fired when kicked from a server stop #3695

Closed Elikill58 closed 2 weeks ago

Elikill58 commented 3 weeks ago

Bungeecord version

git:BungeeCord-Bootstrap:1.20-R0.3-SNAPSHOT:18eae8a:1842

Server version

all

Client version

1.20.6

Bungeecord plugins

The bug

On proxy, with this code:

@EventHandler
public void onServerKick(ServerKickEvent event) {
     MyPlugin.getInstance().getLogger().info("ServerKickEvent fired for " + event.getPlayer() + " who was kicked from "+  event.getPlayer().getServer().getInfo().getName());
}

When stop your spigot server, this event will not be fired. There is no event to manage those players, as ServerDisconnectEvent is called when the player is disconnected from proxy.

Checking

Outfluencer commented 3 weeks ago

@md-5 Is there a reason we don't call the event at this point? I also noticed that the event is not always called (if the server is closed for example). And that's why fallback plugins don't always work properly on BungeeCord

md-5 commented 3 weeks ago

If there's actually a kick it should be called

Outfluencer commented 3 weeks ago

Yes there is no kick i have read your old comments on old prs, but what do we do? how can we handle it so the player does not get disconnected from the proxy

Outfluencer commented 3 weeks ago

I am also a bit confused if i kill the backend server, the first event called is the PlayerDisconnectEvent and the handlerboss first closes the players connection and after that the connection to the server.And than the ServerDisconnectEvent is called

Shoulnt it be the other way around?

https://outfluencer.dev/data/bungeelog.txt

for contect bungee is on 25577 backend on 25565

md-5 commented 3 weeks ago

If there's no kick then a different event should be used and this bug is invalid.

Wouldn't the event be ServerDisconnectEvent?

Outfluencer commented 3 weeks ago

yes but you cant cancel the disconnection of the player

Outfluencer commented 3 weeks ago

look into my log please the PlayerDisconnectEvent is called before the ServerDisconnectEvent and the handlerboss has instantly marked the connection from the player as closed

Edit: I printed the executed events in the logs also i printed channelInactive of HandlerBoss

Edit of edit: ignore that it was bullshit

Elikill58 commented 3 weeks ago

If there's actually a kick it should be called

In fact, it's not a kick by the code, but a kick by the force connection closed with the backend server.

It's not a basic move or basic disconnect, so it more apply to kick, and that would be helpful for more compatibility with waterfall users and less dev for you to use the ServerKickEvent

Outfluencer commented 3 weeks ago

If there's actually a kick it should be called

In fact, it's not a kick by the code, but a kick by the force connection closed with the backend server.

It's not a basic move or basic disconnect, so it more apply to kick, and that would be helpful for more compatibility with waterfall users and less dev for you to use the ServerKickEvent

nvm i think i was just stupid and dont understand what actually happend

You need to put the priorities inyou bungeecord config.

If en exception is thrown it will send the player to one of the specified server. if we get a kick the ServerKickEvent is called

Outfluencer commented 3 weeks ago

@md-5 i made a test spigot plugin to show the exact problem

This is the bukkit code. I close the connection without kicking the player

@EventHandler
public void onChat(AsyncPlayerChatEvent event) {
    CraftPlayer player = (CraftPlayer) event.getPlayer();
    player.getHandle().playerConnection.networkManager.channel.close();
}

this will only call the disconnected(ChannelWrapper channel) method in the DownstreamBridge of BungeeCord

THE PROBLEM IS THAT: server.isObsolete() returns false and BungeeCord handels it like this

if ( !server.isObsolete() )
{
    con.disconnect( bungee.getTranslation( "lost_connection" ) );
}

ServerDisconnectEvent serverDisconnectEvent = new ServerDisconnectEvent( con, server.getInfo() );
bungee.getPluginManager().callEvent( serverDisconnectEvent );

It closes the connection of the player without calling any events before it (i printed all events) here:

11:48:28 [INFORMATION] ChatEvent(super=TargetedEvent(sender=Outfluencer, receiver=net.md_5.bungee.ServerConnection@5ec7547f), cancelled=false, message=gf)

11:48:28 [INFORMATION] localhost/127.0.0.1:25565 [frame-decoder, decompress, packet-decoder, timeout, frame-prepender, compress, packet-encoder, inbound-boss, DefaultChannelPipeline$TailContext#0]

11:48:28 [INFORMATION] MinecraftDecoder channelInactive GAME false

11:48:28 [INFORMATION] inactive localhost/127.0.0.1:25565 -> ChannelHandlerContext(inbound-boss, [id: 0x99867119, L:/127.0.0.1:57169 ! R:localhost/127.0.0.1:25565])

11:48:28 [INFORMATION] disconnect !server.isObsolete()

11:48:28 [INFORMATION] [Outfluencer] disconnected with: Proxy lost connection to server.

so you cant cancel the disconnection of the player by any chance

We would need to call the ServerDisconnectEvent before

we execute this code

if ( !server.isObsolete() )
{
    con.disconnect( bungee.getTranslation( "lost_connection" ) );
}

and add a way to cancel the disconnection

Outfluencer commented 3 weeks ago

@Elikill58 i hope #3696 will help you out

Elikill58 commented 3 weeks ago

@Elikill58 i hope #3696 will help you out

So, with that, I should simply set the reconnect server ?

Also, why doesn't simply call the kick event as waterfall did ?

Outfluencer commented 3 weeks ago

because we do not receive a Kick packet from the backend and we already have a fallback functionality with priorities in bungeecord that we can use. (what i did in my pr)

Outfluencer commented 3 weeks ago

@Elikill58 i hope #3696 will help you out

So, with that, I should simply set the reconnect server ?

Also, why doesn't simply call the kick event as waterfall did ?

Also waterfall is a fork of bungeecord It is not our responsibility to adapt to them.

Outfluencer commented 3 weeks ago

@Elikill58 i hope #3696 will help you out

So, with that, I should simply set the reconnect server ?

Also, why doesn't simply call the kick event as waterfall did ?

and yes not it searches for the next server to fallback like it should, i looked it ub btw watefall does the same but also calling a server kick event. But you dont need that you can use the ServerDisconnectEvent with this PR