Skip to content
This repository has been archived by the owner on Aug 31, 2019. It is now read-only.

Fix skull items not displaying skins #146

Open
wants to merge 1 commit into
base: real1.8
Choose a base branch
from
Open

Fix skull items not displaying skins #146

wants to merge 1 commit into from

Conversation

ShinyDialga
Copy link
Contributor

In 1.8, they added player skull previews to the inventory item. Currently, if you use SkullMeta in order to have a custom SkullOwner, it'll appear in the inventory as a steve skin (it looks like the SkullOwner when placed down though). This patch uses TileEntitySkull.b() to update the skin, and it won't update skin inventory items if it is disabled in bukkit.yml (fetch-skulls) because of the base b method.

Before patch:
image

After patch:
image

The skulls in the screenshots use the same method (SkullMeta setSkullOwner()), and both have the fetch-skulls setting enabled in bukkit.yml. Thanks.

@tonybruess
Copy link
Member

So this PR respects the fetch-skins setting? These skins are populated when the world (er, chunk) is loaded?

@ShinyDialga
Copy link
Contributor Author

Yes, if fetch-skins is set to false, it won't render the skins. It looks like this:
image

The skins are loaded when they're called in the SkullMeta.

@Yukon
Copy link
Member

Yukon commented May 21, 2015

There are a few lines that remove this which is unnecessary.

@ShinyDialga
Copy link
Contributor Author

Sorry about that, wasn't sure if it was necessary as I wanted it the same throughout. I'll see if I can fix it later.

@ryanwarsaw
Copy link

@tonybruess @jedediah Would it be possible to get this merged? I'd like to file a PR after this gets merged to expand on the current Skull API to support setting Skull owners by UUID rather than only by usernames.

@jedediah
Copy link
Member

If the unnecessary diffs get fixed, I'll merge it.

import net.minecraft.server.GameProfileSerializer;
import net.minecraft.server.NBTTagCompound;

+import net.minecraft.server.TileEntitySkull;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be using fully qualified names rather than imports for patches

@ShinyDialga
Copy link
Contributor Author

Feel free to do whatever with this patch. I don't even know if I have the code anymore. I didn't look hard, but maybe you could find what Spigot used to fix this as theirs works.

@ryanwarsaw
Copy link

@ShinyDialga If you're cool with this, I can file a new PR with these changes + diffs and also take a look at how Spigot fixes this issue, this weekend.

@ShinyDialga
Copy link
Contributor Author

Go for it

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants