tth05 / morerefinedstorage

Continuation of RefinedStorage for 1.12 with focus on performance and new features.
MIT License
16 stars 7 forks source link

Auto-crafting of oredict items #26

Open hjae78 opened 3 years ago

hjae78 commented 3 years ago

Issue description: I just transfered my quite large save to use your fork. I've noticed that quite a few of the recipes I had to use oredict (for instance steel, bronze plates) think they can't be crafted.

What happens: Try to craft an item that has an incorrect item with the correct oredict in the recipe. The crafter doesn't recognize the recipe to craft an item with the same oredict. In my case I was trying to craft a Mixed Metal Ingot from GTCE which uses Steel, Bronze and Aluminium plates. The recipe had Thermal Steel and Bronze plates in the pattern. I had existing patterns to craft GTCE Steel and Bronze plates.

Now maybe this is just something that is unavoidable giving the transfer to your fork but I wanted to bring it to your attention just in case.

tth05 commented 3 years ago

Crafting oredict items is currently not supported in the crafting engine. So if you have for example "oak log" in the pattern, then the engine will extract everything that is a log, but it will only try to craft "oak log" (I think that's what your problem is).

I just haven't gotten around to implementing this because it would be somewhat annoying.

embeddedt commented 3 years ago

@tth05 Maybe it would be best to have this feature as an option for each pattern? In a SkyFactory 4 playthrough with morerefinedstorage added, we ended up with a pattern using Refined Storage silicon, but our pattern for crafting silicon produced NuclearCraft silicon. As such, the first pattern would frequently complain about a lack of silicon due to the fact that it wouldn't use the silicon from NuclearCraft.

tth05 commented 3 years ago

I mean, there's already the "oredict" option

hjae78 commented 3 years ago

The issue for me is just not that my base is so large and the fact that I have thousands of patterns already but that in GregTech the majority of progression is done through circuits. As you get access to better machines you get access to make the same oredict circuit but easier. Once you unlock the next circuit it would be quite a task to go through all your patterns to change out the circuit to the new one.

I would very much like to see oredict autocrafting back

embeddedt commented 3 years ago

The oredict option works if you already have the silicon in stock, but if more needs to be crafted, it will only craft the one that was requested in the pattern.

I haven't returned to the SF4 world in a while, but if I recall correctly, the issue was something like this:

tth05 commented 3 years ago

Crafting oredict items is currently not supported in the crafting engine. So if you have for example "oak log" in the pattern, then the engine will extract everything that is a log, but it will only try to craft "oak log" (I think that's what your problem is).

I just haven't gotten around to implementing this because it would be somewhat annoying.

That's what I described here and it's what the oredict option did originally I think.

hjae78 commented 3 years ago

No, what @embeddedt described works in normal RS

hjae78 commented 3 years ago

I look through this which is the only place that I could see where normal RS uses oredict but I can't see a difference. I would be happy to try and create PR for this if you could point me in the right direction

tth05 commented 3 years ago

Well, this conversation is a bit all over the place, but anyways. Here's a starting point of how it would work:

The missing logic would be right here: https://github.com/tth05/morerefinedstorage/blob/mc1.12/src/main/java/com/raoulvdberge/refinedstorage/apiimpl/autocrafting/engine/task/Task.java#L511-L550

This needs some kind of loop over all oredict possibilities, which are located here: https://github.com/tth05/morerefinedstorage/blob/mc1.12/src/main/java/com/raoulvdberge/refinedstorage/apiimpl/autocrafting/engine/task/inputs/Input.java#L34

The Task#calculate could be a boolean simulate parameter, or you can call Task#onCancelled after calling calculate so it puts the extracted items back into the system.

The loop could do something like this:

for each possibility of the input: 
    if no pattern for this exists: 
        continue

    create a task requesting Input#getMissingAmount() of items and let it calculate

    Using the MasterCraftingTask#getMissingItems/Fluids determine how much the Task could craft of the wanted item (could also use Input#getAmountMissing from each Task, although getMissingItems contains all missing items from all inputs). 
    Probably better to make this a method in MasterCraftingTask because it can access more things. 
    This requires a bit of math because you need to account for all inputs (items and fluids) and all outputs.
    Both of these can use or produce multiple items per craft. 
    50 missing items of some input could mean that the Task would be able to craft 110/111 of the requested items, because each requires 50 of the missing input. 
    You also need to calculate this in a way where you account for all Tasks that are part of this MasterCraftingTask.
    While the first root Task might have everything it needs, the missing items will be in the sub tasks which need to be traversed from the bottom of the tree upwards, properly account for each Input/Output#getQuantityPerCraft to then determine how many sets of crafting the root Task could perform.

    call Input#increaseToCraftAmount with how much of this item you can get
    for this amount Input#getQuantityPerCraft % amount == 0 MUST be true. Machines won't accept 2 oak log and 1 birch log (if the machine needs 3 logs at once to process)

    //if there's no simulate parameter for Task#calculate then call Task#onCancelled here...

    create a new Task with the calculated amount and add it as a child to this current task

I hope this shows why I haven't tackled this yet (mainly because I wasn't using it at all). Hopefully this is the starting point you might need. We can also discuss here if you find an easier way or think about edge cases (DurabilityInput or RestockableInput like to do this :D). We can use this and the above pseudo code as a tracking comment.

P.S.:

embeddedt commented 3 years ago

or you can call Task#onCancelled after calling calculate so it puts the extracted items back into the system.

I'm not that familiar with how Minecraft internals work, but unless there is some type of caching, this sounds like it has potential for lag whenever an autocraft is being calculated, due to the need to search for which storage device the item should be returned to.

tth05 commented 3 years ago

I'm not that familiar with how Minecraft internals work, but unless there is some type of caching, this sounds like it has potential for lag whenever an autocraft is being calculated, due to the need to search for which storage device the item should be returned to.

It wouldn't be different from any other insertion into the RS network (like importers etc.), but of course this update would worsen the performance for any pattern which uses oredict mode.

Kreatifchk commented 2 years ago

So the Orediсt doesn't work? Need to remove it from all recipes?

tth05 commented 2 years ago

No it should be fine. It's just that if you need wood and your recipe uses Oak Wood, the engine won't try to craft Acacia Wood. It will only be able to use existing Acacia Wood.