The bug
Goat horns with any NBT data but without the instrument
NBT data fails to play.
To reproduce
/give @p goat_horn
Rename the given goat horn using an anvil.
Expected result
The goat horn would still play.
Observed result
The goat horn no longer plays.
Original description
I named my Goat Horn using an anvil after that, I tried using it, but I can't blow it anymore. No sound comes out, nor does the animation for it work. It's just a useless object now.
Code analysis
Code analysis by @unknown can be found in this comment.
Attachments
Comments 11
I am also not able to reproduce this issue. Please attach a video of this issue occurring with the F3 screen enabled.
I can reproduce this issue; it requires a /give-en goat horn without additional NBT data. You can blow it before renaming, but not after.
The actual cause is not the renaming though: If ANY NBT in the tag tag is present, but not the instrument string, it fails to play anything.
example:
You can play it: /give @s goat_horn
You can't play it: /give @s goat_horn{test:1b}
Note: That even the instrument in the lore disappears.
That's weird, I tried it again and now I was able to reproduce. Maybe it was because I was in creative mode and now I'm in survival?
No, I was in creative when I tried it. It's absolutely only the case for command-given ones without instrument.
I suggest updating the report to address the actual issue (goat horns with NBT, but without instrument do not default to Ponder, and fail to play).
Code analysis (1.19 yarn):
This is due to a misunderstanding on what a valid identifier/resource location is. Identifier#tryParse
returns null
for invalid identifiers, such as ones with non-ASCII characters. The problem here is that empty string is considered valid identifier. Thus Identifier.tryParse("") != null
. I don't think this is intended.
Let's look at GoatHornItem
:
GoatHornItem.java
private Optional<RegistryEntry<Instrument>> getInstrument(ItemStack stack) {
NbtCompound lv = stack.getNbt();
if (lv != null) {
Identifier lv2 = Identifier.tryParse(lv.getString("instrument"));
// problematic code
if (lv2 != null)
return Registry.INSTRUMENT.getEntry(RegistryKey.of(Registry.INSTRUMENT_KEY, lv2));
}
/* code for getting first instrument in tag, the fallback behavior */
}
NbtCompound#getString
returns an empty string if there is no string with the key in the compound. If the stack does not have NBT, the earlier if block will not be executed, which makes reproduction not possible in such cases. Finally, Registry#getEntry
is returned, which returns Optional#empty
for unknown/unregistered registry keys. Since no instrument has an empty id, this will return an empty optional.
There are two ways to fix this bug: one is to replace lv != null
with lv != null && lv.contains("instrument", NbtElement.STRING_TYPE)
to check for the existence of the key in addition to existence of NBT. The other fix is to make empty string IDs/ID paths invalid. Either one should fix the bug.
I'd say empty ID should be invalid as well, but, a specific input to have a non-playable goat horn is an interesting thought mojang might want to consider (in which case, I'd suggest a proper ID for it (eg "none") rather than an empty string).
Just my 2 cents on the matter.
It works for me