Artisan Worktables 1.12

Artisan Worktables 1.12

3M Downloads

CraftTweaker key-value iteration failing

Parcipal opened this issue · 14 comments

commented

What Happens

Error when using the new RecipeBuilder within an Array

What You Expect to Happen

I expect to have a working script

Script

Link to the Script HERE

Crash Log

Link to the Crash HERE

Affected Versions

  • Minecraft: 1.12.2
  • Forge: 14.23.2.2613
  • CraftTweaker: 1.12-4.1.1
  • Artisan Worktables: 1.12.2-1.13.21
  • Athenaeum: 1.12.2-1.8.4

I did follow your template and I'm pretty sure either a RecipeBuilder cannot be used within a loop or I'm missing some documentation

commented

@codetaylor wrote:

Since builders are not designed to be reused, it is not a good idea to keep them around anyway.

@Parcipal tried to "create a new builder for each recipe." as recommended here in ZENSCRIPT_ADVANCED.

As recipes are registered in a loop, each iteration is a new recipe.
My understanding is that: the Recipe Builder needs a dedicated instance for each recipe.
This is the reason Recipe Builder instances are stored into an array/map so as, each iteration has its very own dedicated Recipe Builder.
What @Parcipal experienced if it was not, is exactly that as you document, each iteration overwrite the recipe.

About the use of a Map here, the method conforms to CraftTweaker's documented way of iterating over an associative array.

commented

@leagris You are correct about everything. There is nothing technically wrong with the way that @Parcipal wrote the script. It should work, but it doesn't.

The problem is in CraftTweaker (CT). Yes, the iteration code is correct according to CT's documented way of iterating over an associative array.

See my minimal reproduction of the issue here: #61 (comment)

That script will fail during iteration of the map and produce the same error. This has been tested in an isolated development environment using only the absolute latest CT code. The point of failure has been identified as being within CT's domain and the developer has been contacted. I spoke directly to Jared, the CT developer, yesterday and we were unable to determine what causes this behavior in CT. At this point, there is little I can do to fix the problem with CT.

Since I cannot fix the problem in CT, I provided a way to work around the problem that @Parcipal is having here: #61 (comment)

Since builders are not designed to be reused, it is not a good idea to keep them around anyway.

What I mean by this, is that since a new instance of the builder object should be used for each recipe, there is no need to persist the instances outside the scope of the loop. Technically, it will (should) work, but it isn't needed.

@leagris I don't understand what you are trying to tell me in your comment other than: it should work. I absolutely agree. It should work, but it doesn't.

commented

If you do not persist distinct RecipeBuilders for each recipe, recipes overwrites at next iteration of the loop

commented

Thanks for the Help @codetaylor ,
While the working version of your script is internally working (not producing an error) the array still either fails to properly work. As I'm using it with some TE plates and oredict entried ingots, it refuses to output the right plate, example following up

Having applied your workaround solution to my script , I decided to give it a go.
As the Result shows, using Steel Ingots (which is defined as an IIngredient) technically it should output a Steel Plate (thermalfoundation:material:352) while it outputs a tin plate (thermalfoundation:material:321). Tin Plates are being outputted to any entry in the array

commented

If you do not persist distinct RecipeBuilders for each recipe, recipes overwrites at next iteration of the loop

@leagris That is not correct. When you call create() on the builder, the recipe that you have defined with the builder is created and stored internally.

@Parcipal You're welcome.

<thermalfoundation:material:356> : <ore:Constantan>,

That should probably be <ore:ingotConstantan>.

I'm unable to reproduce the problem that you are having using this script. I get the expected results.

image

Have you tried updating to CraftTweaker 4.1.2?

commented

@Parcipal I just noticed that your example image was taken in the table itself, whereas mine was taken in JEI. I'm testing the table now to see if I can reproduce it.

commented

I tested several of the script's recipes in the table and could find no issues.

image

commented

Yeah that was totally my bad, sorry for that!

commented

No worries. :)

commented

I was able to reproduce the error using a minimal version of the script you provided.

import crafttweaker.item.IItemStack;
import crafttweaker.item.IIngredient;
import crafttweaker.oredict.IOreDictEntry;
import mods.artisanworktables.Worktable;
import mods.artisanworktables.IRecipeBuilder;

val plateIngots = {
    <minecraft:gravel> : <ore:ingotGold>
} as IIngredient[IItemStack];

val plateRecipeBuilder = {} as IRecipeBuilder[IItemStack];

for plate, ingot in plateIngots {
    plateRecipeBuilder[plate] = Worktable.createRecipeBuilder("blacksmith");
    plateRecipeBuilder[plate].setShaped([
        [ingot],
        [ingot],
        [ingot]
    ])
    .addTool(<ore:blacksmiths_hammer>, 5)
    .addOutput(plate)
    .create();
}

However, was able to get it to work by removing the map and using this instead:

for plate, ingot in plateIngots {
    val builder = Worktable.createRecipeBuilder("blacksmith");
    builder.setShaped([
        [ingot],
        [ingot],
        [ingot]
    ])
    .addTool(<ore:blacksmiths_hammer>, 5)
    .addOutput(plate)
    .create();
}

This indicates that the call to Worktable.createRecipeBuilder("blacksmith") is returning the builder object correctly.

My preliminary conclusion is that the error has something to do with crafttweaker and its handling of maps. I will continue to investigate this issue, but for now I suggest removing the map and using the solution that I provided. Since builders are not designed to be reused, it is not a good idea to keep them around anyway.

Hope this helps. :)

commented

I have been able to reproduce the error without using anything from this mod, verifying that the problem is definitely within CraftTweaker's domain.

Used this script:

import crafttweaker.item.IItemStack;

val map1 = {
    <minecraft:gravel> : "hello world"
} as string[IItemStack];

val map2 as string[IItemStack] = {};

for a, b in map1 {
    map2[a] = b;
}
commented

I'm deferring to the CraftTweaker developers on this issue.

commented

I just wanted to point out that "var" is used in ZenScript for variables, while things declared as "val" are constant and cannot be modified.

This includes not being able to append to arrays, so maybe it affects associative arrays too? Really tripped me up for a few hours. I'm used to "const", not a one-letter difference.

commented

@Yukitty Thank you for your input. You made me curious, so I went back and tried the same script using CraftTweaker 1.12-4.1.6.457.

import crafttweaker.item.IItemStack;

val map1 = {
    <minecraft:gravel> : "hello world"
} as string[IItemStack];

val map2 as string[IItemStack] = {};

for a, b in map1 {
    map2[a] = b;
}

print(map2[<minecraft:gravel>]);

And I got the expected (desired) result:

[INITIALIZATION][CLIENT][INFO] CraftTweaker: Building registry
[INITIALIZATION][CLIENT][INFO] CraftTweaker: Successfully built item registry
[INITIALIZATION][CLIENT][INFO] Loading scripts
[INITIALIZATION][CLIENT][INFO] [crafttweaker | SIDE_CLIENT]: Loading Script: {[0:crafttweaker]: issue61.zs}
[INITIALIZATION][CLIENT][INFO] hello world
[INITIALIZATION][CLIENT][INFO] Completed script loading in: 483ms

It looks like this isn't an issue anymore and I'm going to go ahead and close this.

¯\_(ツ)_/¯