feat: add stable session UUID to MinecraftConnection#1840
Conversation
|
You're using FQDN's in the code instead of importing, and I'd rather like to see the use of UUIDs instead of an incrementing ID here. This way the connection IDs are also unique across reboots (useful contract to have when exposing this to plugins) |
|
UUIDs also generally have the advantage that they're more useful should plugins want to externalise that identity for other systems, especially in a network |
|
I initially chose long (AtomicLong) thinking it could serve dual purpose as both a connection identifier and a rough connection counter for server operators who use such metrics. But your point about UUIDs being more useful for external systems and surviving reboots makes sense, I'm going to change it |
|
@electronicboy Done, please review again |
|
I think we should shift from a per-connection id to a session id: one UUID that's stable for the player's entire session on the proxy, and shared across the client<->proxy connection and every proxy<->backend connection they open. So I'd name it Right now that shared/stable property doesn't actually hold. Since every
It happens to work for the login-phase events only because the What I'd propose:
The null case is never reachable through // VelocityInboundConnection
default UUID getSessionId() {
return Objects.requireNonNull(getConnection().getSessionId(), "sessionId");
}
One thing to note is that this shouldn't be confused with Minecraft 26.2's new session ID (a shared "play session" UUID across every connected player that resets when the server goes from empty->non-empty). I'm not too sure if "session id" would be the right name here, but this could probably also be solved by a javadoc comment on |
|
Hmmm… indeed, this is a scenario I didn't test before. I only tested multiple players joining the same backend server, but I didn't consider the case of switching between backend servers. I will test this scenario and make the necessary adjustments accordingly. Thanks for pointing this out! |
…nce random generation
|
I also wrote a plugin for test |
|
Let me rename this PR |
xfy2412
changed the title
|
Maybe call it ”clientConnectionId” will be better? |
3 participants
Add a stable connection identifier to MinecraftConnection
A unique connectionId (AtomicLong) is held in MinecraftConnection,
the shared underlying instance across all InboundConnection wrappers,
surfaced via InboundConnection.getSessionId().
This allows plugins to reliably track the same TCP connection across
PreLoginEvent, GameProfileRequestEvent, and LoginEvent, even when
the InboundConnection wrapper object differs between events.
Tested with both online-mode and offline-mode connections: