Yarn: Make NBT serialisation methods follow the conventions

[Juuxel]

The NBT serialisation methods are currently a mess. Most of them were mapped before the conventions were a thing, and as a result they're pretty inconsistent.

For example, the conventions say that you should use read/write for non-static methods like the ones in BlockEntity - but BlockEntity uses fromTag and toTag instead. (The names should also probably use -Nbt instead of -Tag, as that seems to be the new standard.)

Should we make a bunch of PRs fixing this, or change the conventions to adapt to current naming? (I'd prefer the former for clean mappings, but the latter is better for modders.)

[Technici4n]

I would be in favor of changing the conventions. That part was written 2 years ago anyway.

If the parameter is going to be a Tag or a CompoundTag, then keeping fromTag and toTag is better imo.

As for confusion between nm.nbt.Tag and nm.tag.Tag, I don't think you are deserializing from datapack tags anyway, and I don't think toTag returning nm.tag.Tag is supposed to happen either.

[Earthcomputer]

As for confusion between nm.nbt.Tag and nm.tag.Tag, I don't think you are deserializing from datapack tags anyway, and I don't think toTag returning nm.tag.Tag is supposed to happen either.

There have been multiple times where I have had to use both classes in the same class, and I am only able to import one. They should have different names.

[Prospector]

If the parameter is going to be a Tag or a CompoundTag, then keeping fromTag and toTag is better imo.

The format is NBT, though. When serializing to JSON, you wouldn't call that method toObject, you would call it toJson because the format is JSON, and that is what you care about, not the arbitrary name that the format chooses to call its objects/elements/tags/nodes/properties/values/entries.

As for confusion between nm.nbt.Tag and nm.tag.Tag, I don't think you are deserializing from datapack tags anyway, and I don't think toTag returning nm.tag.Tag is supposed to happen either.

Until you work with items or entities where the phrase "get tag" could refer to either getting an NBT tag or a datapack tag. Really, the only time this isn't an issue is with blocks, because they don't store NBT.

"Tag" is not an ideal name for anything because it's so generic, but in the datapacks case it sorta works because really it is quite generic. But in the case of NBT, calling it just a "Tag" is entirely unhelpful because not only does the word "tag" not a term that entirely makes sense for what it is, it's also a generic term that is also used by the same game for a different concept and often in the same context as NBT.

It would be bad enough if NBT elements were called something unconventional like a "potato" instead of "tag". But if you also have potatoes in the game that can store NbtPotatoes, then why would you not clarify which you're referring to? Or why would you even bring up the arbitrary element name when it is not at all relevant to someone who wants to store "NBT" and will be searching for relevant methods to the name of the format.

I propose two options for class renames:

• Consistency with GSON
  
  
  
  • Tag -> NbtTag (Subclasses can either remain CompoundTag or become CompoundNbtTag. Generally it's okay to drop a prefix for subclasses)
  
  
  • No, this is not redundant. NBT is the format name. Tag is the type. Yes, that is Named Binary Tag Tag. Named Binary Tag is the format. Tag is the type. Compare to JsonObject, which is a JavaScript Object Notation Object. JSON is the format, Object is the type.
  
  
• Tag -> Nbt
  
  
  
  • Since there is no element of NBT that is not a tag, the Tag suffix is potentially unnecessary, and doesn't need to be used at all.
  
  
  • Subclasses are tricker here, would they become CompoundNbt if Nbt is a noun or NbtCompound if it is a compound of format NBT? For this reason, I'd prefer the first option, plus it would be consistent with GSON.
  
  

[i509VCB]

I'd personally like to the move towards seperating the two names.

FormatType scheme feels to be more natural sounding english. Atleast how I have learned english, modifiers are applied before a word, such as an orange cat. Similarly I belive this would apply to the classes as you are presenting a type in a specific format, where the format is the modifier. e.x: JsonArray, NbtList.

Other precendent exists in support of the FormatType scheme in DataFixerUpper as mojang has chosen the JsonOps name for the DynamicOps implementation.

For the method names, I would agree with a to/fromNbt standard.

[Prospector]

For the copious amounts of discussion on this topic: see #241, #1077, #1078, and #1275

[Technici4n]

Ok I see. You are right Prospector, it makes more sense to use Nbt, both for the serialization function and for the tag types. :)
We could also go for Tag -> NbtElement for even more Gson consistency, and NbtNumber, NbtCompound, ... for the subtypes.

[liach]

Linking to #2037

[YanisBft]

Has been fixed