PneumaticCraft: Repressurized

PneumaticCraft: Repressurized

43M Downloads

Possible Issue with Recipe Detection in the TPP

MuteTiefling opened this issue · 8 comments

commented

Minecraft Version

1.16.5

Forge Version

36.0.46

Mod Version

pneumaticcraft-repressurized-1.16.5-2.10.3-149

Describe your problem, including steps to reproduce it

I'm having a bit of an issue with recipes in the TPP. We first noticed it after adding some recipes to the machine with KubeJS and the problem did seem to go away when some of the recipes were removed. However, it's starting to look more like some kind of race condition or recipe detection failure.

The issue is that the Potato recipes appear to conflict at times. When the issue occurs, it is -only- possible to create oil from potatoes. The Ethanol and Chips recipes don't work and the TPP reports that they need more pressure. Of course, feeding them pressure results in oil instead of Ethanol or Chips.

In debugging, I initially removed all of our custom recipes and this resolved it. I then added only one and the problem did not come back. Adding them all back in broke it again.

Since recipes are added in a script, they're done with a loop. After a bit more testing it almost seemed like the loop itself was the issue somehow, so I again removed it, moved the recipes out to individual recipe events and that worked for a bit... then stopped?

Reloading the recipes with /reload also doesn't fix them once they break. A full instance restart is required. And on a few occasions I've gone from working to not working with no changes to the scripts, only a client restart... so it really makes me think there might be something goofy in the recipe detection itself.

Kinda confusing what's going on, so I'm not sure I'm explaining this well. I should note, however, that none of our recipes are changing any existing recipes and are not adding conflicting recipes; the inputs all come from different mods.

Let me know what else you might need or if you've got any tests you'd like me to do. I could not reproduce it with only PNC, so it does seem like a conflict between mods, most likely KubeJS. It's just strange how... erratic it is.

Any other comments?

commented

Well, the first thing that springs to mind is the TPP recipe search strategy basically just looks for matching input and output ingredients, and picks the first applicable recipe; it doesn't take current pressure & temperature into account when finding recipes. This could cause conflicts with potato recipes, since there is vegetable_oil_from_crops and ethanol_from_potato recipes. I'm wondering if it just happens to work OK with the default recipe set, but adding more recipes alters the search order sufficiently to cause the wrong recipe to be picked.

Possibly a more correct approach would be to build a shortlist of all potentially matching recipes when recipes need to be searched, then check each of those recipes each tick, based on the TPP's current temperature and pressure. Caching the valid recipe(s) is very important for server performance, especially if the list is large (which if you're adding lots of extra recipes, it will be).

I'll add some experimental changes today and point you at a dev build (which also has a lot of other work for 2.11, so... backups etc. :)

commented

Awesome thanks!

I'm doing all this in a test world anyway, so no risk there. I'll gladly test this out once you can get a build ready

commented

Thinking about this some more, I think I might have a better idea now. The TPP's recipe search is:

@Override
public boolean matches(FluidStack fluidStack, @Nonnull ItemStack stack) {
    return (inputFluid == FluidIngredient.EMPTY || inputFluid.testFluid(fluidStack.getFluid()))
            && (inputItem == Ingredient.EMPTY || inputItem.test(stack));
}

In other words, if the recipe's input fluid or item is EMPTY, then the TPP doesn't care what's actually in the machine's corresponding input (item or fluid). So once it sees potatoes in the TPP, it locks onto the first recipe it finds with potatoes as an input item.

With the default recipe set, it so happens that ethanol_from_potato comes before vegetable_oil_from_crops in the recipe list (and this is purely by luck and probably dependent on the implementation of HashMap). What I suspect is happening is that your recipe additions are changing the order in which recipes are iterated (through no fault of yours - just the hashing implementation) when searched so the TPP finds the vegetable_oil_from_crops recipe and locks onto it, ignoring the ethanol_from_potato recipe even if there's yeast culture in the TPP - simply because the vegetable oil recipe is ignoring the fluid.

I think changing the search to:

@Override
public boolean matches(FluidStack fluidStack, @Nonnull ItemStack stack) {
    return (inputFluid.hasNoMatchingItems() && fluidStack.isEmpty() || inputFluid.testFluid(fluidStack.getFluid()))
                && (inputItem.hasNoMatchingItems() && itemStack.isEmpty()) || inputItem.test(itemStack);}

will fix the problem, but it is a bit of a semantic change; basically always require a match, even if the fluid/item in the recipe is EMPTY. It will need some testing to ensure there's no subtle breakage, so I'll let you know when a dev build is ready...

commented

Oh, just one other thing: you'll need Forge 36.0.42 minimum for the upcoming 2.11.0 dev builds.

commented

no trouble there, already on *.46

commented

OK, build 150 available for testing. (Also adds the flight stabilizers & holding enchant changes you asked for - check the changelog :) )

commented

Beautiful! Works like a charm for the potatoes.

:D

commented

Fixed in 2.11.0