xyzz / acquisition

http://get.acquisition.today/
GNU General Public License v3.0
271 stars 100 forks source link

'Type' drop down menu not visible in SSF standard #547

Closed adrianjtaylor closed 4 years ago

adrianjtaylor commented 4 years ago

Works ok in Standard. Can see all items, but can't select by type

flesyMeM commented 4 years ago

The types are gone for me in Standard. The only option is "Any". This is new as of patch 3.8.

xyzz commented 4 years ago

My understanding is that the JSON fields this feature relied on were removed from the API - see https://www.pathofexile.com/forum/view-thread/2627531

DaneelTrevize commented 4 years ago

Do you intend to reimplement this feature? It seems Procurement uses hardcoded substring matches to derive the category from the typeLine
https://github.com/Stickymaddness/Procurement/blob/master/POEApi.Model/GearType/GearTypeRunner.cs

I think another, more brittle option would be to use the icon 2D Art URL.

xyzz commented 4 years ago

The fix is unlikely to come from me, no. If somebody contributes the feature I'll merge it of course.

ericsium commented 4 years ago

I just checked, actually I was already using the 2D Art URL for this feature, maybe they started quoting the slashes in the icon paths or something else must have subtly changed. This probably isn't a difficult fix. If you want to take this Daneel let me know, else I'll try to look at it this weekend.

DaneelTrevize commented 4 years ago

I started to look at it, currently the art URL seems only to really be used for maps, and that may not be accounting for the 3.5.0 changes https://www.pathofexile.com/forum/view-thread/2254801.
It's trivially otherwise used to subcategorise vaal gems, Breach/Essence currency.

I tested out using the typeLine, it seems possible, but probably more brittle than parsing the icon path at first glance, because you need a huge list of most type (sub)strings, and to manually subcategorise them for Armour. and handedness, etc. And this is once you've determined the item's gear vs other stuff. Procurement first applies some simple tests to do this before handling an item as being certainly gear/gem/jewel/currency.

So I'll test out using the test & icon path approach before the weekend.

ericsium commented 4 years ago

Hmm, I'm confused I changed this a few years ago to use icon path but I only see the changes in my branch.

https://github.com/ericsium/acquisition/blob/master/src/item.cpp

So maybe somehow this didn't get merged. I don't use git that often so have to look further to see what's up.

What do you mean that URL seems to be only used for maps? It looks to me like all items still have 'icon' attribute with paths that seems to be the same as before.

ericsium commented 4 years ago

The commit was here:

https://github.com/ericsium/acquisition/commit/1c5f4534e872b17510e6bbbec72212a081c31de4#diff-f7ca61cf749eba039da961e8a7be1630

xyzz commented 4 years ago

It was changed in https://github.com/xyzz/acquisition/pull/507

ericsium commented 4 years ago

Ok, I see you re-implemented Daneel so I'll let you fix this.

ericsium commented 4 years ago

I had previously found the iconpath seems to have detailed classification information for all items with possibly multiple levels of information, for example:

2DItems\/Gems 2DItems\/Gems\/VaalGems 2DItems\/Gems\/Support 2DItems\/Rings 2DItems\/Amulets 2DItems\/Currency 2DItems\/Currency\/Oils 2DItems\/Weapons\/OneHandWeapons\/Claws 2DItems\/Armours\/Gloves

The advantage of using iconpaths is it's easy to dynamically generate the information, So when new stuff shows up like Oils it should 'just work' and the code is compact and doesn't need to be maintained as long the icon paths don't go away.

DaneelTrevize commented 4 years ago

Ok, I think I've got the new system done, I'm asking here first for a look-over before I faff with squashing it into a single change & PR.

I also developed a new Rarity dropdown filter, and an Unidentified checkbox filter, as I find them useful in competing tools & there are outstanding raised Issues for such things here. Feedback on those would be appreciated, and whether they want to be held back for a different PR or absorbed into a single branch, or even a single change?

ericsium commented 4 years ago

Nice! Looks ok to me. I'd do two separate pull requests, one for the categorization work, the other for the filtering.

DaneelTrevize commented 4 years ago

Had an unexpectedly busy IRL weekend. PRs are raised, the second one being based on the first, hopefully easy to just accept both in order rather than wait for me to update my master inbetween.

adrianjtaylor commented 4 years ago

Hi Guys, thanks for the great work, but I don't understand how to get the modified files back into the App. Could someone give me the idiot's guide to this. I downloaded Visual Stdio etc but don't know what to do with it

ericsium commented 4 years ago

You need to wait for a new acquisition release which xyzz will get to when he gets to it.

adrianjtaylor commented 4 years ago

OK thanks for getting back to me

DaneelTrevize commented 4 years ago

PR #552 is also a part of this. Forgot to retest having restored the PoB export button functionality after tweaking category names.

indeed317 commented 4 years ago

Eta on this getting pushed to public release?