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

Revert to old createProfile(UUID, String) logic #7723

Merged

Conversation

lynxplay
Copy link
Contributor

The recent change to the createProfile method set, which allowed plugins
to properly create profiles with differing names/uuids for players
currently on the server broke an implicit method contract for plugins
providing the lowercased version of a player name to the method.

Previously, the returned profile would simply be pulled from an online
player with the same name (when compared case-insensitively), however
the previously mentioned fix to the creation logic currently returns
a profile with the lowercased name instead.

To not break plugins that relied on this specific internal logic, the
ability to create pre-filled profiles with different names or uuids for
players that are currently online is moved by this commit to a new
method called createProfileExact, which simply mirrors the current
createProfile logic.

The createProfile logic is hence returned to its old functionality, once
again allowing plugins to grab an online players profile from their
lowercased name.

See: #7558
Resolves: #7700


Potential points of discussions:

This PR actively brings back (in my opinion) unwanted behaviour to the createProfile method. While I see the use of the logic in said method, I am uncertain whether moving the (in my opinion) correct logic to a new method like createProfileExact is the way to go here or if we should move the old logic into a new method. Both sides have their benefits, however the patch currently introduces a new method as not break plugins that relied on the lowercase hack.

@lynxplay lynxplay requested a review from a team as a code owner April 10, 2022 01:39
@r4nx
Copy link

r4nx commented Apr 22, 2022

So is anyone against? Me and, I guess, other people who liked the PR would be thankful if it could be merged

And thanks for your work guys, I'm very surprised that you not only not refused the issue, but provided the fix. That deserves respect.

@electronicboy
Copy link
Member

the PR needs rebasing

The recent change to the createProfile method set, which allowed plugins
to properly create profiles with differing names/uuids for players
currently on the server broke an implicit method contract for plugins
providing the lowercased version of a player name to the method.

Previously, the returned profile would simply be pulled from an online
player with the same name (when compared case-insensitively), however
the previously mentioned fix to the creation logic currently returns
a profile with the lowercased name instead.

To not break plugins that relied on this specific internal logic, the
ability to create pre-filled profiles with different names or uuids for
players that are currently online is moved by this commit to a new
method called createProfileExact, which simply mirrors the current
createProfile logic.

The createProfile logic is hence returned to its old functionality, once
again allowing plugins to grab an online players profile from their
lowercased name.
@lynxplay lynxplay force-pushed the bugfix/lower-cased-create-profile branch from d9f2536 to 548ddae Compare April 22, 2022 18:49
@lynxplay
Copy link
Contributor Author

Rebased onto latest 👍

@electronicboy electronicboy merged commit ef6a1a5 into PaperMC:master Apr 22, 2022
@lynxplay lynxplay deleted the bugfix/lower-cased-create-profile branch April 22, 2022 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New profile creation logic breaks the plugins
3 participants