Support creating recipes from existing crafting recipes, and then removing them
ChiriVulpes opened this issue Β· 27 comments
It's likely the main use for most Artisan Worktables packs will be to move existing recipes over to the Worktable. However, doing it is a pain:
The workflow of creating a recipe is currently to copy the code of an existing builder and replace the stuff in it with the new recipe information. Alternatively you can use /ct recipes hand
, however this puts the recipes into a file instead of copying them to your clipboard. It's usually only faster (for me, anyway) if the recipe is very complex.
To improve this workflow I suggest adding support for a multi-purpose RecipeBuilder method, which will inherit all the ingredients and output of an existing recipe (probably selecting the recipe by name), and then remove that existing recipe.
Here's an example of how it looks now:
recipes.remove(<minecraft:piston>);
Worktable.createRecipeBuilder("engineer")
.setShaped([
[<ore:plankWood>, <ore:plankWood>, <ore:plankWood>],
[<ore:cobblestone>, <minecraft:iron_ingot>, <ore:cobblestone>],
[<ore:cobblestone>, <minecraft:redstone>, <ore:cobblestone>],
])
.addTool(<ore:engineers_driver>, 1)
.addOutput(<minecraft:piston>)
.create();
And here's after my suggestion:
Worktable.createRecipeBuilder("engineer")
.fromRecipe("minecraft:piston")
.addTool(<ore:engineers_driver>, 1)
.create();
This has the benefit of making the ZenScript code much simpler and more succinct, while being many times easier to write.
Adding recipes always happens after removing recipes, split into sections by mod, after vanilla, correct? Is it possible to do it the way you describe?
recipes.removeByRecipeName(recipeName);
builder.copyRecipe(recipeName);
Based on my current understanding this wouldn't be able to copy the recipe because the recipe would be removed before it has a chance to copy it. This is why I was suggesting recipe removal as part of the functionality, simply because I figured it would be easier to guarantee the functionality of. The API you describe is objectively better, so if it can work, I'm all for it. =)
It's likely the main use for most Artisan Worktables packs will be to move existing recipes over to the Worktable.
I don't presume to know how people will want to use the mod and I don't have enough data to extrapolate an assumption such as this. That being said, I know that I would use a feature like this.
I should have phrased this better, I was sorta rushing. Hindsight is 20-20. Think I would have said "I expect a common use for Artisan Worktables in packs"
When the builder method is invoked, no recipes have yet been removed and transferring the existing recipe's information can safely occur at that time.
Recipes added by other CraftTweaker scripts will not be usable by these methods, only recipes added via the JSON recipe system.
Recipe removals and additions are aggregated as IAction
objects and not actually removed or added when the respective methods are called in scripts.
Yea, I got that. I figured that could be a solution, but wasn't sure if grabbing recipes when CT is initialising would work (I don't know offhandedly when it does registration and when normal IRecipe registration is done, relative to it. I haven't needed to know that yet)
I recommend that the method copyRecipeLayout
be renamed to copyRecipeInput
to maintain consistency with the method copyRecipeOutput
.
It's likely the main use for most Artisan Worktables packs will be to move existing recipes over to the Worktable.
I don't presume to know how people will want to use the mod and I don't have enough data to extrapolate an assumption such as this. That being said, I know that I would use a feature like this.
The builder has a singular responsibility to aggregate parameters for and simplify the instantiation of recipes with a large constructor parameter count. Removing recipes lies outside the scope of the builder's responsibility.
Your suggested syntax is brief, but it does not have clarity; both are needed to be succinct.
Worktable.createRecipeBuilder("engineer")
.fromRecipe("minecraft:piston")
.addTool(<ore:engineers_driver>, 1)
.create();
When I read this, I am not immediately aware that the code is going to remove a recipe. That behavior is obfuscated behind a black box method.
I propose the following:
recipes.remove(<minecraft:piston>);
Worktable.createRecipeBuilder("engineer")
.copyRecipe("minecraft:piston")
.addTool(<ore:engineers_driver>, 1)
.create();
This example would create a new worktable recipe using the ingredients and output of the copied recipe and remove said recipe.
recipes.remove(<minecraft:piston>);
Worktable.createRecipeBuilder("engineer")
.copyRecipeLayout("minecraft:piston")
.addTool(<ore:engineers_driver>, 1)
.create();
This example would create a new worktable recipe using only the ingredients of the copied recipe and remove said recipe.
// Parameters: String recipeName, @Optional int quantity, @Optional int weight
IRecipeBuilder#copyRecipeOutput(String);
IRecipeBuilder#copyRecipeOutput(String, int);
IRecipeBuilder#copyRecipeOutput(String, int, int);
This last overloaded method will give the user control over copying and manipulating a recipe's output.
Splitting the methods and responsibilities in such a way not only adheres to the spirit of the SRP, it increases clarity as well as flexibility. When I read either of the first two examples, I can immediately discern that it removes and copies a recipe or part of a recipe. Exposing multiple methods allows the end user flexibility in how they want to remove recipes, which recipes to remove, and which parts of a recipe to copy.
The complete method list is as follows:
// Parameters: String recipeName
IRecipeBuilder#copyRecipe(String);
This method copies ingredients, layout and output from an existing recipe. If this method is invoked after setShaped
, setShapeless
, or setOutput
, an error will occur.
// Parameters: String recipeName
IRecipeBuilder#copyRecipeLayout(String);
This method copies only the ingredients and layout from an existing recipe. If this method is invoked after setShaped
or setShapeless
, an error will occur.
// Parameters: String recipeName, @Optional int quantity, @Optional int weight
IRecipeBuilder#copyRecipeOutput(String);
IRecipeBuilder#copyRecipeOutput(String, int);
IRecipeBuilder#copyRecipeOutput(String, int, int);
This method copies only the output from an existing recipe. If this method is invoked after setOutput
, an error will occur.
If a different syntax is desired, CraftTweaker functions can be used. Again, this is flexibility in the user's domain.
function fromRecipe(toRemove as IItemStack, tableName as string, toCopy as string, tool as IIngredient, damage as int) {
recipes.remove(toRemove);
Worktable.createRecipeBuilder(tableName)
.copyRecipeLayout(toCopy)
.addTool(tool, damage)
.create();
}
fromRecipe(<minecraft:piston>, "engineer", "minecraft:piston", <ore:engineers_driver>, 1);
A different approach:
function copyAndRemove(recipeName as string, builder as IRecipeBuilder) {
recipes.removeByRecipeName(recipeName);
builder.copyRecipe(recipeName);
builder.create();
}
copyAndRemove("minecraft:piston", Worktable.createRecipeBuilder("engineer")
.addTool(<ore:engineers_driver>, 1));
The advantage of this implementation lies in the flexibility that it allows the end user and the clarity of the api.
Can they also support targeting all recipes to make an item by providing the stack, actually? I just realised that if it supported that I could do this, or similar:
for item in <ore:carpenters_hammer>.items {
recipes.remove(item);
Worktable.createRecipeBuilder("basic")
.copyRecipeLayout(item)
.addTool(<some random tool here>, 1)
.addOutput(item)
.create();
}
Being able to move all of these recipes over to a custom table with a custom tool this way would be fantastic. It would also pair well with being able to differentiate the material of the tool by ore dictionary, as then you could use a different table depending on the material. #74
What would you expect the behavior to be if there were multiple recipes for item
?
A builder creates one recipe.
Yea, that's the main issue.
What if the copying methods could take ICraftingRecipe
s as well as strings? Then you could loop over those to do it, so that the Worktables API wouldn't have to be adjusted to work for multiple.
Is it possible to retrieve a collection of ICraftingRecipe
objects from an IItemStack
in CraftTweaker?
IIRC, CraftTweaker doesn't handle overloaded methods with the same parameter count. If this is true, the method names for copyRecipe
and copyRecipeInput
and copyRecipeOutput
would have to be different based on the method's parameter type.
Something like:
copyRecipeByName(String)
andcopyRecipe(ICraftingRecipe)
copyRecipeInputByName(String)
andcopyRecipeInput(ICraftingRecipe)
copyRecipeOutputByName(String)
andcopyRecipeOutput(ICraftingRecipe)
Development of this feature is temporarily blocked by an NPE in CraftTweaker when calling:
recipes.getRecipesFor(<minecraft:iron_ingot>);
This is confirmed working:
import mods.artisanworktables.Worktable;
recipes.remove(<minecraft:furnace>);
Worktable.createRecipeBuilder("basic")
.copyRecipeByName("minecraft:furnace")
.addTool(<ore:artisansHammer>, 1)
.create();
Worktable.createRecipeBuilder("mason")
.copyRecipeInputByName("minecraft:furnace")
.addTool(<ore:artisansHammer>, 1)
.addOutput(<minecraft:diamond>)
.create();
Worktable.createRecipeBuilder("carpenter")
.setShapeless([<minecraft:diamond>])
.addTool(<ore:artisansHammer>, 1)
.copyRecipeOutputByName("minecraft:furnace")
.create();
Worktable.createRecipeBuilder("jeweler")
.copyRecipeInputByName("minecraft:anvil")
.addTool(<ore:artisansHammer>, 1)
.copyRecipeOutputByName("minecraft:furnace")
.create();
ByName
methods available in alpha 1.12.2-1.15.26-28-gc5ac37f
Will be available in beta 1.12.2-1.16.x
I've added a new method to the RecipeBuilder
:
/**
* Copies recipe ingredients and outputs for all the recipe outputs provided.
* <p>
* This method is mutually exclusive with the following methods and can't be called
* on the same builder: {@link IZenRecipeBuilder#copyRecipeByName(String)},
* {@link IZenRecipeBuilder#copyRecipeInputByName(String)},
* {@link IZenRecipeBuilder#copyRecipeOutputByName(String)}.
*
* @param recipeOutput outputs of the recipes to copy
* @param excludeOutput true to not copy recipe outputs, defaults to false
* @return builder
*/
@ZenMethod
IZenRecipeBuilder copyRecipes(IIngredient[] recipeOutput, @Optional boolean excludeOutput);
It should copy multiple recipes if they exists for the output given. I haven't tested it yet with multiple recipes though. I would love it if someone could test this for me!
I can confirm that it works with these recipes:
import mods.artisanworktables.Worktable;
recipes.remove(<minecraft:furnace>);
Worktable.createRecipeBuilder("basic")
.copyRecipes([<minecraft:furnace>])
.addTool(<ore:artisansHammer>, 1)
.create();
Worktable.createRecipeBuilder("mason")
.copyRecipes([<minecraft:furnace>], true)
.addTool(<ore:artisansHammer>, 1)
.addOutput(<minecraft:diamond>)
.create();
I'll test when I have a minute later but I recommend using gold or iron ingots as a test recipe because they can be made from 9 nuggets or a block
import mods.artisanworktables.Worktable;
recipes.remove(<minecraft:furnace>);
Worktable.createRecipeBuilder("basic")
.copyRecipes([<ore:ingotIron>])
.addTool(<ore:artisansHammer>, 1)
.create();
import mods.artisanworktables.Worktable;
recipes.remove(<minecraft:furnace>);
Worktable.createRecipeBuilder("mason")
.copyRecipes([<ore:ingotGold>], true)
.addTool(<ore:artisansHammer>, 1)
.addOutput(<minecraft:diamond>)
.create();
I wonder if there could be a succinct way to change the output item without changing the count? Cause of that issue in the diamond example
What about this:
@ZenMethod
IZenRecipeBuilder copyRecipesWithOutput(IIngredient[] toCopy);
@ZenMethod
IZenRecipeBuilder copyRecipesReplaceOutput(IIngredient[] toCopy, IItemStack newOutput);
@ZenMethod
IZenRecipeBuilder copyRecipesRemoveOutput(IIngredient[] toCopy);
So copyRecipesReplaceOutput
would replace the output item but not the quantity?
Whatβs the benefit of providing copyRecipesRemoveOutput
, out of curiosity? Since you could just use addOutput
to overwrite it later anyway?
// Copies recipe input and output with quantity
@ZenMethod
IZenRecipeBuilder copyRecipesWithOutput(IIngredient[] toCopy);
// Copies recipe input and replaces output with copied quantity
@ZenMethod
IZenRecipeBuilder copyRecipesReplaceOutput(IIngredient[] toCopy, IItemStack newOutput);
// Copies recipe input, but not output
@ZenMethod
IZenRecipeBuilder copyRecipesRemoveOutput(IIngredient[] toCopy);
Whatβs the benefit of providing copyRecipesRemoveOutput, out of curiosity? Since you could just use addOutput to overwrite it later anyway?
addOutput
does not overwrite anything, it ... adds output. Since recipes can have multiple weighted outputs, addOutput
adds outputs.