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

Added a simple way to get a player by their xuid #1642

Merged
merged 10 commits into from
Dec 17, 2020

Conversation

janrabek
Copy link
Contributor

@janrabek janrabek commented Dec 9, 2020

Simple method to get a player by their Xbox UUID. Can be used by Bungee plugins that need to access the GeyserSession

@Tim203
Copy link
Member

Tim203 commented Dec 9, 2020

While this method won't be called a lot, I still recommend using plain for loops

@janrabek
Copy link
Contributor Author

janrabek commented Dec 9, 2020

Any reasons for that?

@Tim203
Copy link
Member

Tim203 commented Dec 9, 2020

For loops are better for performance.
While it might look fancy, I prefer performance xd

@rtm516
Copy link
Member

rtm516 commented Dec 9, 2020

Might also be better to name it GeyserConnector#getSessionByXboxUuid since that's more descriptive.
Also please add a JavaDoc comment to the method and if you can find the few for loops in the code that do this and replace them with this method. I think there are ones in SkinUtils.java and some of the commands

@janrabek
Copy link
Contributor Author

janrabek commented Dec 9, 2020

Might also be better to name it GeyserConnector#getSessionByXboxUuid since that's more descriptive.
Also please add a JavaDoc comment to the method and if you can find the few for loops in the code that do this and replace them with this method. I think there are ones in SkinUtils.java and some of the commands

I named this method getPlayer... because the corresponding methods are named "addPlayer" and "removePlayer". But I can easily change the name of it and replace the for loops.

@rtm516
Copy link
Member

rtm516 commented Dec 9, 2020

I named this method getPlayer... because the corresponding methods are named "addPlayer" and "removePlayer".

Ah your correct, leave the method name as-is for now then.

@janrabek
Copy link
Contributor Author

janrabek commented Dec 9, 2020

I named this method getPlayer... because the corresponding methods are named "addPlayer" and "removePlayer".

Ah your correct, leave the method name as-is for now then.

And just found out that nowhere the player is retrieved by the XUID, only by the UUID of the player entity. Should I create an extra method for that?

@rtm516
Copy link
Member

rtm516 commented Dec 9, 2020

And just found out that nowhere the player is retrieved by the XUID, only by the UUID of the player entity. Should I create an extra method for that?

If you don't mind, would make sense to roll it into this PR since they are very simmilar.

@janrabek
Copy link
Contributor Author

janrabek commented Dec 9, 2020

done

@rtm516
Copy link
Member

rtm516 commented Dec 9, 2020

Can you also add {} to some of the other if statements to follow the styling of the project?

@Tim203 Tim203 changed the title Added GeyserConnector#getPlayerByXboxUuid Added a simple way to get a player by their xuid Dec 9, 2020
@Camotoy Camotoy dismissed stale reviews from lukeeey and rtm516 December 17, 2020 16:52

Resolved

@Camotoy Camotoy merged commit 9f6182f into GeyserMC:master Dec 17, 2020
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.

5 participants