as title

currently PH displays "Spawned #bat's Head"
presumably there is a lang entry that has "Spawned _'s Head" where _ is the name passed

Head names are generally controlled by lang entries in the form "_ Head" (for mobs) or "_'s Head" (for players)

To fix this we would need a new lang entry for the spawn message in the form "Spawned _" to allow the above head name format to display correctly.

it might be nice if it also displayed the number of items spawned and who it was given to... (which it currently does not - heldover from legacy code)

as title

Wither skeleton drop code incorrectly reads the wither boss drop rate (witherdroprate) instead of the correct rate property (witherskeletondroprate) following corrections to entity type names in 1bffd3f

I'm not sure if this issue pre-existed this change or not, but after the fix, the wither skeleton should never drop a skull if witherskeletondroprate: 0.0 is set.

When you view the javadoc description for the API methods (for example, PlayerHeadsAPI interface), the @nullable annotation appears both above the method signature and before the return type for the method.

This doesn't seem to happen in other major javadocs and I'm not sure why this occurs.

as title, the input value for this config command is validated incorrectly, as STRINGLIST checks against Bukkit Material names (behavior for 'requireditems' config).

ignoredheadnames and ignoredheaduuids needs to be properly validated as possible using separate method, perhaps a separate config type.

depends on issue #15

ignoredheadnames,ignoredheaduuids require a restart to take effect, they should be updated when a reload is triggered by the user.

affects both 5.2.1 branch (5.2.3) and master branch (5.3 WIP)

as title

as title.

example:

    /**
     * Gets the version string for NMS & OBC class paths
     *
     * @return The version string of OBC and NMS packages
     * @author sainttx
     */
    public static String getVersion() {
        if (versionString == null) {
            String name = Bukkit.getServer().getClass().getPackage().getName();
            versionString = name.substring(name.lastIndexOf('.') + 1) + ".";
        }

        return versionString;
    }

reminder to myself

Duplicate heads are dropped despite settings to avoid this (when player drops and BH head drops are both enabled).

This issue can be avoided by disabling player-head drops on one of the two plugins, currently.

Recent changes do not affect this as it's assumed the BH plugin is intercepting the death event early, which puts this plugin in the sticky situation of changing its priority to be able to intercept it first - and last, which is not the best idea in design.

In 1.15, players who aggro neutral mobs are considered to be the killer, allowing head drops.
this means some forms of farms can farm heads now.

Some advanced checking needs to occur to block this new mojang "feature"

Reference:
#14 (also test null drops)
#4 (test behavior settings and modifier and headroll values)

• perform initial usage testing to discover issues.
• test #14
• test null drops
• test #4 chargedcreeperbehavior
• test witherskeletonbehavior
• test inbuilt/custom modifiers
• add testcases for modifier behavior !!!
• test multiple modifier support
• test multiple drop-item support
• test wiither delay support after change (and with modified items)
• test custom modifier support in external plugins
• test extension support API (when complete)

Add more villager's drop heads. What profession the villagers killed the head of that profession and you get.

Example:

Title says it all, this means that the bigger the slime, the player gets multiple chances.

There was never any code in the plugin detecting slime size, so this is original behavior.

I could detect slime size and suppress drops conditionally, and possibly make this optional as well if it's really an issue - or maybe it's just up to server admins to nerf the droprate?

as title

1.17 support needs to be added eventually.
4 new mob heads to add:

• Axolotl
• Glow squid
• Goat
• Warden

pending head texture availability.

From the original code:

        if (plugin.configFile.getBoolean("pkonly") && ((killer == null) || (killer == player) || !killer.hasPermission("playerheads.canbehead"))) {
            return;
        }

So if pkonly is disabled, then a player can behead regardless of their permission - is this supposed to be intended behavior?

see title

checking externalheadbehavior and iscustomhead in PlayerHeadsListener is now redundant because SkullConverter checks this when detecting the head type.

The API method also now rejects custom and external heads

as title

[11:31:00 ERROR]: Could not pass event BlockBreakEvent to PlayerHeads v5.3.0-SNAPSHOT
java.lang.NullPointerException: null
        at com.github.crashdemons.playerheads.DropManager.requestDrops(DropManager.java:62) ~[?:?]
        at com.github.crashdemons.playerheads.DropManager.requestDrops(DropManager.java:69) ~[?:?]
        at com.github.crashdemons.playerheads.DropManager.blockDrop(DropManager.java:114) ~[?:?]
        at org.shininet.bukkit.playerheads.PlayerHeadsListener.onBlockBreak(PlayerHeadsListener.java:395) ~[?:?]
        at com.destroystokyo.paper.event.executor.MethodHandleEventExecutor.execute(MethodHandleEventExecutor.java:37) ~[patched_1.15.1.jar:git-Paper-25]
        at co.aikar.timings.TimedEventExecutor.execute(TimedEventExecutor.java:80) ~[patched_1.15.1.jar:git-Paper-25]
        at org.bukkit.plugin.RegisteredListener.callEvent(RegisteredListener.java:70) ~[patched_1.15.1.jar:git-Paper-25]
        at org.bukkit.plugin.SimplePluginManager.callEvent(SimplePluginManager.java:545) ~[patched_1.15.1.jar:git-Paper-25]
        at net.minecraft.server.v1_15_R1.PlayerInteractManager.breakBlock(PlayerInteractManager.java:313) ~[patched_1.15.1.jar:git-Paper-25]
        at net.minecraft.server.v1_15_R1.PlayerInteractManager.a(PlayerInteractManager.java:272) ~[patched_1.15.1.jar:git-Paper-25]
        at net.minecraft.server.v1_15_R1.PlayerInteractManager.a(PlayerInteractManager.java:240) ~[patched_1.15.1.jar:git-Paper-25]
        at net.minecraft.server.v1_15_R1.PlayerConnection.a(PlayerConnection.java:1319) ~[patched_1.15.1.jar:git-Paper-25]
        at net.minecraft.server.v1_15_R1.PacketPlayInBlockDig.a(SourceFile:40) ~[patched_1.15.1.jar:git-Paper-25]
        at net.minecraft.server.v1_15_R1.PacketPlayInBlockDig.a(SourceFile:10) ~[patched_1.15.1.jar:git-Paper-25]
        at net.minecraft.server.v1_15_R1.PlayerConnectionUtils.lambda$ensureMainThread$0(PlayerConnectionUtils.java:23) ~[patched_1.15.1.jar:git-Paper-25]
        at net.minecraft.server.v1_15_R1.TickTask.run(SourceFile:18) ~[patched_1.15.1.jar:git-Paper-25]
        at net.minecraft.server.v1_15_R1.IAsyncTaskHandler.executeTask(IAsyncTaskHandler.java:136) ~[patched_1.15.1.jar:git-Paper-25]
        at net.minecraft.server.v1_15_R1.IAsyncTaskHandlerReentrant.executeTask(SourceFile:23) ~[patched_1.15.1.jar:git-Paper-25]
        at net.minecraft.server.v1_15_R1.IAsyncTaskHandler.executeNext(IAsyncTaskHandler.java:109) ~[patched_1.15.1.jar:git-Paper-25]
        at net.minecraft.server.v1_15_R1.MinecraftServer.aZ(MinecraftServer.java:1037) ~[patched_1.15.1.jar:git-Paper-25]
        at net.minecraft.server.v1_15_R1.MinecraftServer.executeNext(MinecraftServer.java:1030) ~[patched_1.15.1.jar:git-Paper-25]
        at net.minecraft.server.v1_15_R1.IAsyncTaskHandler.awaitTasks(IAsyncTaskHandler.java:119) ~[patched_1.15.1.jar:git-Paper-25]
        at net.minecraft.server.v1_15_R1.MinecraftServer.sleepForTick(MinecraftServer.java:1014) ~[patched_1.15.1.jar:git-Paper-25]
        at net.minecraft.server.v1_15_R1.MinecraftServer.run(MinecraftServer.java:937) ~[patched_1.15.1.jar:git-Paper-25]
        at java.lang.Thread.run(Thread.java:748) [?:1.8.0_171]

Ref:

Code incorrectly assumes drop can be performed without an event and without a location.
Later method reinforces this belief. need rewrite to enforce either an event or location being provided.

see https://pastebin.com/5hwdYYT8

Happens because the listener for PlayerInteractEvent passes an string array of 1 null object to formatMsg/format.

Confirmed with testing

This is most likely caused by this line

However, a previous line in the block-break handler expects it to unconditionally drop the head (the event was already cancelled. The blockdrop method returns false for other situations, so there is no clear messaging of the failure reason back to the handler.

possible options:

• events for head type determination (from entity, blockstate, itemstack)
• an option is to shim getCompatibleName and detect custom-format names for special logic
• reserved head enum type for existing API results (check via HeadType.isCustomHead() ?)
• registering custom uuiid/texture entries - necessary at all?
• creation of heads from custom ID string
• other ideas?

Core needs:

• hook head detection and present a custom head drop (with complete meta) based on third-party plugin logic on the entity/blockstate (death event / blockbreak)
• hook head detection for blockstate/itemstack to allow third-party plugin logic to modify the displayed name of the head (interact / itemspawn fixing)
• spawn command # hook (+rename)
• API methods to detect/get third-party heads
• Event changes to detect/get third-party head type
• localization and configuration access for custom heads? possibly handled by the third-party plugin itself.

p = 0.004*(1+0.2*(3+2)), n=400, C=1-(1-p)^n
C = ~96%
no drops regularly with high probability, need to perform verification of:

• droprate modification
• roll results.

Head rotation works alright on 1.13+
Head rotation on 1.8 does not.

Some tests:

/ph setblock ~ 0 84 0 #skeleton north floor results in a wall-head facing north
/ph setblock ~ 0 84 0 #skeleton east floor results in a floor-head facing northeast
/ph setblock ~ 0 84 0 #skeleton south floor (result: floor east-southeast)
/ph setblock ~ 0 84 0 #skeleton west floor (result: floor west-southwest)
/ph setblock ~ 0 84 0 #skeleton south_south_west floor (result: floor south-south-west - CORRECT)

And concerningly, some results depended on the prior state:
/ph setblock ~ 0 84 0 #skeleton south wall (after north floor: north wall unchanged result)
/ph setblock ~ 0 84 0 #skeleton south wall (after south floor: east floor)

Some positions were indeterminant / updated only after a prior position update.
same behavior with a username head and custom-textured head.

Logs:
north floor

[12:26:12 INFO]: crashdemons issued server command: /ph setblock ~ 0 84 0 #skeleton north floor
[12:26:12 INFO]: [PlayerHeads] setblock FLOOR NORTH
[12:26:12 INFO]: [PlayerHeads] setblock profile null null null
[12:26:12 INFO]: SkullDetails setblock - got material SKULL for attachment FLOOR rot NORTH
[12:26:12 INFO]: LegacyData set on block NORTH FLOOR => FLOOR
[12:26:12 INFO]: MaterialData set to rotation NORTH
[12:26:12 INFO]: SkullState set to rotation NORTH
[12:26:12 INFO]: SkullType set to rotation SKELETON
[12:26:12 WARN]: [PlayerHeads] Blockstate wasn't a skull or profile wasn't valid true false

south floor

[12:27:08 INFO]: crashdemons issued server command: /ph setblock ~ 0 84 0 #skeleton south floor
[12:27:08 INFO]: [PlayerHeads] setblock FLOOR SOUTH
[12:27:08 INFO]: [PlayerHeads] setblock profile null null null
[12:27:08 INFO]: SkullDetails setblock - got material SKULL for attachment FLOOR rot SOUTH
[12:27:08 INFO]: LegacyData set on block SOUTH FLOOR => FLOOR
[12:27:08 INFO]: MaterialData set to rotation SOUTH
[12:27:08 INFO]: SkullState set to rotation SOUTH
[12:27:08 INFO]: SkullType set to rotation SKELETON
[12:27:08 WARN]: [PlayerHeads] Blockstate wasn't a skull or profile wasn't valid true false

south wall (after north floor)

[12:31:56 INFO]: crashdemons issued server command: /ph setblock ~ 0 84 0 #skeleton south wall
[12:31:56 INFO]: [PlayerHeads] setblock WALL SOUTH
[12:31:56 INFO]: [PlayerHeads] setblock profile null null null
[12:31:56 INFO]: SkullDetails setblock - got material SKULL for attachment WALL rot SOUTH
[12:31:56 INFO]: LegacyData set on block SOUTH WALL => WALL_SOUTH
[12:31:56 INFO]: LegacyData did not need additional tile data
[12:31:56 INFO]: SkullType set to rotation SKELETON
[12:31:56 WARN]: [PlayerHeads] Blockstate wasn't a skull or profile wasn't valid true false

south wall (after south floor) - final direction was different than above.

[12:35:12 INFO]: crashdemons issued server command: /ph setblock ~ 0 84 0 #skeleton south wall
[12:35:12 INFO]: [PlayerHeads] setblock WALL SOUTH
[12:35:12 INFO]: [PlayerHeads] setblock profile null null null
[12:35:12 INFO]: SkullDetails setblock - got material SKULL for attachment WALL rot SOUTH
[12:35:12 INFO]: LegacyData set on block SOUTH WALL => WALL_SOUTH
[12:35:12 INFO]: LegacyData did not need additional tile data
[12:35:12 INFO]: SkullType set to rotation SKELETON
[12:35:12 WARN]: [PlayerHeads] Blockstate wasn't a skull or profile wasn't valid true false

matters are probably complicated by having 3 types of rotation:
A. raw block datavalue (LegacyHeadData) - ref https://bukkit.org/threads/setting-blocktype-to-skull-places-a-bugged-skull-fix.145637/

        FLOOR(0x1,true),// On the floor (rotation is stored in the tile entity)
        WALL_NORTH(0x2,false),// On a wall, facing north
        WALL_SOUTH(0x3,false),// On a wall, facing south
        WALL_EAST(0x4,false),// On a wall, facing east
        WALL_WEST(0x5,false)// On a wall, facing west

B. Skull (MaterialData, Directional) : setFacingDirection
C. Skull (BlockState) : setRotation

note that the latter two are only changed when the attachment is FLOOR, since it was indicated by a few posts that orientation was only needed on Floor heads...
so if this actually needs to be updated when not floor, that would explain a lot

Looking through the code, it looks like the intended behavior was to remove vanilla wither skeleton skull drops (even from charged creepers) - and replace them with custom drop rates.

However, creeper, skeleton, and zombie vanilla drops (using a charged creeper) are untouched by the original code.

So the question here is - should the plugin also ignore charged-creeper kills of wither skeletons? should there be an option to disable charged-creeper drops?
(currently <0 should not modify witherskeleton drops at all if it can be loaded, 0 would remove all witherskeleton drops, >0 would replace vanilla drop rate)

https://github.com/crashdemons/PlayerHeads/tree/feature-setblock-2
https://github.com/crashdemons/PlayerHeads/tree/91187fe431a9f107ac43c7ef72959e2761b8e446

/ph setblock ~ 0 85 0 #skeleton north wall

Caused by: java.lang.IllegalArgumentException: Name and ID cannot both be blank
        at com.mojang.authlib.GameProfile.<init>(GameProfile.java:25) ~[spigot-1.8.jar:git-Spigot-550ebace-7019900e]
        at com.github.crashdemons.playerheads.compatibility.craftbukkit.CompatibleProfileCB.createInternalObject(CompatibleProfileCB.java:36) ~[?:?]
        at com.github.crashdemons.playerheads.compatibility.craftbukkit.CompatibleProfileCB.toInternalObject(CompatibleProfileCB.java:61) ~[?:?]
        at com.github.crashdemons.playerheads.compatibility.craftbukkit.ProfileUtils.setProfile(ProfileUtils.java:59) ~[?:?]
        at com.github.crashdemons.playerheads.compatibility.craftbukkit_1_8.Provider_craftbukkit_18.setCompatibleProfile(Provider_craftbukkit_18.java:73) ~[?:?]
        at org.shininet.bukkit.playerheads.InventoryManager.setBlock(InventoryManager.java:111) ~[?:?]
        at org.shininet.bukkit.playerheads.PlayerHeadsCommandExecutor.onCommandSetblock(PlayerHeadsCommandExecutor.java:123) ~[?:?]
        at org.shininet.bukkit.playerheads.PlayerHeadsCommandExecutor.onCommand(PlayerHeadsCommandExecutor.java:407) ~[?:?]
        at org.bukkit.command.PluginCommand.execute(PluginCommand.java:44) ~[spigot-1.8.jar:git-Spigot-550ebace-7019900e]

for example: ""crashdemons's head"
because this is controlled by lang formatting, we will have to added a separate _S lang entries to do this.

Wither Boss head does drop, but is incorrectly named as "Wither Skeleton Skull" just like the vanilla Wither Skeleton Skull item, but is a different color name and has the correct head texture.

lang.properties:

HEAD_WITHER=Wither Skeleton Skull
HEAD_WITHER_BOSS=Wither Boss Head

Judging by the code of the default case of onEntityDeath,
EntityType.name() is being checked for an entry in the lang file and checks HEAD_<entityname> and <entityname without _>droprate
from the spigot 1.13 api:

WITHER,
WITHER_SKELETON

So,
WITHER (boss) dies -> loads name from HEAD_WITHER -> loads rate from witherdroprate
WITHER_SKELETON dies (with old code) -> loads name from HEAD_WITHER_SKELETON -> loads rate from witherskeletondroprate

as title

Me and the ops on my server have all perms because it makes things easier but there is no way to turn this off with it being part of the perms instead of the config. Can you please add this so people with all perms don't just behead everything? We are currently using minecraft version 1.14.4.

because of the number of modifiers, we should implement them in a list, not hardcoded
maybe

private HashMap<String name, double value> modifiers;

void recordModifier(name, double value)
double getModifier(name)

instead of dropping, give directly to killer

using the config-set command forces the input value to be lowercase, which is not desirable for head names in 'ignoredheadnames'

pigzombie in 1.13,16 and cats in 1.13,1.14,1.16
profile changes across the board in craftbukkit and glowstone.

it determines the valid directions, and it is first in internal standards, so it would be easier to make the command be
/ph setblock ... WALL SOUTH rather than the reverse.

Multiple items should be permitted to drop from headdrop events similar to entitydeath

for example:
List getDrops()
void setDrops(List)

modification may be possible through the list reference directly, in which case it may need to be concurrent.

effective roll/rate values should represent the roll or rate responsible for the roll's success or failure in most cases - but plugins that modify success are unable to report their changes.

together with #16 this would allow better third-party plugin reporting of roll modifications.

see title

18.07 17:36:39 [Server] INFO [PlayerHeads-stat-sim] non-test BEFORE Head Roll:  killer=crashdemons target=Wandering Trader success=false droprateO=0.05 droprollO=0.6948116462349225 droprateE=0.05 droprollE=0.6948116462349225
18.07 17:36:39 [Server] INFO [PlayerHeads-stat-sim] non-test AFTER Head Roll:  killer=crashdemons target=Wandering Trader success=false droprateO=0.05 droprollO=0.6948116462349225 droprateE=0.05 droprollE=0.6948116462349225

killing a wanderingtrader with wanderingtraderdroprate: 0.05 and lootingrate: 0.2
we see that the effective droprate is still 0.05
PlayerHeads 5.2.10, TrophyLuckModifier 0.1.1

from event records for HeadRollEvent, looting doesn't change the rate anymore

18.07 17:42:58 [Server] INFO [PlayerHeads-stat-sim] non-test BEFORE Head Roll:  killer=crashdemons target=Wandering Trader success=false droprateO=0.05 droprollO=0.8568048274890377 droprateE=0.05 droprollE=0.8568048274890377
18.07 17:42:58 [Server] INFO [PlayerHeads-stat-sim] non-test AFTER Head Roll:  killer=crashdemons target=Wandering Trader success=false droprateO=0.05 droprollO=0.8568048274890377 droprateE=0.05 droprollE=0.8568048274890377

using a luck item doesn't seem to change the effective droprate either with TrophyLuckModifier which is odd because it explicitly sets the droprate value.

I need to test also with TrophyLuckModifier 0.1.2 and with debugging

ref #52

new issue to indicate the lower priority nature of this

at title, need a way to get the set of all supported head types from the API.

Note: This issue does not affect the master branch / the plugin. This is an internal issue of a Work-in-progress feature which will not be released until this issue is fixed.

Common-support cannot see getBlockMaterial in implementations during compilation despite no warnings from the editor.

the method is defined in the interface and the class is abstract so there should be no issue: other APIs do the same thing without problems. (perhaps defining an abstract method also would help?)

Modern-support also has an error overriding the getBlockMaterial method despite pulling in the correct compatibility-api version... perhaps either from transitive deps or from common-support not being built yet.