use json system for machine recipes

[AEnterprise]

The json recipes system is extremely powerfull already. Modders can use custom factories, constants, conditions and custom ingredients to make very powerfull recipe input definitions.

It would be great to leverage this system for machine crafting as well, i have started expermenting with this but found one major issue with this: everything loaded from JSON is mixed together in 1 general recipe registry.

This means it could all work currently but it means that your IRecipe would need to always return false in the matches function to prevent recipes from being crafted in a regular crafting table and use a custom function (or instanceof check on the inventory)

If we where to use it for machine recipes as well like it is right now it would also mean that they would all be in one giant list and slow down finding a maching crafting recipe significatly as next to all the vanilla recipes there would a a huge list of machine recipes to iterate through as well

The sugestion: allow the factory to handle registring the recipe or configuring where/how to register in the registration of the factory. That way mods can leverage the power of this json system and its ingredient parsing easier and expose machine recipes for pack makers in the way other tweak mods did in the past natively

Is this something the forge team is already considering/working on or would a PR be prefered?

[Alpvax]

Could be an additional optional value recipeType which would be a resourcelocation of the recipe registry to use. If it isn't defined it would default to "minecraft:crafting" or something similar, and if the recipe type doesn't exist it could silently (or maybe log a low level message) skip it. That way you could add e.g. macerator recipes, but if IndustrialCraft isn't installed they would just be ignored. It would negate the need for mods to expose APIs purely for crafting purposes.

That way custom machine recipes could take advantage of existing IRecipe types such as OreDict easily.

[AEnterprise]

you can already do that: you can have conditions defined in your json recipe, forge has some pre-defined ones, one of those is checking if a specific mod is loaded or not

[Alpvax]

Yes, but adding a new recipe saying if IC2 is loaded register this macerator recipe seems unnecessary, when you could just say register this to the macerator recipe registry if it exists.

It then allows mods to create a semi generic crusher registry that multiple mods could create (if it doesn't exist). Otherwise you would have to check for each mod that exposes that recipe registry.

[AlexIIL]

The current issue is not that we only have one registry, it's that all recipes loaded from JSON must implement IRecipe - which is completely useless for other types of recipes, as they don't necessarily deal with itemstacks. For example BuildCraft has a combustion engine, which takes in a FluidStack and produces power, and optionally a fluid stack residue. An example change which could allow this is in https://github.com/AlexIIL/MinecraftForge/tree/1.12-alexiil-custom-recipes. Note that the current example registers a smelting recipe, which probably isn't recommended as vanilla doesn't do this (and we have no way of clearing the smelting list automatically)

[liach]

I don't support this. This is modders' freedom.

[AEnterprise]

i think it could still return IRecipe as this json entire system is made for crafting recipes and i'm not sure if powergen would fall under that, allowing fluids and/or other arbitrairy outputs and such would probably require yet another rewrite of the crafting system

the entire point of this is modders freedom. Allowing the factory to decide where to register the recipe will allow modders to leverage all the posibilities of Ingredient inputs, conditional registration and (in the future) easy syncing of these between client and server

[mcenderdragon]

Well you can already implement such a system completly by yourself, nothing is stopping you.

[AlexIIL]

@mcenderdragon yes we can, but we would have to duplicate most of forge's file searching and loading code... this would make it a lot simpler, as then we could use it with just some registration code rather than copy-pasting the entire thing (which has licensing issues).

[KingLemming]

Modders are already free to do this if they wish, and if it makes sense for their recipes.

There is absolutely zero sense in forcing it on everyone though, just to be clear.

So I'm okay with this idea if it's a totally optional thing. Absolutely not if it isn't.

[Prospector]

Nobody said anything about forcing it

[raoulvdberge]

It's not even possible to force it, nor does it make sense.

[AEnterprise]

nobody is saying it should/would be forced on anyone, just that the current system isn't the best for it

every mod that would want to do this would have to do the same exact same thing: wait until all the factories are loaded and then load their own custom receipes from somewhere else

having native support for this from within forge would greatly simplify and encourage this for mods to convert machine recipes to json

the only thing this would kinda 'force' is that these recipes are getting loaded, synced and managed like any regular crafting recipe, and that in my opinion would be a good thing

[JonathanCSmith]

Yeah, not sure I understand the 'freedom' argument. Recipes cannot be added to custom machines unless the mod providing the machine also provides a recipe factory which implies that moders maintain control over their machines.

[Elix-x]

We could use recipe registry registry. Each of registered registries would be populated with recipes by forge - The registry registered by owningmodid:registryname would have its' recipes located in $nextmodid:recipes/$owningmodid/$registryname, where $nextmodid is any loaded modid. Now, when a mod wants to use custom recipe registry, it gives forge all the loaders (ingredient, grid, etc...) and receives recipe registry which it then registers to recipe registry registry. Whenever it's time, forge loads all recipes to all registries with location syntax above, with exception of minecraft:crafting (vanilla crafting registry) which is loaded from $nextmodid:recipes.

On top of no need to manage recipe loading yourself, this system would allow for very easy mod compatibility. You just ship recipes for all mods. If the mod that uses recipes is installed, they are loaded. Otherwise they are simply ignored.

[Alpvax]

If a modder doesn't want json recipes for their machine, they just create a recipe list the old fashioned way. But this idea would make it much easier for those who DID want to allow json recipe customisation.

[Alpvax]

I'm not hugely keen on adding more stuff to the vanilla recipes folder, I would rather keep that as strictly crafting recipes to avoid confusion, but adding another folder (modrecipes or forgerecipes or something along those lines) with subfolders works far better than my original suggestion of a recipetype value.

[AlexIIL]

Should I create a PR implementing this based off of https://github.com/AlexIIL/MinecraftForge/tree/1.12-alexiil-custom-recipes?

While it doesn't implement @Elix-x 's suggestion, it would be a start. Additionally I would have to add an event RecipeReload.Pre for custom machine recipe lists to be cleared. It would also be the event for registering custom factories, conditionals and ingredient handlers.

There could be a Post even for after all recipes have been reloaded - perhaps it would be of use for mods like JEI? I'd have to run it past mezz though, as I don't need it for any of the mods I work on.

[raoulvdberge]

Very nice @AlexIIL!

[Elix-x]

@AlexIIL It is a nice start! Though if you can separate it into different registries, then reload events will not be necessary, as registry will already hold all the recipes and for only that machine.

[AlexIIL]

@Elix-x It's to allow the recipes to be reloaded multiple times in one game instance - if recipe reloading is moved to server init then it will be necessary. (Not my idea, Its in lex's comments in the code)

[Elix-x]

@AlexIIL The events will be useful for mods like JEI, but if we add a way to get all the recipes loaded by given factory (identified by ResLoc), then machines do not need to cache recipes themselves (and events for them will not be necessary). Because even if we reload the registry in the middle of gameplay, registries will be reloaded by Forge, and if the mod does not cache recipes in its' own cache and uses recipe registry directly instead, then all the recipes will be updated for it. (i hope that makes sense). Eventually IGenericRecipeFactory#parseAndRegister is no longer needed as Forge will call parse and register recipe to this recipe factory's recipe registry. If we want to go further, current smelting system could be changed to use this instead of FurnaceRecipes cache.

---

For everybody - direct link for all changes in progress (in case you're lazy or something): https://github.com/MinecraftForge/MinecraftForge/compare/1.12.x...AlexIIL:1.12-alexiil-custom-recipes

[Alpvax]

+1 for migrating FurnaceRecipes to json

[Alpvax]

I suggest that you keep the methods that don't have a resourcelocation parameter, and just make them call the new ones with "minecraft:crafting". But I would much rather have different registries for different recipe types.

[AlexIIL]

@Elix-x the issue with storing all recipes in forge is that we would have to store them all as Map<ResourceLocation, Object>, which isn't very helpful for custom recipe types. Additionally it might be better to store some recipes in a different format, for example smelting recipes are stored as a map from the input item to the output item - which couldn't be done generically.

@Alpvax unfortunatly I think furnace recipes need a bit more work to be supported properly (as otherwise they won't work with reloads) but I'd be happy to investigate it for a different, later PR.

[Elix-x]

@AlexIIL

interface IIRecipeRegistry<R extends IGenericRecipe> extends IRegistry<R>, IForgeRegistryEntry<IIRecipeRegistry<R>>
...
GameData#recipeRegistryRegistry = FMLNamespacedRegisty<IIRecipeRegistry>
...
<R extends IGenericRecipe> IIRecipeRegistry<R> GameRegistry.getRecipeRegistry(ResourceLocation recRegName){ /*Possibly cast*/ return GameData.recipeRegistryRegistry.get(recRegName);}

Crafting:

interface IRecipe extends IGenericRecipe
...
CraftingRecipeRegistry extends IForgeRegistyEntry.Impl< IIRecipeRegistry<IRecipe>> implements IIRecipeRegistry<IRecipe>
...
/*in reg events*/ GameRegistry.register(new CraftingRecipeRegistry())

Similar for smelting, but:

interface ISmeltingRecipe extends IGenericRecipe {
    ItemStack getInput();
    ItemStack getOutput();
}

[AlexIIL]

@Elix-x That would work, but I'd rather leave recipe storage to the mods themselves. (It's much simpler to port existing recipe code if we don't mandate where the recipes are stored.)

Edit: oh right, looks like I missed your comment... I'm still not sure about forcing registries to extend iregistry though, but I haven't looked into what that implies yet.

[Elix-x]

@AlexIIL But you also have new mods (or the old mods under rewrite). And for them, it would be much much simpler if Forge stored the recipes (especially with 1.13 recipe sync).

So, to cover both cases, we can implement this system + add load/reload events marked as deprecated / for removal in 1.13 (recipe sync). This way, old mods can still cache recipes locally with the events. And they have time to move to new system (which should not be very hard, considering that registries are iterable).

Now, Forge managing all recipes is quite important for the future. Mojang said that they will (very likely) sync recipes from server to client in 1.13. If Forge is the one managing the recipes, then mods won't have to write entire recipe sync system for their all of their machines.

[Alpvax]

We could always add a RecipeRegistryWrapper which would be an IRecipeRegistry implementation which just passed all the functionality off to the existing recipe storage (similar to how IInventories are wrapped to produce IItemHandlers). Putting all the recipe registries in the same place massively simplifies the forge side of things, as when the recipe type is registered it would then be able to load all the json recipes. Otherwise you would need to register a recipe adapter, which would be called for each json recipe in order for it to be registered with your implementation. If you are writing that adapter anyway, why not just write your recipe factory implementation?

[AEnterprise]

i think a key part of this would be having these recipes stored in a forge managed IRegistry (that isn't saved to disk), at this point it is almost a certainty that 1.13 will have these new content packs where content will be synced to the client by the server, since we are reworking crafting at this point we might as well prepare now for it for recipes

[AlexIIL]

I'd rather not assume how the sync system will work before we actually see it - as we might assume it works differently to how it actually does, and then we will have wasted our time getting mods to work on a system that will have to be rewritten again anyway.

[AEnterprise]

well forge registries (especially with the current rewrite) are as far as i can see already perfectly syncable as it does this for blocks/items and more already

if we move all recipes to forge registries it also will not be a problem if the syncing doesn't turn out to work like we thought (for modders that is), at that point the registries stuff would not have to be changed realy, only when/how it syncs but as long as modders don't cache and always use the registry (like they would be suposed to, they should eighter use this new system or keep using their old version, a inbetween version wouldn't work anyways)

[Elix-x]

If that happens, Forge will only need to rewrite how it handles sync & make IGenericRecipe extend NBTSerializeable. The registries would stay intact.

Also, as i said, no need to break the old mods. Add the load events you currently have. And they'll work just fine. And new mods will benefit from Forge handling everything for them.

[AlexIIL]

@AEnterprise they sync the integer ID's, they currently don't sync the actual objects - which is what we are interested in. (Which requires a bunch more work that I'd rather not get into)

[Alpvax]

@AlexIIL is correct, the factories can be synched easily (any RegistryEntry can be sent to the client without too much work), but the tricky bit would be actually sending the individual recipes.

One approach would be to send any valid outputs from server-client whenever an item is changed in a slot (or other crafting input is changed). But that wouldn't allow the recipebook to work (or JEI) as the client would have no idea how to craft something until it actually tried.

[Elix-x]

@Alpvax Make IGenericRecipe extend NBTSerializeable. Create a NBTTagCompound map with keys being recipes resource locations and values being serialization result. Send in handshake event with all other registries. Done.

[Alpvax]

You are correct. For some reason I was thinking that the recipe objects themselves weren't in the registry.

[modmuss50]

Using IRecipe isnt useful at all, something way more generic would need to be made to handle the following,

• Ingredients(could be items, fluids, anything?)
• Power usage, Could be FE, but might need to be another power system
• Output, could be anything again, some machines might even need to change something in the world
• How would machines that require things like multiblock's work?

Thats just what I can think of up in 2 mins, the list is almost endless.

I think its almost impossible to get something that every machine could use. There are just too many possibilities on what it would need to handle. I think it would be best to allow the mod to handle it.

Having some helper methods to allow mods to do their own json parsing might not be a bad idea though.

[AlexIIL]

@Elix-x That doesn't handle the case where the registry may contain entires which aren't all the same class. (Which is common in BuildCraft's recipe registries, and defiantly in vanilla crafting).

@modmuss50 We aren't talking about making all custom recipes extend IRecipe - that would be silly.

[AlexIIL]

@Elix-x I would assume that the server would send over the raw JSON files rather than serialise registry entries though - which wouldn't require special support from anything, and could work without forcing custom recipes to be in a forge registry.

[Elix-x]

@AlexIIL You actually reminded me of something! Server -> Client resource pack sync! And it just makes everything simpler... A lot simpler...

@modmuss50 So far i've only talked about IGenericRecipe and IRecipe extends IGenericRecipe. So quite the opposite.

[modmuss50]

Ah ok, I should have read the comments, sorry about that.

Thats looking like something that could be useful.

[AlexIIL]

@Elix-x I'll make a separate branch to test the registry of recipe registries idea - I'm just arguing based on my assumptions at the moment, and I'd like to argue based on how it exactly what your suggesting.

[Elix-x]

@AlexIIL Thanks! I would have liked to code it myself, but i'm unable to write&code anything till tuesday :( . So thanks!

[Alpvax]

@AlexIIL The issue with sending the raw json data is that you then need to decode the recipe data on both sides.

The way I imagine it would be as follows:

• Forge could add an IRecipeList registry.
• Each IRecipeList entry in the registry would be an instance of a recipe list.
• For each IRecipeList registered forge would load all the json files pertaining to that specific recipelist.
• Each IRecipeList would implement a way of reading recipes from the json files (A lot of code could be in common locations such as loading ItemStacks)
• Each IRecipeList would implement a getRecipes method, which would return a list of IGenericRecipe.
• All implementations of IGenericRecipe would implement a getDataForClient method (similar to how TileEntity data is synched).
• The data passed into getDataForClient (along with some other data potentially) would be passed into the IRecipeList implementation on the client, which could generate the required data on the client.

Whenever the recipes need to be synched, the data would be sent to client as follows: (pseudocode) for each IRecipeList in registry: add list identifying data to packet (already implemented by it being a forge registry entry) for each recipe in recipelist: add recipe identifying data to packet (probably implemented in IRecipeList) recipe.getDataForClient() (adds recipe specific data to the packet) send packet to client

And retrieved on the client: for each recipelist in packet: add recipe to list using following data (implemented in IRecipeList, should also load recipe specific data)

We can sync recipes on client connecting (and potentially on recipelist reload). Each recipe "type" (e.g. crafting, smelting, alloying, grinding, AE2 in-world crafting, infusion crafting etc.) would have its own IRecipeList implementation. All json recipes would be loaded for all registered registries.

[Elix-x]

@Alpvax Sending raw JSONs is not a problem in any way. Minecraft already syncs resource pack from server to client and client does have to reparse all jsons and reload all textures, models, blockstates...

[AEnterprise]

@mezz / @LexManos i'm sorry to ping you guys directly but would this be considered a breaking change? if so i see on twitter that the deadline for this is getting closer. We had some discussions above but can we get an official stance on what way would be the prefered way of handeling this for a PR?

[raoulvdberge]

After discussing with Lex:

• Problem with these changes is that every recipe would be a "generic random thing";
• We can still have JSON recipes, but not managed by Forge;
• Lex would make a helper method available to get the JSONs out of the jar in a good way;
• We can reuse methods from CraftingHelper so we get equivalent parsing.

[AlexIIL]

@raoulvdberge Where? I'd like to see the whole conversation if that's ok?

[raoulvdberge]

I can't dump the entire conversation since we discussed in a private IRC channel.

[AlexIIL]

Oh ok