Skip to content
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

Floodgate Velocity fixes #1223

Merged
merged 3 commits into from
Jul 8, 2024
Merged

Conversation

Smart123s
Copy link
Contributor

@Smart123s Smart123s commented Jul 6, 2024

Summary of your change

First of all, I'm sorry for this awful reflection heavy code. I didn't want to add too many dependencies, so there are some type safety related warnings.

Two commits in this PR, but the second one is a one liner, and I'm lazy.

9a40cf0: After running the LOGIN event, it didn't get marked as run due to an early return. In Velocity, this caused all proxy commands to have no effect.

2d177e3: Most of the Floodgate checks are run at the PreLoginEvent, which requires precise usernames. However, Floodgate only sets the correct username in GameProfileRequestEvent (see here). If username prefixes are used, this made FastLogin unable to detect Floodgate players at this stage of the login process.

Related issue

Should fix pretty much every Floodgate & Velocity related issue.
Fixes #1222
Fixes #1176
Fixes #1070
Fixes #991
Fixes #935
Fixes #729

@Clyde6790pGIT
Copy link

Hi, I am testing this and it worked almost instantly, it did tell me to register a few times but then it registered automatically and logged me in, this also worked with authmevelocity.

* Use logging instead of raw exception printing
* Extract method
* Shorter var names
* More precise generics if possible
* Don't restore accessible state could be in conflict with other plugins
if we don't restore the exact value
@games647
Copy link
Owner

games647 commented Jul 8, 2024

Oh wow, that's really nasty reflection. Sadly we cannot use the other ProfileEvent, because the decision needs to made earlier. I guess we cannot use the API at that instant?

Nevertheless, thank you very much for the research.

@Smart123s
Copy link
Contributor Author

Smart123s commented Jul 8, 2024

I guess we cannot use the API at that instant?

Both the name and the UUID are set at the GameProfile stage.

The only thing we have access to at this step is the username, and an offline UUID.
The purpose of username prefixes is that a Floodgate user and a Java user can be online at the same time, so that's not really useful if we don't know which platform the player is on.
As for offline UUID, AFAIK, Minecraft generates them based on usernames, so same problem there.

Sadly we cannot use the other ProfileEvent, because the decision needs to made earlier.

Yeah, I know. The Minecraft login procedure is pretty strict.

A prettier solution would be to change Floodgates code to directly expose that cache variable, but they have previously stated that they don't want to support offline-mode servers.

@games647 games647 merged commit 1a56741 into games647:main Jul 8, 2024
1 of 2 checks passed
@Tim203
Copy link

Tim203 commented Oct 30, 2024

I was just checking some Floodgate related issues/PRs to see if GeyserMC/Floodgate#143 is still an issue, and I saw this PR. This indeed looks like a lot of reflection and looks very prune to breaking. Instead, on all platforms we support, we have a channel attribute key called floodgate-player that returns FloodgatePlayer. This will probably also break in Floodgate 3.0 (because we'll be combining the Floodgate API with the Geyser API) as it'll return Connection (a base api class) instead, but that seems much more future proof.
This AttributeKey is set when we handle the handshake packet, so that is quite early on.
Btw please send me a DM on discord if you have further questions, as I literally have 1000 unread notifications on GH.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants