-
-
Notifications
You must be signed in to change notification settings - Fork 647
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pass through packets in config phase #1459
Conversation
logger.error("Error forwarding known packs response to backend:", ex); | ||
return null; | ||
}); | ||
if (player.getConnectionInFlight() != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think some aspects of this need reverting, the connection isn't always going to be inflight when we jump through configuration, but, events should still be fired, etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So that IF could just be moved in front of
player.getConnectionInFlight().ensureConnected().write(packet);
tho... it might affect plugins listening to config event?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that would prevent plugins from being able to interact with reconfiguration while on a server, this block basically needs reverting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right theoretically it shouldn't matter if I not handle the packet (handle it via handleGeneric), or handle the packet by default and send the KnownPacksPacket using connectedServer
But for some reason this doesn't work; The only difference is that handleGeneric also checks serverConnection.getPhase().consideredComplete() before sending
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, does the latest commit work or not, because, that looks pretty on par with what I'd expect from this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't be calling handleGeneric, however, we should ideally be handling all packets inside of this phase, as that we, any packet we don't know about is going to be a modded packet, which is where the special handling gets into play here
This is very good, thank you! |
32c5baf
to
8bca421
Compare
8bca421
to
13e05ae
Compare
Closing this, as this is not a proper fix. |
Re-adds #1326 which got reverted by #1371
also adds pass through for the PluginMessagePacket