mojira.dev
MC-252773

Goat Horn without instrument NBT and with other NBT data (such as text) does not play

The bug

Goat horns with any NBT data but without the instrument NBT data fails to play.

To reproduce

  1. /give @p goat_horn

  2. 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.

1 more comments

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.

Can confirm in 1.19.1.

Can confirm in 1.19.2.

sappheric

Panda4994

Confirmed

Low

Items

goat-horn

1.19, 1.19.1 Pre-release 1, 1.19.1 Pre-release 4, 1.19.1 Pre-release 5, 1.19.1, 1.19.2, 1.19.3 Release Candidate 1

23w03a

Retrieved