Open GaracktheMad opened 4 years ago
I know this is a super old issue but I remember having this years ago as well, and now that I got back into ESO for a bit I run into it again.
I dug into the code for a bit and I may have found the issue? From what I can tell there are 2 relevant places concerning recipes.
The first is this block of code in OnInventorySingleSlotUpdate
:
https://github.com/iFedix/Dustman/blob/28f92a2354e3bbc80d4eb969e1aa597fcc20b206/Dustman.lua#L1366-L1375
I tested in game using /script d(GetItemLinkItemType("<itemlink here>")); d(ITEMTYPE_RECIPE)
and both return 29, so this certainly evaluates true. The secondary condition is self-explanatory and does return false
for unknown recipes.
So this means the entire condition evaluates to false and this block is never executed. Which is correct though because this whole block deals with recipes that are already known, which of course is not the case here.
So I dug a little further (or rather, further up), and found this in the IsItemLinkStolen
section:
https://github.com/iFedix/Dustman/blob/28f92a2354e3bbc80d4eb969e1aa597fcc20b206/Dustman.lua#L858-L878
Relevant is only the first block since itemType ~= ITEMTYPE_TREASURE
(i.e. "item is not a treasure") evaluates true here.
That entire next condition is frankly unreadable for me but I think what it comes down to is the last part:
(itemType == ITEMTYPE_RECIPE and quality <= Dustman.GetSettings().stolenRecipeQuality)
The first condition is the same check as the one above, and the second checks for this setting here (in the consumable section) from what I can tell:
However, here's the kicker: Which of these two is it referring to? Dustman isn't doing separate checks for Provisioning and Furnishing, or Jewelcrafting for that matter. Going by the variable name, it seems to go by the Recipe setting (top).
Additionally, it never actually checks whether the recipe is known or not, and so it just goes by whatever quality was set (again, for Recipes). And since the check is for "less or equal" it basically always defaults to true (for white recipes that is).
Speaking of which, this should by that logic also affect Provisioning and Jewelcrafter recipes, but I have no way of confirming that (I already know pretty much all the stolen provisioning recipes and I don't have Summerset or ESO+ right now).
I don't think there's an easy way to fix this without making this condition massively complex. It would probably be best to rip that condition out of the single line up top and create an entire new block for this.
Basically, Dustman needs to check: [x] the item type [ ] whether the recipe is known or not [ ] which of the three disciplines it is [ ] what the configured quality for that discipline is
Off the top of my head it would have to be something like
itemType and isNotKnown and ((isCooking and isCookingQuality) or (isHousing and isHousingQuality) or (isJewelry and isJewelryQuality))
but maybe I'm tripping or I've just been up too long :)
Looking into this further, this seems to affect all recipes, even provisioning:
which makes sense given:
it never actually checks whether the recipe is known or not
Aside from that, I was wrong in my first post. Those are not the options it checks, I completely forgot there is a separate Recipe-Quality Setting under the Stolen category, and that is what it checks.
Also, it seems Jewelry Writs is already handled separately, although I can't really test it (can they even be stolen?).
Also, further digging into this I don't think we need a check for whether it's housing and what quality in the second block of code, because that is already done in the first block (which we currently just don't get to because our recipes are falsely handled by the second block already).
edit: Nevermind, this won't ever be tested when the recipe is unknown, and if it is known it just goes by the recipe-quality in for stolen items.
Given that, I think all we actually need is a check whether the recipe is known or not:
diff --git a/Dustman.lua b/Dustman.lua
index 6d4e53a..f8a0a7f 100644
--- a/Dustman.lua
+++ b/Dustman.lua
@@ -896,7 +896,8 @@ local function OnInventorySingleSlotUpdate(_, bagId, slotId, isNewItem)
-- Stolen item to do not launder, not in the main block because it must be re-evaluated.
if IsItemLinkStolen(itemLink) then
if itemType ~= ITEMTYPE_TREASURE then
- if (Dustman.GetSettings().excludeLaunder[itemType] and quality <= ITEM_QUALITY_NORMAL and (itemType ~= ITEMTYPE_STYLE_MATERIAL or (itemType == ITEMTYPE_STYLE_MATERIAL and Dustman.GetSettings().styleMaterial[itemId] ~= nil))) or (itemType == ITEMTYPE_RECIPE and quality <= Dustman.GetSettings().stolenRecipeQuality) then
+ if (Dustman.GetSettings().excludeLaunder[itemType] and quality <= ITEM_QUALITY_NORMAL and (itemType ~= ITEMTYPE_STYLE_MATERIAL or (itemType == ITEMTYPE_STYLE_MATERIAL and Dustman.GetSettings().styleMaterial[itemId] ~= nil)))
+ or (itemType == ITEMTYPE_RECIPE and IsItemLinkRecipeKnown(itemLink) and quality <= Dustman.GetSettings().stolenRecipeQuality) then
if Dustman.GetSettings().destroyNonLaundered then
HandleJunk(bagId, slotId, itemLink, sellPrice, true, "FENCE-TO-DESTROY") -- Will destroy the item directly
return
With this, unknown recipes should bypass the FENCE-TO-DESTROY
path and therefore be passed down further the chain, eventually getting to the provisioning/housing recipes block, which it would also bypass due to the same check being in place, and hence the item is kept.
If it is known, it continues down the FENCE-TO-DESTROY
path, which is effectively what we get now already.
edit: It also occurs to me that for recipes it doesn't check whether they should be laundered or not either. The excludeLaunder
array is only checked in the first bracket and therefore outside the check for whether it is a recipe or not. The current implementation only checks for quality, and while the quality selector is only accessible when the option is on, the variable is still saved and accessible even after turning it off. Effectively, the on/off option for recipes doesn't do anything.
It also doesn't respect the "quality for stolen items" setting under the Destroy section.
Honestly there are so many settings that play into this, I have lost track. It is very confusing not just looking at the code but even as a user.
Either way, I'll test the above patch until I got a couple unknown recipes/diagrams collected and then I can PR it.
edit:
Lookin' good :eyes:
And that's handy - the second right after I learned the first one:
Although arguably the correct way to do this would be doing away with the "Stolen Recipe Quality" setting entirely and using the respective "non-stolen" Recipe Qualities instead :thinking:
on a different note: The "Destroy items previously selected" setting is extremely unclear in what it actually does (and the description just repeats the same thing, which isn't helpful at all).
Reading the variable name makes so much more sense for this, and it makes me wonder why the option isn't just called "Destroy non-laundered items".
"Previously selected" sounds like either manually selected (i.e. marked as junk) - which is what the "Remember" options under Misc do - or automatically selected by Dustman by whatever rules were configured. But what it actually means is "selected as ON
in the settings right above", i.e. "Don't launder xyz". So why not just call the option that.
/ramble
I use the "Destroy Stolen Items" option when they are set to not be junk or laundered. I have not disabled laundering of recipes, however, all white quality blueprints get automatically destroyed by Dustman. My destruction value for stolen items is set to 4g, 1g less than the 5g value of these recipes.