AlexFolland/AutoGear

Untalented rogue requires a dagger in the main hand in pre-Cataclysm WoW

Closed this issue · 19 comments

function AutoGearIsMainHandASkinningKnife()  ; or call it ...ATradeskillTool
	local mainHandID = GetInventoryItemID("player", INVSLOT_MAINHAND)
	if mainHandID then
		local itemClassID, itemSubClassID = select(6, GetItemInfoInstant(GetInventoryItemID("player", INVSLOT_MAINHAND)))
		return ((itemClassID == Enum.ItemClass.Miscellaneous) and (itemSubClassID == Enum.ItemMiscellaneousSubclass.Junk))
	end
end

I added this function to the code to check if the item is a skinning knife. It could be modified to test for the mining pick as well.
This could prevent the addon from equipping the knife/pick every time you acquire something new. It's really annoying.

I was thinking you could put the function call around lines 2378 (where you test for non-gear and fishing pole).
I'm not sure how exactly how we would write the call.

Hi. Thank you for reporting this. While you may already know that you can lock the slot with the AutoGear slot-locking feature, I'm interested in improving this situation by detecting more trade skill tools in the main hand as you have suggested. My plan is to go with your suggestion of changing AutoGearIsMainHandAFishingPole to AutoGearIsMainHandATradeSkillTool and checking for more matching item classes and subclasses.

Would you mind enabling debug mode in AutoGear's settings and taking a screenshot of the Skinning Knife's tooltip so I can see the item class and item subclass? I just want to confirm that they are indeed "Miscellaneous" and "Junk" respectively, which seems like it should be wrong for an important trade skill tool. Furthermore, would you mind doing the same thing with a Mining Pick? These screenshots to document the real in-game item class and item subclass will allow me to improve this.

Locking the slot would require me to check my bags for any item picked up that might be better. I'd rather not have to do that.

I'll grab your screenshots later tonight when I get home from work.

I can't see the screenshots. To paste them here, please use your paste keybind while the github comment box is active. This will cause github to upload the screenshot and generate the necessary markdown code for displaying the screenshot in the comment. You can then use the "Preview" button to preview the comment. If you would like to easily manage multiple screenshots in your clipboard, I suggest considering using a clipboard manager, like Ditto for Windows or CopyQ for Linux.

I believe I had my script blocker turned on. I'll try again.

image
image
image

Explains why I was having trouble pasting them... lol

I looked into this and found that these items are just treated as generic one-handed weapons and have no specific enum value for specifying that they are necessary for trade skills like fishing poles do. If detection is necessary for these, it is not obvious how to do it like it was with the fishing poles. I'd prefer to avoid needing to maintain a list of specific item IDs.

That being said, do you even need to equip the skinning knife to do the skinning, arclight spanner for engineering, and the other items for their professions? I don't remember ever having to do that. I remember just needing them to be in the bag to be usable. I think if that is the case, there is a good reason to not avoid equipping something else instead, which is that the player may get an item later which is better for combat, and having the profession item locked in would be worse in that situation.

If there is a reason for needing these items equipped, please let me know, as I may be unaware.

Locking the slot would require me to check my bags for any item picked up that might be better. I'd rather not have to do that.

By the way, I just reconsidered this earlier comment with the idea that you may need something in particular equipped temporarily. You could just run /ag off at the beginning of your skinning knife equip macro and then run /ag on at the end of your usual gear equip macro. Alternatively, if it is a toggle macro, there is a /ag toggle command that can be used. This would prevent AutoGear from changing your gear while you are doing whatever you need to do with profession-specific gear.

Tradeskill items do not need to be equipped to be used.

There is a way to test for them. Enum.ItemWeaponSubclass can be used to test for Miscellaneous (14)
https://warcraft.wiki.gg/wiki/API_GetItemInfo

https://wowpedia.fandom.com/wiki/ItemType#2:_Weapon -> This one gives a better explanation.

Your tooltip actually shows it:
image

Basically, if the ItemSubclass = 14, skip it. This would apply to all profession tools

Wait, I seem to have misunderstood the issue here. Are you saying it's a problem that AutoGear is evaluating and equipping your profession tool when it's the best weapon available? What is it replacing that is worse? It is better to equip a skinning knife than nothing at all, so that behavior would be expected and approved.

Yes, I realize what my tooltip shows and I cross-referenced the item subclass chart on that very page. Item subclass 14 is "Miscellaneous" which is seemingly not reserved for trade skill tools, so blanket-ignoring anything in that subclass would be wrong. Also, expected behavior is to equip whichever item is better for combat, and if the trade skill tool is better for combat, then equipping it is expected. Ignoring a trade skill tool over nothing makes no sense.

Please clarify the issue if you can. To do so, please tell me the steps you took to encounter the issue, what you consider to be the in-game behavior which was not expected, and what behavior you expected instead. This is necessary because as this ticket stands, it sounds to me like AutoGear's current trade skill item equipping behavior is intended.

AG is equipping my skinning knife over another weapon and preventing me from using abilities that require a 'weapon' to be equipped. I agree, something is better than nothing. This is happening every time I loot something that requires AG to test.

I'll see if I can get a screenshot showing what I mean.

This classification 'miscellaneous' is basically 1 step above junk. I'd call it novelty.

I've checked with WH and there 52 items in Retail that fall into this category (25 of them are beer Steins).
If you are using one of these items, it's for a specific purpose and you want it equipped.

I thought about using itemName (or itemID), but Blizz has a bad habit of messing with those.

I'll come up with something; I'm good at that.
FYI... I'm one of the guide authors for WowPro.

It sounds like this has nothing to do with the weapon that's being equipped being a trade skill tool, but instead the problem is that an inferior weapon is being equipped over a superior one, which is a real issue. Please tell me your steps to reproduce, including class, spec (/ag spec command output ideally), which weapon you are seeing replaced by which other weapon, and what AutoGear says in the chat when it does the replacement, ideally with verbosity level 2 or higher.

I'll do it when I get home from work tonight.

Created a new rogue toon and sent a skinning knife to him.

As soon as I opened the mail:

  • AutoGear: [Skinning Knife] (8.75) was determined to be better than [Worn Dirk] (0.00). Equipping.
  • AutoGear: [Skinning Knife]:
  • AutoGear: Pawn "Rogue: Subtlety" score: 8.75
  • AutoGear: [Worn Dirk]:
  • AutoGear: Pawn "Rogue: Subtlety" score: 0
  • AutoGear: Equipping [Skinning Knife].

I disabled using pawn and I got this:

  • AutoGear: [Skinning Knife] (3.69) was determined to be better than [Worn Dagger] (2.77). Equipping.
  • AutoGear: [Skinning Knife]:
  • AutoGear: DPS: 1.20 * 3.075 = 3.69
  • AutoGear: [Worn Dagger]:
  • AutoGear: DPS: 0.90 * 3.075 = 2.77
  • AutoGear: Equipping [Skinning Knife].

It does that with any weapon that has a lower score than the skinning knife. Not only can I not use my special abilities, I no longer earn skill points towards my daggers because of it.

This might be fixed in the latest version which is not yet tagged for release. Would you mind trying with the latest git master?

I ended up releasing the latest version of the addon. Would you mind testing the latest release?

I've been tinkering with this issue a little further because I found it was equipping my skinning knife on my Druid because I had an OH that scored higher than my equipped staff.
image

I found a way to make it work.
image
It now considers those named items as 'unusable'. It will still tell me that I have an OH that 's better than my currently equipped item.

image
It will not attempt to equip your OH if you do not have a weapon in your MH. (I believe a 2H weapon is treated differently compared to the MH slot)

I'm doing some more tweaking to make it work with your git version. Plug and play did not go smoothly. But, it's a place to start.

This is the next thing I need to address...
image

It says it's equipping it, but I've stopped that .

I've been tinkering with this issue a little further because I found it was equipping my skinning knife on my Druid because I had an OH that scored higher than my equipped staff. image

I found a way to make it work.

Hold up. The behavior you've shown here looks like the expected behavior to me. If the [Tear of Grief] is worth 0.35 and the [Gritroot Staff] is worth only 0.1, obviously you'd want to equip the [Tear of Grief]. Since the [Skinning Knife] is better than nothing in the main hand, then why not equip the [Skinning Knife]? Heck, that even saves you a bag slot.

It looks like AutoGear did the right thing in this case, to me. Feral gets no particular benefit from a staff over a 1-handed weapon. They just blend into its cat-formed body. Can you explain your rationale for wanting AutoGear to equip worse gear in this situation? If there is a reason, why not just change the weights with Pawn and/or use a spec override to guarantee a 2-hander?

In this circumstance equipping the Tears of Grief does absolutely nothing for you (DPS stays exactly the same). Intellect is the lowest stat of importance for a Feral DPS Druid.
Equipping a profession tool as a weapon does not earn weapon skill.

If int's not valuable, why is [Tear of Grief] scored higher? I checked AutoGear's built-in weights, and stam is valued at 10 times the value of int for "Feral Combat", so it would value the [Gritroot Staff] over the [Tear of Grief]. It sounds like your Pawn scale is not ideal and you might want to consider either solving that for your own use and/or reporting an incorrect default scale to the Pawn author if int is not valuable as a DPS feral druid.

Optimizing the leveling rate of weapon skills is outside the scope of what I will try to make AutoGear do. It only takes a few minutes playing with new weapons for the weapon skill to become viable, and an heuristic that equips weapons depending on which weapon skills you might need would be convoluted and issue-prone, so it's not something I consider worth optimizing.

I can see that the stat weights were respected in that calculation, the gear which scored highest with those weights was equipped, and the built-in stat weights would not have equipped the same items. This is all the expected behavior. As a result of this analysis, this does not seem to me to be an issue with AutoGear.