search

Serilum / .issue-tracker

Tracks all issues for Serilum's Minecraft mods on CurseForge and Modrinth.
https://serilum.com/
153 stars 40 forks source link

Collective (randomly?) breaks Keep Head Names #2490

Open Fourmisain opened 1 week ago

Fourmisain commented 1 week ago

Information

Minecraft version: 1.20.1 Modloader: Fabric Fabric loader version: 0.15.11 Environment: Singleplayer and Multiplayer

First mod name: Collective First mod version: 7.70

Second mod name: Keep Head Names Second mod version: 1.5.1

Description

Original issue: https://github.com/Fourmisain/KeepHeadNames/issues/5

Keep Head Names is a bugfix mod of mine which is supposed to retain a player head's display name and lore tags when placing and mining the head. With Collective (and Fabric API) installed, Keep Head Names appears to partially or completely break - not sure what exactly that depends on.

To hopefully reproduce the issue, give yourself a player head with display name and lore tag: /give @s minecraft:player_head{display:{Name:"{\"text\":\"Pink Panther\"}",Lore:['[{"text":"Might or might not dye his hair"}]']},SkullOwner:"dfgsteam"} 1

Place the head down and look at the block entity NBT with the /data get block <coords> command. It should have a CustomName and Lore subtag, like so: blockdata

Breaking the head should give back the original head, with the "Pink Panther" name and lore text, but this does not (always?) happen when Collective is installed.

In the original issue, the head had no display name, a SkullOwner of jeb ("jeb's head") and some lore. The lore wasn't retained and weirdly "jeb_'s head" turned italics, meaning it became a display name. This might only happen in combination with some other mod, I had it happen too, but now I can't reproduce that one either, even with all mods turned on. The original reporter also did some more tests and it appeared to work just fine there, leading me to believe that this whole issue might be random, maybe changing with restarts, number of mods (load order, mixin order) etc.

I scanned through your mixins, but can't see an obvious conflict. Keep Head Names only modifies PlayerSkullBlock and SkullBlockEntity, registers its own loot function and overwrites the player_head loot table via a datapack.

Fourmisain commented 1 week ago

I just disabled all of Collective's mixins and the issue still happens. Only when I disable the mod's entry point does the issue disappear.

Fourmisain commented 1 week ago

Got it, Collective registers a default event which replaces mined player heads: https://github.com/Serilum/Collective/blob/a018ac406d027e935cd046802614b2aefb7e1ab5/Common/src/main/java/com/natamus/collective/events/CollectiveEvents.java#L281-L283

HeadFunctions also incorrectly sets a display/custom name instead of the skull owner, explaining why the head names have italics (plus the "'s head" won't be translated in non-English languages and the name can be completely removed by anvil): https://github.com/Serilum/Collective/blob/a018ac406d027e935cd046802614b2aefb7e1ab5/Common/src/main/java/com/natamus/collective/functions/HeadFunctions.java#L62-L63

What is the purpose of this event? There might be a more direct/compatible way of doing what it does.

Fourmisain commented 1 week ago

HeadFunctions also incorrectly sets a display/custom name instead of the skull owner, explaining why the head names have italics (plus the "'s head" won't be translated in non-English languages and the name can be completely removed by anvil):

This also happens in 1.21 without any other mods, so it's not just a compatibility thing. (Equivalent of SkullOwner is the profile component)