TheGeneticsGuy / Custom_Item_Notes

Add custom notes to any item tooltip in Warcraft.
Other
1 stars 0 forks source link

Items with randomized suffixes share the same notes #1

Closed RabbleCode closed 5 months ago

RabbleCode commented 6 months ago

Love this addon and have been using it extensively the last few weeks since I found it, thank you for making this!

I've encountered one small issue whereby items with randomized suffixes (e.g. Sacrificial Kris) all share the same note as a result of having the same item ID (ID 3187 in this case).

Would you consider updating the addon to use the full item ID string (or making this an opt-in option) to differentiate between various suffixes?

For example:

I'm not sure how much extra work that would be, but I'd certainly appreciate being able to keep separate notes per suffix for things like tracking AH sales since some suffixes are far more valuable than others.

TheGeneticsGuy commented 6 months ago

You know what, I didn't even really consider this as I kind of forgot that Blizz has done items like this... It's definitely something I can consider. Thanks for the heads up.

RabbleCode commented 6 months ago

I appreciate it, thanks! If you need any more info I'm happy to help however I can.

TheGeneticsGuy commented 6 months ago

Interestingly, the way I get the item number will not work with this... Just mouseover something and type /dump GameTooltip:GetItem() and you will see that no matter which item it is, it doesn't show any variation in the dump result for the item. I'll probably have to parse the full tooltip - I'll look into it.

RabbleCode commented 6 months ago

I'm in Classic Era (SoD), in case that makes a difference, but GameTooltip:GetItem() returns two values for me: item name and item link.

The item link then contains within it the complete item string (between the |H and |h).

image

In this case "of the Tiger" suffix ID is 681 and "of Agility" suffix ID is 167.

This would require parsing the item link, though, which looks annoying. More info on item strings and suffix IDs available here. I totally understand if this makes things way too complicated. It's a convenience feature, don't tear your hair out refactoring the entire addon lol

TheGeneticsGuy commented 6 months ago

Weird, I wonder if it's an anomaly since I was just mousing over the items in the AH and the hyperlink text was exactly the same

TheGeneticsGuy commented 6 months ago

I'll look into later this afternoon when I have some time.

RabbleCode commented 6 months ago

I was hovering over them in the Auction House frame too (specifically, my own pending listings).

RabbleCode commented 6 months ago

I suppose it's possible I have some other addon that is manipulating /dump output of item links. I'll try disabling everything and see if it makes a difference.

RabbleCode commented 6 months ago

No difference with all addons disabled, looks the same as in the previous screenshot.

TheGeneticsGuy commented 6 months ago

image

So, I mouseover any of these 3 items - this is in retail, and it doesn't even give me the full name lol:

image

TheGeneticsGuy commented 6 months ago

Anyway, I gotta get some work done, but I'll look into it again this afternoon when I have a chance. I'm sure I can figure something out as clearly there is some underlying data to parse out or else how could the AH frames know which to display properly...

RabbleCode commented 6 months ago

Ah, probably a difference in API between Retail and Classic. I think they revamped how suffixes work at some point over the decades. And no worries, no rush, I appreciate your time and enjoy the troubleshooting process.

TheGeneticsGuy commented 6 months ago

Ya prob. There's always a solution though, probably just dig through the API see what I can find.

TheGeneticsGuy commented 5 months ago

I have another issue I was just thinking ... am I just going to lose notes by using bonusID specificity instead of just item ID, every time the item gets upgraded? For example, every time gets upgraded, or the heirlooms gear gets upgraded to the next level, or the artifact item goes up a level, or the legendary gear is enhanced, the bonus ID is going to change, thus if I go by that, then I will lose the item notes. This adds a LOT of nuance to when or when not to carryover the notes or to ignore bonus ID.. I'd be curious to hear your thoughts on that as this can get really complicated really fast if I try to start adding rules for every edge case consideration of when to and when not to consider ignoring bonus ID and/or carrying over the notes.

RabbleCode commented 5 months ago

Huh, I didn't think of that, that sounds like a nightmare. I agree the edge case handling sounds like it would spiral out of control.

According to the item link structure section on the wow wiki page, it should be possible to distinguish between suffix IDs and bonus IDs as they are at different locations in the item link, but it looks like retail isn't giving you the suffix IDs at all in your screenshot so I don't know. It'd be hard to parse much of anything if the API isn't even returning you a complete link. Even if retail changed where those IDs are stored in the link, it doesn't appear to matter since it's returning you the exact same string for different suffixed items. 🤷

I don't think this will be worth it. I certainly wouldn't be enthused about it were I the author fielding this request.

TheGeneticsGuy commented 5 months ago

Well the funn thing is I already figured out a solution in retail. The problem is just the AH - mousing over won't give you the full details, but if item is anywhere but the AH, linked in chat, your bags, the full link is visible, so I parsed the bonus ID, but then I realized as I was testing that I now am creating unique itemID/bonusID memory considerations, but what if the items get upgraded... The name stays the same, the itemID stays the same, the stats go up, and the bonusID changes... but in this case I don't actually want the note to be unique to each version of the gear lmao. So, I just need to think about it for a day to ensure my headspace is right, but the ONLY way it makes sense on my end is that if I then store the text string of the full name and only give it a unique note if the text name varies as well... if the name is the same, not to change the note. But I will have to rewrite the DB slightly.

It's funny how rather simple changes can actually be sizable modifications in the coding world lol. I just need to sleep on it for a day to make sure it all makes sense in my head.

TheGeneticsGuy commented 5 months ago

Ya, I might make the dictionary references to be the string name of the item instead of the item ID. It's just not as clean looking doing that, but it would resolve this issue...

RabbleCode commented 5 months ago

Oh that's clever! Indexing by full item name avoids all the complication of trying to parse by the various suffix and bonus IDs.

Off the top of my head I can only think of two concerns with that approach:

  1. Localization issues if someone happens to swap back and forth between languages on the same account. String matching would produce some false negatives here.
  2. Item names in practice are probably unique, but they aren't guaranteed to be unique as far as I know

The first one doesn't personally affect me and I'm not sure how common it is to swap languages, but WoW (and by extension its addons) does have a global audience.

The second one is probably a non-issue, but there's always that sliver of an edge case poking at the boundaries, isn't there?Definitely strange how simple requests can spiral into complex logic!

TheGeneticsGuy commented 5 months ago

Haha ya - I will have some time later tonight to review this stuff though.

TheGeneticsGuy commented 5 months ago

Ok so I got it updated, I'll push a release in about 10 minutes, just doing a quick check in Classic era and Cata to ensure it works on all builds, though it should. I changed the DB to use the string name of the items. I also wrote a patch to convert your existing DB of item IDs and convert it over to the new format, so you shouldn't lose any existing notes. The ONLY exception is the notes on the ones that were shared were generic for all the items and those just won't pull now because the only info I have to link to that item is the base item ID, not bonus ID. So Sacrificial Kris will not carry over properly. But the rest will be fine, so minimal impact for future ease.

I DID include the store of all the item IDs as well, so if in the future if I ever decided to add localization support, I can update that way, but I'll keep that on the backburner for now lol. I will mark this as closed now. Thanks again for the help on this!

RabbleCode commented 5 months ago

Hey thanks so much for the update! I'll give it a go later today.

RabbleCode commented 5 months ago

Following up, everything seems to be working great in Classic! I manually copy and pasted my Sacrificial Kris notes from the lua file and then pasted them back in to their "of the Tiger" and "of Agility" counterparts and all is well. Thanks again for the responsiveness and back-and-forth.