Yarn: TYPE_CODEC -> REGISTRY_CODEC

[liach]

Currently, we have a lot of TYPE_CODEC, which are general registry-backed codecs.

It works like this:
Decoding: decodes object type with a registry instance, then use the object type's provided codec to decode the actual object
Encoding: obtains the object type, uses the type's provided codec to encode object (including the type information)

So those codecs are full codecs for any instance of those objects (game event, etc.). The name TYPE_CODEC would sound like a codec dedicated to the type and be a bit confusing.

[haykam821]

Could it be renamed to REGISTRY_CODEC or TYPE_DISPATCH_CODEC then?

[liach]

My main concern is that we need a straightforward way to convey its property as ser/deser based on the type of the object. Of course, mentioning that the type is in registry is good.

Imo referring to dispatch helps little. This is an implementation detail referring to a confusing implementation (dfu has no javadocs and has mojang grade confusing names).

An alternative solution is to add a description about these on the registry javadoc and have those codec fields' docs link to that explanation.

[mschae23]

Maybe you could rename the codecs in the subclasses to CONFIG_CODEC (or something similar), so you can use CODEC instead of TYPE_CODEC.

[liach]

What prevents the renames is enigma's check system. So most of the time we indeed can name both the root class Codec and children class Codecs CODEC without problems (except in an edge case).

The edge case is when a child class calls the parent class's codec (denote as TYPE_CODEC here) without qualification, like TYPE_CODEC.doStuff() as opposed to Parent.TYPE_CODEC.doStuff(), then the java compiler will (most likely) make the TYPE_CODEC field access as getting field from the child class, and in this case, if we name the 2 fields the same, things break.

This is a bloody lesson from #1262. Mojang added entity attributes static factory methods. Everywhere else in the attribute creation methods, they qualify call to the presumed createDefaultAttributes with the class name (like LivingEntity.createLivingAttributes etc in original code) but they forgot to do that in the donkey class. And the result is https://gist.github.com/fa02c52d84c4e45b494a53a26bb4274b

---

After all, if we can confirm that the parent class codec is not used in subclasses, then we can just name them CODEC without any concern. Apparently you can look up usages in enigma, so that would be easy.

[haykam821]

I think the use is pretty clear; the codec dispatches to a different codec in a registry based on the type of the serialized data, and vice versa. Essentially, it is the basis of Registry<Codec<? extends T>>.

[liach]

Uh, I mean, now Registry<T> is an instance of Codec<T>. So, for instance, you are calling the dispatch on a registry of rule test type. You have code like Codec<RuleTest> codec = getRuleTestTypeRegistry().dispatch(RuleTest::getType, RuleTestType::getCodec); the getRuleTestTypeRegistry() returns a Registry<RuleTestType>, which implements Codec<RuleTestType>. There is no registry of codecs in this case.

[haykam821]

Well, RuleTestType and other ThingType classes are just storing the codec, so they are no different. An example of directly using the codec so that it is Registry<Codec<? extends T>> is Nucleoid's spleef minigame. The only thing the spleef minigame does differently to achieve direct access to the codec is using Function.identity() rather than ThingType::getCodec.

[liach]

So I now support REGISTRY_CODEC. The requirement for the type to be in the registry is crucial for the codec to work, as opposed to subclass codecs that can accept any instance of those subclasses.