refinedstorage icon indicating copy to clipboard operation
refinedstorage copied to clipboard

Implement pattern reconstruction

Open Darkere opened this issue 4 years ago • 7 comments

This addresses potential issues with autocrafting and reusable items. Currently, there is potential for crafting to produce a different item than the original input if there are 2 options for the item in a non-exact pattern.

This happens because output and the byproduct that get produced are not based on the initially selected pattern not necessarily the pattern actually used for the craft.

This implementation changes that by reconstructing the pattern with the ingredients extracted from the internal storage.

For this the number of items in each slot is saved with the ingredient. The ingredient is then passed on through calculation into the node's requirement. Then when the items/fluids are extracted from internal storage they get then split up into the slots they belong into and then pushed/crafted.

This also implements #1937

It might have been a good idea to implement a fluid ingredient instead of making Ingredient generic.. but it is working now :)

Darkere avatar Aug 22 '20 20:08 Darkere

Since making this I've been made aware of more recipes that are affected by this.

  • Mystical Agr. essence upgrades can return the wrong crystal
  • Bucket recipes can return the wrong bucket if for example clay buckets are registered
  • Modular routers modules can take the wrong pickaxe for crafting turning them into the wrong tier
  • Autocrafting with tanks containing fluid can run without the needed fluid :/ (afaik only pneumaticcraft affected)

Darkere avatar Oct 03 '20 20:10 Darkere

It looks like this is a prerequisite to have "crafting tools" properly supported. I made a patch that allows it to properly ignore durability changes when choosing patterns, but (without this PR) due to the wrong byproduct being produced this results in the tool that returns to storage having the original durability when the pattern was created, so it doesn't get worn down over time.

The other issue with crafting tools (that I didn't look into) was making it notice that a byproduct can be used again in a subsequent crafting operation when making multiple items.

According to the git history, both of these issues have been solved before, however..... they were just reintroduced at some point.

uecasm avatar Jan 01 '21 04:01 uecasm

When I initially made this I also put in (almost) full support for reusable items.

The issue was "solved". As in, it had issues that weren't easily fixable. The issue is that every mod has full control over what is changed when a craft goes through. Nothing stopping them from making the durability use random for example.

The most accurate way I was able to come up with is simulating a craft and comparing the resulting against an expandable list of options and extrapolating the resulting number of uses from that. This way we could add support for new crafting types over time (for example fluid tank crafting).

That said even this PR still needs some work. Autocrafting with fluid tanks will currently work without tank, but with the PR will just straight up crash.

Darkere avatar Jan 01 '21 12:01 Darkere

It shouldn't need any kind of second-guessing.

If a basic crafting recipe has the same item (ignoring NBT) in both input and byproduct, then always assume a chain crafting where the output tool is used for the next crafting operation rather than crafting in parallel.

The pattern grid appears to already calculate the correct byproduct according to whatever the mod's rules are (ticking down durability or whatever other change) at the time the pattern is created -- it shouldn't need to try to figure out what it did, just recursively feed the byproduct of one craft to the input of the next during the actual craft (or pre-craft simulation), rather than trying to repeat the original pattern verbatim. (Maybe a tool "levels up" as it crafts more stuff and gets better durability or more output items per craft -- again, RS shouldn't have to care, it just asks the mod what the byproducts are, and tries to reuse the same tool in successive crafting operations as much as possible.)

There's two special cases: the tool could "break" by no longer appearing as a byproduct of the recipe (either completely disappearing or changing to a different item type). The other special case is if the craft itself becomes invalid (by not producing the output item), for tools that don't actually disappear when broken (to allow repair/recharge). In both of these cases, the craft is valid but it then has to be able to find or craft another tool to continue past that point (or show a missing items error).

This should work just as well for tools that drain "charge" or something else other than damage, since you never care what the actual data change actually is.

(And yes, a mod could make seemingly or completely random changes to data on each craft. If so, that's a mechanic of the mod and it should be allowed to do that.)

This could then be cached so that when executing the crafting plan it just simulates the appropriate amount of time and then spits out the final results, without needing to reprocess all the crafting logic. (Unless there is a difference between simulated crafting and actual crafting from the perspective of the other mod.)

The only thing that'd trip it up is if the mod tries to update something on the player or some other object unrelated to the stack being crafted when you ask it for the recipe update. But hopefully that's not a thing.

uecasm avatar Jan 01 '21 15:01 uecasm

Yes. There is just the tiny detail in that RS needs to know how many reusable items it needs to request to fulfill the craft before any crafting happens.

"pre-craft simulation" is not (or no longer) a thing. RS figures out how much it needs for an individual recipe and scales up the amounts to the number of items requested.

Darkere avatar Jan 01 '21 15:01 Darkere

Perhaps pre-craft simulation should still be a thing, at least just for the patterns that appear to generate tool byproducts?

It seems likely that there would be fewer of these recipes than "normal" recipes (although I guess that depends on the modpacks being used), and also less of these items crafted at once (due to limited tool durability), so it shouldn't cause too much of a performance hit, which I assume is the main reason you might not want to do that.

uecasm avatar Jan 01 '21 15:01 uecasm

It would be incredibly ugly to support both ways of calculation (not to mention prone to bugs).

less of these items crafted at once

An assumption which is incorrect. Think Mystical Agriculture, equivalent exchange, gregtech...

Darkere avatar Jan 01 '21 15:01 Darkere

this pull request is pretty old already and the main issue it would fix, #2955, still persists to this day, almost three years later

motz0815 avatar Apr 11 '23 10:04 motz0815

I discussed with Darkere. We will not be merging this for RS 1 because RS 2 is in active development and we don't want to risk breakage of any sort on RS 1 at this point.

raoulvdberge avatar Feb 12 '24 20:02 raoulvdberge