Player Heads

Player Heads

897k Downloads

Keep player head skin

ehetteNandaYo opened this issue ยท 10 comments

commented

Describe the solution you'd like
When players change their skin, the head also changes the skin, It would be better if it saved the head skin forever, maybe add a setting for those who dont want this behavior?

commented

@rayzr522

You may have already found out, but I wanted to mention that, much to my displeasure, the newest Authlib versions now disallow passing null as the username when creating a GameProfile (the data structure that carries the head UUID/name/texture) - so I'm probably going to go the way of DropHeads and HeadDB plugins where the username will be "PlayerHeads:uuid".

As the attached commit shows, ":" is what I'm filtering-out at the moment when detecting an actual player-named head.

I'm not sure why they decided on this change since nameless heads seem like they would make perfect sense for a head that is not attached to an account. I'm hoping the invalid head name + uuid will reduce the amount of Mojang API calls servers make to resolve heads, but I worry about that.

commented

hey, author of DecoHeads here -- i see you modify the output of BlockBreakEvents so that the dropped item has the correct name, lore, etc. when you break a placed PlayerHead. i also see you have an option convertvanillaheads in the config which appears to default to false, yet looking at your BlockBreakEvent listener, I don't see this option being used anywhere.

would it be possible to make this option or some other option that simply prevents custom handling of BlockBreakEvents? or, perhaps even better, fetches the GameProfile from the block's state (I think you can do this) and then uses this information directly to set the GameProfile on the item? custom GameProfile data is how plugins like DecoHeads use custom textures without associating usernames

would be nice to have a way to make your plugin play nice with other plugins which deal in custom heads :)

EDIT: I'm not sure if it's as easily accessible in all versions, but I just checked a 1.8 JAR i had lying around, and it looks like this is trivial to do:

public class CraftSkull extends CraftBlockState implements Skull {
  private static final int MAX_OWNER_LENGTH = 16;
  
  private final TileEntitySkull skull;
  
  private GameProfile profile;

and then there are pretty easy version-independent ways to set GameProfiles on skull items with reflection so you could preserve the GameProfile itself when creating the custom item to drop

commented

hey, author of DecoHeads here -- i see you modify the output of BlockBreakEvents so that the dropped item has the correct name, lore, etc. when you break a placed PlayerHead. i also see you have an option convertvanillaheads in the config which appears to default to false, yet looking at your BlockBreakEvent listener, I don't see this option being used anywhere.

'convertvanillaheads' is used by admins that want PH to use all-custom or mixed-vanilla-custom heads (depending on dropvanillaheads) - (the default behavior is to only use custom heads to extend the existing head list)

If you look in the method you linked, you will see it calls blockDrop which can modify the dropped item.
blockDrop itself calls createConvertedMobhead and by extension canConversionHappen which does check the "convertvanillaheads" as a condition for using nonvanilla head

createConvertedMobhead is the same method used in the ItemSpawn listener so that item conversion either form block-breaking or normal item drops is the same (players dropping - or even pistons/water popping head blocks, which historically was not capturable despite being an event now).

would it be possible to make this option or some other option that simply prevents custom handling of BlockBreakEvents? or, perhaps even better, fetches the GameProfile from the block's state (I think you can do this) and then uses this information directly to set the GameProfile on the item? custom GameProfile data is how plugins like DecoHeads use custom textures without associating usernames

For nonvanilla mobheads, PH pretty much relies on a reserved-UUID (unused UUID) system for heads and no username. getOwner() generally fails for these types of heads, and this seems to be the same way things like the MCHeads db do custom heads.
Beginning with version 4.x, username-based mobheads were phased out for texture/uuid pairs and in 5.x support for username-mobheads was removed entirely - so PH doesn't associate usernames... except when the head is legitimately owned by a user, which was the intended purpose I think. If we want to convert a user's head into a custom one with no name it should be possible with some work, but it wouldn't be recognizable in any other plugin if you change both name/uuid.
If you're having a conflict with your/other plugins, I'd be interested in specific examples.

For re-setting the profile should be possible, I think. I believe both methods were already tested and coded into the ProfileUtils class. (PlayerHeads-craftbukkit-support)
However a couple concerns first:

  • If I remember correctly, Paper-api (at one time) didn't support the profile methods for either item or block (I can't remember which now though). This would only affect GlowstoneMC support though which... is it even an issue?

  • In the blockbreak event there are only a couple different outcomes:

  1. it thinks the head is a Player's head with a name/player owner. [in which case it is replaced with a styled version] (decoration shouldn't fall into this category - but maintaing the profile would improve it)
  2. it thinks it's a custom head without a known player and is not a PH-reserved UUID. [in which case it is completely ignored](decoration should fall into this category if you are giving it an unused UUID and no username).
  3. it thinks the head is a mobhead [ie: it has a PH-reserved UUID or it is a vanilla mob head item]. [In this case the head is 'converted' if it is either custom or convertvanillaheads is forced.] This last one has been an issue for some plugins that make decorated/renamed versions of vanilla items
  • there is a slight issue with disabling all blockbreak events/itemspawn events in that items dropped from blockbreaks and water/pistons effectively lose their display meta (in this case, I'm only concerned about name/lore). There could be a setting for it, but a broken PH head would remain broken unless you choose some other event for updating it.

that said, resetting the game profiile like you mention should be possible to prevent re-retrieving the skin for players (like in OPs concern - detecting custom heads is a different issue), I didn't think of it saving the block profile for later, but that's totally doable.

would be nice to have a way to make your plugin play nice with other plugins which deal in custom heads :)

EDIT: I'm not sure if it's as easily accessible in all versions, but I just checked a 1.8 JAR i had lying around, and it looks like this is trivial to do:

public class CraftSkull extends CraftBlockState implements Skull {
  private static final int MAX_OWNER_LENGTH = 16;
  
  private final TileEntitySkull skull;
  
  private GameProfile profile;

and then there are pretty easy version-independent ways to set GameProfiles on skull items with reflection so you could preserve the GameProfile itself when creating the custom item to drop

indeed and I use an implementation already because of textures support and issues with bukkiti api owner resolution.
https://github.com/meiskam/PlayerHeads/blob/master/PlayerHeads-craftbukkit-support/src/main/java/com/github/crashdemons/playerheads/compatibility/craftbukkit/ProfileUtils.java
This has been tested on craftbukkit builds from 1.8 to 1.16.3 without issue. Apparently authlib and profile fields haven't changed very much as long as you don't use the rare first-day server build with broken mapping.

I should be able to modify it to do what you suggest, I think.

So again, two takeaways I see:

  • Maintaining a profile for Player's heads is something I can work on implementing in PH-core.
  • There may be some compatibility issues you allude to which may be mis-detected by PH as something that wipes out the profile when it shouldn't be doing so. I need more information in this case.

EDIT: Although I will comment though that even when heads are not from the plugin, and their profile is maintained, their texture can still expire in line with the timestamp field. Although it may(?) be possible to update the timestamp to delay skin updates.
EDIT 2: Paper-API in GlowstoneMC does not support a method for getPlayerProfile for Skull (blockstate), so it is not possible to get that. (Paper-api 1.12.2-R0.1-SNAPSHOT / Glowkit 1.12.2-R0.1-SNAPSHOT / Glowstone 2018.10.0-SNAPSHOT) - the profile field I believe may not be present since this is a novel implementation of paper-api.
EDIT 3: Correction for convertvanillaheads behavior - dependent on another setting.

commented

Here is a development build that adds two configuration options:

  • "fixbrokenheads" (if you set this to false it disables blockbreak actions - similar to the existing "fixdroppedheads") [default: true]
  • "restoreprofile" (if this is enabled, it reads the block/item's gameprofile before replacing any item and attempts to set the profile on the new item replacement), though testing with a changed skin is a little difficult because of caching. (this should do absolutely nothing on GlowstoneMC) [default: true]

I don't notice any immediate big change (or benefit) and this has not been tested heavily, but I can tell you that if "fixdroppedheads" and "fixbrokenheads" are both disabled, head-blocks that are broken will just become undecorated "Player Head" (but retain the texture/uuid).

Dev build/CI: https://ci.meme.tips/job/PlayerHeads-5.x/1088/
Code snapshot: https://github.com/crashdemons/PlayerHeads/tree/43d89a04326710e5bcd1ce1ce17db5beda19e496

The easiest way to implement the second feature was to set the profile on the meta right before setting the meta on the item [1][2] - hopefully wiping out the previous owner information with the restored version, but I still need the verify the behavior.

commented

Regarding issues with PH and other plugins, I don't know if you were referring to your own plugin or not, but when I test DecoHeads, you appear to set the SkullOwner.Name tag with your item-name. See below:

image
This is not the displayname/customname, but the owner username. This was generated with only DecoHeads on the server.

SkullOwner.Name is typically blank for custom heads, and reserved for putting usernames into.
This is what confuses PH because it thinks (because a username exists on the head), it thinks it belongs to a user - and PH is a plugin which handles Player drops also, not just mob drops.
I don't use the username to determine the texture of mobs/custom-heads, but when a player dies and drops their own head, their own head should have the correct username in it...

I don't see a way to avoid the issue outside of:

  • looking up the name/uuid and see that they don't match (that would then break ChangeSkin head detection).
  • getting a list of all of your custom head UUIDs and blacklisting them in PH so I ignore them (which I don't really want to do).
    I don't think you would need to set the Name field though - you already know the name if you can match the UUID to your plugin's list... the username field isn't the right place for it though IMO.

Aside:
In this line https://github.com/Rayzr522/DecoHeads/blob/master/src/main/java/me/rayzr522/decoheads/util/CustomHead.java#L57 you construct the GameProfile for a head using the UUID and Username, but the username you pass is your item name https://github.com/Rayzr522/DecoHeads/blob/2b50ac3be36f04bfd12257db0562b316c3e58bde/src/main/java/me/rayzr522/decoheads/data/Head.java#L144
Setting a name is not required at all - refer to this thread: https://www.spigotmc.org/threads/using-custom-head-textures.311562/#post-2947987 which uses a null name.

commented

I won't guarantee it fixes all issues, but it should fix the issue where PH changes DecoHeads into "Microwave's Head" etc. I didn't notice any other immediate issues though.

So I think that's a good start.

Regarding HeadDB, I'll admit I haven't looked at their code yet (it's a paid plugin), but their website (minecraft-heads.com, similar to freshcoal) presents the generated commands without a name field. (and the resulting head does not have one set)

commented

I see, I originally copied the custom head code from mrCookieSlime, and was not aware that you could simply pass null for the name. that actually makes things a good bit easier :) I've opened a corresponding issue to address this: rayzr522/DecoHeads#30

so assuming I make that change, along with the new options present here, this should eliminate any compatibility issues? at least with decoheads, I'm unsure of how heads database handles custom heads or whether or not this will work for them directly.

commented

unfortunately this behavior is controlled by your server - PlayerHeads does not update the head texture for players, your server does - and there isn't usually much configuration for that. Somewhat ironically, a lot of people have the reverse problem also - the server failing to update head skins.

This will happen as long as the head's name or UUID match the user - and if you change it, it would no longer be their head...

commented

huh, so there is no way to make it work like the plugin head database, which has tons of heads which I think they never change?

commented

If the head is linked to a name/uuid of a player they will change (PH player drops) - it shouldn't matter the plugin.

but PH mob heads / custom heads (vanilla or custom UUID/texture) won't change since there is no mojang profile to download.

It's important to distinguish between a player's head and a custom head since they have different behaviors.

I imagine the head database plugin handles both player-linked (updating) and custom heads.

It's possible to make a 'custom' head with the old texture for a player, but with different UUID. However the plugin would need to generate and store a log of UUIDs for each old head/texture to be recognized the same and prevent updating - and they wouldn't stack or work in villager trades with more updated heads.