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

[Bug] [General] Selling two sets sometimes results in 2 regal and 2 chaos #321

Open
Davst opened this issue Feb 22, 2022 · 10 comments
Open
Labels
bug Something isn't working low priority We want to prioritize this class of issue well after others need reproduction steps 🔃

Comments

@Davst
Copy link

Davst commented Feb 22, 2022

I've noticed an issue, where if you sell a full inventory you'll get 2 chaos and 2 regals.

This is likely cause the greedy mode will take one item into consideration for the chaos recipie.

I'd be nice if there was a toggle where you could compoind match 2 sets at a time, where it'd choose the same item type if possible for the low level items. This way the sales tab couldnt' recalculate them to 2 regal 2 chaos.

@HiMarioLopez HiMarioLopez added enhancement New feature or request low priority We want to prioritize this class of issue well after others revisit An issue we want to revisit in a few months due to limitations (of GGG APIs, .NET, Windows, etc.) labels May 10, 2022
@HiMarioLopez HiMarioLopez added bug Something isn't working more details required We'd like some clarification on what the issue is to help us find a root cause. and removed enhancement New feature or request low priority We want to prioritize this class of issue well after others revisit An issue we want to revisit in a few months due to limitations (of GGG APIs, .NET, Windows, etc.) labels May 17, 2022
@HiMarioLopez
Copy link
Collaborator

Tested this myself and could not get it to replicate. Going to leave this open since we have had a few users report this over the past couple months (emphasis on few, probably less than 5).

My main takeaway is to write tests for all of our set composition logic.

Keeping this open in case others report the issue

@HiMarioLopez HiMarioLopez changed the title Selling two sets sometimes results in 2 regal and 2 chaos [Bug] [General] Selling two sets sometimes results in 2 regal and 2 chaos May 17, 2022
@tnielsen
Copy link

I have seen this in the past. When it happened to me, preserve low level items was on, and I was just finishing up using one type and started on the next. So for example I used the last low-level weapon, and started the first low-level gloves. When this happened, the recipe used two low-level items (weapon and gloves) in the first recipe, and then all 75+ items in the second.

No real idea if this can be fixed, other than maybe some warning/pop-up. It's more an interaction with PoE than a problem with ChaosRecipeEnhancer.

@Davst
Copy link
Author

Davst commented Sep 13, 2022

Ok so I think I nailed this down to how it is happening.

When selling two sets, if you have the sets using higher level items with ONE item bringing the level down. The vendor logic will try to offer you the "best" set first.

This means.. that if you have different item slots pulling the item level down, it will sub the item into a full regal set and use the two chaos leveled items for a chaos set.

the workaround here, is to use the same slot for the lower level item, so the logic can't make a smart substitution but is locked to use one of the low level amulets, boots or such.


suggestion for improvement:

Two set sell mode:

  1. Toggle two set sell mode
  2. Enhancer will choose sets if possible where it will select the same item (avoiding rings or weapons if possible since they are 4 items) of a lower tier.
  3. Sell

IF It can't find 2 sets, warn user that they should pick up one set and then sell it.

@Irfy
Copy link
Contributor

Irfy commented Apr 11, 2023

Tested this myself and could not get it to replicate. Going to leave this open since we have had a few users report this over the past couple months (emphasis on few, probably less than 5).

My main takeaway is to write tests for all of our set composition logic.

Keeping this open in case others report the issue

@HiMarioLopez This is not something you can test with the existing codebase, because it's a "feature" of PoE to sometimes combine items across the two sets into one and thus offer 2 regal orbs. We would have to implement exactly what @Davst says and that would prevent the regals and would be testable.

@Davst
Copy link
Author

Davst commented Aug 18, 2023

So this issue is still tagged as needs more information. Do you need anything more from me?

I've thought about the issue and I think that pritoitizing low level filler items that aren't rings or 1h weapons and selecting the same item type where possible would eliminate most incidents where this happen.

If this can't be automaticly chosen, it'd be good to be alerted that you can't sell 2 sets at once with the items actually selected.

@HiMarioLopez
Copy link
Collaborator

I think the 3rd scenario might be the best case for users.

The issue is the comment left by Irfy, where sometimes PoE 'overrides' the recipe combinations. I think it's similar to when you vendor 2 mirrored 6-socket items, PoE will 'prioritize' the chance orb recipe as opposed to giving you back jeweler orbs.

I'm guessing in this case the Regal orb recipe will take precedence

@HiMarioLopez HiMarioLopez added low priority We want to prioritize this class of issue well after others and removed more details required We'd like some clarification on what the issue is to help us find a root cause. labels Aug 18, 2023
@HiMarioLopez
Copy link
Collaborator

you're right - i mismarked this as more detail required from you. we should be good on that front. i am marking it as low -priority for now as there is a simple workaround for the user to simply exit the vendor screen and sell 1 set at a time (as opposed to 2) for that one 'trip'

@sclient
Copy link

sclient commented Dec 21, 2023

Im curious about progress on this if it is what caused the regal recipe to be disabled. I really need this back so I can craft heist gear this league (was just about to switch from c to reg bc nobody will sell me stuff for c in my PL anyways).

@HiMarioLopez
Copy link
Collaborator

It will be resolved ahead of 3.24. I usually take 1-2 month breaks on development once the new league releases , so I wouldn’t count on it making it in any time soon.

Appreciate your patience

@sclient
Copy link

sclient commented Dec 21, 2023

Fair enough. Guess I cant heist, then. Oh well. Guess Ill see the tool next league.
Edit: I guess I would add a request that any feature breaking bugs that can possibly be fixed before end of league development concludes get those in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working low priority We want to prioritize this class of issue well after others need reproduction steps 🔃
Projects
None yet
Development

No branches or pull requests

5 participants