Cooking recipes working with count.

[SufficientlyPositive]

Alters the processing of cooking recipes so that they accept the following JSON formats for deserialising recipe results:

Vanilla

"result": "minecraft:example"

Additional (Added by this pull request)

"result": {
    "item": minecraft:example",      (required)
    "count": 1                       (optional)
 }

Serialisation of recipe results is altered from the vanilla format to the additional object format.

There has been fruitful discussion on NBT recipe additions in the following thread:
Add result NBT and count to crafting and smelting recipes
This modification adds the Mojang crafting result Codec to the cooking result deserialisation Codec. Should a Codec that also accepts NBT be produced for crafting, it should be a simple matter to edit CookingRecipeSerializerMixin to use that instead. This should give you NBT smithing recipes for (nearly, see below) free.

Suggested modifications should NBT recipes be enabled:
(Yarn names) canAcceptRecipeOutput in AbstractFurnaceBlockEntity.class does not check NBT data when checking whether a recipe is valid. In order to avoid unintended gain/loss of NBT data for recipes, this function would need to check NBT of the outputs too.

[maityyy]

I'm assuming it should be fabric:count instead of count

[SufficientlyPositive]

This is using the Vanilla Codec for processing crafting recipe items with count. There is merit to renaming it in order to differentiate it from Vanilla datapack processing, but personally I prefer the consistency with vanilla.
I'm open to changing this however, but for now I am partial towards the proposal Shadows-of-Fire mentioned in the NBT thread.

[maityyy]

Well, I personally agree with your point of view, but fabric adds a suffix to all of its changes to vanilla formats (look at resource conditions and custom ingredients), so consistency should be assessed from fabric's point of view, not vanilla's

[SufficientlyPositive]

Thanks for pointing me to these, I will take a look at them.
I agree that consistency should be prioritised over the mod as a whole, so I will look into building a separate codec to use with fabric:count and build and test it over the next few days.

[SufficientlyPositive]

How about changing the new result format to fabric:result?, Leaving out the normal result.
So something like this:

{
  "type": "minecraft:smelting",
  "group": "iron_ingot",
  "ingredient": {
    "item": "minecraft:deepslate_iron_ore"
  },
  "fabric:result": {
    "item": "minecraft:iron_ingot",
    "count": 3
  },
  "experience": 0,
  "cookingtime": 1
}

My reasoning is as follows: This change does not simply add to an additional option to an existing format, but instead adds an entirely new way of expressing results. For example, simply naming the (optional) count value fabric:count does not imply that a result of the format

"result": {
  "item": "minecraft:iron_ingot"
}

would fail to compile in Vanilla (which it would). Renaming to fabric:result over result would express that this is a fabric-specific datapack file, even in the case where an item count is not provided.

[maityyy]

How will this work with datagen?

[SufficientlyPositive]

Some form of additional functionality would be required for modders to be able to use this functionality with datagen. I do not intend to implement this within this PR.

[maityyy]

I mean, to what extent is fabric:result a realistic option? I would prefer the option that is easier from a datagen point of view, because a large number of recipes are created through datagen. I'm not saying you need to implement datagen support, I'm saying we can't make a choice without datagen

[maityyy]

You change the codec, okay, but what about networking?

[SufficientlyPositive]

You change the codec, okay, but what about networking?

Iirc networking uses the codecs for both serialisation and deserialisation also?

I assume there may be some slowdown from packets switching from the String format to the JSON object format, but I don't imaging this is particularly significant. Crafting uses the same Codec.

[maityyy]

I decided to recheck the code and everything seems fine, so I'm sorry. :D My concern was that network packets might ignore the item NBT. And btw, no, recipe network packets doesn't use codecs

[SufficientlyPositive]

I decided to recheck the code and everything seems fine, so I'm sorry :D Btw, no, recipe network packets doesn't use codecs

Ah thanks, must me my faulty memory. I was under the impression that the pipeline was something like (object) --codec--> (JSON) --magic--> (packets) but it's been a while. I will need to recheck it 😄

[modmuss50]

• Datagen support is needed
• The tests should be improved, have a look at adding some game or unit tests to ensure that vanilla behaviour is presevered. (let me know if you need help with this)

Generally we would prefix it with fabric:

[Technici4n]

This changes the default format, you should use an xmapped either codec. To give you some ideas:

(this also supports NBT)

[maityyy]

Here is a link for easy copy :DDDD neoforged/NeoForge#300