-
Notifications
You must be signed in to change notification settings - Fork 551
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
General Fix for Head Textures #4026
Conversation
Note: I do not believe the fix is completely comprehensive.
Pro Tip!
If your changes do not fall into any of these categories, don't worry. You can just ignore this message in that case! 👀 |
If the issue is in dough we will have to fix it on dough side. |
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.
Small review since i still think its better to be fixed on doughs side
import java.util.List; | ||
import java.util.Optional; | ||
import java.util.OptionalInt; | ||
import java.util.*; |
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.
dont star import
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.
apologies, my IDE did that. I've fixed it.
|
||
PlayerProfile profile = Bukkit.createPlayerProfile(RANDOM_UUID); // Create a new player profile | ||
PlayerTextures textures = profile.getTextures(); | ||
URL urlObject; |
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.
URL is deprecated please use URI
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.
Are you asking for me to make it a URI and then convert it to URL? <PlayerTextures>.setSkin only accepts a URL parameter.
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.
This is also a straight up breaking change, you are throwing an exception in getProfile, and then using that in a method that returns non null. Afaik previously it would send a message to log and give the Steve head if anything went wrong, now it will just stop completely which is a pretty big breaking change.
Slimefun preview buildA Slimefun preview build is available for testing! https://preview-builds.walshy.dev/download/Slimefun/4026/840e69d1
|
Have now fixed |
Thanks for the PR but this is getting superceded by #4027 |
Note: I do not believe the fix is completely comprehensive.
Description
I am making this pull request to fix the "Alex/Steve" heads issue that Slimefun currently has on spigot builds > 246. Heads are not being turned into their custom textures.
Proposed changes
I have changed the code to mainly stop using dough-api and instead to use plain Bukkit code to request the textures. I am aware that this probably not a comprehensive fix (meaning I did not remove every instance of DoughAPI's method calls. I just replaced the main SlimefunUtils function and the SlimefunItem function for custom textures), however, I hope this can serve as a base.
Related Issues (if applicable)
Resolves #4024
Checklist
Nonnull
andNullable
annotations to my methods to indicate their behaviour for null values