Artisan Worktables 1.12

Artisan Worktables 1.12

3M Downloads

Reset builder objects when calling .create()

mayonnaisefarmer opened this issue ยท 6 comments

commented

What Happens

After the second createRecipeBuilder in my script, all of my recipes bug out with a crafttweaker.log error of "unable to register recipe, can't calculate recipe tier"

What You Expect to Happen

I expected my recipes to work properly as setting a global variable should be creating a new builder. I have a feeling this is probably due to me not quite understanding what creating a new createRecipeBuilder means. If I had to guess, using variables like this doesn't actually "create" a new IRecipeBuilder despite implying that it should.

Script

https://pastebin.com/LaczkqxK

Crash Log

Not a crash log, but the error in question in crafttweaker.log

https://pastebin.com/MU85qy7Y

Affected Versions

Do not use latest; please supply accurate version numbers.

  • Minecraft: 1.12.2
  • Forge: 14.23.2.2611
  • CraftTweaker: 4.1.3
  • Artisan Worktables: 1.14.23
  • Athenaeum: 1.9.5
commented

Yep.

Don't reuse builders. Builders are designed to be disposable.

From the documentation:

It's a good idea to create a new builder for each recipe. If you don't create a new builder for each recipe, parameters set on the previous recipe may leak into subsequent recipes if not explicitly reset.

What's happening in your script is each time you add a tool, you've already added a tool to that builder in the previous call to addTool, so you end up with recipes that require many, many tools. Same thing with addOutput, you're adding an output and it's compounding with the previous recipe. Then, when the system tries to calculate the minimum table tier required to create the recipe, it looks at the recipe and doesn't know what to do with a recipe that requires 5+ tools.

See: Example #1

Create a new recipe builder for each recipe.

I am going to make a change in the next version that will require you to use a new recipe builder for each recipe and fail with a clear message in the log if you don't.

Hope this helps. :)

commented

Yeah the lack of a clear error message definitely threw me for a loop, since everything checked out fine in game with /ct syntax and everything, and didn't give me any meaningful error messages. I figured it was the case. I'm not sure how feasible it would be, but is there potential for a method to clear builders to allow for global variables like this, sort of like a .clearBuilder("str"); after the .clear();? Or is it just simply not possible in an easy way?

commented

What if calling .create() reset the builder?

Also, if the .create() method was changed to return an IRecipeBuilder, you could create a single builder and simply continuously chain methods to create recipes. Of course, creating a builder and storing it in a val, var, or global should also work.

So you could do something like this:

Worktable.createRecipeBuilder("carpenter")
    .setShaped([
        [<minecraft:planks>],
        [<minecraft:planks>],
        [<minecraft:planks>]])
    .addTool(<ore:carpenters_hammer>, 3)
    .addOutput(<minecraft:planks>)
    .create()

    .setShaped([
        [<minecraft:planks>],
        [<minecraft:planks>],
        [<minecraft:planks>]])
    .addTool(<ore:carpenters_hammer>, 3)
    .addOutput(<minecraft:planks>)
    .create()

    .setShaped([
        [<minecraft:planks>],
        [<minecraft:planks>],
        [<minecraft:planks>]])
    .addTool(<ore:carpenters_hammer>, 3)
    .addOutput(<minecraft:planks>)
    .create();

Using different recipes, of course.

commented

since everything checked out fine in game with /ct syntax and everything, and didn't give me any meaningful error messages

Yeah, /ct syntax just checks that you're syntax is correct. It doesn't do any static analysis or anything.

commented

That would be fantastic honestly. Yeah, adding the ability for .create() to simply finalize the recipe and then reset the builder would be better than adding a new method for resetting it. It's 100% my fault for it not working the way I inferred it would since this is probably the only example of something I've seen where you can't actually set it to a global and have it work as expected vs other things, and that definitely tripped me up for a while.

I'm working on a very large scale pack and I don't exactly envy the idea of creating literally thousands of 'var' at the moment is all, that new '.create();' method sounds fantastic in that regard since it would let me store like 10 globals instead of probably hundreds / thousands of 'IRecipeBuilder' with how things currently are.

commented

This is an easy change and I can slip it into the next feature version release.