Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Item counts in NEI pinned recipe chains are buggy #17148

Open
3 tasks done
Meegooo opened this issue Aug 29, 2024 · 9 comments · May be fixed by GTNewHorizons/NotEnoughItems#530
Open
3 tasks done

Item counts in NEI pinned recipe chains are buggy #17148

Meegooo opened this issue Aug 29, 2024 · 9 comments · May be fixed by GTNewHorizons/NotEnoughItems#530
Labels
Status: Triage Tasks awaiting triage. Removed once a dev confirms the issue as valid. Type: bugMinor

Comments

@Meegooo
Copy link

Meegooo commented Aug 29, 2024

Your GTNH Discord Username

Meegoo

Your Pack Version

2.6.1 (Reproduced with only NEI in dev environment)

Your Server

SP

Java Version

Java 17

Type of Server

Single Player

Your Expectation

Something consistent. Here is what I think is the intended behavior.
Screenshots with messy item counts for reference
image
image

  1. With Ctrl held down:
  • For pure inputs, scrolling up or down should increase/decrease item count by one recipe amount (i.e. 5 iron for vanilla hopper)
  • For intermediate items. Same as pure inputs, plus increasing/decreasing amount of that item in recipe, that gives this item as output.
  • For outputs, item count should go up/down by one recipe amount, without requesting extra inputs
  1. With Shift+Ctrl held down:
  • For any item, scrolling should increase/decrease amount of items by one recipe for all of inputs, intermediates and outputs. Without changing amount of requested items (shown in bottom right with blue background), except for the recipe I scrolled on.
  1. With extra Alt, multiplier from NEI bottom right field is added to all increases/decreases.

The Reality

Scrolling can randomly lead to item counts breaking. I don't have exact steps to reproduce, but it seems to be some sort of race condition. Did not debug too deep, but BookmarkCraftingChain::calculatedItems seems to become complete nonsense after some fiddling with Ctrl+scroll.

image

In this state, scrolling down on input planks for a chest, causes plank amount to go up by 8. The reason for that is the mapping 232x -> 248x in BookmarkCraftingChain::calculatedItems. BookmarkGrid::realItems, as seen from this method, contains 232x, but getSlotMouseOver(mousex, mousey) (which eventually looks into BookmarkCraftingChain::calculatedItems returns x248. 248 gets decreased by 8 to 240, and then I guess calculatedItems gets updated, and 232x -> 248x becomes 240x -> 256x.

There is also some other nonsense, like 0x oreIron -> 1x oreIron, which is visible in game as ore count going down as I scroll down on iron ingots, ore count dropping to zero, and then, on next scroll down, setting itself from 0 to 1, without touching iron ingots.

Toggling recipe chain seems to temporarily fix it. I suppose because BookmarkCraftingChain::calculatedItems gets... recalculated

Your Proposal

Not having that bug would be nice, but I have other proposals to improve UX of this feature.

On first screenshots there is 4 chests in input for hopper crafts, and 5 as output from planks. 5 is calculated as 4 requested by hopper plus one extra craft that is "requested" by me (shown in blue with 1 in bottom right). When i scroll chests in hopper inputs up, amount of chests that is requested by me decreases by 1. Same in opposite direction, but not always. If I previously messed with counts of chests, it goes down up to the amount I messed with, and then starts adding items as if they were requested by me. That doesn't make much sense

Also, I would argue that input count in recipe should max out at whatever recipe requests, so in my example, for hopper recipe, chests should be maxed at 2 and iron at 10. But the amounts should be adjustable individually, not in steps of "one recipe", and this adjustment should be shown explicitly, as an offset from requested amount. This would be useful when I have 3 iron, so I can set iron requirement to 7, and with shift pressed it would show -3 somewhere. For intermediates it's slightly more complex, because you can have multiple recipes requesting iron. So maybe forcing amount of intermediates in inputs to always be equal to requested amount, but allowing adjustment on recipe that produces said intermediate. With similar -3 shown.

And last but not least, this

  • For outputs, item count should go or down by one recipe amount, without requesting extra inputs

doesn't make much sense either. At the very least extra hoppers without associated inputs should not be marked as requested
image
image

Final Checklist

  • I have searched this issue tracker and there is nothing similar already. Posting on a closed issue saying the bug still exists will prompt us to investigate and reopen it once we confirm your report.
  • I can reproduce this problem consistently by follow the exact steps I described above, or this does not need reproducing, e.g. recipe loophole.
  • I have asked other people and they confirm they also have this problem by follow the exact steps I described above, or this does not need reproducing, e.g. recipe loophole.
@Meegooo Meegooo added Status: Triage Tasks awaiting triage. Removed once a dev confirms the issue as valid. Type: bugMinor labels Aug 29, 2024
@Meegooo
Copy link
Author

Meegooo commented Aug 29, 2024

This is an issue purely with NEI, so it might make more sense in NotEnoughItems' repo. Whatever you guys decide

@Meegooo Meegooo changed the title Item counts in NEI recipe chains are buggy Item counts in NEI pinned recipe chains are buggy Aug 30, 2024
@Meegooo
Copy link
Author

Meegooo commented Aug 30, 2024

Looked a bit through the code, found a bunch of problems and inconsistensies (like using ItemStack as key in hashable containers). So, if everyone is alright with it, I'm gonna try to rewrite that part of code. And maybe make a recipe graph, as described here GTNewHorizons/NotEnoughItems#487

@slprime
Copy link
Member

slprime commented Sep 7, 2024

  1. This is create specifically so that you can scroll intermediate recipes, as well as ingredients separately. Also it is possible to add items to the chain without recipes to indicate that you already have them and do not need to calculate them.
  2. What version did you check on because I corrected some errors in the calculations after the release of 2.6.1?
  3. How will you solve the problem with looped recipes?
  4. How will you solve the problem if there is more than one final recipe in the chain?

P.S. I think you haven't grasped all the possibilities and flexibility of this functionality

@Meegooo
Copy link
Author

Meegooo commented Sep 7, 2024

This is create specifically so that you can scroll intermediate recipes, as well as ingredients separately.

You mean Shift+Scroll? Yeah, I got that. But I'm not sure if that's really necessary. Because (assuming vanilla recipes), I added a chest recipe, a hopper recipe and a chest minecart recipe. Then I shift+scrolled on planks in chest recipe. What is supposed to happen now? Does hopper take priority? Or minecart?

Also it is possible to add items to the chain without recipes to indicate that you already have them and do not need to calculate them.

Aha. I was planning to do this as well, but with completely locked ingredient amounts. So if you need to get a sum of items, or reduce/increase it, you pin that item.

What version did you check on because I corrected some errors in the calculations after the release of 2.6.1?

Whatever was in main branch at the time of creating this ticket.

How will you solve the problem if there is more than one final recipe in the chain?

In general speak. I want to add a separate counter for requested items into metadata, decoupled from current ItemStack size, stored in BookmarkGrid::realItems. Then, when crafting mode is enabled, I traverse the graph, starting from each item that is requested more than once, accumulating number of crafts, inputs and outputs for each recipe.

How will you solve the problem with looped recipes?

I looked into how Applied Energestics appears to do it (by behaviour, I did not look into code). Which is, go until you encounter a loop, and instead of continuing on, just don't add any more recipes to the queue from this branch. Which means, that adding Block->Ingot recipe and Ingot->Block recipe to the chain and requesting 10 blocks, you will see two recipes: 10 blocks <- 90 ingots, and 90 ingots <- 10 blocks.

P.S. I think you haven't grasped all the possibilities and flexibility of this functionality

In my book this is just a relatively simple graph traversal with a couple extra steps. The biggest hurdle is just minecraft code in general.

@chochem chochem mentioned this issue Sep 7, 2024
66 tasks
@slprime
Copy link
Member

slprime commented Sep 7, 2024

You mean Shift+Scroll? Yeah, I got that. But I'm not sure if that's really necessary. Because (assuming vanilla recipes), I added a chest recipe, a hopper recipe and a chest minecart recipe. Then I shift+scrolled on planks in chest recipe. What is supposed to happen now? Does hopper take priority? Or minecart?

  1. In the case of your example, how do you propose to change the quantity?
  2. The scroll will not work on intermediate recipes, only on latest ones? Do you have an idea how to visualize this so as not to break user experience?

P.S. It would make sense to remove ctrl+scroll then. And for change count always use shift+scroll

@Meegooo
Copy link
Author

Meegooo commented Sep 7, 2024

In the case of your example, how do you propose to change the quantity?

I was honestly thinking ctrl+scroll to adjust single output item quantities, to match non crafting chain mode. Meaning you can "request" 5 planks, and get told that you need 2 logs, you'll get your 5 planks plus 3 remaining (will be shown in the tooltip).

As for shift, I don't know. To match current behavior as closely as possible, I am thinking about increasing requested amount by "one craft" (i.e. 4 for planks). And probably allowing to scroll on inputs as well (it will increase requested output amount). If recipe has multiple outputs, all outputs will increase by one craft.

The scroll will not work on intermediate recipes, only on latest ones?

I'm not planning to distinguish between intermediate and final recipes, because any intermediate recipe can become both intermediate and final recipe at any moment, if you request some of that item. The exception is background colors while holding shift:

  • light blue on outputs, for when you need to do at least one craft.
  • nothing on outptus for recipes that are not participating in crafting chain (including cases where recipe asks for X chests, but you already have X chests and marked it as '-X' requested items.
  • green on pure inputs.
  • nothing on inputs that are intermediate.

The thing I haven't figured out yet, is what numbers to show. Currently the plan is as follows:

  • without shift
    • bottom right shows number of items that the craft will take/produce. Repeats current behavior if user did not mess with ctrl+scroll.
    • top left shows how many items you requested, in the form of +5/-9, etc. (you can request negative amounts to show that you already have some). It overrides "R" marker.
  • with shift
    • bottom right shows the same thing, as without shift, even for intermediate items (that is different from current behavior).
    • top left shows number of crafts, like it does now.

@slprime
Copy link
Member

slprime commented Sep 7, 2024

I am confused by the many numbers on the item. Could it look dirty and unreadable? The negative amounts also confuses me. I need to see what it looks like.

How will an item added to the chain without a recipe behave? (Now it signals the algorithm how much of this item the player has. now it works like yours "negative amounts")

P.S. ItemStack may be more the millions
P.S.2. I don't know if you've seen the new tooltip:
image

@slprime
Copy link
Member

slprime commented Sep 7, 2024

...because any intermediate recipe can become both intermediate and final recipe at any moment, if you request some of that item.

What do you mean by the word "request"?

P.S. English is not my main language :)

@Meegooo
Copy link
Author

Meegooo commented Sep 8, 2024

As for Millions, Billions, etc, I reused the existing code, so big numbers are displayed the way they currently are.

I'm still working on the highlighting, but here are some screenshots
image
+8.0M on hoppers means I want 8 million hoppers.
-4M on iron blocks means I have 4M iron blocks. 444K in the bottom right means I still need to get/craft 444K iron blocks.

Here is with shift held.
image.
This says that I need to do 8M crafts of hoppers, 4.4M crafts of iron ingots (4.4M * 9 per craft will get me 40M ingots), 8M chest crafts, 16M wood plank crafts (16M * 4 per craft = 64M).

PS. Speaking of highlights. How about showing highlights without having to hover over items? For all groups at once, when mouse is over bookmark panel in general. Thinking this because hovering over item shows tooltip, which is in the way.

P.S.2. I don't know if you've seen the new tooltip:

Yep

P.S. English is not my main language :)

Saw in the discord that your main language is Russian. I do speak Russian, so feel free to ping/DM me in discord for any details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Triage Tasks awaiting triage. Removed once a dev confirms the issue as valid. Type: bugMinor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants