evil-morfar / RCLootCouncil2

RCLootCouncil - addon for World of Warcraft
https://rclootcouncil.com
GNU Lesser General Public License v3.0
19 stars 29 forks source link

Autopass Owned Items #153

Closed SafeteeWoW closed 6 years ago

SafeteeWoW commented 6 years ago

This PR adds a new autopass option which autopasses owned items. (Enabled by default) This feature autopasses item when all following conditions are met:

  1. You have collected the appearance of the item or the item doesn't have an appearance.
  2. You have collected an equal or better version of the item. That means, You has an item equipped or in your bag with the same item id and the item level and any single stat of your owned item is not worse than the item being rolled, including secondary stats, #sockets, leech, etc. (Enchant and gems on the item are ignored before comparison.)
  3. The item is equippable (not set token, not artifact relic). Because non-equippable items are usually not permanent and are consumed on use.

TODO:

  1. Localization text of this PR needs to be adjusted. I prefer you to adjust it if you accept this PR.
  2. Currently, this PR does not check your item in the bank or void storage. This is not hard to do, but in order to do this, I would have to store bank item info in saved variable whenever you open the bank because Bank API doesn't return if you aren't at the bank. I'm not sure if you'll like this.
evil-morfar commented 6 years ago

A fine idea. How extensively have you tested it? I'm especially nervous about the implications of adding a new response type.

I'll probably want to change some of text, no biggie though.

I don't think it's worth checking the bank or void.

SafeteeWoW commented 6 years ago

I did some basic test when I pushed this PR, but I am not available to do any extensively test or make any major changes right now. You can add your reviewed stuff if you want to merge this PR. However, this aren't many changes, so it should be easy to find bugs if sth is wrong I miss a comment for IsEqualOrBetterItem that it also compares item by ilvl

evil-morfar commented 6 years ago

Roger that. I'm thinking it'll go in v2.8 with the other new stuff.

SafeteeWoW commented 6 years ago

Another "tiny" problem: patterns and replacment in GetItemLinkWithoutEnchant should probably excludes "|H" in case input is item string instead of item link

SafeteeWoW commented 6 years ago

Another small feature suggestion: Autopass owned mount/toy. My current idea is to do a tooltip parsing ITEM_SPELL_KNOWN =="Already known"

SafeteeWoW commented 6 years ago

Above suggestion is done.

SafeteeWoW commented 6 years ago

I have made another improvement that I have loose the item comparison requirement that it does not always require the two items to have the same id.

If both items are not restricted (uniqueness/tournament gear, etc) and are similar (Same type/subtype/equipLoc) and the item being compared to is a stat stick (equippable item that not set piece, no on-use effect, no on-equip effect), then we compare the stats of two items.

This change has potential to give false autopass. Many tests are needed and more consideration needed. This is an aggressive optimization that still has some problems.

SafeteeWoW commented 6 years ago

TODO: Fix dual-wield weapon. Items that can be double equipped if two or more better items are found.

SafeteeWoW commented 6 years ago

This PR won't be ready for v2.8. After more consideration, I have found that this feature is more complicated than I imagined before. Many stuffs needs to be considered to avoid false autopass. Meanwhile, I really dislike the current code structure to handle the item information. Current implementation makes the code very redundant, slow and easy to make mistake.


Let me summarize when the item will be autopassed by this PR. I haven't implement the entire definition yet. I am not explaining the reason behind this definition, but you should be able to understand it. IMPORTANT: All following ignores the item gems, enchants of both item being rolled and item being checks.

  1. The item has no appearance or the item appearance has been collected.
  2. The item being rolled is equipable (equipLoc ~= "")
  3. If the item being rolled is an one-hand weapon not specifying main or off-hand, the player needs to have 2 equal or better items. If the item is ring or trinket, the player needs to have one equal or better items with the SAME ID or 2 equal or better items with the different id. (TODO: Not implemented yet). Otherwise, only need 1 equal or better item.

The definition of equal or better item:

  1. The item being checked must be usable by the player. (TODO: Not implemented yet)
  2. The item being checked must not be tradable. (TODO: Not implemented yet)
  3. The ilvl of the item being rolled is not greater than the item being checked.
  4. Every single stat (i.e., secondary stats, #sockets, etc) as returned by GetItemStats of the item being rolled is not greater than the item being checked.
  5. The item being rolled and checked are similar.
  6. Item upgrade should be properly considered. (TODO: Not implemented yet, but Legion expansion doesn't have item upgrades.)

The definition of similar items.


Do you have any case that my definition will cause false autopass? Btw, while I want to 100% avoid false autopass, I do want to autopass as aggressively as possible. TODO: Not implemented. Also check item in bank/void storage.