aRTy42 / POE-ItemInfo

Item Info Script for Path of Exile
166 stars 224 forks source link

Item info for shaped maps shows incorrect level and "upgrades to" #8

Closed xanthics closed 7 years ago

xanthics commented 8 years ago

See attached image, Shaped Jungle Valley (t6, level 73) upgrades to Arachnid Tomb. However iteminfo states it is level 68 and upgrades to beach(the unshaped map).

This happens with all shaped maps I have available to test with.

screenshot-0008

aRTy42 commented 8 years ago

Thanks for the report. I looked and the wiki has documented the shaped maps, so I have the data needed to update the script.

aRTy42 commented 7 years ago

I wanted to implement the data today and it turns out the shaped map wiki articles were created, but a lot of them are missing the upgrade info. The few ones I initially checked had them, so I assumed most should have them. Consequently that will leave a lot of "?" in the tooltip data for the script.

Eruyome commented 7 years ago

There are still problems with some maps, at least with Ivory Temple. It gets parsed as Temple Map. I fixed the Item.Name parsing for magic/rare items (ParseItemName()) a while ago and had to call this function a second time after knowing the number of affixes the item has. This second call happens after ParseItemType() is called, that function though is called before we know the number of affixes. But ParseItemType() would need the right Item.Name information to use it instead of the NamePlateText to parse Item.SubType and Item.BaseType without fail.

I'm sure there are multiple solutions to this problem, the shortest ones more or less bandaid patches while the proper solutions could require a bit more refactoring. A shorter one could be not stopping the matching loop when a match is found but to continue until all maps have been compared, then the "better"/longer match is used if there are more than one...

Eruyome commented 7 years ago

Well, here is a quick bandaid-fix: https://github.com/aRTy42/POE-ItemInfo/pull/18/commits/cce341726547f6eee1deebca1bffa3a4c7350c5f

aRTy42 commented 7 years ago

Well, the other solution would be to simply check "Ivory Temple Map" before "Temple Map". I had the same issue when implementing all the "Shaped ..." maps, because initially I had them in the "matchlist" after their corresponding map, so "Arcade Map" was matched against the item info before "Shaped Arcade Map" and consequently both types matched the regular map version.

The affix matching has similar manual fine tuning where shorter strings are checked later. I simply didn't notice the Ivory Temple Map/Temple Map issue before.

Edit: it's also the Vaal Temple Map

Eruyome commented 7 years ago

Well, the best solution would still be to get the correct Item.Name and
RegExMatch(Trim(LoopField), "i)^"%Item.Name%"$", match)

Or just LoopField = Item.Name

Should be safer for future changes/additions.

aRTy42 commented 7 years ago

Currently the map list is matched against the item name, not the other way around. Even if the item name was parsed/prepared correctly, the mismatch would occur, because the more general map name is listed earlier in the match list. Of course one could change the way the whole matching works. On the other hand, both methods can be broken by updates. The current variant is vulnerable to GGG renaming maps and other variants might get broken if any assumption about the original ctrl+c clipboard fails and/or when GGG makes changes to the clipboard (which are often enough not documented in the patch notes)

Eruyome commented 7 years ago

Do both and compare results :p

aRTy42 commented 7 years ago

I double checked the map names with a program. Apart from the potential "Shaped" issue, there is:

In the previous map list, only the temple map was listed too early. I moved all three candidates to the very end though, for now.

cirf commented 7 years ago

tested all missing shaped maps upgrades and updated the wiki. An overview can be found her: https://pathofexile.gamepedia.com/User:Cirf_eu/ShapedMaps

aRTy42 commented 7 years ago

Thanks