Improve categories #507
Improve categories #507
Conversation
Reached feature parity with previous category approach. Actually define an Item Wearable function, and use it to control PoB export button. Remove old category name mappings. Refactor around Item baseType, constants, function calls. Don't draw a socket layer if there aren't any sockets. Factor out some magic numbers. More intuitive painter image name. Cleaner switch statements rather than recreate a small set of paths. Add "Copy for Path of Building" button and grouping widget to UI.
DaneelTrevize
changed the title
There was a problem hiding this comment.
Please, follow the code style of surrounding code, for example, you have a lot of if( which should be if (, same for switch. Function calls also shouldn't have extra spaces like in ui->imageLabel->setPixmap( GenerateItemIcon(*current_item_, image_cache_->Get(icon)) ) it should be ui->imageLabel->setPixmap(GenerateItemIcon(*current_item_, image_cache_->Get(icon))) and if you feel there's too many levels of brackets, then the argument should be extracted to a variable, i.e. auto icon = GenerateItemIcon(...); ui->imageLabel->setPixmap(icon)
Also, the logic in CalculateCategories is pretty complicated, it definitely deserves a bunch of new tests.
|
I'm used to different style guidelines w.r.t. using spaces to make things more human-friendly, and avoiding polluting scopes with used-once variables, but it's your codebase to maintain, and apologies where I slip between & mix the different styles as I either get into a flow or revert to referencing other sections. |
Stop iterating over properties once the Map Tier is found. Add unit tests for item category parsing. Cleaner functions. Cleaner range looping. Remove ineffective legacy condition while there remains dedicated Gem Level and Map Tier attributes and search filters. Test assumptions about Shaper & Elder attributes.
|
I'm not sure if you were already notified of the commit earlier that resolved your change requests, but this is to hopefully ping you that it's ready. |
|
there's still a bunch of |
|
Adjusted, as well as a last few |
|
FWIW, I asked a few questions around this icon path parsing approach to GGG, and had a response from Rory. https://www.reddit.com/r/pathofexiledev/comments/a1jnoy/apiwebcdn_question_around_map_icons/ |
Disable PoB export button, so other changes can be accepted.
Reached feature parity with previous category approach.
Define an Item Wearable function, and use it to control PoB export button.
Remove old category name mappings.
Refactor around Item baseType, constants, function calls.
Don't draw a socket layer if there aren't any sockets.
Factor out some magic numbers. More intuitive painter image name.
Cleaner switch statements rather than recreate a small set of paths.
Add "Copy for Path of Building" button and grouping widget to UI.